From 135553bd6b6f4e9f18cf9c34b57699d13ba6f9ee Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Fri, 12 Jan 2024 12:10:39 +0200 Subject: [PATCH] feat: Redirect users without feature flag from template cred setup (no-changelog) (#8302) --- cypress/composables/featureFlags.ts | 12 ++++++++++ .../e2e/34-template-credentials-setup.cy.ts | 9 +------- cypress/pages/template-credential-setup.ts | 10 ++++----- cypress/pages/template-workflow.ts | 12 ++++++++-- .../editor-ui/src/stores/posthog.store.ts | 22 +++++-------------- .../__tests__/templateActions.test.ts | 15 +++---------- .../src/utils/templates/templateActions.ts | 6 +---- packages/editor-ui/src/views/NodeView.vue | 12 ++++++++++ .../SetupWorkflowFromTemplateView.vue | 17 ++++++++++---- .../setupTemplate.store.ts | 10 --------- 10 files changed, 63 insertions(+), 62 deletions(-) create mode 100644 cypress/composables/featureFlags.ts diff --git a/cypress/composables/featureFlags.ts b/cypress/composables/featureFlags.ts new file mode 100644 index 0000000000000..ea8e95064e4d1 --- /dev/null +++ b/cypress/composables/featureFlags.ts @@ -0,0 +1,12 @@ +export const overrideFeatureFlag = (name: string, value: boolean | string) => { + cy.window().then((win) => { + // If feature flags hasn't been initialized yet, we store the override + // in local storage and it gets loaded when the feature flags are + // initialized. + win.localStorage.setItem('N8N_EXPERIMENT_OVERRIDES', JSON.stringify({ [name]: value })); + + if (win.featureFlags) { + win.featureFlags.override(name, value); + } + }); +}; diff --git a/cypress/e2e/34-template-credentials-setup.cy.ts b/cypress/e2e/34-template-credentials-setup.cy.ts index f65d93e3daf7a..863cc61c18310 100644 --- a/cypress/e2e/34-template-credentials-setup.cy.ts +++ b/cypress/e2e/34-template-credentials-setup.cy.ts @@ -38,6 +38,7 @@ describe('Template credentials setup', () => { it('can be opened from template workflow page', () => { templateWorkflowPage.actions.visit(testTemplate.id); + templateCredentialsSetupPage.enableTemplateCredentialSetupFeatureFlag(); templateWorkflowPage.getters.useTemplateButton().should('be.visible'); templateCredentialsSetupPage.enableTemplateCredentialSetupFeatureFlag(); templateWorkflowPage.actions.clickUseThisWorkflowButton(); @@ -57,14 +58,6 @@ describe('Template credentials setup', () => { .should('be.visible'); }); - it('can be opened with a direct url', () => { - templateCredentialsSetupPage.visitTemplateCredentialSetupPage(testTemplate.id); - - templateCredentialsSetupPage.getters - .title(`Set up 'Promote new Shopify products on Twitter and Telegram' template`) - .should('be.visible'); - }); - it('has all the elements on page', () => { templateCredentialsSetupPage.visitTemplateCredentialSetupPage(testTemplate.id); diff --git a/cypress/pages/template-credential-setup.ts b/cypress/pages/template-credential-setup.ts index 5453c1025d880..d673261fdf77b 100644 --- a/cypress/pages/template-credential-setup.ts +++ b/cypress/pages/template-credential-setup.ts @@ -1,5 +1,6 @@ import { CredentialsModal, MessageBox } from './modals'; import * as formStep from '../composables/setup-template-form-step'; +import { overrideFeatureFlag } from '../composables/featureFlags'; export type TemplateTestData = { id: number; @@ -28,15 +29,14 @@ export const getters = { }; export const enableTemplateCredentialSetupFeatureFlag = () => { - cy.window().then((win) => { - win.featureFlags.override('017_template_credential_setup_v2', true); - }); + overrideFeatureFlag('017_template_credential_setup_v2', true); }; export const visitTemplateCredentialSetupPage = (templateId: number) => { - cy.visit(`/templates/${templateId}/setup`); - formStep.getFormStep().eq(0).should('be.visible'); + cy.visit(`templates/${templateId}/setup`); enableTemplateCredentialSetupFeatureFlag(); + + formStep.getFormStep().eq(0).should('be.visible'); }; /** diff --git a/cypress/pages/template-workflow.ts b/cypress/pages/template-workflow.ts index cc40c00975ad9..ff54e1e3d4d60 100644 --- a/cypress/pages/template-workflow.ts +++ b/cypress/pages/template-workflow.ts @@ -5,7 +5,7 @@ export class TemplateWorkflowPage extends BasePage { getters = { useTemplateButton: () => cy.get('[data-test-id="use-template-button"]'), - description: () => cy.get('[data-test-id="template-description"]') + description: () => cy.get('[data-test-id="template-description"]'), }; actions = { @@ -17,7 +17,15 @@ export class TemplateWorkflowPage extends BasePage { this.getters.useTemplateButton().click(); }, - openTemplate: (template: {workflow: {id: number, name: string, description: string, user: {username: string}, image: {id: number, url: string}[] }}) => { + openTemplate: (template: { + workflow: { + id: number; + name: string; + description: string; + user: { username: string }; + image: { id: number; url: string }[]; + }; + }) => { cy.intercept('GET', `https://api.n8n.io/api/templates/workflows/${template.workflow.id}`, { statusCode: 200, body: template, diff --git a/packages/editor-ui/src/stores/posthog.store.ts b/packages/editor-ui/src/stores/posthog.store.ts index ea58fa2417d86..8596c142c163d 100644 --- a/packages/editor-ui/src/stores/posthog.store.ts +++ b/packages/editor-ui/src/stores/posthog.store.ts @@ -35,7 +35,7 @@ export const usePostHog = defineStore('posthog', () => { }; const getVariant = (experiment: keyof FeatureFlags): FeatureFlags[keyof FeatureFlags] => { - return featureFlags.value?.[experiment]; + return overrides.value[experiment] ?? featureFlags.value?.[experiment]; }; const isVariantEnabled = (experiment: string, variant: string) => { @@ -46,7 +46,7 @@ export const usePostHog = defineStore('posthog', () => { * Checks if the given feature flag is enabled. Should only be used for boolean flags */ const isFeatureEnabled = (experiment: keyof FeatureFlags) => { - return featureFlags.value?.[experiment] === true; + return getVariant(experiment) === true; }; if (!window.featureFlags) { @@ -55,7 +55,10 @@ export const usePostHog = defineStore('posthog', () => { if (cachedOverrides) { try { console.log('Overriding feature flags', cachedOverrides); - overrides.value = JSON.parse(cachedOverrides); + const parsedOverrides = JSON.parse(cachedOverrides); + if (typeof parsedOverrides === 'object') { + overrides.value = JSON.parse(cachedOverrides); + } } catch (e) { console.log('Could not override experiment', e); } @@ -65,10 +68,6 @@ export const usePostHog = defineStore('posthog', () => { // since features are evaluated serverside, regular posthog mechanism to override clientside does not work override: (name: string, value: string | boolean) => { overrides.value[name] = value; - featureFlags.value = { - ...featureFlags.value, - [name]: value, - }; try { useStorage(LOCAL_STORAGE_EXPERIMENT_OVERRIDES).value = JSON.stringify(overrides.value); } catch (e) {} @@ -93,13 +92,6 @@ export const usePostHog = defineStore('posthog', () => { window.posthog?.identify?.(id, traits); }; - const addExperimentOverrides = () => { - featureFlags.value = { - ...featureFlags.value, - ...overrides.value, - }; - }; - const trackExperiment = (featFlags: FeatureFlags, name: string) => { const variant = featFlags[name]; if (!variant || trackedDemoExp.value[name] === variant) { @@ -160,13 +152,11 @@ export const usePostHog = defineStore('posthog', () => { }; // does not need to be debounced really, but tracking does not fire without delay on page load - addExperimentOverrides(); trackExperimentsDebounced(featureFlags.value); } else { // depend on client side evaluation if serverside evaluation fails window.posthog?.onFeatureFlags?.((keys: string[], map: FeatureFlags) => { featureFlags.value = map; - addExperimentOverrides(); // must be debounced because it is called multiple times by posthog trackExperimentsDebounced(featureFlags.value); diff --git a/packages/editor-ui/src/utils/templates/__tests__/templateActions.test.ts b/packages/editor-ui/src/utils/templates/__tests__/templateActions.test.ts index 1f7c537c9a1ea..6c48ca8683798 100644 --- a/packages/editor-ui/src/utils/templates/__tests__/templateActions.test.ts +++ b/packages/editor-ui/src/utils/templates/__tests__/templateActions.test.ts @@ -67,6 +67,7 @@ describe('templateActions', () => { templateId, templatesStore, router, + source: 'workflow', }); }); @@ -76,18 +77,6 @@ describe('templateActions', () => { params: { id: templateId }, }); }); - - it("should track 'User inserted workflow template'", async () => { - expect(telemetry.track).toHaveBeenCalledWith( - 'User inserted workflow template', - { - source: 'workflow', - template_id: templateId, - wf_template_repo_session_id: '', - }, - { withPostHog: true }, - ); - }); }); describe('When feature flag is enabled and template has nodes requiring credentials', () => { @@ -111,6 +100,7 @@ describe('templateActions', () => { templateId, templatesStore, router, + source: 'workflow', }); }); @@ -144,6 +134,7 @@ describe('templateActions', () => { templateId, templatesStore, router, + source: 'workflow', }); }); diff --git a/packages/editor-ui/src/utils/templates/templateActions.ts b/packages/editor-ui/src/utils/templates/templateActions.ts index e0f715d32d2b3..96c8e7b247c5c 100644 --- a/packages/editor-ui/src/utils/templates/templateActions.ts +++ b/packages/editor-ui/src/utils/templates/templateActions.ts @@ -98,9 +98,8 @@ async function openTemplateWorkflowOnNodeView(opts: { templatesStore: TemplatesStore; router: Router; inNewBrowserTab?: boolean; - telemetry: Telemetry; }) { - const { externalHooks, templateId, templatesStore, telemetry, inNewBrowserTab, router } = opts; + const { externalHooks, templateId, templatesStore, inNewBrowserTab, router } = opts; const routeLocation: RouteLocationRaw = { name: VIEWS.TEMPLATE_IMPORT, params: { id: templateId }, @@ -111,9 +110,6 @@ async function openTemplateWorkflowOnNodeView(opts: { wf_template_repo_session_id: templatesStore.currentSessionId, }; - telemetry.track('User inserted workflow template', telemetryPayload, { - withPostHog: true, - }); await externalHooks.run('templatesWorkflowView.openWorkflow', telemetryPayload); if (inNewBrowserTab) { diff --git a/packages/editor-ui/src/views/NodeView.vue b/packages/editor-ui/src/views/NodeView.vue index 0012a5abb6b0a..e611064c152fa 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -1308,6 +1308,18 @@ export default defineComponent({ return; } + this.$telemetry.track( + 'User inserted workflow template', + { + source: 'workflow', + template_id: templateId, + wf_template_repo_session_id: this.templatesStore.previousSessionId, + }, + { + withPostHog: true, + }, + ); + this.blankRedirect = true; await this.$router.replace({ name: VIEWS.NEW_WORKFLOW, query: { templateId } }); diff --git a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue index b8cc56f905a49..1869070eb4b93 100644 --- a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue +++ b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue @@ -1,5 +1,5 @@