Просмотр исходного кода

Merge pull request #8957 from overleaf/ab-split-test-controls-badge

[web] SplitTestBadge based on split test phase and badge config

GitOrigin-RevId: e178ca864fd6619ff61a2a84fc1ccb5d54e0a814
Alexandre Bourdin 4 лет назад
Родитель
Сommit
e9e36737e6

+ 8 - 0
services/web/app/src/Features/SplitTests/LocalsHelper.js

@@ -5,6 +5,14 @@ function setSplitTestVariant(locals, splitTestName, variant) {
   locals.splitTestVariants[splitTestName] = variant
 }
 
+function setSplitTestInfo(locals, splitTestName, info) {
+  if (!locals.splitTestInfo) {
+    locals.splitTestInfo = {}
+  }
+  locals.splitTestInfo[splitTestName] = info
+}
+
 module.exports = {
   setSplitTestVariant,
+  setSplitTestInfo,
 }

+ 17 - 1
services/web/app/src/Features/SplitTests/SplitTestHandler.js

@@ -80,13 +80,15 @@ async function getAssignment(req, res, splitTestName, { sync = false } = {}) {
     splitTestName,
     assignment.variant
   )
+  await _loadSplitTestInfoInLocals(res.locals, splitTestName)
   return assignment
 }
 
 /**
  * Get the assignment of a user to a split test by their user ID.
  *
- * Warning: this does not support query parameters override. Wherever possible, `getAssignment` should be used instead.
+ * Warning: this does not support query parameters override, nor makes the assignment and split test info available to
+ * the frontend through locals. Wherever possible, `getAssignment` should be used instead.
  *
  * @param userId the user ID
  * @param splitTestName the unique name of the split test
@@ -105,6 +107,9 @@ async function getAssignmentForUser(
 /**
  * Get the assignment of a user to a split test by their pre-fetched mongo doc.
  *
+ * Warning: this does not support query parameters override, nor makes the assignment and split test info available to
+ * the frontend through locals. Wherever possible, `getAssignment` should be used instead.
+ *
  * @param user the user
  * @param splitTestName the unique name of the split test
  * @param options {Object<sync: boolean>} - for test purposes only, to force the synchronous update of the user's profile
@@ -373,6 +378,17 @@ async function _getUser(id) {
   })
 }
 
+async function _loadSplitTestInfoInLocals(locals, splitTestName) {
+  const splitTest = await SplitTestCache.get(splitTestName)
+  if (splitTest) {
+    const phase = splitTest.getCurrentVersion().phase
+    LocalsHelper.setSplitTestInfo(locals, splitTestName, {
+      phase,
+      badgeInfo: splitTest.toObject().badgeInfo?.[phase],
+    })
+  }
+}
+
 module.exports = {
   getAssignment: callbackify(getAssignment),
   getAssignmentForMongoUser: callbackify(getAssignmentForMongoUser),

+ 17 - 1
services/web/app/src/Features/SplitTests/SplitTestManager.js

@@ -34,7 +34,12 @@ async function getSplitTest(query) {
   }
 }
 
-async function createSplitTest(name, configuration, info = {}) {
+async function createSplitTest({
+  name,
+  configuration,
+  badgeInfo = {},
+  info = {},
+}) {
   const stripedVariants = []
   let stripeStart = 0
   _checkNewVariantsConfiguration([], configuration.variants)
@@ -61,6 +66,7 @@ async function createSplitTest(name, configuration, info = {}) {
     ticketUrl: info.ticketUrl,
     reportsUrls: info.reportsUrls,
     winningVariant: info.winningVariant,
+    badgeInfo,
     versions: [
       {
         versionNumber: 1,
@@ -118,6 +124,15 @@ async function updateSplitTestInfo(name, info) {
   return _saveSplitTest(splitTest)
 }
 
+async function updateSplitTestBadgeInfo(name, badgeInfo) {
+  const splitTest = await getSplitTest({ name })
+  if (!splitTest) {
+    throw new OError(`Cannot update split test '${name}': not found`)
+  }
+  splitTest.badgeInfo = badgeInfo
+  return _saveSplitTest(splitTest)
+}
+
 async function switchToNextPhase(name) {
   const splitTest = await getSplitTest({ name })
   if (!splitTest) {
@@ -294,6 +309,7 @@ module.exports = {
   createSplitTest,
   updateSplitTestConfig,
   updateSplitTestInfo,
+  updateSplitTestBadgeInfo,
   switchToNextPhase,
   revertToPreviousVersion,
   archive,

+ 27 - 0
services/web/app/src/models/SplitTest.js

@@ -16,6 +16,29 @@ const RolloutPercentType = {
   required: true,
 }
 
+const BadgeSchema = new Schema(
+  {
+    tooltipText: {
+      type: String,
+      required: false,
+    },
+    url: {
+      type: String,
+      required: false,
+    },
+  },
+  { _id: false }
+)
+
+const BadgeInfoSchema = new Schema(
+  {
+    alpha: BadgeSchema,
+    beta: BadgeSchema,
+    release: BadgeSchema,
+  },
+  { _id: false }
+)
+
 const VariantSchema = new Schema(
   {
     name: {
@@ -118,6 +141,10 @@ const SplitTestSchema = new Schema({
     type: Boolean,
     required: false,
   },
+  badgeInfo: {
+    type: BadgeInfoSchema,
+    required: false,
+  },
 })
 
 SplitTestSchema.methods.getCurrentVersion = function () {

+ 1 - 0
services/web/app/views/layout-base.pug

@@ -47,6 +47,7 @@ html(
 			//- Expose some settings globally to the frontend
 			meta(name="ol-ExposedSettings" data-type="json" content=ExposedSettings)
 			meta(name="ol-splitTestVariants" data-type="json" content=splitTestVariants || {})
+			meta(name="ol-splitTestInfo" data-type="json" content=splitTestInfo || {})
 
 			if (typeof(settings.algolia) != "undefined")
 				meta(name="ol-algolia" data-type="json" content={

+ 4 - 0
services/web/frontend/js/shared/components/beta-badge.tsx

@@ -24,6 +24,10 @@ function BetaBadge({
     case 'release':
       badgeClass = 'info-badge'
       break
+    case 'alpha':
+      badgeClass = 'alpha-badge'
+      break
+    case 'beta':
     default:
       badgeClass = 'beta-badge'
   }

+ 53 - 0
services/web/frontend/js/shared/components/split-test-badge.tsx

@@ -0,0 +1,53 @@
+import { useSplitTestContext } from '../context/split-test-context'
+import BetaBadge from './beta-badge'
+import { OverlayTriggerProps } from 'react-bootstrap'
+
+type TooltipProps = {
+  id?: string
+  placement?: OverlayTriggerProps['placement']
+  className?: string
+}
+
+type SplitTestBadgeProps = {
+  splitTestName: string
+  displayOnVariants: string[]
+  tooltip?: TooltipProps
+}
+
+export default function SplitTestBadge({
+  splitTestName,
+  displayOnVariants,
+  tooltip = {},
+}: SplitTestBadgeProps) {
+  const { splitTestVariants, splitTestInfo } = useSplitTestContext()
+
+  const testInfo = splitTestInfo[splitTestName]
+  if (!testInfo) {
+    return null
+  }
+
+  const variant = splitTestVariants[splitTestName]
+  if (!variant || !displayOnVariants.includes(variant)) {
+    return null
+  }
+
+  return (
+    <BetaBadge
+      tooltip={{
+        id: tooltip.id || `${splitTestName}-badge-tooltip`,
+        className: `split-test-badge-tooltip ${tooltip.className}`,
+        text: testInfo.badgeInfo?.tooltipText || (
+          <>
+            We are testing this new feature.
+            <br />
+            Click to give feedback
+          </>
+        ),
+      }}
+      phase={testInfo.phase}
+      url={
+        testInfo.badgeInfo?.url?.length ? testInfo.badgeInfo?.url : undefined
+      }
+    />
+  )
+}

+ 2 - 0
services/web/frontend/js/shared/context/split-test-context.js

@@ -7,6 +7,7 @@ export const SplitTestContext = createContext()
 SplitTestContext.Provider.propTypes = {
   value: PropTypes.shape({
     splitTestVariants: PropTypes.object.isRequired,
+    splitTestInfo: PropTypes.object.isRequired,
   }),
 }
 
@@ -14,6 +15,7 @@ export function SplitTestProvider({ children }) {
   const value = useMemo(
     () => ({
       splitTestVariants: getMeta('ol-splitTestVariants') || {},
+      splitTestInfo: getMeta('ol-splitTestInfo') || {},
     }),
     []
   )

+ 137 - 0
services/web/frontend/stories/split-test-badge.stories.js

@@ -0,0 +1,137 @@
+import SplitTestBadge from '../js/shared/components/split-test-badge'
+import { ScopeDecorator } from './decorators/scope'
+import { SplitTestContext } from '../js/shared/context/split-test-context'
+
+const splitTestContextValue = {
+  splitTestVariants: {
+    'storybook-test': 'active',
+  },
+  splitTestInfo: {
+    'storybook-test': {
+      phase: 'alpha',
+      badgeInfo: {
+        url: '/alpha/participate',
+        tooltipText: 'This is an alpha feature',
+      },
+    },
+  },
+}
+
+export const Alpha = args => {
+  splitTestContextValue.splitTestVariants = {
+    'storybook-test': 'active',
+  }
+  splitTestContextValue.splitTestInfo = {
+    'storybook-test': {
+      phase: 'alpha',
+      badgeInfo: {
+        url: '/alpha/participate',
+        tooltipText: 'This is an alpha feature',
+      },
+    },
+  }
+
+  return <SplitTestBadge {...args} />
+}
+
+export const AlphaNotDisplayed = args => {
+  splitTestContextValue.splitTestVariants = {
+    'storybook-test': 'default',
+  }
+  splitTestContextValue.splitTestInfo = {
+    'storybook-test': {
+      phase: 'alpha',
+      badgeInfo: {
+        url: '/alpha/participate',
+        tooltipText: 'This is an alpha feature',
+      },
+    },
+  }
+
+  return <SplitTestBadge {...args} />
+}
+
+export const Beta = args => {
+  splitTestContextValue.splitTestVariants = {
+    'storybook-test': 'active',
+  }
+  splitTestContextValue.splitTestInfo = {
+    'storybook-test': {
+      phase: 'beta',
+      badgeInfo: {
+        url: '/beta/participate',
+        tooltipText: 'This is a beta feature',
+      },
+    },
+  }
+
+  return <SplitTestBadge {...args} />
+}
+
+export const BetaNotDisplayed = args => {
+  splitTestContextValue.splitTestVariants = {
+    'storybook-test': 'default',
+  }
+  splitTestContextValue.splitTestInfo = {
+    'storybook-test': {
+      phase: 'beta',
+      badgeInfo: {
+        url: '/beta/participate',
+        tooltipText: 'This is a beta feature',
+      },
+    },
+  }
+
+  return <SplitTestBadge {...args} />
+}
+
+export const Release = args => {
+  splitTestContextValue.splitTestVariants = {
+    'storybook-test': 'active',
+  }
+  splitTestContextValue.splitTestInfo = {
+    'storybook-test': {
+      phase: 'release',
+      badgeInfo: {
+        url: '/feedback/form',
+        tooltipText: 'This is a new feature',
+      },
+    },
+  }
+
+  return <SplitTestBadge {...args} />
+}
+
+export const ReleaseNotDisplayed = args => {
+  splitTestContextValue.splitTestVariants = {
+    'storybook-test': 'default',
+  }
+  splitTestContextValue.splitTestInfo = {
+    'storybook-test': {
+      phase: 'release',
+      badgeInfo: {
+        url: '/feedback/form',
+        tooltipText: 'This is a new feature',
+      },
+    },
+  }
+
+  return <SplitTestBadge {...args} />
+}
+
+export default {
+  title: 'Shared / Components / Split Test Badge',
+  component: SplitTestBadge,
+  args: {
+    splitTestName: 'storybook-test',
+    displayOnVariants: ['active'],
+  },
+  decorators: [
+    (Story, context) => (
+      <SplitTestContext.Provider value={splitTestContextValue}>
+        <Story />
+      </SplitTestContext.Provider>
+    ),
+    ScopeDecorator,
+  ],
+}

+ 15 - 1
services/web/frontend/stylesheets/components/beta-badges.less

@@ -1,5 +1,6 @@
 .info-badge,
-.beta-badge {
+.beta-badge,
+.alpha-badge {
   display: inline-block;
   width: @line-height-computed * 0.75;
   height: @line-height-computed * 0.75;
@@ -46,6 +47,19 @@
   }
 }
 
+.alpha-badge {
+  background-color: @ol-green;
+  border-radius: @border-radius-base;
+
+  &::before {
+    content: 'α';
+  }
+}
+
+.split-test-badge-tooltip .tooltip-inner {
+  white-space: pre-wrap;
+}
+
 .tooltip-wide .tooltip-inner {
   min-width: 275px;
 }

+ 13 - 10
services/web/test/acceptance/src/PrimaryEmailCheckTests.js

@@ -12,16 +12,19 @@ describe('PrimaryEmailCheck', function () {
   // Create the primary-email-check split test because this is now required for the query string override to work. See
   // https://github.com/overleaf/internal/pull/7545#discussion_r848575736
   before(async function () {
-    await SplitTestManager.createSplitTest('primary-email-check', {
-      active: true,
-      analyticsEnabled: true,
-      phase: 'release',
-      variants: [
-        {
-          name: 'active',
-          rolloutPercent: 0,
-        },
-      ],
+    await SplitTestManager.createSplitTest({
+      name: 'primary-email-check',
+      configuration: {
+        active: true,
+        analyticsEnabled: true,
+        phase: 'release',
+        variants: [
+          {
+            name: 'active',
+            rolloutPercent: 0,
+          },
+        ],
+      },
     })
   })
 

+ 228 - 0
services/web/test/frontend/components/shared/split-test-badge.spec.tsx

@@ -0,0 +1,228 @@
+import SplitTestBadge from '../../../../frontend/js/shared/components/split-test-badge'
+import { EditorProviders } from '../../helpers/editor-providers'
+
+describe('split test badge', function () {
+  beforeEach(function () {
+    window.metaAttributesCache = new Map()
+  })
+
+  it('renders an alpha badge with the url and tooltip text', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'active',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {
+        'cypress-test': {
+          phase: 'alpha',
+          badgeInfo: {
+            url: '/alpha/participate',
+            tooltipText: 'This is an alpha feature',
+          },
+        },
+      })
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('a.badge.alpha-badge[href="/alpha/participate"]').contains(
+      'This is an alpha feature'
+    )
+  })
+
+  it('does not render the alpha badge when user is not assigned to the variant', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'default',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {
+        'cypress-test': {
+          phase: 'alpha',
+          badgeInfo: {
+            url: '/alpha/participate',
+            tooltipText: 'This is an alpha feature',
+          },
+        },
+      })
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('.badge').should('not.exist')
+  })
+
+  it('renders a beta badge with the url and tooltip text', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'active',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {
+        'cypress-test': {
+          phase: 'beta',
+          badgeInfo: {
+            url: '/beta/participate',
+            tooltipText: 'This is a beta feature',
+          },
+        },
+      })
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('a.badge.beta-badge[href="/beta/participate"]').contains(
+      'This is a beta feature'
+    )
+  })
+
+  it('does not render the beta badge when user is not assigned to the variant', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'default',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {
+        'cypress-test': {
+          phase: 'beta',
+          badgeInfo: {
+            url: '/beta/participate',
+            tooltipText: 'This is a beta feature',
+          },
+        },
+      })
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('.badge').should('not.exist')
+  })
+
+  it('renders an info badge with the url and tooltip text', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'active',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {
+        'cypress-test': {
+          phase: 'release',
+          badgeInfo: {
+            url: '/feedback/form',
+            tooltipText: 'This is a new feature',
+          },
+        },
+      })
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('a.badge.info-badge[href="/feedback/form"]').contains(
+      'This is a new feature'
+    )
+  })
+
+  it('does not render the info badge when user is not assigned to the variant', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'default',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {
+        'cypress-test': {
+          phase: 'release',
+          badgeInfo: {
+            url: '/feedback/form',
+            tooltipText: 'This is a new feature',
+          },
+        },
+      })
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('.badge').should('not.exist')
+  })
+
+  it('does not render the badge when no split test info is available', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'active',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {})
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('.badge').should('not.exist')
+  })
+
+  it('default badge url and text are used when not provided', function () {
+    cy.window().then(win => {
+      win.metaAttributesCache.set('ol-splitTestVariants', {
+        'cypress-test': 'active',
+      })
+      win.metaAttributesCache.set('ol-splitTestInfo', {
+        'cypress-test': {
+          phase: 'release',
+        },
+      })
+    })
+
+    cy.mount(
+      <EditorProviders>
+        <SplitTestBadge
+          splitTestName="cypress-test"
+          displayOnVariants={['active']}
+        />
+      </EditorProviders>
+    )
+
+    cy.get('a.badge.info-badge[href="/beta/participate"]')
+      .contains('We are testing this new feature.')
+      .contains('Click to give feedback')
+  })
+})