From 6188079122b8e3eb79ae8a1624c7d6fa4ea5366f Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Mon, 30 Sep 2024 10:49:34 +0200 Subject: [PATCH] feat: add project owners to personal dashboard (#8293) This PR adds all user-type owners of projects that you have access to to the personal dashboard payload. It adds the new `projectOwners` property regardless of whether you have access to any projects or not because it required less code and fewer conditionals, but we can do the filtering if we want to. To add the owners, it uses the private project checker to get accessible projects before passing those to the project owner read model, which has a new method to fetch user owners for projects. --- .../createPersonalDashboardService.ts | 7 +- .../personal-dashboard-controller.ts | 13 +-- .../personal-dashboard-service.ts | 22 +++- .../project/fake-project-owners-read-model.ts | 5 + .../project/project-owners-read-model.test.ts | 106 ++++++++++++++++-- .../project/project-owners-read-model.ts | 27 ++++- .../project/project-owners-read-model.type.ts | 4 + .../openapi/spec/personal-dashboard-schema.ts | 29 +++++ src/lib/services/index.ts | 2 +- 9 files changed, 196 insertions(+), 19 deletions(-) diff --git a/src/lib/features/personal-dashboard/createPersonalDashboardService.ts b/src/lib/features/personal-dashboard/createPersonalDashboardService.ts index b24e9512b982..4b438c14c0b4 100644 --- a/src/lib/features/personal-dashboard/createPersonalDashboardService.ts +++ b/src/lib/features/personal-dashboard/createPersonalDashboardService.ts @@ -1,5 +1,5 @@ import type { Db } from '../../db/db'; -import type { IUnleashConfig } from '../../types'; +import type { IUnleashConfig, IUnleashStores } from '../../types'; import { PersonalDashboardService } from './personal-dashboard-service'; import { PersonalDashboardReadModel } from './personal-dashboard-read-model'; import { FakePersonalDashboardReadModel } from './fake-personal-dashboard-read-model'; @@ -10,10 +10,13 @@ import { FakeProjectReadModel } from '../project/fake-project-read-model'; import EventStore from '../../db/event-store'; import { FeatureEventFormatterMd } from '../../addons/feature-event-formatter-md'; import FakeEventStore from '../../../test/fixtures/fake-event-store'; +import { FakePrivateProjectChecker } from '../private-project/fakePrivateProjectChecker'; +import { PrivateProjectChecker } from '../private-project/privateProjectChecker'; export const createPersonalDashboardService = ( db: Db, config: IUnleashConfig, + stores: IUnleashStores, ) => { return new PersonalDashboardService( new PersonalDashboardReadModel(db), @@ -24,6 +27,7 @@ export const createPersonalDashboardService = ( unleashUrl: config.server.unleashUrl, formatStyle: 'markdown', }), + new PrivateProjectChecker(stores, config), ); }; @@ -37,5 +41,6 @@ export const createFakePersonalDashboardService = (config: IUnleashConfig) => { unleashUrl: config.server.unleashUrl, formatStyle: 'markdown', }), + new FakePrivateProjectChecker(), ); }; diff --git a/src/lib/features/personal-dashboard/personal-dashboard-controller.ts b/src/lib/features/personal-dashboard/personal-dashboard-controller.ts index dacfc0f81aa9..8408945b6d91 100644 --- a/src/lib/features/personal-dashboard/personal-dashboard-controller.ts +++ b/src/lib/features/personal-dashboard/personal-dashboard-controller.ts @@ -83,18 +83,17 @@ export default class PersonalDashboardController extends Controller { ): Promise { const user = req.user; - const flags = await this.personalDashboardService.getPersonalFeatures( - user.id, - ); - - const projects = - await this.personalDashboardService.getPersonalProjects(user.id); + const [flags, projects, projectOwners] = await Promise.all([ + this.personalDashboardService.getPersonalFeatures(user.id), + this.personalDashboardService.getPersonalProjects(user.id), + this.personalDashboardService.getProjectOwners(user.id), + ]); this.openApiService.respondWithValidation( 200, res, personalDashboardSchema.$id, - { projects, flags }, + { projects, flags, projectOwners }, ); } diff --git a/src/lib/features/personal-dashboard/personal-dashboard-service.ts b/src/lib/features/personal-dashboard/personal-dashboard-service.ts index d28aa4fdc1d1..8c5f391b78fe 100644 --- a/src/lib/features/personal-dashboard/personal-dashboard-service.ts +++ b/src/lib/features/personal-dashboard/personal-dashboard-service.ts @@ -1,10 +1,14 @@ -import type { IProjectOwnersReadModel } from '../project/project-owners-read-model.type'; +import type { + IProjectOwnersReadModel, + UserProjectOwner, +} from '../project/project-owners-read-model.type'; import type { IPersonalDashboardReadModel, PersonalFeature, PersonalProject, } from './personal-dashboard-read-model-type'; import type { IProjectReadModel } from '../project/project-read-model-type'; +import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType'; import type { IEventStore } from '../../types'; import type { FeatureEventFormatter } from '../../addons/feature-event-formatter-md'; @@ -19,6 +23,8 @@ export class PersonalDashboardService { private projectReadModel: IProjectReadModel; + private privateProjectChecker: IPrivateProjectChecker; + private eventStore: IEventStore; private featureEventFormatter: FeatureEventFormatter; @@ -29,12 +35,14 @@ export class PersonalDashboardService { projectReadModel: IProjectReadModel, eventStore: IEventStore, featureEventFormatter: FeatureEventFormatter, + privateProjectChecker: IPrivateProjectChecker, ) { this.personalDashboardReadModel = personalDashboardReadModel; this.projectOwnersReadModel = projectOwnersReadModel; this.projectReadModel = projectReadModel; this.eventStore = eventStore; this.featureEventFormatter = featureEventFormatter; + this.privateProjectChecker = privateProjectChecker; } getPersonalFeatures(userId: number): Promise { @@ -62,6 +70,18 @@ export class PersonalDashboardService { return normalizedProjects; } + async getProjectOwners(userId: number): Promise { + const accessibleProjects = + await this.privateProjectChecker.getUserAccessibleProjects(userId); + + const filter = + accessibleProjects.mode === 'all' + ? undefined + : new Set(accessibleProjects.projects); + + return this.projectOwnersReadModel.getAllUserProjectOwners(filter); + } + async getPersonalProjectDetails( projectId: string, ): Promise { diff --git a/src/lib/features/project/fake-project-owners-read-model.ts b/src/lib/features/project/fake-project-owners-read-model.ts index cab7bd59ad8b..7fe734214aab 100644 --- a/src/lib/features/project/fake-project-owners-read-model.ts +++ b/src/lib/features/project/fake-project-owners-read-model.ts @@ -1,5 +1,6 @@ import type { IProjectOwnersReadModel, + UserProjectOwner, WithProjectOwners, } from './project-owners-read-model.type'; @@ -12,4 +13,8 @@ export class FakeProjectOwnersReadModel implements IProjectOwnersReadModel { owners: [{ ownerType: 'system' }], })); } + + async getAllUserProjectOwners(): Promise { + return []; + } } diff --git a/src/lib/features/project/project-owners-read-model.test.ts b/src/lib/features/project/project-owners-read-model.test.ts index 831310cc573e..3d9b8854ca4d 100644 --- a/src/lib/features/project/project-owners-read-model.test.ts +++ b/src/lib/features/project/project-owners-read-model.test.ts @@ -135,7 +135,7 @@ afterEach(async () => { describe('integration tests', () => { test('returns an empty object if there are no projects', async () => { - const owners = await readModel.getAllProjectOwners(); + const owners = await readModel.getProjectOwnersDictionary(); expect(owners).toStrictEqual({}); }); @@ -150,7 +150,7 @@ describe('integration tests', () => { projectId, ); - const owners = await readModel.getAllProjectOwners(); + const owners = await readModel.getProjectOwnersDictionary(); expect(owners).toMatchObject({ [projectId]: expect.arrayContaining([ expect.objectContaining({ name: 'Owner Name' }), @@ -168,7 +168,7 @@ describe('integration tests', () => { projectId, ); - const owners = await readModel.getAllProjectOwners(); + const owners = await readModel.getProjectOwnersDictionary(); expect(owners).toMatchObject({ [projectId]: [ @@ -201,7 +201,7 @@ describe('integration tests', () => { projectId, ); - const owners = await readModel.getAllProjectOwners(); + const owners = await readModel.getProjectOwnersDictionary(); expect(owners).toMatchObject({ [projectId]: [{ name: 'Owner Name' }], @@ -219,7 +219,7 @@ describe('integration tests', () => { projectId, ); - const owners = await readModel.getAllProjectOwners(); + const owners = await readModel.getProjectOwnersDictionary(); expect(owners).toMatchObject({ [projectId]: [ @@ -248,7 +248,7 @@ describe('integration tests', () => { projectId, ); - const owners = await readModel.getAllProjectOwners(); + const owners = await readModel.getProjectOwnersDictionary(); expect(owners).toMatchObject({ [projectId]: [ @@ -299,7 +299,7 @@ describe('integration tests', () => { projectId, ); - const owners = await readModel.getAllProjectOwners(); + const owners = await readModel.getProjectOwnersDictionary(); expect(owners).toMatchObject({ [projectId]: [ @@ -361,4 +361,96 @@ describe('integration tests', () => { { name: projectIdB, owners: [{ ownerType: 'user' }] }, ]); }); + + test('filters out system and group owners when getting all user project owners', async () => { + const createProject = async () => { + const id = randomId(); + return db.stores.projectStore.create({ + id, + name: id, + }); + }; + + const projectA = await createProject(); + const projectB = await createProject(); + const projectC = await createProject(); + await createProject(); // <- no owner + + await db.stores.accessStore.addUserToRole( + owner.id, + ownerRoleId, + projectA.id, + ); + + await db.stores.accessStore.addUserToRole( + owner2.id, + ownerRoleId, + projectB.id, + ); + + await db.stores.accessStore.addGroupToRole( + group.id, + ownerRoleId, + '', + projectC.id, + ); + + const userOwners = await readModel.getAllUserProjectOwners(); + userOwners.sort((a, b) => a.name.localeCompare(b.name)); + + expect(userOwners).toMatchObject([ + { + name: owner.name, + ownerType: 'user', + email: owner.email, + imageUrl: 'https://image-url-1', + }, + { + name: owner2.name, + ownerType: 'user', + email: owner2.email, + imageUrl: 'https://image-url-3', + }, + ]); + }); + + test('only returns projects listed in the projects input if provided', async () => { + const createProject = async () => { + const id = randomId(); + return db.stores.projectStore.create({ + id, + name: id, + }); + }; + + const projectA = await createProject(); + const projectB = await createProject(); + + await db.stores.accessStore.addUserToRole( + owner.id, + ownerRoleId, + projectA.id, + ); + + await db.stores.accessStore.addUserToRole( + owner2.id, + ownerRoleId, + projectB.id, + ); + + const noOwners = await readModel.getAllUserProjectOwners(new Set()); + expect(noOwners).toMatchObject([]); + + const onlyProjectA = await readModel.getAllUserProjectOwners( + new Set([projectA.id]), + ); + expect(onlyProjectA).toMatchObject([ + { + name: owner.name, + ownerType: 'user', + email: owner.email, + imageUrl: 'https://image-url-1', + }, + ]); + }); }); diff --git a/src/lib/features/project/project-owners-read-model.ts b/src/lib/features/project/project-owners-read-model.ts index 038b97a67a0f..3bb053f0b190 100644 --- a/src/lib/features/project/project-owners-read-model.ts +++ b/src/lib/features/project/project-owners-read-model.ts @@ -104,7 +104,7 @@ export class ProjectOwnersReadModel implements IProjectOwnersReadModel { return groupsDict; } - async getAllProjectOwners(): Promise { + async getProjectOwnersDictionary(): Promise { const ownerRole = await this.db(T.ROLES) .where({ name: RoleName.OWNER }) .first(); @@ -127,10 +127,33 @@ export class ProjectOwnersReadModel implements IProjectOwnersReadModel { return dict; } + async getAllUserProjectOwners( + projects?: Set, + ): Promise { + const allOwners = await this.getProjectOwnersDictionary(); + + const owners = projects + ? Object.entries(allOwners) + .filter(([projectId]) => projects.has(projectId)) + .map(([_, owners]) => owners) + : Object.values(allOwners); + + const ownersDict = owners.flat().reduce( + (acc, owner) => { + if (owner.ownerType === 'user') { + acc[owner.email || owner.name] = owner; + } + return acc; + }, + {} as Record, + ); + return Object.values(ownersDict); + } + async addOwners( projects: T[], ): Promise> { - const owners = await this.getAllProjectOwners(); + const owners = await this.getProjectOwnersDictionary(); return ProjectOwnersReadModel.addOwnerData(projects, owners); } diff --git a/src/lib/features/project/project-owners-read-model.type.ts b/src/lib/features/project/project-owners-read-model.type.ts index c26536648c1c..8aa9bab6a9fb 100644 --- a/src/lib/features/project/project-owners-read-model.type.ts +++ b/src/lib/features/project/project-owners-read-model.type.ts @@ -29,4 +29,8 @@ export interface IProjectOwnersReadModel { addOwners( projects: T[], ): Promise>; + + getAllUserProjectOwners( + projects?: Set, + ): Promise; } diff --git a/src/lib/openapi/spec/personal-dashboard-schema.ts b/src/lib/openapi/spec/personal-dashboard-schema.ts index 8277ab53c2bf..7c4040a64195 100644 --- a/src/lib/openapi/spec/personal-dashboard-schema.ts +++ b/src/lib/openapi/spec/personal-dashboard-schema.ts @@ -7,6 +7,35 @@ export const personalDashboardSchema = { additionalProperties: false, required: ['projects', 'flags'], properties: { + projectOwners: { + type: 'array', + description: + 'Users with the project owner role in Unleash. Only contains owners of projects that are visible to the user.', + items: { + type: 'object', + required: ['ownerType', 'name'], + properties: { + ownerType: { + type: 'string', + enum: ['user'], + }, + name: { + type: 'string', + example: 'User Name', + }, + imageUrl: { + type: 'string', + nullable: true, + example: 'https://example.com/image.jpg', + }, + email: { + type: 'string', + nullable: true, + example: 'user@example.com', + }, + }, + }, + }, projects: { type: 'array', items: { diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index d84327a0be11..9c6ef7845e72 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -407,7 +407,7 @@ export const createServices = ( onboardingService.listen(); const personalDashboardService = db - ? createPersonalDashboardService(db, config) + ? createPersonalDashboardService(db, config, stores) : createFakePersonalDashboardService(config); return {