From 2ac9c701c338b07635f587b40026516a42284254 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Fri, 4 Oct 2024 15:43:02 +0200 Subject: [PATCH] fix: return 404 if the project doesn't exist (#8362) 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. --- .../onboarding/fake-onboarding-read-model.ts | 6 +-- .../onboarding/onboarding-read-model-type.ts | 4 +- .../onboarding/onboarding-read-model.ts | 11 +++- .../personal-dashboard-controller.e2e.test.ts | 8 +++ .../personal-dashboard-service.ts | 51 ++++++++++--------- src/lib/features/project/project-service.ts | 4 +- 6 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/lib/features/onboarding/fake-onboarding-read-model.ts b/src/lib/features/onboarding/fake-onboarding-read-model.ts index 61318602de..2f0139f522 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 f88f522722..177ca5d346 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 a124e520c3..8e2792b0d9 100644 --- a/src/lib/features/onboarding/onboarding-read-model.ts +++ b/src/lib/features/onboarding/onboarding-read-model.ts @@ -82,7 +82,16 @@ export class OnboardingReadModel implements IOnboardingReadModel { async getOnboardingStatusForProject( projectId: string, - ): Promise { + ): Promise { + const projectExists = await this.db('projects') + .select(1) + .where('id', projectId) + .first(); + + if (!projectExists) { + return null; + } + const feature = await this.db('features') .select('name') .where('project', projectId) 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 f4267d7db1..21e8ffb5b3 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 316d070364..17a341db7b 100644 --- a/src/lib/features/personal-dashboard/personal-dashboard-service.ts +++ b/src/lib/features/personal-dashboard/personal-dashboard-service.ts @@ -21,6 +21,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; @@ -105,6 +106,17 @@ export class PersonalDashboardService { userId: number, projectId: string, ): Promise { + const onboardingStatus = + await this.onboardingReadModel.getOnboardingStatusForProject( + projectId, + ); + + if (!onboardingStatus) { + throw new NotFoundError( + `No project with id "${projectId}" exists.`, + ); + } + const formatEvents = (recentEvents: IEvent[]) => recentEvents.map((event) => ({ summary: this.featureEventFormatter.format(event).text, @@ -122,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; diff --git a/src/lib/features/project/project-service.ts b/src/lib/features/project/project-service.ts index 46f0ffae6d..a3c664b780 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,