From 8e76a9e2fca5a73b21e6bf167bde406ac8faed91 Mon Sep 17 00:00:00 2001 From: Alexandre Bourdin Date: Thu, 10 Feb 2022 16:39:37 +0100 Subject: [PATCH] Merge pull request #6587 from overleaf/ab-split-test-create-edit Split tests admin - create/edit GitOrigin-RevId: a256bf6fe8350214b1ef01ff5e6fa68a812a59be --- package-lock.json | 141 +++++++++++++++++- .../Features/SplitTests/SplitTestManager.js | 6 +- .../UserMembershipMiddleware.js | 15 ++ services/web/app/src/models/SplitTest.js | 2 +- services/web/app/src/models/User.js | 2 + .../web/app/views/layout/navbar-marketing.pug | 18 ++- services/web/app/views/layout/navbar.pug | 17 ++- .../frontend/stylesheets/app/admin-hub.less | 49 ++++++ services/web/package.json | 4 +- 9 files changed, 231 insertions(+), 23 deletions(-) diff --git a/package-lock.json b/package-lock.json index 1928470649..521c03b90f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18559,6 +18559,47 @@ "url": "https://ko-fi.com/tunnckoCore/commissions" } }, + "node_modules/formik": { + "version": "2.2.9", + "resolved": "https://registry.npmjs.org/formik/-/formik-2.2.9.tgz", + "integrity": "sha512-LQLcISMmf1r5at4/gyJigGn0gOwFbeEAlji+N9InZF6LIMXnFNkO42sCI8Jt84YZggpD4cPWObAZaxpEFtSzNA==", + "funding": [ + { + "type": "individual", + "url": "https://opencollective.com/formik" + } + ], + "dependencies": { + "deepmerge": "^2.1.1", + "hoist-non-react-statics": "^3.3.0", + "lodash": "^4.17.21", + "lodash-es": "^4.17.21", + "react-fast-compare": "^2.0.1", + "tiny-warning": "^1.0.2", + "tslib": "^1.10.0" + }, + "peerDependencies": { + "react": ">=16.8.0" + } + }, + "node_modules/formik/node_modules/deepmerge": { + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/deepmerge/-/deepmerge-2.2.1.tgz", + "integrity": "sha512-R9hc1Xa/NOBi9WRVUWg19rl1UB7Tt4kuPd+thNJgFZoxXsTz7ncaPaeIm+40oSGuP33DfMb4sZt1QIGiJzC4EA==", + "engines": { + "node": ">=0.10.0" + } + }, + "node_modules/formik/node_modules/react-fast-compare": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/react-fast-compare/-/react-fast-compare-2.0.4.tgz", + "integrity": "sha512-suNP+J1VU1MWFKcyt7RtjiSWUjvidmQSlqu+eHslq+342xCbGTYmC0mEhPCOHxlW0CywylOC1u2DFAT+bv4dBw==" + }, + "node_modules/formik/node_modules/tslib": { + "version": "1.14.1", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz", + "integrity": "sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==" + }, "node_modules/forwarded": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/forwarded/-/forwarded-0.2.0.tgz", @@ -26502,6 +26543,11 @@ "resolved": "https://registry.npmjs.org/nan/-/nan-2.15.0.tgz", "integrity": "sha512-8ZtvEnA2c5aYCZYd1cvgdnU6cqwixRoYg70xPLWUws5ORTa/lnw+u4amixRS/Ac5U5mQVgp9pnlSUnbNWFaWZQ==" }, + "node_modules/nanoclone": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/nanoclone/-/nanoclone-0.2.1.tgz", + "integrity": "sha512-wynEP02LmIbLpcYw8uBKpcfF6dmg2vcpKqxeH5UcoKEYdExslsdUA4ugFauuaeYdTB76ez6gJW8XAZ6CgkXYxA==" + }, "node_modules/nanoid": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.2.0.tgz", @@ -30179,6 +30225,11 @@ "node >= 0.8.1" ] }, + "node_modules/property-expr": { + "version": "2.0.5", + "resolved": "https://registry.npmjs.org/property-expr/-/property-expr-2.0.5.tgz", + "integrity": "sha512-IJUkICM5dP5znhCckHSv30Q4b5/JA5enCtkRHYaOVOAocnH/1BQEYTC5NMfT3AVl/iXKdr3aqQbQn9DxyWknwA==" + }, "node_modules/property-information": { "version": "5.6.0", "resolved": "https://registry.npmjs.org/property-information/-/property-information-5.6.0.tgz", @@ -36690,6 +36741,11 @@ "integrity": "sha512-YXXAAhmF9zpQbC7LEcREFtXfGq5K1fmd+4PHkBq8NUqmzW3G+Dq10bI/i0KucLRwss3YYFQ0fSfoxBZYiGUqtQ==", "deprecated": "This module has moved and is now available at @hapi/hoek. Please update your dependencies as this version is no longer maintained an may contain bugs and security issues." }, + "node_modules/toposort": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/toposort/-/toposort-2.0.2.tgz", + "integrity": "sha1-riF2gXXRVZ1IvvNUILL0li8JwzA=" + }, "node_modules/toposort-class": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/toposort-class/-/toposort-class-1.0.1.tgz", @@ -40795,6 +40851,23 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/yup": { + "version": "0.32.11", + "resolved": "https://registry.npmjs.org/yup/-/yup-0.32.11.tgz", + "integrity": "sha512-Z2Fe1bn+eLstG8DRR6FTavGD+MeAwyfmouhHsIUgaADz8jvFKbO/fXc2trJKZg+5EBjh4gGm3iU/t3onKlXHIg==", + "dependencies": { + "@babel/runtime": "^7.15.4", + "@types/lodash": "^4.14.175", + "lodash": "^4.17.21", + "lodash-es": "^4.17.21", + "nanoclone": "^0.2.1", + "property-expr": "^2.0.4", + "toposort": "^2.0.2" + }, + "engines": { + "node": ">=10" + } + }, "node_modules/zeparser": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/zeparser/-/zeparser-0.0.5.tgz", @@ -43817,6 +43890,7 @@ "express-bearer-token": "^2.4.0", "express-http-proxy": "^1.6.0", "express-session": "^1.17.1", + "formik": "^2.2.9", "fs-extra": "^4.0.2", "fuse.js": "^3.0.0", "globby": "^5.0.0", @@ -43899,7 +43973,8 @@ "xml-crypto": "^2.1.2", "xml2js": "^0.4.22", "xregexp": "^4.3.0", - "yauzl": "^2.10.0" + "yauzl": "^2.10.0", + "yup": "^0.32.11" }, "devDependencies": { "@babel/register": "^7.14.5", @@ -51236,6 +51311,7 @@ "express-session": "^1.17.1", "fetch-mock": "^9.10.2", "file-loader": "^5.0.2", + "formik": "^2.2.9", "fs-extra": "^4.0.2", "fuse.js": "^3.0.0", "glob": "^7.1.6", @@ -51361,7 +51437,8 @@ "xml-crypto": "^2.1.2", "xml2js": "^0.4.22", "xregexp": "^4.3.0", - "yauzl": "^2.10.0" + "yauzl": "^2.10.0", + "yup": "^0.32.11" }, "dependencies": { "@eslint/eslintrc": { @@ -63534,6 +63611,37 @@ "integrity": "sha512-KcpbcpuLNOwrEjnbpMC0gS+X8ciDoZE1kkqzat4a8vrprf+s9pKNQ/QIwWfbfs4ltgmFl3MD177SNTkve3BwGQ==", "dev": true }, + "formik": { + "version": "2.2.9", + "resolved": "https://registry.npmjs.org/formik/-/formik-2.2.9.tgz", + "integrity": "sha512-LQLcISMmf1r5at4/gyJigGn0gOwFbeEAlji+N9InZF6LIMXnFNkO42sCI8Jt84YZggpD4cPWObAZaxpEFtSzNA==", + "requires": { + "deepmerge": "^2.1.1", + "hoist-non-react-statics": "^3.3.0", + "lodash": "^4.17.21", + "lodash-es": "^4.17.21", + "react-fast-compare": "^2.0.1", + "tiny-warning": "^1.0.2", + "tslib": "^1.10.0" + }, + "dependencies": { + "deepmerge": { + "version": "2.2.1", + "resolved": "https://registry.npmjs.org/deepmerge/-/deepmerge-2.2.1.tgz", + "integrity": "sha512-R9hc1Xa/NOBi9WRVUWg19rl1UB7Tt4kuPd+thNJgFZoxXsTz7ncaPaeIm+40oSGuP33DfMb4sZt1QIGiJzC4EA==" + }, + "react-fast-compare": { + "version": "2.0.4", + "resolved": "https://registry.npmjs.org/react-fast-compare/-/react-fast-compare-2.0.4.tgz", + "integrity": "sha512-suNP+J1VU1MWFKcyt7RtjiSWUjvidmQSlqu+eHslq+342xCbGTYmC0mEhPCOHxlW0CywylOC1u2DFAT+bv4dBw==" + }, + "tslib": { + "version": "1.14.1", + "resolved": "https://registry.npmjs.org/tslib/-/tslib-1.14.1.tgz", + "integrity": "sha512-Xni35NKzjgMrwevysHTCArtLDpPvye8zV/0E4EyYn43P7/7qvQwPh9BGkHewbMulVntbigmcT7rdX3BNo9wRJg==" + } + } + }, "forwarded": { "version": "0.2.0", "resolved": "https://registry.npmjs.org/forwarded/-/forwarded-0.2.0.tgz", @@ -69879,6 +69987,11 @@ "resolved": "https://registry.npmjs.org/nan/-/nan-2.15.0.tgz", "integrity": "sha512-8ZtvEnA2c5aYCZYd1cvgdnU6cqwixRoYg70xPLWUws5ORTa/lnw+u4amixRS/Ac5U5mQVgp9pnlSUnbNWFaWZQ==" }, + "nanoclone": { + "version": "0.2.1", + "resolved": "https://registry.npmjs.org/nanoclone/-/nanoclone-0.2.1.tgz", + "integrity": "sha512-wynEP02LmIbLpcYw8uBKpcfF6dmg2vcpKqxeH5UcoKEYdExslsdUA4ugFauuaeYdTB76ez6gJW8XAZ6CgkXYxA==" + }, "nanoid": { "version": "3.2.0", "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.2.0.tgz", @@ -72818,6 +72931,11 @@ "integrity": "sha1-F6EW0l2vIJRCbX1oRNJiMsnWkms=", "dev": true }, + "property-expr": { + "version": "2.0.5", + "resolved": "https://registry.npmjs.org/property-expr/-/property-expr-2.0.5.tgz", + "integrity": "sha512-IJUkICM5dP5znhCckHSv30Q4b5/JA5enCtkRHYaOVOAocnH/1BQEYTC5NMfT3AVl/iXKdr3aqQbQn9DxyWknwA==" + }, "property-information": { "version": "5.6.0", "resolved": "https://registry.npmjs.org/property-information/-/property-information-5.6.0.tgz", @@ -78127,6 +78245,11 @@ } } }, + "toposort": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/toposort/-/toposort-2.0.2.tgz", + "integrity": "sha1-riF2gXXRVZ1IvvNUILL0li8JwzA=" + }, "toposort-class": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/toposort-class/-/toposort-class-1.0.1.tgz", @@ -81359,6 +81482,20 @@ "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz", "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==" }, + "yup": { + "version": "0.32.11", + "resolved": "https://registry.npmjs.org/yup/-/yup-0.32.11.tgz", + "integrity": "sha512-Z2Fe1bn+eLstG8DRR6FTavGD+MeAwyfmouhHsIUgaADz8jvFKbO/fXc2trJKZg+5EBjh4gGm3iU/t3onKlXHIg==", + "requires": { + "@babel/runtime": "^7.15.4", + "@types/lodash": "^4.14.175", + "lodash": "^4.17.21", + "lodash-es": "^4.17.21", + "nanoclone": "^0.2.1", + "property-expr": "^2.0.4", + "toposort": "^2.0.2" + } + }, "zeparser": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/zeparser/-/zeparser-0.0.5.tgz", diff --git a/services/web/app/src/Features/SplitTests/SplitTestManager.js b/services/web/app/src/Features/SplitTests/SplitTestManager.js index 0695b9f7ea..2c075b45e2 100644 --- a/services/web/app/src/Features/SplitTests/SplitTestManager.js +++ b/services/web/app/src/Features/SplitTests/SplitTestManager.js @@ -35,7 +35,7 @@ async function createSplitTest(name, configuration, info = {}) { _checkNewVariantsConfiguration([], configuration.variants) for (const variant of configuration.variants) { stripedVariants.push({ - name: variant.name, + name: (variant.name || '').trim(), rolloutPercent: variant.rolloutPercent, rolloutStripes: [ { @@ -47,7 +47,7 @@ async function createSplitTest(name, configuration, info = {}) { stripeStart += variant.rolloutPercent } const splitTest = new SplitTest({ - name, + name: (name || '').trim(), description: info.description, expectedEndDate: info.expectedEndDate, ticketUrl: info.ticketUrl, @@ -71,7 +71,7 @@ async function updateSplitTestConfig(name, configuration) { const lastVersion = splitTest.getCurrentVersion().toObject() if (configuration.phase !== lastVersion.phase) { throw new OError( - `Cannot update with different phase - use switchToNextPhase endpoint instead` + `Cannot update with different phase - use /switch-to-next-phase endpoint instead` ) } _checkNewVariantsConfiguration(lastVersion.variants, configuration.variants) diff --git a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js index d84730b927..e80c4e8757 100644 --- a/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js +++ b/services/web/app/src/Features/UserMembership/UserMembershipMiddleware.js @@ -155,6 +155,21 @@ const UserMembershipMiddleware = { fetchEntityConfig('institution'), ], + requireSplitTestMetricsAccess: [ + AuthenticationController.requireLogin(), + allowAccessIfAny([ + UserMembershipAuthorization.hasStaffAccess('splitTestMetrics'), + UserMembershipAuthorization.hasStaffAccess('splitTestManagement'), + ]), + ], + + requireSplitTestManagementAccess: [ + AuthenticationController.requireLogin(), + allowAccessIfAny([ + UserMembershipAuthorization.hasStaffAccess('splitTestManagement'), + ]), + ], + // graphs access is an edge-case: // - the entity id is in `req.query.resource_id`. It must be set as // `req.params.id` diff --git a/services/web/app/src/models/SplitTest.js b/services/web/app/src/models/SplitTest.js index f302ad81ce..a84fcc0d50 100644 --- a/services/web/app/src/models/SplitTest.js +++ b/services/web/app/src/models/SplitTest.js @@ -6,7 +6,7 @@ const MIN_NAME_LENGTH = 3 const MAX_NAME_LENGTH = 200 const MIN_VARIANT_NAME_LENGTH = 3 const MAX_VARIANT_NAME_LENGTH = 255 -const NAME_REGEX = /^[a-zA-Z0-9\-_]+$/ +const NAME_REGEX = /^[a-z0-9-]+$/ const RolloutPercentType = { type: Number, diff --git a/services/web/app/src/models/User.js b/services/web/app/src/models/User.js index 9a02a46929..dabaf91c7e 100644 --- a/services/web/app/src/models/User.js +++ b/services/web/app/src/models/User.js @@ -49,6 +49,8 @@ const UserSchema = new Schema({ groupMetrics: { type: Boolean, default: false }, groupManagement: { type: Boolean, default: false }, adminMetrics: { type: Boolean, default: false }, + splitTestMetrics: { type: Boolean, default: false }, + splitTestManagement: { type: Boolean, default: false }, }, signUpDate: { type: Date, diff --git a/services/web/app/views/layout/navbar-marketing.pug b/services/web/app/views/layout/navbar-marketing.pug index 063c3f5b26..c62ddae9b1 100644 --- a/services/web/app/views/layout/navbar-marketing.pug +++ b/services/web/app/views/layout/navbar-marketing.pug @@ -15,9 +15,11 @@ nav.navbar.navbar-default.navbar-main else a(href='/', aria-label=settings.appName).navbar-brand - .navbar-collapse.collapse(data-ol-navbar-main-collapse) + - var canDisplayAdminMenu = getSessionUser() && getSessionUser().isAdmin + - var canDisplaySplitTestMenu = hasFeature('saas') && (canDisplayAdminMenu || (getSessionUser() && getSessionUser().staffAccess && (getSessionUser().staffAccess.splitTestMetrics || getSessionUser().staffAccess.splitTestManagement))) + .navbar-collapse.collapse(collapse="navCollapsed") ul.nav.navbar-nav.navbar-right - if (getSessionUser() && getSessionUser().isAdmin) + if (canDisplayAdminMenu || canDisplaySplitTestMenu) li.dropdown.subdued a.dropdown-toggle( href="#", @@ -29,15 +31,15 @@ nav.navbar.navbar-default.navbar-main | Admin span.caret ul.dropdown-menu - li - a(href="/admin") Manage Site - li - a(href="/admin/user") Manage Users - if hasFeature('saas') + if canDisplayAdminMenu + li + a(href="/admin") Manage Site + li + a(href="/admin/user") Manage Users + if canDisplaySplitTestMenu li a(href="/admin/split-test") Manage Split Tests - // loop over header_extras each item in ((splitTestVariants && (splitTestVariants['unified-navigation'] === 'show-unified-navigation')) ? nav.header_extras_unified : nav.header_extras) - diff --git a/services/web/app/views/layout/navbar.pug b/services/web/app/views/layout/navbar.pug index 156b1744e2..7ae635ba40 100644 --- a/services/web/app/views/layout/navbar.pug +++ b/services/web/app/views/layout/navbar.pug @@ -10,24 +10,25 @@ nav.navbar.navbar-default.navbar-main else a(href='/', aria-label=settings.appName).navbar-brand + - var canDisplayAdminMenu = getSessionUser() && getSessionUser().isAdmin + - var canDisplaySplitTestMenu = hasFeature('saas') && (canDisplayAdminMenu || (getSessionUser() && getSessionUser().staffAccess && (getSessionUser().staffAccess.splitTestMetrics || getSessionUser().staffAccess.splitTestManagement))) .navbar-collapse.collapse(collapse="navCollapsed") - ul.nav.navbar-nav.navbar-right - if (getSessionUser() && getSessionUser().isAdmin) + if (canDisplayAdminMenu || canDisplaySplitTestMenu) li.dropdown(class="subdued", dropdown) a.dropdown-toggle(href, dropdown-toggle) | Admin b.caret ul.dropdown-menu - li - a(href="/admin") Manage Site - li - a(href="/admin/user") Manage Users - if hasFeature('saas') + if canDisplayAdminMenu + li + a(href="/admin") Manage Site + li + a(href="/admin/user") Manage Users + if canDisplaySplitTestMenu li a(href="/admin/split-test") Manage Split Tests - // loop over header_extras each item in ((splitTestVariants && (splitTestVariants['unified-navigation'] === 'show-unified-navigation')) ? nav.header_extras_unified : nav.header_extras) - diff --git a/services/web/frontend/stylesheets/app/admin-hub.less b/services/web/frontend/stylesheets/app/admin-hub.less index d27a899536..a4f199d623 100644 --- a/services/web/frontend/stylesheets/app/admin-hub.less +++ b/services/web/frontend/stylesheets/app/admin-hub.less @@ -105,3 +105,52 @@ display: list-item; } } + +.material-switch { + input[type='checkbox'] { + display: none; + + &:checked + label::before { + background: inherit; + opacity: 0.5; + } + &:checked + label::after { + background: inherit; + left: 20px; + } + } + + label { + cursor: pointer; + height: 0; + position: relative; + width: 40px; + + &:before { + background: rgb(0, 0, 0); + box-shadow: inset 0 0 10px rgba(0, 0, 0, 0.5); + border-radius: 8px; + content: ''; + height: 16px; + margin-top: -2px; + position: absolute; + opacity: 0.3; + transition: all 0.2s ease-in-out; + width: 40px; + } + + &:after { + background: rgb(255, 255, 255); + border-radius: 16px; + box-shadow: 0 0 5px rgba(0, 0, 0, 0.3); + content: ''; + height: 24px; + left: -4px; + margin-top: -2px; + position: absolute; + top: -4px; + transition: all 0.2s ease-in-out; + width: 24px; + } + } +} diff --git a/services/web/package.json b/services/web/package.json index 836893120b..b94076feb4 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -117,6 +117,7 @@ "express-bearer-token": "^2.4.0", "express-http-proxy": "^1.6.0", "express-session": "^1.17.1", + "formik": "^2.2.9", "fs-extra": "^4.0.2", "fuse.js": "^3.0.0", "globby": "^5.0.0", @@ -199,7 +200,8 @@ "xml-crypto": "^2.1.2", "xml2js": "^0.4.22", "xregexp": "^4.3.0", - "yauzl": "^2.10.0" + "yauzl": "^2.10.0", + "yup": "^0.32.11" }, "devDependencies": { "@babel/register": "^7.14.5",