From a368a02b0fb29fe70e24d401e08cd81b4f52c9e5 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Thu, 11 Jan 2024 14:28:16 +0200 Subject: [PATCH 1/4] fix: Move 'User inserted workflow template' to NodeView This way the event gets fired in all scenarios and we don't have to remember to add it everywhere. --- .../templates/__tests__/templateActions.test.ts | 15 +++------------ .../src/utils/templates/templateActions.ts | 6 +----- packages/editor-ui/src/views/NodeView.vue | 12 ++++++++++++ .../setupTemplate.store.ts | 10 ---------- 4 files changed, 16 insertions(+), 27 deletions(-) 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 f06e4c7e31c22..e202c8fd85492 100644 --- a/packages/editor-ui/src/views/NodeView.vue +++ b/packages/editor-ui/src/views/NodeView.vue @@ -1305,6 +1305,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/setupTemplate.store.ts b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/setupTemplate.store.ts index 898fd81402bf4..b0b1715792fdb 100644 --- a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/setupTemplate.store.ts +++ b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/setupTemplate.store.ts @@ -153,16 +153,6 @@ export const useSetupTemplateStore = defineStore('setupTemplate', () => { wf_template_repo_session_id: templatesStore.currentSessionId, }); - telemetry.track( - 'User inserted workflow template', - { - source: 'workflow', - template_id: templateId.value, - wf_template_repo_session_id: templatesStore.currentSessionId, - }, - { withPostHog: true }, - ); - telemetry.track('User closed cred setup', { completed: false, creds_filled: 0, From e25e18a62106bb30194e90b7028e217ca6674249 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Thu, 11 Jan 2024 14:28:47 +0200 Subject: [PATCH 2/4] feat: Redirect users without feature flag from template cred setup to NodeView --- .../SetupWorkflowFromTemplateView.vue | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue index b8cc56f905a49..25273b197e001 100644 --- a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue +++ b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue @@ -7,19 +7,28 @@ import N8nLink from 'n8n-design-system/components/N8nLink'; import AppsRequiringCredsNotice from './AppsRequiringCredsNotice.vue'; import SetupTemplateFormStep from './SetupTemplateFormStep.vue'; import TemplatesView from '../TemplatesView.vue'; -import { VIEWS } from '@/constants'; +import { TEMPLATE_CREDENTIAL_SETUP_EXPERIMENT, VIEWS } from '@/constants'; import { useI18n } from '@/composables/useI18n'; -import { useTelemetry } from '@/composables/useTelemetry'; +import { usePostHog } from '@/stores/posthog.store'; // Store const setupTemplateStore = useSetupTemplateStore(); const i18n = useI18n(); -const telemetry = useTelemetry(); +const posthogStore = usePostHog(); // Router const route = useRoute(); const router = useRouter(); +const currentTemplateId = Array.isArray(route.params.id) ? route.params.id[0] : route.params.id; + +if (!posthogStore.isFeatureEnabled(TEMPLATE_CREDENTIAL_SETUP_EXPERIMENT)) { + void router.replace({ + name: VIEWS.TEMPLATE_IMPORT, + params: { id: currentTemplateId }, + }); +} + //#region Computed const templateId = computed(() => From 982ad81f4370581321b0ccb6dd224d0c7a5fbc5b Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Fri, 12 Jan 2024 10:58:56 +0200 Subject: [PATCH 3/4] test: Fix template cred setup e2e tests --- 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 +++++++++-- .../SetupWorkflowFromTemplateView.vue | 20 +++++++++---------- 5 files changed, 38 insertions(+), 25 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/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue index 25273b197e001..1869070eb4b93 100644 --- a/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue +++ b/packages/editor-ui/src/views/SetupWorkflowFromTemplateView/SetupWorkflowFromTemplateView.vue @@ -1,5 +1,5 @@