Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Redirect users without feature flag from template cred setup (no-changelog) #8302

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions cypress/composables/featureFlags.ts
Original file line number Diff line number Diff line change
@@ -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);
}
});
};
9 changes: 1 addition & 8 deletions cypress/e2e/34-template-credentials-setup.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);

Expand Down
10 changes: 5 additions & 5 deletions cypress/pages/template-credential-setup.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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');
};

/**
Expand Down
12 changes: 10 additions & 2 deletions cypress/pages/template-workflow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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,
Expand Down
22 changes: 6 additions & 16 deletions packages/editor-ui/src/stores/posthog.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand All @@ -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) {
Expand All @@ -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);
}
Expand All @@ -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) {}
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe('templateActions', () => {
templateId,
templatesStore,
router,
source: 'workflow',
});
});

Expand All @@ -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', () => {
Expand All @@ -111,6 +100,7 @@ describe('templateActions', () => {
templateId,
templatesStore,
router,
source: 'workflow',
});
});

Expand Down Expand Up @@ -144,6 +134,7 @@ describe('templateActions', () => {
templateId,
templatesStore,
router,
source: 'workflow',
});
});

Expand Down
6 changes: 1 addition & 5 deletions packages/editor-ui/src/utils/templates/templateActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand All @@ -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) {
Expand Down
12 changes: 12 additions & 0 deletions packages/editor-ui/src/views/NodeView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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 } });

Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
<script setup lang="ts">
import { computed, onMounted, watch } from 'vue';
import { computed, onBeforeMount, onMounted, watch } from 'vue';
import { useRoute, useRouter } from 'vue-router';
import { useSetupTemplateStore } from './setupTemplate.store';
import N8nHeading from 'n8n-design-system/components/N8nHeading';
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();
Expand Down Expand Up @@ -79,6 +79,15 @@ const skipIfTemplateHasNoCreds = async () => {

setupTemplateStore.setTemplateId(templateId.value);

onBeforeMount(async () => {
if (!posthogStore.isFeatureEnabled(TEMPLATE_CREDENTIAL_SETUP_EXPERIMENT)) {
void router.replace({
name: VIEWS.TEMPLATE_IMPORT,
params: { id: templateId.value },
});
}
});

onMounted(async () => {
await setupTemplateStore.init();
await skipIfTemplateHasNoCreds();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading