diff --git a/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.js b/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.js index 7510b6686a..1b9bf5597f 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.js +++ b/services/web/app/src/Features/SplitTests/SplitTestSessionHandler.js @@ -60,12 +60,18 @@ async function getAssignments(session) { if (!assignments[splitTestName]) { assignments[splitTestName] = [] } - assignments[splitTestName].push({ - versionNumber, - variantName, - phase: 'release', // anonymous users can only be exposed to tests in release phase - assignedAt, - }) + if ( + !_.find(assignments[splitTestName], { + versionNumber, + variantName, + }) + ) + assignments[splitTestName].push({ + versionNumber, + variantName, + phase: 'release', // anonymous users can only be exposed to tests in release phase + assignedAt, + }) } catch (error) { logger.error( { err: error, token }, @@ -193,13 +199,15 @@ async function _convertAnonymousAssignmentsIfNeeded(session) { session.sta = '' } for (const [splitTestName, assignments] of Object.entries( - session.splitTests + session.splitTests || {} )) { const splitTest = splitTests.get(splitTestName) for (const assignment of assignments) { const assignmentString = _buildAssignmentString(splitTest, assignment) const separator = session.sta.length > 0 ? TOKEN_SEP : '' - session.sta += `${separator}${assignmentString}` + if (!session.sta.includes(assignmentString)) { + session.sta += `${separator}${assignmentString}` + } } } delete session.splitTests diff --git a/services/web/test/unit/src/SplitTests/SplitTestSessionHandlerTests.js b/services/web/test/unit/src/SplitTests/SplitTestSessionHandlerTests.js index b9216c8464..ac1159a78d 100644 --- a/services/web/test/unit/src/SplitTests/SplitTestSessionHandlerTests.js +++ b/services/web/test/unit/src/SplitTests/SplitTestSessionHandlerTests.js @@ -120,7 +120,7 @@ describe('SplitTestSessionHandler', function () { }) }) - it('should not read from the sta field', async function () { + it('should read from the sta field', async function () { this.SplitTestCache.get = sinon.stub().resolves( new Map( Object.entries({ @@ -190,6 +190,76 @@ describe('SplitTestSessionHandler', function () { }) }) + it('should deduplicate entries from the sta field', async function () { + this.SplitTestCache.get = sinon.stub().resolves( + new Map( + Object.entries({ + 'anon-test-1': { + _id: '661f92a4669764bb03f73e37', + name: 'anon-test-1', + versions: [ + { + versionNumber: 1, + variants: [ + { + name: 'enabled', + }, + ], + }, + ], + }, + 'anon-test-2': { + _id: '661f92a9d68ea711d6bf2df4', + name: 'anon-test-2', + versions: [ + { + versionNumber: 1, + variants: [ + { + name: 'v-1', + }, + { + name: 'v-2', + }, + ], + }, + ], + }, + }) + ) + ) + const session = { + sta: `Zh+SpGaXZLsD9z43_1=d:sbrvs0;Zh+SqdaOpxHWvy30_1=d:sbtqg0;Zh+SqdaOpxHWvy30_1=d:sbtqg0;Zh+SpGaXZLsD9z43_1=d:sbrvs0;Zh+SqdaOpxHWvy30_2=1:sbsi00;Zh+SqdaOpxHWvy30_1=d:sbtqg0`, + } + + const assignments = + await this.SplitTestSessionHandler.promises.getAssignments(session) + expect(assignments).to.deep.equal({ + 'anon-test-1': [ + { + variantName: 'default', + versionNumber: 1, + phase: 'release', + assignedAt: new Date(1712829600000), + }, + ], + 'anon-test-2': [ + { + variantName: 'default', + versionNumber: 1, + phase: 'release', + assignedAt: new Date(1712916000000), + }, + { + variantName: 'v-2', + versionNumber: 2, + phase: 'release', + assignedAt: new Date(1712858400000), + }, + ], + }) + }) + it('should merge assignments from both splitTests and sta fields', async function () { const session = { splitTests: {