diff --git a/packages/cli/eslint.config.mjs b/packages/cli/eslint.config.mjs index 4dbb7aef590..7a10f9f0e2e 100644 --- a/packages/cli/eslint.config.mjs +++ b/packages/cli/eslint.config.mjs @@ -351,7 +351,6 @@ export default defineConfig( './src/modules/dynamic-credentials.ee/services/credential-resolver.service.ts', './src/modules/external-secrets.ee/secrets-providers-connections.service.ee.ts', './src/modules/favorites/favorites.service.ts', - './src/modules/insights/insights-collection.service.ts', './src/modules/instance-ai/instance-ai.adapter.service.ts', './src/modules/instance-ai/mcp/instance-ai-mcp-registry.service.ts', './src/modules/instance-ai/storage/typeorm-agent-checkpoint-store.ts', diff --git a/packages/cli/src/modules/insights/__tests__/insights-bigint-migration.integration.test.ts b/packages/cli/src/modules/insights/__tests__/insights-bigint-migration.integration.test.ts index 769b8f301f2..0616a6469af 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-bigint-migration.integration.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-bigint-migration.integration.test.ts @@ -2,7 +2,7 @@ import { createTeamProject, createWorkflow, testDb, testModules } from '@n8n/bac import { Container } from '@n8n/di'; import { DateTime } from 'luxon'; -import { InsightsRawRepository } from '@/modules/insights/database/repositories/insights-raw.repository'; +import { InsightsRawRepository } from '../database/repositories/insights-raw.repository'; import { createRawInsightsEvent, diff --git a/packages/cli/src/modules/insights/__tests__/insights-collection.service.integration.test.ts b/packages/cli/src/modules/insights/__tests__/insights-collection.service.integration.test.ts index dc9913f9488..7485ca8a531 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-collection.service.integration.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-collection.service.integration.test.ts @@ -8,17 +8,15 @@ import { import type { Project, WorkflowEntity, IWorkflowDb, SharedWorkflowRepository } from '@n8n/db'; import type { WorkflowExecuteAfterContext } from '@n8n/decorators'; import { Container } from '@n8n/di'; -import { In } from '@n8n/typeorm'; import { createDeferredPromise } from '@n8n/utils/promise/deferred-promise'; import { DateTime } from 'luxon'; import { type ExecutionStatus, type IRun, type WorkflowExecuteMode } from 'n8n-workflow'; import assert from 'node:assert'; import { mock } from 'vitest-mock-extended'; -import type { TypeUnit } from '@/modules/insights/database/entities/insights-shared'; -import { InsightsMetadataRepository } from '@/modules/insights/database/repositories/insights-metadata.repository'; -import { InsightsRawRepository } from '@/modules/insights/database/repositories/insights-raw.repository'; - +import type { TypeUnit } from '../database/entities/insights-shared'; +import { InsightsMetadataRepository } from '../database/repositories/insights-metadata.repository'; +import { InsightsRawRepository } from '../database/repositories/insights-raw.repository'; import { InsightsCollectionService } from '../insights-collection.service'; import { InsightsConfig } from '../insights.config'; @@ -283,9 +281,9 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { // Mock the repositories functions const repositoryMocks = { - find: vi.fn(), - findBy: vi.fn(), - upsert: vi.fn(), + findOwnerProjectsByWorkflowIds: vi.fn(), + findByWorkflowIds: vi.fn(), + upsertWorkflowMetadata: vi.fn(), insert: vi.fn(), }; const sharedWorkflowRepositoryMock = mock(repositoryMocks); @@ -319,15 +317,12 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { project = await createTeamProject(); workflow = await createWorkflow({}, project); - repositoryMocks.find = vi.fn().mockResolvedValue([ - { - workflow, - workflowId: workflow.id, - projectId: 'project-id', - project: { name: 'project-name' }, - }, - ]); - repositoryMocks.findBy = vi.fn().mockResolvedValue([ + repositoryMocks.findOwnerProjectsByWorkflowIds.mockReset(); + repositoryMocks.findOwnerProjectsByWorkflowIds.mockResolvedValue( + new Map([[workflow.id, mock({ id: 'project-id', name: 'project-name' })]]), + ); + repositoryMocks.findByWorkflowIds.mockReset(); + repositoryMocks.findByWorkflowIds.mockResolvedValue([ { metaId: 'meta-id', workflowId: workflow.id, @@ -336,6 +331,8 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { projectName: 'project-name', }, ]); + repositoryMocks.upsertWorkflowMetadata.mockClear(); + repositoryMocks.insert.mockClear(); }); test('reuses cached metadata for subsequent executions of the same workflow', async () => { @@ -350,11 +347,8 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { await insightsCollectionService.flushEvents(); // ASSERT - expect(repositoryMocks.find).toHaveBeenCalledWith({ - where: { workflowId: In([workflow.id]), role: 'workflow:owner' }, - relations: { project: true }, - }); - expect(repositoryMocks.upsert).toHaveBeenCalledWith( + expect(repositoryMocks.findOwnerProjectsByWorkflowIds).toHaveBeenCalledWith([workflow.id]); + expect(repositoryMocks.upsertWorkflowMetadata).toHaveBeenCalledWith( expect.arrayContaining([ { workflowId: workflow.id, @@ -363,18 +357,21 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { projectName: 'project-name', }, ]), - ['workflowId'], ); + repositoryMocks.findOwnerProjectsByWorkflowIds.mockClear(); + repositoryMocks.findByWorkflowIds.mockClear(); + repositoryMocks.upsertWorkflowMetadata.mockClear(); + // ACT AGAIN with the same workflow await insightsCollectionService.handleWorkflowExecuteAfter(ctx); await insightsCollectionService.flushEvents(); // ASSERT AGAIN - repositoryMocks.find.mockClear(); - repositoryMocks.upsert.mockClear(); - expect(repositoryMocks.find).not.toHaveBeenCalled(); - expect(repositoryMocks.upsert).not.toHaveBeenCalled(); + expect(repositoryMocks.findOwnerProjectsByWorkflowIds).toHaveBeenCalledOnce(); + expect(repositoryMocks.findOwnerProjectsByWorkflowIds).toHaveBeenCalledWith([workflow.id]); + expect(repositoryMocks.findByWorkflowIds).not.toHaveBeenCalled(); + expect(repositoryMocks.upsertWorkflowMetadata).not.toHaveBeenCalled(); }); test('updates cached metadata if workflow details change', async () => { @@ -386,8 +383,8 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { await insightsCollectionService.flushEvents(); // ASSERT - expect(repositoryMocks.find).toHaveBeenCalled(); - expect(repositoryMocks.upsert).toHaveBeenCalled(); + expect(repositoryMocks.findOwnerProjectsByWorkflowIds).toHaveBeenCalled(); + expect(repositoryMocks.upsertWorkflowMetadata).toHaveBeenCalled(); // Change the workflow name workflow.name = 'new-workflow-name'; @@ -397,11 +394,8 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { await insightsCollectionService.flushEvents(); // ASSERT AGAIN - expect(repositoryMocks.find).toHaveBeenCalledWith({ - where: { workflowId: In([workflow.id]), role: 'workflow:owner' }, - relations: { project: true }, - }); - expect(repositoryMocks.upsert).toHaveBeenCalledWith( + expect(repositoryMocks.findOwnerProjectsByWorkflowIds).toHaveBeenCalledWith([workflow.id]); + expect(repositoryMocks.upsertWorkflowMetadata).toHaveBeenCalledWith( expect.arrayContaining([ { workflowId: workflow.id, @@ -410,7 +404,6 @@ describe('workflowExecuteAfterHandler - cacheMetadata', () => { projectName: 'project-name', }, ]), - ['workflowId'], ); }); }); @@ -427,11 +420,11 @@ describe('workflowExecuteAfterHandler - flushEvents', () => { insertInsightsRaw: vi.fn(), }; const sharedWorkflowRepositoryMock = mock({ - find: repoMocks.findSharedWorkflowRepositoryMock, + findOwnerProjectsByWorkflowIds: repoMocks.findSharedWorkflowRepositoryMock, }); const metadataRepositoryMock = mock({ - findBy: repoMocks.findByMetadata, - upsert: repoMocks.upsertMetadata, + findByWorkflowIds: repoMocks.findByMetadata, + upsertWorkflowMetadata: repoMocks.upsertMetadata, }); const insightsRawRepositoryMock = mock({ insert: repoMocks.insertInsightsRaw, @@ -459,14 +452,9 @@ describe('workflowExecuteAfterHandler - flushEvents', () => { beforeEach(async () => { project = await createTeamProject(); workflow = await createWorkflow({ settings: { timeSavedPerExecution: 1 } }, project); - repoMocks.findSharedWorkflowRepositoryMock.mockResolvedValue([ - { - workflow, - workflowId: workflow.id, - projectId: 'project-id', - project: { name: 'project-name' }, - }, - ]); + repoMocks.findSharedWorkflowRepositoryMock.mockResolvedValue( + new Map([[workflow.id, mock({ id: 'project-id', name: 'project-name' })]]), + ); repoMocks.findByMetadata.mockResolvedValue([ { metaId: 'meta-id', diff --git a/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts b/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts index dffedc6eacf..012a41568e4 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-collection.service.test.ts @@ -1,11 +1,5 @@ import { mockLogger } from '@n8n/backend-test-utils'; -import type { - Project, - SharedWorkflow, - SharedWorkflowRepository, - IWorkflowDb, - WorkflowEntity, -} from '@n8n/db'; +import type { Project, SharedWorkflowRepository, IWorkflowDb, WorkflowEntity } from '@n8n/db'; import type { WorkflowExecuteAfterContext } from '@n8n/decorators'; import { DateTime } from 'luxon'; import type { IRun } from 'n8n-workflow'; @@ -90,15 +84,11 @@ describe('initialization safeguards', () => { }); // mock shared workflow repository for the flushing process - sharedWorkflowRepository.find.mockResolvedValueOnce([ - mock({ - workflow, - project: mock(), - role: 'workflow:editor', - }), - ]); + sharedWorkflowRepository.findOwnerProjectsByWorkflowIds.mockResolvedValueOnce( + new Map([[workflow.id, mock()]]), + ); // mock insights metadata repository for the flushing process - insightsMetadataRepository.findBy.mockResolvedValueOnce([ + insightsMetadataRepository.findByWorkflowIds.mockResolvedValueOnce([ mock({ workflowId: ctx.workflow.id, metaId: 1 }), ]); @@ -285,15 +275,10 @@ describe('flushEvents with an insight that has no shared workflow', () => { service.init(); // Only "known-workflow" has an owner row, so "gone-workflow" gets no metadata. - sharedWorkflowRepository.find.mockResolvedValue([ - mock({ - workflowId: 'known-workflow', - projectId: 'project-id', - project: mock({ name: 'Project' }), - role: 'workflow:owner', - }), - ]); - insightsMetadataRepository.findBy.mockResolvedValue([ + sharedWorkflowRepository.findOwnerProjectsByWorkflowIds.mockResolvedValue( + new Map([['known-workflow', mock({ id: 'project-id', name: 'Project' })]]), + ); + insightsMetadataRepository.findByWorkflowIds.mockResolvedValue([ mock({ workflowId: 'known-workflow', metaId: 1 }), ]); }); @@ -326,8 +311,8 @@ describe('flushEvents with an insight that has no shared workflow', () => { }); test('does not re-buffer the event on the next flush', async () => { - sharedWorkflowRepository.find.mockResolvedValue([]); - insightsMetadataRepository.findBy.mockResolvedValue([]); + sharedWorkflowRepository.findOwnerProjectsByWorkflowIds.mockResolvedValue(new Map()); + insightsMetadataRepository.findByWorkflowIds.mockResolvedValue([]); bufferEvent('gone-workflow'); await service.flushEvents(); diff --git a/packages/cli/src/modules/insights/__tests__/insights-compaction.service.integration.test.ts b/packages/cli/src/modules/insights/__tests__/insights-compaction.service.integration.test.ts index fa266356113..9f726aaa159 100644 --- a/packages/cli/src/modules/insights/__tests__/insights-compaction.service.integration.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights-compaction.service.integration.test.ts @@ -5,7 +5,7 @@ import { Container } from '@n8n/di'; import { DateTime } from 'luxon'; import { mock } from 'vitest-mock-extended'; -import { InsightsRawRepository } from '@/modules/insights/database/repositories/insights-raw.repository'; +import { InsightsRawRepository } from '../database/repositories/insights-raw.repository'; import { createMetadata, diff --git a/packages/cli/src/modules/insights/__tests__/insights.module.test.ts b/packages/cli/src/modules/insights/__tests__/insights.module.test.ts index 62e63f2eaae..f1f7ebedf8e 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.module.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.module.test.ts @@ -48,6 +48,7 @@ describe('InsightsModule', () => { mockInstanceSettings, Container.get(Logger), mock(), + mock(), ), ); insightsModule = Container.get(InsightsModule); diff --git a/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts b/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts index cf34c7f3b80..be7f2c23f95 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.service.integration.test.ts @@ -19,7 +19,7 @@ import type { MockProxy } from 'vitest-mock-extended'; import { mock } from 'vitest-mock-extended'; import { ForbiddenError } from '@n8n/errors'; -import type { WorkflowSharingService } from '@n8n/backend-services'; +import type { ProjectScopeService, WorkflowSharingService } from '@n8n/backend-services'; import { createMember } from '@test-integration/db/users'; import { createCompactedInsightsEvent } from '../database/entities/__tests__/db-utils'; @@ -69,6 +69,7 @@ describe('InsightsService (Integration)', () => { instanceSettings, mockLogger(), mock(), + mock(), ); // Get the real service from the container and spy on it @@ -1629,6 +1630,7 @@ describe('InsightsService (Integration)', () => { mock(), mockLogger(), mock(), + mock(), ); }); @@ -1728,6 +1730,7 @@ describe('InsightsService (Integration)', () => { mock({ instanceType: 'main' }), mockLogger(), mock(), + mock(), ); }); diff --git a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts index 0c6a7cc1121..1ebbfba73a0 100644 --- a/packages/cli/src/modules/insights/__tests__/insights.service.test.ts +++ b/packages/cli/src/modules/insights/__tests__/insights.service.test.ts @@ -5,17 +5,12 @@ import type { InstanceSettings } from 'n8n-core'; import type { MockProxy } from 'vitest-mock-extended'; import { mock } from 'vitest-mock-extended'; -import { userHasScopes } from '@/permissions.ee/check-access'; -import type { WorkflowSharingService } from '@n8n/backend-services'; +import type { ProjectScopeService, WorkflowSharingService } from '@n8n/backend-services'; import { TypeToNumber, type TypeUnitNumber } from '../database/entities/insights-shared'; import type { InsightsByPeriodRepository } from '../database/repositories/insights-by-period.repository'; import { InsightsService } from '../insights.service'; -vi.mock('@/permissions.ee/check-access', () => ({ - userHasScopes: vi.fn(), -})); - const user = mock({ id: 'user-1' }); describe('InsightsService', () => { @@ -25,6 +20,7 @@ describe('InsightsService', () => { let mockLicenseState: MockProxy; let mockInstanceSettings: MockProxy; let mockWorkflowSharingService: MockProxy; + let mockProjectScopeService: MockProxy; beforeEach(() => { vi.clearAllMocks(); @@ -33,7 +29,8 @@ describe('InsightsService', () => { mockLicenseState = mock(); mockInstanceSettings = mock(); mockWorkflowSharingService = mock(); - vi.mocked(userHasScopes).mockResolvedValue(true); + mockProjectScopeService = mock(); + mockProjectScopeService.getProjectIds.mockResolvedValue(null); insightsService = new InsightsService( mockInsightsByPeriodRepository, @@ -41,6 +38,7 @@ describe('InsightsService', () => { mockInstanceSettings, mockLogger(), mockWorkflowSharingService, + mockProjectScopeService, ); }); @@ -869,11 +867,11 @@ describe('InsightsService', () => { it('should not check project access when no project is requested', async () => { await insightsService.getInsightsSummary({ user, startDate, endDate }); - expect(userHasScopes).not.toHaveBeenCalled(); + expect(mockProjectScopeService.getProjectIds).not.toHaveBeenCalled(); }); it('should throw a forbidden error when the requested project is not accessible', async () => { - vi.mocked(userHasScopes).mockResolvedValue(false); + mockProjectScopeService.getProjectIds.mockResolvedValue([]); await expect( insightsService.getInsightsSummary({ user, startDate, endDate, projectId: 'project-1' }), @@ -892,9 +890,7 @@ describe('InsightsService', () => { projectId: 'project-1', }); - expect(userHasScopes).toHaveBeenCalledWith(user, ['workflow:read'], false, { - projectId: 'project-1', - }); + expect(mockProjectScopeService.getProjectIds).toHaveBeenCalledWith(user, ['workflow:read']); expect( mockInsightsByPeriodRepository.getPreviousAndCurrentPeriodTypeAggregates, ).toHaveBeenCalledWith(expect.objectContaining({ projectId: 'project-1' })); @@ -1045,11 +1041,11 @@ describe('InsightsService', () => { it('should not check project access when no project is requested', async () => { await insightsService.getInsightsByWorkflow({ user, startDate, endDate }); - expect(userHasScopes).not.toHaveBeenCalled(); + expect(mockProjectScopeService.getProjectIds).not.toHaveBeenCalled(); }); it('should throw a forbidden error when the requested project is not accessible', async () => { - vi.mocked(userHasScopes).mockResolvedValue(false); + mockProjectScopeService.getProjectIds.mockResolvedValue([]); await expect( insightsService.getInsightsByWorkflow({ @@ -1106,11 +1102,11 @@ describe('InsightsService', () => { it('should not check project access when no project is requested', async () => { await insightsService.getInsightsByTime({ user, startDate, endDate }); - expect(userHasScopes).not.toHaveBeenCalled(); + expect(mockProjectScopeService.getProjectIds).not.toHaveBeenCalled(); }); it('should throw a forbidden error when the requested project is not accessible', async () => { - vi.mocked(userHasScopes).mockResolvedValue(false); + mockProjectScopeService.getProjectIds.mockResolvedValue([]); await expect( insightsService.getInsightsByTime({ diff --git a/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.integration.test.ts b/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.integration.test.ts index 0d87a9b1333..fe652585124 100644 --- a/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.integration.test.ts +++ b/packages/cli/src/modules/insights/database/repositories/__tests__/insights-by-period.repository.integration.test.ts @@ -14,7 +14,7 @@ import { DataSource } from '@n8n/typeorm'; import { sleep } from '@n8n/utils/sleep'; import { DateTime } from 'luxon'; -import { InsightsConfig } from '@/modules/insights/insights.config'; +import { InsightsConfig } from '../../../insights.config'; import { createMember } from '@test-integration/db/users'; import { diff --git a/packages/cli/src/modules/insights/database/repositories/insights-metadata.repository.ts b/packages/cli/src/modules/insights/database/repositories/insights-metadata.repository.ts index f21cd5ddc9b..bcab0382734 100644 --- a/packages/cli/src/modules/insights/database/repositories/insights-metadata.repository.ts +++ b/packages/cli/src/modules/insights/database/repositories/insights-metadata.repository.ts @@ -1,5 +1,5 @@ import { Service } from '@n8n/di'; -import { DataSource, Repository } from '@n8n/typeorm'; +import { DataSource, In, Repository } from '@n8n/typeorm'; import { InsightsMetadata } from '../entities/insights-metadata'; @@ -8,4 +8,16 @@ export class InsightsMetadataRepository extends Repository { constructor(dataSource: DataSource) { super(InsightsMetadata, dataSource.manager); } + + async upsertWorkflowMetadata(metadata: InsightsMetadata[]) { + if (metadata.length === 0) return; + + await this.upsert(metadata, ['workflowId']); + } + + async findByWorkflowIds(workflowIds: string[]) { + if (workflowIds.length === 0) return []; + + return await this.findBy({ workflowId: In(workflowIds) }); + } } diff --git a/packages/cli/src/modules/insights/insights-collection.service.ts b/packages/cli/src/modules/insights/insights-collection.service.ts index 6a8428fd904..f13e5d9fdb4 100644 --- a/packages/cli/src/modules/insights/insights-collection.service.ts +++ b/packages/cli/src/modules/insights/insights-collection.service.ts @@ -3,13 +3,11 @@ import { isBillableExecution } from '@n8n/backend-services'; import { SharedWorkflowRepository } from '@n8n/db'; import { OnLifecycleEvent, type WorkflowExecuteAfterContext } from '@n8n/decorators'; import { Service } from '@n8n/di'; -import { In } from '@n8n/typeorm'; import { DateTime } from 'luxon'; import { IRun, type ExecutionStatus, type WorkflowExecuteMode } from 'n8n-workflow'; -import { InsightsMetadata } from '@/modules/insights/database/entities/insights-metadata'; -import { InsightsRaw } from '@/modules/insights/database/entities/insights-raw'; - +import { InsightsMetadata } from './database/entities/insights-metadata'; +import { InsightsRaw } from './database/entities/insights-raw'; import { InsightsMetadataRepository } from './database/repositories/insights-metadata.repository'; import { InsightsRawRepository } from './database/repositories/insights-raw.repository'; import { InsightsConfig } from './insights.config'; @@ -233,40 +231,44 @@ export class InsightsCollectionService { workflowIdNames.set(event.workflowId, event.workflowName); } - const sharedWorkflows = await this.sharedWorkflowRepository.find({ - where: { workflowId: In([...workflowIdNames.keys()]), role: 'workflow:owner' }, - relations: { project: true }, - }); + const ownerProjects = await this.sharedWorkflowRepository.findOwnerProjectsByWorkflowIds([ + ...workflowIdNames.keys(), + ]); // Upsert metadata for the workflows that are not already in the cache or have // different project or workflow names - const metadataToUpsert = sharedWorkflows.reduce((acc, workflow) => { - const cachedMetadata = this.cachedMetadata.get(workflow.workflowId); + const metadataToUpsert: InsightsMetadata[] = []; + for (const [workflowId, workflowName] of workflowIdNames) { + const project = ownerProjects.get(workflowId); + if (!project) continue; + + const cachedMetadata = this.cachedMetadata.get(workflowId); if ( !cachedMetadata || - cachedMetadata.projectId !== workflow.projectId || - cachedMetadata.projectName !== workflow.project.name || - cachedMetadata.workflowName !== workflowIdNames.get(workflow.workflowId) + cachedMetadata.projectId !== project.id || + cachedMetadata.projectName !== project.name || + cachedMetadata.workflowName !== workflowName ) { const metadata = new InsightsMetadata(); - metadata.projectId = workflow.projectId; - metadata.projectName = workflow.project.name; - metadata.workflowId = workflow.workflowId; - metadata.workflowName = workflowIdNames.get(workflow.workflowId)!; + metadata.projectId = project.id; + metadata.projectName = project.name; + metadata.workflowId = workflowId; + metadata.workflowName = workflowName; - acc.push(metadata); + metadataToUpsert.push(metadata); } - return acc; - }, [] as InsightsMetadata[]); + } - this.logger.debug(`Saving ${metadataToUpsert.length} insights metadata for workflows`); - await this.insightsMetadataRepository.upsert(metadataToUpsert, ['workflowId']); + if (metadataToUpsert.length > 0) { + this.logger.debug(`Saving ${metadataToUpsert.length} insights metadata for workflows`); + await this.insightsMetadataRepository.upsertWorkflowMetadata(metadataToUpsert); - const upsertMetadata = await this.insightsMetadataRepository.findBy({ - workflowId: In(metadataToUpsert.map((m) => m.workflowId)), - }); - for (const metadata of upsertMetadata) { - this.cachedMetadata.set(metadata.workflowId, metadata); + const upsertMetadata = await this.insightsMetadataRepository.findByWorkflowIds( + metadataToUpsert.map((metadata) => metadata.workflowId), + ); + for (const metadata of upsertMetadata) { + this.cachedMetadata.set(metadata.workflowId, metadata); + } } const events: InsightsRaw[] = []; diff --git a/packages/cli/src/modules/insights/insights.service.ts b/packages/cli/src/modules/insights/insights.service.ts index 746e1b1d394..3d20a45fb79 100644 --- a/packages/cli/src/modules/insights/insights.service.ts +++ b/packages/cli/src/modules/insights/insights.service.ts @@ -4,7 +4,7 @@ import { type RestrictedInsightsByTime, } from '@n8n/api-types'; import { LicenseState, Logger } from '@n8n/backend-common'; -import { WorkflowSharingService } from '@n8n/backend-services'; +import { ProjectScopeService, WorkflowSharingService } from '@n8n/backend-services'; import type { User } from '@n8n/db'; import { Container, Service } from '@n8n/di'; import { DateTime } from 'luxon'; @@ -12,8 +12,6 @@ import { InstanceSettings } from 'n8n-core'; import { UserError } from 'n8n-workflow'; import { ForbiddenError } from '@n8n/errors'; -import { userHasScopes } from '@/permissions.ee/check-access'; - import type { PeriodUnit, TypeUnit, ByTimeInsightType } from './database/entities/insights-shared'; import { NumberToType } from './database/entities/insights-shared'; import type { InsightsAccessFilter } from './database/repositories/insights-by-period.repository'; @@ -42,6 +40,7 @@ export class InsightsService { private readonly instanceSettings: InstanceSettings, private readonly logger: Logger, private readonly workflowSharingService: WorkflowSharingService, + private readonly projectScopeService: ProjectScopeService, ) { this.logger = this.logger.scoped('insights'); } @@ -85,10 +84,10 @@ export class InsightsService { projectId?: string, ): Promise { if (projectId) { - const userHasRequiredProjectScopes = await userHasScopes(user, ['workflow:read'], false, { - projectId, - }); - if (!userHasRequiredProjectScopes) { + const accessibleProjectIds = await this.projectScopeService.getProjectIds(user, [ + 'workflow:read', + ]); + if (accessibleProjectIds !== null && !accessibleProjectIds.includes(projectId)) { throw new ForbiddenError('You do not have access to insights for this project.'); } }