From 2bf995e7310aca85b512d22231f693fef1d5f06b Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Mon, 25 Sep 2023 15:50:44 +0300 Subject: [PATCH] feat: context/segment usage private (#4826) --- .../component/segments/SegmentFormStepOne.tsx | 1 - .../segments/SegmentProjectAlert.tsx | 16 ++++++--- .../createExportImportService.ts | 14 ++++++-- .../features/segment/createSegmentService.ts | 9 +++++ .../features/segment/segment-controller.ts | 6 ++-- src/lib/routes/admin-api/context.ts | 6 +++- src/lib/segments/segment-service-interface.ts | 2 +- src/lib/services/context-service.ts | 36 ++++++++++++++++++- src/lib/services/index.ts | 14 +++++--- src/lib/services/segment-service.ts | 27 ++++++++++++-- .../e2e/services/access-service.e2e.test.ts | 7 +++- .../services/api-token-service.e2e.test.ts | 7 +++- .../feature-toggle-service-v2.e2e.test.ts | 12 ++++--- .../e2e/services/playground-service.test.ts | 12 ++++--- .../project-health-service.e2e.test.ts | 7 +++- .../e2e/services/project-service.e2e.test.ts | 7 +++- 16 files changed, 150 insertions(+), 33 deletions(-) diff --git a/frontend/src/component/segments/SegmentFormStepOne.tsx b/frontend/src/component/segments/SegmentFormStepOne.tsx index 7f2bd4ab38..8101053ec6 100644 --- a/frontend/src/component/segments/SegmentFormStepOne.tsx +++ b/frontend/src/component/segments/SegmentFormStepOne.tsx @@ -77,7 +77,6 @@ export const SegmentFormStepOne: React.FC = ({ const projectsUsed = new Set( strategies.map(({ projectId }) => projectId!).filter(Boolean) ); - const availableProjects = projects.filter( ({ id }) => !projectsUsed.size || diff --git a/frontend/src/component/segments/SegmentProjectAlert.tsx b/frontend/src/component/segments/SegmentProjectAlert.tsx index da87d4ce50..7c4a255d42 100644 --- a/frontend/src/component/segments/SegmentProjectAlert.tsx +++ b/frontend/src/component/segments/SegmentProjectAlert.tsx @@ -5,6 +5,7 @@ import { IFeatureStrategy } from 'interfaces/strategy'; import { Link } from 'react-router-dom'; import { formatStrategyName } from 'utils/strategyNames'; import { usePlausibleTracker } from 'hooks/usePlausibleTracker'; +import { ConditionallyRender } from 'component/common/ConditionallyRender/ConditionallyRender'; const StyledUl = styled('ul')(({ theme }) => ({ listStyle: 'none', @@ -37,7 +38,6 @@ export const SegmentProjectAlert = ({ }, }); }; - const projectList = ( {Array.from(projectsUsed).map(projectId => ( @@ -78,11 +78,19 @@ export const SegmentProjectAlert = ({ ); - if (projectsUsed.length > 1) { + if (projectsUsed.length > 0) { return ( - You can't specify a project for this segment because it is used - in multiple projects: + 1} + show={ + + You can't specify a project for this segment because + it is used in multiple projects: + + } + elseShow={Usage of this segment:} + /> {projectList} ); diff --git a/src/lib/features/export-import-toggles/createExportImportService.ts b/src/lib/features/export-import-toggles/createExportImportService.ts index 3937ff76d9..c06b619308 100644 --- a/src/lib/features/export-import-toggles/createExportImportService.ts +++ b/src/lib/features/export-import-toggles/createExportImportService.ts @@ -38,11 +38,15 @@ import FakeFeatureStrategiesStore from '../../../test/fixtures/fake-feature-stra import FakeFeatureEnvironmentStore from '../../../test/fixtures/fake-feature-environment-store'; import FakeStrategiesStore from '../../../test/fixtures/fake-strategies-store'; import EventStore from '../../db/event-store'; +import { + createFakePrivateProjectChecker, + createPrivateProjectChecker, +} from '../private-project/createPrivateProjectChecker'; export const createFakeExportImportTogglesService = ( config: IUnleashConfig, ): ExportImportService => { - const { getLogger } = config; + const { getLogger, flagResolver } = config; const importTogglesStore = {} as ImportTogglesStore; const featureToggleStore = new FakeFeatureToggleStore(); const tagStore = new FakeTagStore(); @@ -57,6 +61,7 @@ export const createFakeExportImportTogglesService = ( const featureEnvironmentStore = new FakeFeatureEnvironmentStore(); const accessService = createFakeAccessService(config); const featureToggleService = createFakeFeatureToggleService(config); + const privateProjectChecker = createFakePrivateProjectChecker(); const featureTagService = new FeatureTagService( { @@ -74,7 +79,8 @@ export const createFakeExportImportTogglesService = ( contextFieldStore, featureStrategiesStore, }, - { getLogger }, + { getLogger, flagResolver }, + privateProjectChecker, ); const strategyService = new StrategyService( { strategyStore, eventStore }, @@ -152,6 +158,7 @@ export const createExportImportTogglesService = ( const eventStore = new EventStore(db, getLogger); const accessService = createAccessService(db, config); const featureToggleService = createFeatureToggleService(db, config); + const privateProjectChecker = createPrivateProjectChecker(db, config); const featureTagService = new FeatureTagService( { @@ -169,7 +176,8 @@ export const createExportImportTogglesService = ( contextFieldStore, featureStrategiesStore, }, - { getLogger }, + { getLogger, flagResolver }, + privateProjectChecker, ); const strategyService = new StrategyService( { strategyStore, eventStore }, diff --git a/src/lib/features/segment/createSegmentService.ts b/src/lib/features/segment/createSegmentService.ts index bd3d441f24..5306d3f1b5 100644 --- a/src/lib/features/segment/createSegmentService.ts +++ b/src/lib/features/segment/createSegmentService.ts @@ -11,6 +11,10 @@ import { createChangeRequestAccessReadModel, createFakeChangeRequestAccessService, } from '../change-request-access-service/createChangeRequestAccessReadModel'; +import { + createFakePrivateProjectChecker, + createPrivateProjectChecker, +} from '../private-project/createPrivateProjectChecker'; export const createSegmentService = ( db: Db, @@ -34,11 +38,13 @@ export const createSegmentService = ( db, config, ); + const privateProjectChecker = createPrivateProjectChecker(db, config); return new SegmentService( { segmentStore, featureStrategiesStore, eventStore }, changeRequestAccessReadModel, config, + privateProjectChecker, ); }; @@ -50,9 +56,12 @@ export const createFakeSegmentService = ( const featureStrategiesStore = new FakeFeatureStrategiesStore(); const changeRequestAccessReadModel = createFakeChangeRequestAccessService(); + const privateProjectChecker = createFakePrivateProjectChecker(); + return new SegmentService( { segmentStore, featureStrategiesStore, eventStore }, changeRequestAccessReadModel, config, + privateProjectChecker, ); }; diff --git a/src/lib/features/segment/segment-controller.ts b/src/lib/features/segment/segment-controller.ts index 23d8e7ab4d..ef9d259995 100644 --- a/src/lib/features/segment/segment-controller.ts +++ b/src/lib/features/segment/segment-controller.ts @@ -347,7 +347,8 @@ export class SegmentsController extends Controller { res: Response, ): Promise { const { id } = req.params; - const strategies = await this.segmentService.getStrategies(id); + const { user } = req; + const strategies = await this.segmentService.getStrategies(id, user.id); // Remove unnecessary IFeatureStrategy fields from the response. const segmentStrategies = strategies.map((strategy) => ({ @@ -368,7 +369,8 @@ export class SegmentsController extends Controller { res: Response, ): Promise { const id = Number(req.params.id); - const strategies = await this.segmentService.getStrategies(id); + const { user } = req; + const strategies = await this.segmentService.getStrategies(id, user.id); if (strategies.length > 0) { res.status(409).send(); diff --git a/src/lib/routes/admin-api/context.ts b/src/lib/routes/admin-api/context.ts index d3508c5812..5a39ed2905 100644 --- a/src/lib/routes/admin-api/context.ts +++ b/src/lib/routes/admin-api/context.ts @@ -301,8 +301,12 @@ export class ContextController extends Controller { res: Response, ): Promise { const { contextField } = req.params; + const { user } = req; const contextFields = - await this.contextService.getStrategiesByContextField(contextField); + await this.contextService.getStrategiesByContextField( + contextField, + user.id, + ); this.openApiService.respondWithValidation( 200, diff --git a/src/lib/segments/segment-service-interface.ts b/src/lib/segments/segment-service-interface.ts index 40a7c2a226..3d469d0bd0 100644 --- a/src/lib/segments/segment-service-interface.ts +++ b/src/lib/segments/segment-service-interface.ts @@ -13,7 +13,7 @@ export interface ISegmentService { get(id: number): Promise; - getStrategies(id: number): Promise; + getStrategies(id: number, userId: number): Promise; validateName(name: string): Promise; diff --git a/src/lib/services/context-service.ts b/src/lib/services/context-service.ts index e382478791..775bd893ce 100644 --- a/src/lib/services/context-service.ts +++ b/src/lib/services/context-service.ts @@ -9,6 +9,8 @@ import { IProjectStore } from '../types/stores/project-store'; import { IFeatureStrategiesStore, IUnleashStores } from '../types/stores'; import { IUnleashConfig } from '../types/option'; import { ContextFieldStrategiesSchema } from '../openapi/spec/context-field-strategies-schema'; +import { IFeatureStrategy, IFlagResolver } from '../types'; +import { IPrivateProjectChecker } from '../features/private-project/privateProjectCheckerType'; const { contextSchema, nameSchema } = require('./context-schema'); const NameExistsError = require('../error/name-exists-error'); @@ -30,6 +32,10 @@ class ContextService { private logger: Logger; + private flagResolver: IFlagResolver; + + private privateProjectChecker: IPrivateProjectChecker; + constructor( { projectStore, @@ -43,10 +49,16 @@ class ContextService { | 'contextFieldStore' | 'featureStrategiesStore' >, - { getLogger }: Pick, + { + getLogger, + flagResolver, + }: Pick, + privateProjectChecker: IPrivateProjectChecker, ) { + this.privateProjectChecker = privateProjectChecker; this.projectStore = projectStore; this.eventStore = eventStore; + this.flagResolver = flagResolver; this.contextFieldStore = contextFieldStore; this.featureStrategiesStore = featureStrategiesStore; this.logger = getLogger('services/context-service.js'); @@ -62,9 +74,31 @@ class ContextService { async getStrategiesByContextField( name: string, + userId: number, ): Promise { const strategies = await this.featureStrategiesStore.getStrategiesByContextField(name); + if (this.flagResolver.isEnabled('privateProjects')) { + const accessibleProjects = + await this.privateProjectChecker.getUserAccessibleProjects( + userId, + ); + if (accessibleProjects.mode === 'all') { + return this.mapStrategies(strategies); + } else { + return this.mapStrategies( + strategies.filter((strategy) => + accessibleProjects.projects.includes( + strategy.projectId, + ), + ), + ); + } + } + return this.mapStrategies(strategies); + } + + private mapStrategies(strategies: IFeatureStrategy[]) { return { strategies: strategies.map((strategy) => ({ id: strategy.id, diff --git a/src/lib/services/index.ts b/src/lib/services/index.ts index 6a8899e998..d4818a12bc 100644 --- a/src/lib/services/index.ts +++ b/src/lib/services/index.ts @@ -169,7 +169,15 @@ export const createServices = ( config, lastSeenService, ); - const contextService = new ContextService(stores, config); + const privateProjectChecker = db + ? createPrivateProjectChecker(db, config) + : createFakePrivateProjectChecker(); + + const contextService = new ContextService( + stores, + config, + privateProjectChecker, + ); const emailService = new EmailService(config.email, config.getLogger); const eventService = new EventService(stores, config); const featureTypeService = new FeatureTypeService(stores, config); @@ -201,11 +209,9 @@ export const createServices = ( stores, changeRequestAccessReadModel, config, + privateProjectChecker, ); - const privateProjectChecker = db - ? createPrivateProjectChecker(db, config) - : createFakePrivateProjectChecker(); const clientInstanceService = new ClientInstanceService( stores, config, diff --git a/src/lib/services/segment-service.ts b/src/lib/services/segment-service.ts index 3474e55d22..95fe027846 100644 --- a/src/lib/services/segment-service.ts +++ b/src/lib/services/segment-service.ts @@ -22,6 +22,7 @@ import BadDataError from '../error/bad-data-error'; import { ISegmentService } from '../segments/segment-service-interface'; import { PermissionError } from '../error'; import { IChangeRequestAccessReadModel } from '../features/change-request-access-service/change-request-access-read-model'; +import { IPrivateProjectChecker } from '../features/private-project/privateProjectCheckerType'; export class SegmentService implements ISegmentService { private logger: Logger; @@ -38,6 +39,8 @@ export class SegmentService implements ISegmentService { private flagResolver: IFlagResolver; + private privateProjectChecker: IPrivateProjectChecker; + constructor( { segmentStore, @@ -49,11 +52,13 @@ export class SegmentService implements ISegmentService { >, changeRequestAccessReadModel: IChangeRequestAccessReadModel, config: IUnleashConfig, + privateProjectChecker: IPrivateProjectChecker, ) { this.segmentStore = segmentStore; this.featureStrategiesStore = featureStrategiesStore; this.eventStore = eventStore; this.changeRequestAccessReadModel = changeRequestAccessReadModel; + this.privateProjectChecker = privateProjectChecker; this.logger = config.getLogger('services/segment-service.ts'); this.flagResolver = config.flagResolver; this.config = config; @@ -81,8 +86,26 @@ export class SegmentService implements ISegmentService { } // Used by unleash-enterprise. - async getStrategies(id: number): Promise { - return this.featureStrategiesStore.getStrategiesBySegment(id); + async getStrategies( + id: number, + userId: number, + ): Promise { + const strategies = + await this.featureStrategiesStore.getStrategiesBySegment(id); + if (this.flagResolver.isEnabled('privateProjects')) { + const accessibleProjects = + await this.privateProjectChecker.getUserAccessibleProjects( + userId, + ); + if (accessibleProjects.mode === 'all') { + return strategies; + } else { + return strategies.filter((strategy) => + accessibleProjects.projects.includes(strategy.projectId), + ); + } + } + return strategies; } async create( diff --git a/src/test/e2e/services/access-service.e2e.test.ts b/src/test/e2e/services/access-service.e2e.test.ts index 40119816a5..1557a975a2 100644 --- a/src/test/e2e/services/access-service.e2e.test.ts +++ b/src/test/e2e/services/access-service.e2e.test.ts @@ -253,7 +253,12 @@ beforeAll(async () => { featureToggleService = new FeatureToggleService( stores, config, - new SegmentService(stores, changeRequestAccessReadModel, config), + new SegmentService( + stores, + changeRequestAccessReadModel, + config, + privateProjectChecker, + ), accessService, changeRequestAccessReadModel, privateProjectChecker, 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 4f535c21c5..89ec5bd8ed 100644 --- a/src/test/e2e/services/api-token-service.e2e.test.ts +++ b/src/test/e2e/services/api-token-service.e2e.test.ts @@ -39,7 +39,12 @@ beforeAll(async () => { const featureToggleService = new FeatureToggleService( stores, config, - new SegmentService(stores, changeRequestAccessReadModel, config), + new SegmentService( + stores, + changeRequestAccessReadModel, + config, + privateProjectChecker, + ), accessService, changeRequestAccessReadModel, privateProjectChecker, diff --git a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts index 6bbf855073..766034be32 100644 --- a/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts +++ b/src/test/e2e/services/feature-toggle-service-v2.e2e.test.ts @@ -54,15 +54,17 @@ beforeAll(async () => { db.rawDatabase, accessService, ); - segmentService = new SegmentService( - stores, - changeRequestAccessReadModel, - config, - ); const privateProjectChecker = createPrivateProjectChecker( db.rawDatabase, config, ); + segmentService = new SegmentService( + stores, + changeRequestAccessReadModel, + config, + privateProjectChecker, + ); + service = new FeatureToggleService( stores, config, diff --git a/src/test/e2e/services/playground-service.test.ts b/src/test/e2e/services/playground-service.test.ts index 89b1dc0758..ecd653241b 100644 --- a/src/test/e2e/services/playground-service.test.ts +++ b/src/test/e2e/services/playground-service.test.ts @@ -43,15 +43,17 @@ beforeAll(async () => { db.rawDatabase, accessService, ); - segmentService = new SegmentService( - stores, - changeRequestAccessReadModel, - config, - ); const privateProjectChecker = createPrivateProjectChecker( db.rawDatabase, config, ); + segmentService = new SegmentService( + stores, + changeRequestAccessReadModel, + config, + privateProjectChecker, + ); + featureToggleService = new FeatureToggleService( stores, config, 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 ddc21e7ccd..c2969c2865 100644 --- a/src/test/e2e/services/project-health-service.e2e.test.ts +++ b/src/test/e2e/services/project-health-service.e2e.test.ts @@ -44,7 +44,12 @@ beforeAll(async () => { featureToggleService = new FeatureToggleService( stores, config, - new SegmentService(stores, changeRequestAccessReadModel, config), + new SegmentService( + stores, + changeRequestAccessReadModel, + config, + privateProjectChecker, + ), accessService, changeRequestAccessReadModel, privateProjectChecker, diff --git a/src/test/e2e/services/project-service.e2e.test.ts b/src/test/e2e/services/project-service.e2e.test.ts index 2c33e40af8..773f55bd69 100644 --- a/src/test/e2e/services/project-service.e2e.test.ts +++ b/src/test/e2e/services/project-service.e2e.test.ts @@ -65,7 +65,12 @@ beforeAll(async () => { featureToggleService = new FeatureToggleService( stores, config, - new SegmentService(stores, changeRequestAccessReadModel, config), + new SegmentService( + stores, + changeRequestAccessReadModel, + config, + privateProjectChecker, + ), accessService, changeRequestAccessReadModel, privateProjectChecker,