From 931c53d04c43d79ef5bf3e3e23600555976d84b4 Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Fri, 28 Jan 2022 11:44:10 +0100 Subject: [PATCH] Split Tests Admin UI - test list (#6411) * Admin page to list split tests Admin page to list split tests Update split test model Display optional split test fields + Only display link in admin menu in SaaS mode Add a separate endpoint to update split test info (to avoid creating a new version) Fix split test tests after rebase Add name and activeOnly filtering to split test list Sort split tests by descending order of last modification Fix lint build error Add buttons to copy variant assignment as query string * Move react components to follow our file structure guidelines * Cleanup and improvements from review * Small change of report URLs display * Improve filters display for small screens GitOrigin-RevId: 498531a1f080419de017883e33d6afed05a3f5c9 --- .../Features/SplitTests/SplitTestManager.js | 41 +++++++++++++++---- services/web/app/src/models/SplitTest.js | 26 +++++++++--- .../web/app/views/layout/navbar-marketing.pug | 3 ++ services/web/app/views/layout/navbar.pug | 3 ++ 4 files changed, 60 insertions(+), 13 deletions(-) diff --git a/services/web/app/src/Features/SplitTests/SplitTestManager.js b/services/web/app/src/Features/SplitTests/SplitTestManager.js index 6a321a377b..0695b9f7ea 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestManager.js +++ b/services/web/app/src/Features/SplitTests/SplitTestManager.js @@ -6,9 +6,16 @@ const ALPHA_PHASE = 'alpha' const BETA_PHASE = 'beta' const RELEASE_PHASE = 'release' -async function getSplitTests() { +async function getSplitTests({ name, activeOnly }) { + const filters = {} + if (name && name !== '') { + filters.name = { $regex: _.escapeRegExp(name) } + } + if (activeOnly) { + filters.$where = 'this.versions[this.versions.length - 1].active === true' + } try { - return await SplitTest.find().exec() + return await SplitTest.find(filters).limit(100).exec() } catch (error) { throw OError.tag(error, 'Failed to get split tests list') } @@ -22,14 +29,13 @@ async function getSplitTestByName(name) { } } -async function createSplitTest(name, configuration) { +async function createSplitTest(name, configuration, info = {}) { const stripedVariants = [] let stripeStart = 0 _checkNewVariantsConfiguration([], configuration.variants) for (const variant of configuration.variants) { stripedVariants.push({ name: variant.name, - active: variant.active, rolloutPercent: variant.rolloutPercent, rolloutStripes: [ { @@ -42,6 +48,11 @@ async function createSplitTest(name, configuration) { } const splitTest = new SplitTest({ name, + description: info.description, + expectedEndDate: info.expectedEndDate, + ticketUrl: info.ticketUrl, + reportsUrls: info.reportsUrls, + winningVariant: info.winningVariant, versions: [ { versionNumber: 1, @@ -54,7 +65,7 @@ async function createSplitTest(name, configuration) { return _saveSplitTest(splitTest) } -async function updateSplitTest(name, configuration) { +async function updateSplitTestConfig(name, configuration) { const splitTest = await getSplitTestByName(name) if (splitTest) { const lastVersion = splitTest.getCurrentVersion().toObject() @@ -68,6 +79,7 @@ async function updateSplitTest(name, configuration) { lastVersion.variants, configuration.variants ) + splitTest.versions.push({ versionNumber: lastVersion.versionNumber + 1, phase: configuration.phase, @@ -80,6 +92,20 @@ async function updateSplitTest(name, configuration) { } } +async function updateSplitTestInfo(name, info) { + const splitTest = await getSplitTestByName(name) + if (splitTest) { + splitTest.description = info.description + splitTest.expectedEndDate = info.expectedEndDate + splitTest.ticketUrl = info.ticketUrl + splitTest.reportsUrls = info.reportsUrls + splitTest.winningVariant = info.winningVariant + return _saveSplitTest(splitTest) + } else { + throw new OError(`Cannot update split test '${name}': not found`) + } +} + async function switchToNextPhase(name) { const splitTest = await getSplitTestByName(name) if (splitTest) { @@ -181,7 +207,6 @@ function _updateVariantsWithNewConfiguration( if (!variant) { variantsCopy.push({ name: newVariantConfig.name, - active: newVariantConfig.active, rolloutPercent: newVariantConfig.rolloutPercent, rolloutStripes: [ { @@ -194,7 +219,6 @@ function _updateVariantsWithNewConfiguration( } else if (variant.rolloutPercent < newVariantConfig.rolloutPercent) { const newStripeSize = newVariantConfig.rolloutPercent - variant.rolloutPercent - variant.active = newVariantConfig.active variant.rolloutPercent = newVariantConfig.rolloutPercent variant.rolloutStripes.push({ start: totalRolloutPercentage, @@ -224,7 +248,8 @@ module.exports = { getSplitTestByName, getSplitTests, createSplitTest, - updateSplitTest, + updateSplitTestConfig, + updateSplitTestInfo, switchToNextPhase, revertToPreviousVersion, } diff --git a/services/web/app/src/models/SplitTest.js b/services/web/app/src/models/SplitTest.js index 789b7fb559..f302ad81ce 100644 --- a/services/web/app/src/models/SplitTest.js +++ b/services/web/app/src/models/SplitTest.js @@ -30,11 +30,6 @@ const VariantSchema = new Schema( message: `invalid, cannot be 'default' and must match: ${NAME_REGEX}, got {VALUE}`, }, }, - active: { - type: Boolean, - default: true, - required: true, - }, rolloutPercent: RolloutPercentType, rolloutStripes: [ { @@ -93,6 +88,27 @@ const SplitTestSchema = new Schema({ type: Boolean, required: false, }, + description: { + type: String, + required: false, + }, + expectedEndDate: { + type: Date, + required: false, + }, + ticketUrl: { + type: String, + required: false, + }, + reportsUrls: { + type: [String], + required: false, + default: [], + }, + winningVariant: { + type: String, + required: false, + }, }) SplitTestSchema.methods.getCurrentVersion = function () { diff --git a/services/web/app/views/layout/navbar-marketing.pug b/services/web/app/views/layout/navbar-marketing.pug index 2a69d25457..063c3f5b26 100644 --- a/services/web/app/views/layout/navbar-marketing.pug +++ b/services/web/app/views/layout/navbar-marketing.pug @@ -33,6 +33,9 @@ nav.navbar.navbar-default.navbar-main a(href="/admin") Manage Site li a(href="/admin/user") Manage Users + if hasFeature('saas') + li + a(href="/admin/split-test") Manage Split Tests // loop over header_extras diff --git a/services/web/app/views/layout/navbar.pug b/services/web/app/views/layout/navbar.pug index 5145f30768..156b1744e2 100644 --- a/services/web/app/views/layout/navbar.pug +++ b/services/web/app/views/layout/navbar.pug @@ -23,6 +23,9 @@ nav.navbar.navbar-default.navbar-main a(href="/admin") Manage Site li a(href="/admin/user") Manage Users + if hasFeature('saas') + li + a(href="/admin/split-test") Manage Split Tests // loop over header_extras