mirror of
https://github.com/n8n-io/n8n.git
synced 2024-11-09 22:24:05 -08:00
fix(editor): Fix cloud plan data loading on instance (#7841)
Moving cloud hooks and store initialization logic after users are authenticated. This will ensure user local account is available when their cloud plan data is being fetched. This PR also adds the following error handling improvements: - Added error handling to the same initialization logic - Fixed empty `catch` clauses inside the cloud store which caused it to silently fail and complicated debugging of this bug
This commit is contained in:
parent
90bb6ba417
commit
8b99384367
77
packages/editor-ui/src/__tests__/init.test.ts
Normal file
77
packages/editor-ui/src/__tests__/init.test.ts
Normal file
|
@ -0,0 +1,77 @@
|
||||||
|
import { useUsersStore } from '@/stores/users.store';
|
||||||
|
import { useCloudPlanStore } from '@/stores/cloudPlan.store';
|
||||||
|
import { useSourceControlStore } from '@/stores/sourceControl.store';
|
||||||
|
import { useNodeTypesStore } from '@/stores/nodeTypes.store';
|
||||||
|
import { useRootStore } from '@/stores/n8nRoot.store';
|
||||||
|
import { initializeAuthenticatedFeatures } from '@/init';
|
||||||
|
import type { SpyInstance } from 'vitest';
|
||||||
|
import { createTestingPinia } from '@pinia/testing';
|
||||||
|
import { setActivePinia } from 'pinia';
|
||||||
|
import { useSettingsStore } from '@/stores/settings.store';
|
||||||
|
|
||||||
|
vi.mock('@/stores/users.store', () => ({
|
||||||
|
useUsersStore: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('@/stores/n8nRoot.store', () => ({
|
||||||
|
useRootStore: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
describe('Init', () => {
|
||||||
|
describe('Authenticated Features', () => {
|
||||||
|
let settingsStore: ReturnType<typeof useSettingsStore>;
|
||||||
|
let cloudPlanStore: ReturnType<typeof useCloudPlanStore>;
|
||||||
|
let sourceControlStore: ReturnType<typeof useSourceControlStore>;
|
||||||
|
let nodeTypesStore: ReturnType<typeof useNodeTypesStore>;
|
||||||
|
let cloudStoreSpy: SpyInstance<[], Promise<void>>;
|
||||||
|
let templatesTestSpy: SpyInstance<[], Promise<void>>;
|
||||||
|
let sourceControlSpy: SpyInstance<[], Promise<void>>;
|
||||||
|
let nodeTranslationSpy: SpyInstance<[], Promise<void>>;
|
||||||
|
|
||||||
|
beforeAll(() => {
|
||||||
|
setActivePinia(createTestingPinia());
|
||||||
|
settingsStore = useSettingsStore();
|
||||||
|
cloudPlanStore = useCloudPlanStore();
|
||||||
|
sourceControlStore = useSourceControlStore();
|
||||||
|
nodeTypesStore = useNodeTypesStore();
|
||||||
|
vi.spyOn(settingsStore, 'isCloudDeployment', 'get').mockReturnValue(true);
|
||||||
|
vi.spyOn(settingsStore, 'isTemplatesEnabled', 'get').mockReturnValue(true);
|
||||||
|
vi.spyOn(sourceControlStore, 'isEnterpriseSourceControlEnabled', 'get').mockReturnValue(true);
|
||||||
|
vi.mocked(useRootStore).mockReturnValue({ defaultLocale: 'es' } as ReturnType<
|
||||||
|
typeof useRootStore
|
||||||
|
>);
|
||||||
|
vi.mock('@/hooks/register', () => ({
|
||||||
|
initializeCloudHooks: vi.fn(),
|
||||||
|
}));
|
||||||
|
cloudStoreSpy = vi.spyOn(cloudPlanStore, 'initialize');
|
||||||
|
templatesTestSpy = vi.spyOn(settingsStore, 'testTemplatesEndpoint');
|
||||||
|
sourceControlSpy = vi.spyOn(sourceControlStore, 'getPreferences');
|
||||||
|
nodeTranslationSpy = vi.spyOn(nodeTypesStore, 'getNodeTranslationHeaders');
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should not init authenticated features if user is not logged in', async () => {
|
||||||
|
vi.mocked(useUsersStore).mockReturnValue({ currentUser: null } as ReturnType<
|
||||||
|
typeof useUsersStore
|
||||||
|
>);
|
||||||
|
await initializeAuthenticatedFeatures();
|
||||||
|
expect(cloudStoreSpy).not.toHaveBeenCalled();
|
||||||
|
expect(templatesTestSpy).not.toHaveBeenCalled();
|
||||||
|
expect(sourceControlSpy).not.toHaveBeenCalled();
|
||||||
|
expect(nodeTranslationSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
it('should init authenticated features if user is not logged in', async () => {
|
||||||
|
vi.mocked(useUsersStore).mockReturnValue({ currentUser: { id: '123' } } as ReturnType<
|
||||||
|
typeof useUsersStore
|
||||||
|
>);
|
||||||
|
await initializeAuthenticatedFeatures();
|
||||||
|
expect(cloudStoreSpy).toHaveBeenCalled();
|
||||||
|
expect(templatesTestSpy).toHaveBeenCalled();
|
||||||
|
expect(sourceControlSpy).toHaveBeenCalled();
|
||||||
|
expect(nodeTranslationSpy).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
|
@ -6,7 +6,12 @@ export async function initializeCloudHooks() {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const { n8nCloudHooks } = await import('@/hooks/cloud');
|
try {
|
||||||
extendExternalHooks(n8nCloudHooks);
|
const { n8nCloudHooks } = await import('@/hooks/cloud');
|
||||||
cloudHooksInitialized = true;
|
extendExternalHooks(n8nCloudHooks);
|
||||||
|
} catch (error) {
|
||||||
|
throw new Error(`Failed to extend external hooks: ${error.message}`);
|
||||||
|
} finally {
|
||||||
|
cloudHooksInitialized = true;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -19,13 +19,17 @@ export async function initializeCore() {
|
||||||
}
|
}
|
||||||
|
|
||||||
const settingsStore = useSettingsStore();
|
const settingsStore = useSettingsStore();
|
||||||
const cloudPlanStore = useCloudPlanStore();
|
|
||||||
const usersStore = useUsersStore();
|
const usersStore = useUsersStore();
|
||||||
|
|
||||||
await settingsStore.initialize();
|
await settingsStore.initialize();
|
||||||
await usersStore.initialize();
|
await usersStore.initialize();
|
||||||
|
|
||||||
if (settingsStore.isCloudDeployment) {
|
if (settingsStore.isCloudDeployment) {
|
||||||
await Promise.all([cloudPlanStore.initialize(), initializeCloudHooks()]);
|
try {
|
||||||
|
await initializeCloudHooks();
|
||||||
|
} catch (e) {
|
||||||
|
console.error('Failed to initialize cloud hooks:', e);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
coreInitialized = true;
|
coreInitialized = true;
|
||||||
|
@ -48,6 +52,7 @@ export async function initializeAuthenticatedFeatures() {
|
||||||
const settingsStore = useSettingsStore();
|
const settingsStore = useSettingsStore();
|
||||||
const rootStore = useRootStore();
|
const rootStore = useRootStore();
|
||||||
const nodeTypesStore = useNodeTypesStore();
|
const nodeTypesStore = useNodeTypesStore();
|
||||||
|
const cloudPlanStore = useCloudPlanStore();
|
||||||
|
|
||||||
if (sourceControlStore.isEnterpriseSourceControlEnabled) {
|
if (sourceControlStore.isEnterpriseSourceControlEnabled) {
|
||||||
await sourceControlStore.getPreferences();
|
await sourceControlStore.getPreferences();
|
||||||
|
@ -63,5 +68,13 @@ export async function initializeAuthenticatedFeatures() {
|
||||||
await nodeTypesStore.getNodeTranslationHeaders();
|
await nodeTypesStore.getNodeTranslationHeaders();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (settingsStore.isCloudDeployment) {
|
||||||
|
try {
|
||||||
|
await cloudPlanStore.initialize();
|
||||||
|
} catch (e) {
|
||||||
|
console.error('Failed to initialize cloud plan store:', e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
authenticatedFeaturesInitialized = true;
|
authenticatedFeaturesInitialized = true;
|
||||||
}
|
}
|
||||||
|
|
|
@ -165,11 +165,15 @@ describe('UI store', () => {
|
||||||
const fetchUserCloudAccountSpy = vi
|
const fetchUserCloudAccountSpy = vi
|
||||||
.spyOn(cloudPlanApi, 'getCloudUserInfo')
|
.spyOn(cloudPlanApi, 'getCloudUserInfo')
|
||||||
.mockResolvedValue(getUserCloudInfo(true));
|
.mockResolvedValue(getUserCloudInfo(true));
|
||||||
|
const getCurrentUsageSpy = vi
|
||||||
|
.spyOn(cloudPlanApi, 'getCurrentUsage')
|
||||||
|
.mockResolvedValue({ executions: 1000, activeWorkflows: 100 });
|
||||||
setupOwnerAndCloudDeployment();
|
setupOwnerAndCloudDeployment();
|
||||||
await cloudPlanStore.checkForCloudPlanData();
|
await cloudPlanStore.checkForCloudPlanData();
|
||||||
await cloudPlanStore.fetchUserCloudAccount();
|
await cloudPlanStore.fetchUserCloudAccount();
|
||||||
expect(fetchCloudSpy).toHaveBeenCalled();
|
expect(fetchCloudSpy).toHaveBeenCalled();
|
||||||
expect(fetchUserCloudAccountSpy).toHaveBeenCalled();
|
expect(fetchUserCloudAccountSpy).toHaveBeenCalled();
|
||||||
|
expect(getCurrentUsageSpy).toHaveBeenCalled();
|
||||||
expect(uiStore.bannerStack).toContain('TRIAL');
|
expect(uiStore.bannerStack).toContain('TRIAL');
|
||||||
// There should be no email confirmation banner for trialing users
|
// There should be no email confirmation banner for trialing users
|
||||||
expect(uiStore.bannerStack).not.toContain('EMAIL_CONFIRMATION');
|
expect(uiStore.bannerStack).not.toContain('EMAIL_CONFIRMATION');
|
||||||
|
@ -183,10 +187,15 @@ describe('UI store', () => {
|
||||||
.spyOn(cloudPlanApi, 'getCloudUserInfo')
|
.spyOn(cloudPlanApi, 'getCloudUserInfo')
|
||||||
.mockResolvedValue(getUserCloudInfo(true));
|
.mockResolvedValue(getUserCloudInfo(true));
|
||||||
setupOwnerAndCloudDeployment();
|
setupOwnerAndCloudDeployment();
|
||||||
|
const getCurrentUsageSpy = vi
|
||||||
|
.spyOn(cloudPlanApi, 'getCurrentUsage')
|
||||||
|
.mockResolvedValue({ executions: 1000, activeWorkflows: 100 });
|
||||||
|
setupOwnerAndCloudDeployment();
|
||||||
await cloudPlanStore.checkForCloudPlanData();
|
await cloudPlanStore.checkForCloudPlanData();
|
||||||
await cloudPlanStore.fetchUserCloudAccount();
|
await cloudPlanStore.fetchUserCloudAccount();
|
||||||
expect(fetchCloudSpy).toHaveBeenCalled();
|
expect(fetchCloudSpy).toHaveBeenCalled();
|
||||||
expect(fetchUserCloudAccountSpy).toHaveBeenCalled();
|
expect(fetchUserCloudAccountSpy).toHaveBeenCalled();
|
||||||
|
expect(getCurrentUsageSpy).toHaveBeenCalled();
|
||||||
expect(uiStore.bannerStack).toContain('TRIAL_OVER');
|
expect(uiStore.bannerStack).toContain('TRIAL_OVER');
|
||||||
// There should be no email confirmation banner for trialing users
|
// There should be no email confirmation banner for trialing users
|
||||||
expect(uiStore.bannerStack).not.toContain('EMAIL_CONFIRMATION');
|
expect(uiStore.bannerStack).not.toContain('EMAIL_CONFIRMATION');
|
||||||
|
|
|
@ -10,7 +10,7 @@ function getUserPlanData(trialExpirationDate: Date, isTrial = true): Cloud.PlanD
|
||||||
isActive: true,
|
isActive: true,
|
||||||
displayName: 'Trial',
|
displayName: 'Trial',
|
||||||
metadata: {
|
metadata: {
|
||||||
group: isTrial ? 'trial' : 'pro',
|
group: isTrial ? 'trial' : 'opt-in',
|
||||||
slug: 'trial-1',
|
slug: 'trial-1',
|
||||||
trial: {
|
trial: {
|
||||||
gracePeriod: 3,
|
gracePeriod: 3,
|
||||||
|
|
|
@ -55,7 +55,7 @@ export const useCloudPlanStore = defineStore(STORES.CLOUD_PLAN, () => {
|
||||||
|
|
||||||
const hasCloudPlan = computed(() => {
|
const hasCloudPlan = computed(() => {
|
||||||
const cloudUserId = settingsStore.settings.n8nMetadata?.userId;
|
const cloudUserId = settingsStore.settings.n8nMetadata?.userId;
|
||||||
return usersStore.currentUser?.isOwner && settingsStore.isCloudDeployment && cloudUserId;
|
return usersStore.isInstanceOwner && settingsStore.isCloudDeployment && cloudUserId;
|
||||||
});
|
});
|
||||||
|
|
||||||
const getUserCloudAccount = async () => {
|
const getUserCloudAccount = async () => {
|
||||||
|
@ -68,7 +68,7 @@ export const useCloudPlanStore = defineStore(STORES.CLOUD_PLAN, () => {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
throw new Error(error);
|
throw new Error(error.message);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -143,13 +143,17 @@ export const useCloudPlanStore = defineStore(STORES.CLOUD_PLAN, () => {
|
||||||
if (!userIsTrialing.value) return;
|
if (!userIsTrialing.value) return;
|
||||||
await getInstanceCurrentUsage();
|
await getInstanceCurrentUsage();
|
||||||
startPollingInstanceUsageData();
|
startPollingInstanceUsageData();
|
||||||
} catch {}
|
} catch (e) {
|
||||||
|
throw new Error(e.message);
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
const fetchUserCloudAccount = async () => {
|
const fetchUserCloudAccount = async () => {
|
||||||
try {
|
try {
|
||||||
await getUserCloudAccount();
|
await getUserCloudAccount();
|
||||||
} catch {}
|
} catch (e) {
|
||||||
|
throw new Error(e.message);
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
const redirectToDashboard = async () => {
|
const redirectToDashboard = async () => {
|
||||||
|
@ -163,8 +167,17 @@ export const useCloudPlanStore = defineStore(STORES.CLOUD_PLAN, () => {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
await checkForCloudPlanData();
|
try {
|
||||||
await fetchUserCloudAccount();
|
await checkForCloudPlanData();
|
||||||
|
} catch (error) {
|
||||||
|
console.warn('Error checking for cloud plan data:', error);
|
||||||
|
}
|
||||||
|
|
||||||
|
try {
|
||||||
|
await fetchUserCloudAccount();
|
||||||
|
} catch (error) {
|
||||||
|
console.warn('Error fetching user cloud account:', error);
|
||||||
|
}
|
||||||
|
|
||||||
state.initialized = true;
|
state.initialized = true;
|
||||||
};
|
};
|
||||||
|
|
|
@ -126,7 +126,13 @@ export default defineComponent({
|
||||||
mfaRecoveryCode: form.recoveryCode,
|
mfaRecoveryCode: form.recoveryCode,
|
||||||
});
|
});
|
||||||
this.loading = false;
|
this.loading = false;
|
||||||
await this.cloudPlanStore.checkForCloudPlanData();
|
if (this.settingsStore.isCloudDeployment) {
|
||||||
|
try {
|
||||||
|
await this.cloudPlanStore.checkForCloudPlanData();
|
||||||
|
} catch (error) {
|
||||||
|
console.warn('Failed to check for cloud plan data', error);
|
||||||
|
}
|
||||||
|
}
|
||||||
await this.settingsStore.getSettings();
|
await this.settingsStore.getSettings();
|
||||||
this.clearAllStickyNotifications();
|
this.clearAllStickyNotifications();
|
||||||
this.checkRecoveryCodesLeft();
|
this.checkRecoveryCodesLeft();
|
||||||
|
|
Loading…
Reference in a new issue