fix(onboarding): don't create or change flags in a project already in use

frontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.scss

@@ -7,4 +7,13 @@
   // Top padding gives the theme-toggle row room to breathe from the very edge;
   // sides + bottom keep the reading column off the edges.
   padding: 24px 24px 40px;
+
+  // The terminal states are one short block, so centre them rather than
+  // leaving them stranded at the top of an otherwise empty page.
+  &--message {
+    display: flex;
+    flex-direction: column;
+    justify-content: center;
+    min-height: 70vh;
+  }
 }

frontend/web/components/pages/onboarding/OnboardingFlow/OnboardingFlow.tsx

@@ -1,6 +1,8 @@
 import React, { FC, useState } from 'react'
 import { useHistory } from 'react-router-dom'
 import Button from 'components/base/forms/Button'
+import EmptyState from 'components/EmptyState'
+import Link from 'components/base/link'
 import Icon from 'components/icons/Icon'
 import OnboardingHeader from 'components/pages/onboarding/OnboardingHeader'
 import ThemeToggle from 'components/ThemeToggle'
@@ -178,7 +180,7 @@
 
   if (status === 'creating') {
     return (
-      <div className='onboarding-flow mx-auto text-center'>
+      <div className='onboarding-flow onboarding-flow--message mx-auto text-center'>
         <Loader />
       </div>
     )
@@ -187,12 +189,15 @@
   // Bootstrap failed (e.g. a plan org cap). Recoverable; a reload re-runs it.
   if (status === 'error') {
     return (
-      <div className='onboarding-flow mx-auto text-center'>
-        <h2 className='mb-2'>We couldn’t set up your workspace</h2>
-        <p className='text-muted mb-3'>
-          Something went wrong creating your starter project. Please try again.
-        </p>
-        <Button onClick={() => window.location.reload()}>Try again</Button>
+      <div className='onboarding-flow onboarding-flow--message mx-auto'>
+        <EmptyState
+          icon='warning'
+          title='We couldn’t set up your workspace'
+          description='Something went wrong creating your starter project. Please try again.'
+          action={
+            <Button onClick={() => window.location.reload()}>Try again</Button>
+          }
+        />
       </div>
     )
   }

frontend/web/components/pages/onboarding/bootstrap/__tests__/demoFlag.test.ts

@@ -0,0 +1,66 @@
+import { ProjectFlag, Tag } from 'common/types/responses'
+import {
+  DEMO_FLAG_DESCRIPTION,
+  DEMO_FLAG_NAME,
+  ONBOARDING_TAG,
+  findDemoFlag,
+  findOnboardingTag,
+} from 'components/pages/onboarding/bootstrap/demoFlag'
+
+const flag = (
+  name: string,
+  { description = '', tags = [] }: { description?: string; tags?: number[] } = {},
+): ProjectFlag => ({ description, id: name.length, name, tags } as ProjectFlag)
+
+const onboardingTag = { id: 7, ...ONBOARDING_TAG } as Tag
+
+describe('findOnboardingTag', () => {
+  it('finds the tag a previous run created', () => {
+    expect(
+      findOnboardingTag([{ id: 3, label: 'Backend' } as Tag, onboardingTag]),
+    ).toBe(onboardingTag)
+  })
+
+  it('ignores a tag the customer labelled Onboarding themselves', () => {
+    const theirs = {
+      description: 'Flags behind our signup flow',
+      id: 9,
+      label: 'Onboarding',
+    } as Tag
+    expect(findOnboardingTag([theirs])).toBeUndefined()
+  })
+})
+
+describe('findDemoFlag', () => {
+  it('finds a previous run by its tag, whatever it was renamed to', () => {
+    const renamed = flag('my_own_name', { tags: [onboardingTag.id] })
+    expect(
+      findDemoFlag([flag('checkout_v2'), renamed], onboardingTag),
+    ).toBe(renamed)
+  })
+
+  it('prefers the tag over anything else when both are present', () => {
+    const tagged = flag('renamed_by_hand', { tags: [onboardingTag.id] })
+    const named = flag(DEMO_FLAG_NAME)
+    expect(findDemoFlag([named, tagged], onboardingTag)).toBe(tagged)
+  })
+
+  it('falls back to our description when the tag is gone', () => {
+    // The rename carries the description over, so it outlives the name.
+    const renamed = flag('my_own_name', { description: DEMO_FLAG_DESCRIPTION })
+    expect(findDemoFlag([flag('checkout_v2'), renamed], undefined)).toBe(renamed)
+  })
+
+  it('falls back to the name for a flag seeded before we set a description', () => {
+    const legacy = flag(DEMO_FLAG_NAME)
+    expect(findDemoFlag([flag('checkout_v2'), legacy], undefined)).toBe(legacy)
+  })
+
+  it('finds nothing in a project that never ran the tour', () => {
+    expect(findDemoFlag([flag('checkout_v2')], onboardingTag)).toBeUndefined()
+  })
+
+  it('finds nothing in an empty project', () => {
+    expect(findDemoFlag([], onboardingTag)).toBeUndefined()
+  })
+})

frontend/web/components/pages/onboarding/hooks/bootstrapOnboarding.ts → frontend/web/components/pages/onboarding/bootstrap/bootstrapOnboarding.ts

@@ -16,24 +16,24 @@ import {
   ProjectSummary,
   Tag,
 } from 'common/types/responses'
-import { SmartDefaults } from './useSmartDefaults'
+import {
+  DEMO_FLAG_DESCRIPTION,
+  DEMO_FLAG_NAME,
+  ONBOARDING_TAG,
+  findDemoFlag,
+  findOnboardingTag,
+} from './demoFlag'
+import { SmartDefaults } from 'components/pages/onboarding/hooks/useSmartDefaults'
 import { createOrganisationViaAccountStore } from './createOrganisationViaAccountStore'
 import API from 'project/api'
 import Constants from 'common/constants'
 
 type Store = ReturnType<typeof getStore>
 
-const FLAG_NAME = 'show_demo_button'
 const DEFAULT_ORG_NAME = 'My organisation'
 const DEFAULT_PROJECT_NAME = 'My first project'
 const DEV_ENVIRONMENT_NAME = 'Development'
 const PROD_ENVIRONMENT_NAME = 'Production'
-const ONBOARDING_TAG = {
-  color: '#3cb371',
-  description: 'Created during onboarding',
-  label: 'Onboarding',
-}
-
 type ExistingOrg = { id: number; name: string }
 
 export type BootstrapInput = {
@@ -133,52 +133,71 @@ async function ensureEnvironments(
     .unwrap()
 }
 
-async function findOnboardingTag(
+async function fetchOnboardingTag(
   store: Store,
   projectId: number,
 ): Promise<Tag | undefined> {
   const tags = await store
     .dispatch(tagService.endpoints.getTags.initiate({ projectId }))
     .unwrap()
-  return tags?.find((t) => t.label === ONBOARDING_TAG.label)
+  return findOnboardingTag(tags ?? [])
 }
 
-async function ensureFlag(
+async function listFlags(
   store: Store,
   project: ProjectSummary,
-): Promise<ProjectFlag | undefined> {
+): Promise<ProjectFlag[]> {
   const flags = await store
     .dispatch(
-      projectFlagService.endpoints.getProjectFlags.initiate({
-        project: `${project.id}`,
-      }),
+      projectFlagService.endpoints.getProjectFlags.initiate(
+        { project: `${project.id}` },
+        { forceRefetch: true },
+      ),
     )
     .unwrap()
-  const onboardingTag = await findOnboardingTag(store, project.id)
-  const existing =
-    (onboardingTag &&
-      flags?.results?.find((f) => f.tags?.includes(onboardingTag.id))) ||
-    flags?.results?.find((f) => f.name === FLAG_NAME)
+  return flags?.results ?? []
+}
+
+async function ensureFlag(
+  store: Store,
+  project: ProjectSummary,
+): Promise<ProjectFlag> {
+  // Not fatal: without the tag we fall back to the flag's own description.
+  const onboardingTag = await fetchOnboardingTag(store, project.id).catch(
+    () => undefined,
+  )
+  const existing = findDemoFlag(await listFlags(store, project), onboardingTag)
   if (existing) {
     return existing
   }
-  const isFirstFeature = !flags?.results?.length
-  const created = await store
-    .dispatch(
-      projectFlagService.endpoints.createProjectFlag.initiate({
-        body: {
-          name: FLAG_NAME,
-          project: project.id,
-          type: 'STANDARD',
-        } as Req['createProjectFlag']['body'],
-        project_id: project.id,
-      }),
-    )
-    .unwrap()
-  if (isFirstFeature) {
+  try {
+    const created = await store
+      .dispatch(
+        projectFlagService.endpoints.createProjectFlag.initiate({
+          body: {
+            description: DEMO_FLAG_DESCRIPTION,
+            name: DEMO_FLAG_NAME,
+            project: project.id,
+            type: 'STANDARD',
+          } as Req['createProjectFlag']['body'],
+          project_id: project.id,
+        }),
+      )
+      .unwrap()
     API.trackEvent(Constants.events.CREATE_FIRST_FEATURE)
+    return created
+  } catch (e) {
+    // Names are unique per project, so a clash means our flag is already here
+    // and we failed to recognise it: someone edited its description, or removed
+    // the tag. Take it rather than dropping the user out of the tour.
+    const clash = (await listFlags(store, project)).find(
+      (f) => f.name === DEMO_FLAG_NAME,
+    )
+    if (!clash) {
+      throw e
+    }
+    return clash
   }
-  return created
 }
 
 async function ensureOnboardingTag(
@@ -188,7 +207,7 @@ async function ensureOnboardingTag(
 ): Promise<void> {
   try {
     const tag =
-      (await findOnboardingTag(store, project.id)) ??
+      (await fetchOnboardingTag(store, project.id)) ??
       (await store
         .dispatch(
           tagService.endpoints.createTag.initiate({
@@ -221,13 +240,11 @@ export async function bootstrapOnboarding(
   const project = await ensureProject(store, organisation.id, input.defaults)
   const environment = await ensureEnvironments(store, project)
   const flag = await ensureFlag(store, project)
-  if (flag) {
-    await ensureOnboardingTag(store, project, flag)
-  }
+  await ensureOnboardingTag(store, project, flag)
   AppActions.refreshOrganisation()
   return {
     environment,
-    featureName: flag?.name ?? FLAG_NAME,
+    featureName: flag.name,
     organisationId: organisation.id,
     organisationName: organisation.name,
     project,

frontend/web/components/pages/onboarding/bootstrap/demoFlag.ts

@@ -0,0 +1,34 @@
+import { ProjectFlag, Tag } from 'common/types/responses'
+
+export const DEMO_FLAG_NAME = 'show_demo_button'
+
+// Set on the flag we create, and carried over by the tour's rename, so it
+// identifies our flag after the name has changed. Also tells anyone looking at
+// their flag list why it is there.
+export const DEMO_FLAG_DESCRIPTION = 'Created during onboarding'
+
+export const ONBOARDING_TAG = {
+  color: '#3cb371',
+  description: DEMO_FLAG_DESCRIPTION,
+  label: 'Onboarding',
+}
+
+// Description too: a customer's own tag labelled Onboarding is not ours.
+export const findOnboardingTag = (tags: Tag[]): Tag | undefined =>
+  tags.find(
+    (t) =>
+      t.label === ONBOARDING_TAG.label &&
+      t.description === ONBOARDING_TAG.description,
+  )
+
+// Our flag, in descending order of how much the signal is worth. The tour
+// renames by delete and recreate, carrying the tags and description over, so
+// the name is the one thing that doesn't survive it: it only identifies flags
+// seeded before we set a description.
+export const findDemoFlag = (
+  flags: ProjectFlag[],
+  onboardingTag?: Tag,
+): ProjectFlag | undefined =>
+  (onboardingTag && flags.find((f) => f.tags?.includes(onboardingTag.id))) ||
+  flags.find((f) => f.description === DEMO_FLAG_DESCRIPTION) ||
+  flags.find((f) => f.name === DEMO_FLAG_NAME)

frontend/web/components/pages/onboarding/hooks/useEnsureOnboardingResources.ts

@@ -5,7 +5,7 @@ import useSelectedOrganisation from 'common/hooks/useSelectedOrganisation'
 import { useGetProfileQuery } from 'common/services/useProfile'
 import { Environment } from 'common/types/responses'
 import { useSmartDefaults } from './useSmartDefaults'
-import { bootstrapOnboarding } from './bootstrapOnboarding'
+import { bootstrapOnboarding } from 'components/pages/onboarding/bootstrap/bootstrapOnboarding'
 
 export type OnboardingResourcesStatus = 'creating' | 'ready' | 'error'