From 25db909dfe95496b177a4359c0e6167fa0f5f874 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Oct 2024 07:51:21 +0200 Subject: [PATCH 1/5] fix: return 404 if the project doesn't exist This change adds a check for whether the project exists in the database before trying to fetch data for it. If it doesn't exist, you'll get a 404. --- .../createPersonalDashboardService.ts | 9 +++++++++ .../personal-dashboard-controller.e2e.test.ts | 8 ++++++++ .../personal-dashboard/personal-dashboard-service.ts | 12 ++++++++++++ 3 files changed, 29 insertions(+) diff --git a/src/lib/features/personal-dashboard/createPersonalDashboardService.ts b/src/lib/features/personal-dashboard/createPersonalDashboardService.ts index 8f5955f970fd..7485b7658937 100644 --- a/src/lib/features/personal-dashboard/createPersonalDashboardService.ts +++ b/src/lib/features/personal-dashboard/createPersonalDashboardService.ts @@ -18,6 +18,8 @@ import { OnboardingReadModel } from '../onboarding/onboarding-read-model'; import { FakeOnboardingReadModel } from '../onboarding/fake-onboarding-read-model'; import { AccessStore } from '../../db/access-store'; import FakeAccessStore from '../../../test/fixtures/fake-access-store'; +import ProjectStore from '../project/project-store'; +import FakeProjectStore from '../../../test/fixtures/fake-project-store'; export const createPersonalDashboardService = ( db: Db, @@ -37,6 +39,12 @@ export const createPersonalDashboardService = ( new PrivateProjectChecker(stores, config), new AccountStore(db, config.getLogger), new AccessStore(db, config.eventBus, config.getLogger), + new ProjectStore( + db, + config.eventBus, + config.getLogger, + config.flagResolver, + ), ); }; @@ -54,5 +62,6 @@ export const createFakePersonalDashboardService = (config: IUnleashConfig) => { new FakePrivateProjectChecker(), new FakeAccountStore(), new FakeAccessStore(), + new FakeProjectStore(), ); }; diff --git a/src/lib/features/personal-dashboard/personal-dashboard-controller.e2e.test.ts b/src/lib/features/personal-dashboard/personal-dashboard-controller.e2e.test.ts index f4267d7db1fd..21e8ffb5b304 100644 --- a/src/lib/features/personal-dashboard/personal-dashboard-controller.e2e.test.ts +++ b/src/lib/features/personal-dashboard/personal-dashboard-controller.e2e.test.ts @@ -335,6 +335,14 @@ test('should return personal dashboard project details', async () => { }); }); +test("should return 404 if the project doesn't exist", async () => { + await loginUser('new_user@test.com'); + + await app.request + .get(`/api/admin/personal-dashboard/${randomId()}`) + .expect(404); +}); + test('should return Unleash admins', async () => { await loginUser('new_user@test.com'); const adminRoleId = 1; diff --git a/src/lib/features/personal-dashboard/personal-dashboard-service.ts b/src/lib/features/personal-dashboard/personal-dashboard-service.ts index 316d070364c8..69a4851b98dc 100644 --- a/src/lib/features/personal-dashboard/personal-dashboard-service.ts +++ b/src/lib/features/personal-dashboard/personal-dashboard-service.ts @@ -10,6 +10,7 @@ import type { import type { IProjectReadModel } from '../project/project-read-model-type'; import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType'; import type { + IProjectStore, IAccessStore, IAccountStore, IEvent, @@ -21,6 +22,7 @@ import type { FeatureEventFormatter } from '../../addons/feature-event-formatter import { generateImageUrl } from '../../util'; import type { PersonalDashboardProjectDetailsSchema } from '../../openapi'; import type { IRoleWithProject } from '../../types/stores/access-store'; +import { NotFoundError } from '../../error'; export class PersonalDashboardService { private personalDashboardReadModel: IPersonalDashboardReadModel; @@ -41,6 +43,8 @@ export class PersonalDashboardService { private accessStore: IAccessStore; + private projectStore: IProjectStore; + constructor( personalDashboardReadModel: IPersonalDashboardReadModel, projectOwnersReadModel: IProjectOwnersReadModel, @@ -51,6 +55,7 @@ export class PersonalDashboardService { privateProjectChecker: IPrivateProjectChecker, accountStore: IAccountStore, accessStore: IAccessStore, + projectStore: IProjectStore, ) { this.personalDashboardReadModel = personalDashboardReadModel; this.projectOwnersReadModel = projectOwnersReadModel; @@ -61,6 +66,7 @@ export class PersonalDashboardService { this.privateProjectChecker = privateProjectChecker; this.accountStore = accountStore; this.accessStore = accessStore; + this.projectStore = projectStore; } getPersonalFeatures(userId: number): Promise { @@ -105,6 +111,12 @@ export class PersonalDashboardService { userId: number, projectId: string, ): Promise { + if (!(await this.projectStore.hasProject(projectId))) { + throw new NotFoundError( + `No project with id "${projectId}" exists.`, + ); + } + const formatEvents = (recentEvents: IEvent[]) => recentEvents.map((event) => ({ summary: this.featureEventFormatter.format(event).text, From 2d5fb02822a318afc207aca90a76baf2e5685896 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Oct 2024 09:28:13 +0200 Subject: [PATCH 2/5] fix: return `null` for onboarding if the project doesn't exist --- .../onboarding/fake-onboarding-read-model.ts | 6 ++---- .../onboarding/onboarding-read-model-type.ts | 4 +++- src/lib/features/onboarding/onboarding-read-model.ts | 12 +++++++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/lib/features/onboarding/fake-onboarding-read-model.ts b/src/lib/features/onboarding/fake-onboarding-read-model.ts index 61318602dea6..2f0139f522a6 100644 --- a/src/lib/features/onboarding/fake-onboarding-read-model.ts +++ b/src/lib/features/onboarding/fake-onboarding-read-model.ts @@ -19,9 +19,7 @@ export class FakeOnboardingReadModel implements IOnboardingReadModel { return Promise.resolve([]); } - getOnboardingStatusForProject( - projectId: string, - ): Promise { - throw new Error('Method not implemented.'); + async getOnboardingStatusForProject(): Promise { + return null; } } diff --git a/src/lib/features/onboarding/onboarding-read-model-type.ts b/src/lib/features/onboarding/onboarding-read-model-type.ts index f88f522722f3..177ca5d346bb 100644 --- a/src/lib/features/onboarding/onboarding-read-model-type.ts +++ b/src/lib/features/onboarding/onboarding-read-model-type.ts @@ -26,5 +26,7 @@ export type ProjectOnboarding = { export interface IOnboardingReadModel { getInstanceOnboardingMetrics(): Promise; getProjectsOnboardingMetrics(): Promise>; - getOnboardingStatusForProject(projectId: string): Promise; + getOnboardingStatusForProject( + projectId: string, + ): Promise; } diff --git a/src/lib/features/onboarding/onboarding-read-model.ts b/src/lib/features/onboarding/onboarding-read-model.ts index a124e520c3ac..2fb97ace5b18 100644 --- a/src/lib/features/onboarding/onboarding-read-model.ts +++ b/src/lib/features/onboarding/onboarding-read-model.ts @@ -82,7 +82,17 @@ export class OnboardingReadModel implements IOnboardingReadModel { async getOnboardingStatusForProject( projectId: string, - ): Promise { + ): Promise { + const projectExistsResult = await this.db.raw( + `SELECT EXISTS(SELECT 1 FROM projects WHERE id = ?) AS projectExists`, + [projectId], + ); + const { projectExists } = projectExistsResult.rows[0]; + + if (!projectExists) { + return null; + } + const feature = await this.db('features') .select('name') .where('project', projectId) From 057f0e6e1ac0ed38a72f44a71d26ce630010e4e2 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Oct 2024 09:28:31 +0200 Subject: [PATCH 3/5] fix: use onboarding status to check project existence --- .../createPersonalDashboardService.ts | 9 ---- .../personal-dashboard-service.ts | 51 ++++++++----------- 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/src/lib/features/personal-dashboard/createPersonalDashboardService.ts b/src/lib/features/personal-dashboard/createPersonalDashboardService.ts index 7485b7658937..8f5955f970fd 100644 --- a/src/lib/features/personal-dashboard/createPersonalDashboardService.ts +++ b/src/lib/features/personal-dashboard/createPersonalDashboardService.ts @@ -18,8 +18,6 @@ import { OnboardingReadModel } from '../onboarding/onboarding-read-model'; import { FakeOnboardingReadModel } from '../onboarding/fake-onboarding-read-model'; import { AccessStore } from '../../db/access-store'; import FakeAccessStore from '../../../test/fixtures/fake-access-store'; -import ProjectStore from '../project/project-store'; -import FakeProjectStore from '../../../test/fixtures/fake-project-store'; export const createPersonalDashboardService = ( db: Db, @@ -39,12 +37,6 @@ export const createPersonalDashboardService = ( new PrivateProjectChecker(stores, config), new AccountStore(db, config.getLogger), new AccessStore(db, config.eventBus, config.getLogger), - new ProjectStore( - db, - config.eventBus, - config.getLogger, - config.flagResolver, - ), ); }; @@ -62,6 +54,5 @@ export const createFakePersonalDashboardService = (config: IUnleashConfig) => { new FakePrivateProjectChecker(), new FakeAccountStore(), new FakeAccessStore(), - new FakeProjectStore(), ); }; diff --git a/src/lib/features/personal-dashboard/personal-dashboard-service.ts b/src/lib/features/personal-dashboard/personal-dashboard-service.ts index 69a4851b98dc..17a341db7b95 100644 --- a/src/lib/features/personal-dashboard/personal-dashboard-service.ts +++ b/src/lib/features/personal-dashboard/personal-dashboard-service.ts @@ -10,7 +10,6 @@ import type { import type { IProjectReadModel } from '../project/project-read-model-type'; import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType'; import type { - IProjectStore, IAccessStore, IAccountStore, IEvent, @@ -43,8 +42,6 @@ export class PersonalDashboardService { private accessStore: IAccessStore; - private projectStore: IProjectStore; - constructor( personalDashboardReadModel: IPersonalDashboardReadModel, projectOwnersReadModel: IProjectOwnersReadModel, @@ -55,7 +52,6 @@ export class PersonalDashboardService { privateProjectChecker: IPrivateProjectChecker, accountStore: IAccountStore, accessStore: IAccessStore, - projectStore: IProjectStore, ) { this.personalDashboardReadModel = personalDashboardReadModel; this.projectOwnersReadModel = projectOwnersReadModel; @@ -66,7 +62,6 @@ export class PersonalDashboardService { this.privateProjectChecker = privateProjectChecker; this.accountStore = accountStore; this.accessStore = accessStore; - this.projectStore = projectStore; } getPersonalFeatures(userId: number): Promise { @@ -111,7 +106,12 @@ export class PersonalDashboardService { userId: number, projectId: string, ): Promise { - if (!(await this.projectStore.hasProject(projectId))) { + const onboardingStatus = + await this.onboardingReadModel.getOnboardingStatusForProject( + projectId, + ); + + if (!onboardingStatus) { throw new NotFoundError( `No project with id "${projectId}" exists.`, ); @@ -134,29 +134,22 @@ export class PersonalDashboardService { type: role.type as PersonalDashboardProjectDetailsSchema['roles'][number]['type'], })); - const [latestEvents, onboardingStatus, owners, roles, healthScores] = - await Promise.all([ - this.eventStore - .searchEvents({ limit: 4, offset: 0 }, [ - { - field: 'project', - operator: 'IS', - values: [projectId], - }, - ]) - .then(formatEvents), - this.onboardingReadModel.getOnboardingStatusForProject( - projectId, - ), - this.projectOwnersReadModel.getProjectOwners(projectId), - this.accessStore - .getAllProjectRolesForUser(userId, projectId) - .then(filterRoles), - this.personalDashboardReadModel.getLatestHealthScores( - projectId, - 8, - ), - ]); + const [latestEvents, owners, roles, healthScores] = await Promise.all([ + this.eventStore + .searchEvents({ limit: 4, offset: 0 }, [ + { + field: 'project', + operator: 'IS', + values: [projectId], + }, + ]) + .then(formatEvents), + this.projectOwnersReadModel.getProjectOwners(projectId), + this.accessStore + .getAllProjectRolesForUser(userId, projectId) + .then(filterRoles), + this.personalDashboardReadModel.getLatestHealthScores(projectId, 8), + ]); let avgHealthCurrentWindow: number | null = null; let avgHealthPastWindow: number | null = null; From 49e2371ba1b48554ece34bade6caccfd52e05437 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Oct 2024 09:45:16 +0200 Subject: [PATCH 4/5] fix: fix query --- src/lib/features/onboarding/onboarding-read-model.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lib/features/onboarding/onboarding-read-model.ts b/src/lib/features/onboarding/onboarding-read-model.ts index 2fb97ace5b18..8e2792b0d9bd 100644 --- a/src/lib/features/onboarding/onboarding-read-model.ts +++ b/src/lib/features/onboarding/onboarding-read-model.ts @@ -83,11 +83,10 @@ export class OnboardingReadModel implements IOnboardingReadModel { async getOnboardingStatusForProject( projectId: string, ): Promise { - const projectExistsResult = await this.db.raw( - `SELECT EXISTS(SELECT 1 FROM projects WHERE id = ?) AS projectExists`, - [projectId], - ); - const { projectExists } = projectExistsResult.rows[0]; + const projectExists = await this.db('projects') + .select(1) + .where('id', projectId) + .first(); if (!projectExists) { return null; From c3ed91187224e92fd898ceb460d68b2800b9836a Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Oct 2024 11:33:39 +0200 Subject: [PATCH 5/5] fix: handle type mismatch --- src/lib/features/project/project-service.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index 46f0ffae6d16..a3c664b78026 100644 --- a/src/lib/features/project/project-service.ts +++ b/src/lib/features/project/project-service.ts @@ -1546,7 +1546,9 @@ export default class ProjectService { updatedAt: project.updatedAt, archivedAt: project.archivedAt, createdAt: project.createdAt, - onboardingStatus, + onboardingStatus: onboardingStatus ?? { + status: 'onboarding-started', + }, environments, featureTypeCounts, members,