From 89163b87198975439fb865353122e3e1102c3fdf Mon Sep 17 00:00:00 2001 From: Fredrik Strand Oseberg Date: Wed, 18 Jan 2023 13:22:58 +0100 Subject: [PATCH] refactor/clean-up-get-project (#2925) This PR moves the getProjectOverview method out from the project health controller. It doesn't make sense that this method lives here anymore, as over time it has grown into method that relays all information about a single project. It makes more sense that this now lives on the root of the project api. Also removes unwanted duplication of getProjectOverview from the project-service and the project-health-service. --- .../routes/admin-api/project/health-report.ts | 43 +--------------- src/lib/routes/admin-api/project/index.ts | 41 +++++++++++++++ src/lib/services/index.ts | 12 ++--- src/lib/services/project-health-service.ts | 51 ++----------------- src/lib/services/project-service.ts | 16 +++++- .../api/admin/feature.custom-auth.e2e.test.ts | 2 +- .../__snapshots__/openapi.e2e.test.ts.snap | 2 +- .../e2e/services/access-service.e2e.test.ts | 4 ++ .../services/api-token-service.e2e.test.ts | 5 +- .../project-health-service.e2e.test.ts | 7 +-- .../e2e/services/project-service.e2e.test.ts | 5 ++ 11 files changed, 87 insertions(+), 101 deletions(-) diff --git a/src/lib/routes/admin-api/project/health-report.ts b/src/lib/routes/admin-api/project/health-report.ts index 906fbd6feb..bb2434e7c3 100644 --- a/src/lib/routes/admin-api/project/health-report.ts +++ b/src/lib/routes/admin-api/project/health-report.ts @@ -4,20 +4,15 @@ import { IUnleashServices } from '../../../types/services'; import { IUnleashConfig } from '../../../types/option'; import ProjectHealthService from '../../../services/project-health-service'; import { Logger } from '../../../logger'; -import { IArchivedQuery, IProjectParam } from '../../../types/model'; +import { IProjectParam } from '../../../types/model'; import { NONE } from '../../../types/permissions'; import { OpenApiService } from '../../../services/openapi-service'; import { createResponseSchema } from '../../../openapi/util/create-response-schema'; -import { - healthOverviewSchema, - HealthOverviewSchema, -} from '../../../openapi/spec/health-overview-schema'; import { serializeDates } from '../../../types/serialize-dates'; import { healthReportSchema, HealthReportSchema, } from '../../../openapi/spec/health-report-schema'; -import { IAuthRequest } from '../../unleash-types'; export default class ProjectHealthReport extends Controller { private projectHealthService: ProjectHealthService; @@ -38,22 +33,6 @@ export default class ProjectHealthReport extends Controller { this.projectHealthService = projectHealthService; this.openApiService = openApiService; - this.route({ - method: 'get', - path: '/:projectId', - handler: this.getProjectHealthOverview, - permission: NONE, - middleware: [ - openApiService.validPath({ - tags: ['Projects'], - operationId: 'getProjectHealthOverview', - responses: { - 200: createResponseSchema('healthOverviewSchema'), - }, - }), - ], - }); - this.route({ method: 'get', path: '/:projectId/health-report', @@ -71,26 +50,6 @@ export default class ProjectHealthReport extends Controller { }); } - async getProjectHealthOverview( - req: IAuthRequest, - res: Response, - ): Promise { - const { projectId } = req.params; - const { archived } = req.query; - const { user } = req; - const overview = await this.projectHealthService.getProjectOverview( - projectId, - archived, - user.id, - ); - this.openApiService.respondWithValidation( - 200, - res, - healthOverviewSchema.$id, - serializeDates(overview), - ); - } - async getProjectHealthReport( req: Request, res: Response, diff --git a/src/lib/routes/admin-api/project/index.ts b/src/lib/routes/admin-api/project/index.ts index e5a91fa7fa..dbeb54ce0c 100644 --- a/src/lib/routes/admin-api/project/index.ts +++ b/src/lib/routes/admin-api/project/index.ts @@ -16,6 +16,11 @@ import { OpenApiService } from '../../../services/openapi-service'; import { serializeDates } from '../../../types/serialize-dates'; import { createResponseSchema } from '../../../openapi/util/create-response-schema'; import { IAuthRequest } from '../../unleash-types'; +import { + HealthOverviewSchema, + healthOverviewSchema, +} from '../../../../lib/openapi'; +import { IArchivedQuery, IProjectParam } from '../../../types/model'; export default class ProjectApi extends Controller { private projectService: ProjectService; @@ -43,6 +48,22 @@ export default class ProjectApi extends Controller { ], }); + this.route({ + method: 'get', + path: '/:projectId', + handler: this.getProjectOverview, + permission: NONE, + middleware: [ + services.openApiService.validPath({ + tags: ['Projects'], + operationId: 'getProjectOverview', + responses: { + 200: createResponseSchema('healthOverviewSchema'), + }, + }), + ], + }); + this.use('/', new ProjectFeaturesController(config, services).router); this.use('/', new EnvironmentsController(config, services).router); this.use('/', new ProjectHealthReport(config, services).router); @@ -68,4 +89,24 @@ export default class ProjectApi extends Controller { { version: 1, projects: serializeDates(projects) }, ); } + + async getProjectOverview( + req: IAuthRequest, + res: Response, + ): Promise { + const { projectId } = req.params; + const { archived } = req.query; + const { user } = req; + const overview = await this.projectService.getProjectOverview( + projectId, + archived, + user.id, + ); + this.openApiService.respondWithValidation( + 200, + res, + healthOverviewSchema.$id, + serializeDates(overview), + ); + } } diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 0f4d2950f9..ef15fb68af 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -92,18 +92,18 @@ export const createServices = ( const environmentService = new EnvironmentService(stores, config); const featureTagService = new FeatureTagService(stores, config); const favoritesService = new FavoritesService(stores, config); - const projectHealthService = new ProjectHealthService( - stores, - config, - featureToggleServiceV2, - favoritesService, - ); const projectService = new ProjectService( stores, config, accessService, featureToggleServiceV2, groupService, + favoritesService, + ); + const projectHealthService = new ProjectHealthService( + stores, + config, + projectService, ); const userSplashService = new UserSplashService(stores, config); const openApiService = new OpenApiService(config); diff --git a/src/lib/services/project-health-service.ts b/src/lib/services/project-health-service.ts index 7bad3551ff..86ede7b754 100644 --- a/src/lib/services/project-health-service.ts +++ b/src/lib/services/project-health-service.ts @@ -6,15 +6,13 @@ import { IFeatureOverview, IProject, IProjectHealthReport, - IProjectOverview, } from '../types/model'; import { IFeatureToggleStore } from '../types/stores/feature-toggle-store'; import { IFeatureTypeStore } from '../types/stores/feature-type-store'; import { IProjectStore } from '../types/stores/project-store'; -import FeatureToggleService from './feature-toggle-service'; import { hoursToMilliseconds } from 'date-fns'; import Timer = NodeJS.Timer; -import { FavoritesService } from './favorites-service'; +import ProjectService from './project-service'; export default class ProjectHealthService { private logger: Logger; @@ -29,9 +27,7 @@ export default class ProjectHealthService { private healthRatingTimer: Timer; - private featureToggleService: FeatureToggleService; - - private favoritesService: FavoritesService; + private projectService: ProjectService; constructor( { @@ -43,8 +39,7 @@ export default class ProjectHealthService { 'projectStore' | 'featureTypeStore' | 'featureToggleStore' >, { getLogger }: Pick, - featureToggleService: FeatureToggleService, - favoritesService: FavoritesService, + projectService: ProjectService, ) { this.logger = getLogger('services/project-health-service.ts'); this.projectStore = projectStore; @@ -55,50 +50,14 @@ export default class ProjectHealthService { () => this.setHealthRating(), hoursToMilliseconds(1), ).unref(); - this.featureToggleService = featureToggleService; - this.favoritesService = favoritesService; - } - // TODO: duplicate from project-service. - async getProjectOverview( - projectId: string, - archived: boolean = false, - userId?: number, - ): Promise { - const project = await this.projectStore.get(projectId); - const environments = await this.projectStore.getEnvironmentsForProject( - projectId, - ); - const features = await this.featureToggleService.getFeatureOverview({ - projectId, - archived, - userId, - }); - const members = await this.projectStore.getMembersCountByProject( - projectId, - ); - - const favorite = await this.favoritesService.isFavoriteProject({ - project: projectId, - userId, - }); - return { - name: project.name, - description: project.description, - health: project.health, - favorite: favorite, - updatedAt: project.updatedAt, - environments, - features, - members, - version: 1, - }; + this.projectService = projectService; } async getProjectHealthReport( projectId: string, ): Promise { - const overview = await this.getProjectOverview( + const overview = await this.projectService.getProjectOverview( projectId, false, undefined, diff --git a/src/lib/services/project-service.ts b/src/lib/services/project-service.ts index e1d0cdc19a..37e6b6c4b7 100644 --- a/src/lib/services/project-service.ts +++ b/src/lib/services/project-service.ts @@ -46,6 +46,7 @@ import { IUserStore } from 'lib/types/stores/user-store'; import { arraysHaveSameItems } from '../util/arraysHaveSameItems'; import { GroupService } from './group-service'; import { IGroupModelWithProjectRole, IGroupRole } from 'lib/types/group'; +import { FavoritesService } from './favorites-service'; const getCreatedBy = (user: IUser) => user.email || user.username; @@ -80,6 +81,8 @@ export default class ProjectService { private userStore: IUserStore; + private favoritesService: FavoritesService; + constructor( { projectStore, @@ -105,6 +108,7 @@ export default class ProjectService { accessService: AccessService, featureToggleService: FeatureToggleService, groupService: GroupService, + favoriteService: FavoritesService, ) { this.store = projectStore; this.environmentStore = environmentStore; @@ -114,6 +118,7 @@ export default class ProjectService { this.featureToggleStore = featureToggleStore; this.featureTypeStore = featureTypeStore; this.featureToggleService = featureToggleService; + this.favoritesService = favoriteService; this.tagStore = featureTagStore; this.userStore = userStore; this.groupService = groupService; @@ -590,6 +595,7 @@ export default class ProjectService { async getProjectOverview( projectId: string, archived: boolean = false, + userId?: number, ): Promise { const project = await this.store.get(projectId); const environments = await this.store.getEnvironmentsForProject( @@ -598,13 +604,21 @@ export default class ProjectService { const features = await this.featureToggleService.getFeatureOverview({ projectId, archived, + userId, }); const members = await this.store.getMembersCountByProject(projectId); + + const favorite = await this.favoritesService.isFavoriteProject({ + project: projectId, + userId, + }); return { name: project.name, - environments, description: project.description, health: project.health, + favorite: favorite, + updatedAt: project.updatedAt, + environments, features, members, version: 1, diff --git a/src/test/e2e/api/admin/feature.custom-auth.e2e.test.ts b/src/test/e2e/api/admin/feature.custom-auth.e2e.test.ts index f51767033f..f5e05d59f4 100644 --- a/src/test/e2e/api/admin/feature.custom-auth.e2e.test.ts +++ b/src/test/e2e/api/admin/feature.custom-auth.e2e.test.ts @@ -23,7 +23,7 @@ test('should require authenticated user', async () => { const preHook = (app) => { app.use('/api/admin/', (req, res) => res - .status('401') + .status(401) .json( new AuthenticationRequired({ path: '/auth/demo/login', diff --git a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap index 900a464357..b63cebc297 100644 --- a/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap +++ b/src/test/e2e/api/openapi/__snapshots__/openapi.e2e.test.ts.snap @@ -5291,7 +5291,7 @@ If the provided project does not exist, the list of events will be empty.", }, "/api/admin/projects/{projectId}": { "get": { - "operationId": "getProjectHealthOverview", + "operationId": "getProjectOverview", "parameters": [ { "in": "path", diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 167a793f48..4c1aaad265 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -14,12 +14,14 @@ import { DEFAULT_PROJECT } from '../../../lib/types/project'; import { ALL_PROJECTS } from '../../../lib/util/constants'; import { SegmentService } from '../../../lib/services/segment-service'; import { GroupService } from '../../../lib/services/group-service'; +import { FavoritesService } from '../../../lib/services'; let db: ITestDb; let stores: IUnleashStores; let accessService; let groupService; let featureToggleService; +let favoritesService; let projectService; let editorUser; let superUser; @@ -220,12 +222,14 @@ beforeAll(async () => { new SegmentService(stores, config), accessService, ); + favoritesService = new FavoritesService(stores, config); projectService = new ProjectService( stores, config, accessService, featureToggleService, groupService, + favoritesService, ); editorUser = await createUserEditorAccess('Bob Test', 'bob@getunleash.io'); diff --git a/src/test/e2e/services/api-token-service.e2e.test.ts b/src/test/e2e/services/api-token-service.e2e.test.ts index 094195edbc..e7702f60ff 100644 --- a/src/test/e2e/services/api-token-service.e2e.test.ts +++ b/src/test/e2e/services/api-token-service.e2e.test.ts @@ -10,11 +10,13 @@ import FeatureToggleService from '../../../lib/services/feature-toggle-service'; import { AccessService } from '../../../lib/services/access-service'; import { SegmentService } from '../../../lib/services/segment-service'; import { GroupService } from '../../../lib/services/group-service'; +import { FavoritesService } from '../../../lib/services'; let db; let stores; let apiTokenService: ApiTokenService; let projectService: ProjectService; +let favoritesService: FavoritesService; beforeAll(async () => { const config = createTestConfig({ @@ -39,13 +41,14 @@ beforeAll(async () => { name: 'Some Name', email: 'test@getunleash.io', }); - + favoritesService = new FavoritesService(stores, config); projectService = new ProjectService( stores, config, accessService, featureToggleService, groupService, + favoritesService, ); await projectService.createProject(project, user); diff --git a/src/test/e2e/services/project-health-service.e2e.test.ts b/src/test/e2e/services/project-health-service.e2e.test.ts index a445420ba3..1ba3551ca0 100644 --- a/src/test/e2e/services/project-health-service.e2e.test.ts +++ b/src/test/e2e/services/project-health-service.e2e.test.ts @@ -37,19 +37,20 @@ beforeAll(async () => { new SegmentService(stores, config), accessService, ); + favoritesService = new FavoritesService(stores, config); + projectService = new ProjectService( stores, config, accessService, featureToggleService, groupService, + favoritesService, ); - favoritesService = new FavoritesService(stores, config); projectHealthService = new ProjectHealthService( stores, config, - featureToggleService, - favoritesService, + projectService, ); }); diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index 85fcb07398..f455477997 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -11,6 +11,7 @@ import EnvironmentService from '../../../lib/services/environment-service'; import IncompatibleProjectError from '../../../lib/error/incompatible-project-error'; import { SegmentService } from '../../../lib/services/segment-service'; import { GroupService } from '../../../lib/services/group-service'; +import { FavoritesService } from '../../../lib/services'; let stores; let db: ITestDb; @@ -20,6 +21,7 @@ let groupService: GroupService; let accessService: AccessService; let environmentService: EnvironmentService; let featureToggleService: FeatureToggleService; +let favoritesService: FavoritesService; let user; beforeAll(async () => { @@ -42,6 +44,8 @@ beforeAll(async () => { new SegmentService(stores, config), accessService, ); + + favoritesService = new FavoritesService(stores, config); environmentService = new EnvironmentService(stores, config); projectService = new ProjectService( stores, @@ -49,6 +53,7 @@ beforeAll(async () => { accessService, featureToggleService, groupService, + favoritesService, ); });