From f45454fbfd8798337c9fb4ca65b56725c5b932a0 Mon Sep 17 00:00:00 2001 From: Thomas Heartman Date: Wed, 8 Nov 2023 14:50:12 +0100 Subject: [PATCH] refactor: extract segment usage read model (#5301) This PR adds a way to tell if a specific segment is being used in any active change requests. It's the first step towards preventing segments that are being used in change requests from being deleted. It does that by checking the db for any unclosed CRs and using those CR ids to look for "addStrategy" and "updateStrategy" events in the cr events table. ## Upcoming PRs This only puts in a way to detect it, but doesn't add that to anything. That'll be in an upcoming iteration. --- ...e-request-segment-usage-read-model.test.ts | 135 ++++++++++++++++++ ...change-request-segment-usage-read-model.ts | 3 + ...reateChangeRequestSegmentUsageReadModel.ts | 15 ++ ...change-request-segment-usage-read-model.ts | 16 +++ ...change-request-segment-usage-read-model.ts | 30 ++++ 5 files changed, 199 insertions(+) create mode 100644 src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.test.ts create mode 100644 src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.ts create mode 100644 src/lib/features/change-request-segment-usage-read-model.ts/createChangeRequestSegmentUsageReadModel.ts create mode 100644 src/lib/features/change-request-segment-usage-read-model.ts/fake-change-request-segment-usage-read-model.ts create mode 100644 src/lib/features/change-request-segment-usage-read-model.ts/sql-change-request-segment-usage-read-model.ts diff --git a/src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.test.ts b/src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.test.ts new file mode 100644 index 0000000000..cb9a735ead --- /dev/null +++ b/src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.test.ts @@ -0,0 +1,135 @@ +import { IUser } from 'lib/server-impl'; +import dbInit, { ITestDb } from '../../../test/e2e/helpers/database-init'; +import getLogger from '../../../test/fixtures/no-logger'; +import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model'; +import { createChangeRequestSegmentUsageModel } from './createChangeRequestSegmentUsageReadModel'; +import { randomId } from '../../../lib/util'; + +let db: ITestDb; +let user: IUser; + +const CR_ID = 123456; +const FLAG_NAME = 'crarm-test-flag'; + +let readModel: IChangeRequestSegmentUsageReadModel; + +beforeAll(async () => { + db = await dbInit('change_request_access_read_model_serial', getLogger); + + user = await db.stores.userStore.insert({ + username: 'cr-creator', + }); + + readModel = createChangeRequestSegmentUsageModel(db.rawDatabase); + + await db.stores.featureToggleStore.create('default', { + name: FLAG_NAME, + }); +}); + +afterAll(async () => { + await db.destroy(); +}); + +afterEach(async () => { + await db.rawDatabase.table('change_requests').where('id', CR_ID).delete(); + await db.rawDatabase + .table('change_request_events') + .where('change_request_id', CR_ID) + .delete(); +}); + +const createCR = async (state) => { + await db.rawDatabase.table('change_requests').insert({ + id: CR_ID, + environment: 'default', + state, + project: 'default', + created_by: user.id, + created_at: '2023-01-01 00:00:00', + min_approvals: 1, + title: 'My change request', + }); +}; + +const addChangeRequestChange = async (flagName, action, change) => { + await db.rawDatabase.table('change_request_events').insert({ + feature: flagName, + action, + payload: change, + created_at: '2023-01-01 00:01:00', + change_request_id: CR_ID, + created_by: user.id, + }); +}; + +const addStrategyToCr = async (segmentId: number, flagName: string) => { + await addChangeRequestChange(flagName, 'addStrategy', { + name: 'flexibleRollout', + title: '', + disabled: false, + segments: [segmentId], + variants: [], + parameters: { + groupId: flagName, + rollout: '100', + stickiness: 'default', + }, + constraints: [], + }); +}; + +const updateStrategyInCr = async ( + strategyId: string, + segmentId: number, + flagName: string, +) => { + await addChangeRequestChange(flagName, 'updateStrategy', { + id: strategyId, + name: 'flexibleRollout', + title: '', + disabled: false, + segments: [segmentId], + variants: [], + parameters: { + groupId: flagName, + rollout: '100', + stickiness: 'default', + }, + constraints: [], + }); +}; + +describe.each([ + [ + 'updateStrategy', + (segmentId: number) => + updateStrategyInCr(randomId(), segmentId, FLAG_NAME), + ], + [ + 'addStrategy', + (segmentId: number) => addStrategyToCr(segmentId, FLAG_NAME), + ], +])('Should handle %s changes correctly', (_, addOrUpdateStrategy) => { + test.each([ + ['Draft', true], + ['In Review', true], + ['Scheduled', true], + ['Approved', true], + ['Rejected', false], + ['Cancelled', false], + ['Applied', false], + ])( + 'Changes in %s CRs should make it %s', + async (state, expectedOutcome) => { + await createCR(state); + + const segmentId = 3; + await addOrUpdateStrategy(segmentId); + + expect( + await readModel.isSegmentUsedInActiveChangeRequests(segmentId), + ).toBe(expectedOutcome); + }, + ); +}); diff --git a/src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.ts b/src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.ts new file mode 100644 index 0000000000..08ba2f3217 --- /dev/null +++ b/src/lib/features/change-request-segment-usage-read-model.ts/change-request-segment-usage-read-model.ts @@ -0,0 +1,3 @@ +export interface IChangeRequestSegmentUsageReadModel { + isSegmentUsedInActiveChangeRequests(segmentId: number): Promise; +} diff --git a/src/lib/features/change-request-segment-usage-read-model.ts/createChangeRequestSegmentUsageReadModel.ts b/src/lib/features/change-request-segment-usage-read-model.ts/createChangeRequestSegmentUsageReadModel.ts new file mode 100644 index 0000000000..6865a20ae1 --- /dev/null +++ b/src/lib/features/change-request-segment-usage-read-model.ts/createChangeRequestSegmentUsageReadModel.ts @@ -0,0 +1,15 @@ +import { Db } from 'lib/server-impl'; +import { ChangeRequestSegmentUsageReadModel } from './sql-change-request-segment-usage-read-model'; +import { FakeChangeRequestSegmentUsageReadModel } from './fake-change-request-segment-usage-read-model'; +import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model'; + +export const createChangeRequestSegmentUsageModel = ( + db: Db, +): IChangeRequestSegmentUsageReadModel => { + return new ChangeRequestSegmentUsageReadModel(db); +}; + +export const createFakeChangeRequestAccessService = + (): IChangeRequestSegmentUsageReadModel => { + return new FakeChangeRequestSegmentUsageReadModel(); + }; diff --git a/src/lib/features/change-request-segment-usage-read-model.ts/fake-change-request-segment-usage-read-model.ts b/src/lib/features/change-request-segment-usage-read-model.ts/fake-change-request-segment-usage-read-model.ts new file mode 100644 index 0000000000..d5cb25554a --- /dev/null +++ b/src/lib/features/change-request-segment-usage-read-model.ts/fake-change-request-segment-usage-read-model.ts @@ -0,0 +1,16 @@ +import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model'; + +export class FakeChangeRequestSegmentUsageReadModel + implements IChangeRequestSegmentUsageReadModel +{ + private isSegmentUsedInActiveChangeRequestsValue: boolean; + + constructor(isSegmentUsedInActiveChangeRequests = false) { + this.isSegmentUsedInActiveChangeRequestsValue = + isSegmentUsedInActiveChangeRequests; + } + + public async isSegmentUsedInActiveChangeRequests(): Promise { + return this.isSegmentUsedInActiveChangeRequestsValue; + } +} diff --git a/src/lib/features/change-request-segment-usage-read-model.ts/sql-change-request-segment-usage-read-model.ts b/src/lib/features/change-request-segment-usage-read-model.ts/sql-change-request-segment-usage-read-model.ts new file mode 100644 index 0000000000..47dbf632ae --- /dev/null +++ b/src/lib/features/change-request-segment-usage-read-model.ts/sql-change-request-segment-usage-read-model.ts @@ -0,0 +1,30 @@ +import { Db } from '../../db/db'; +import { IChangeRequestSegmentUsageReadModel } from './change-request-segment-usage-read-model'; + +export class ChangeRequestSegmentUsageReadModel + implements IChangeRequestSegmentUsageReadModel +{ + private db: Db; + + constructor(db: Db) { + this.db = db; + } + + public async isSegmentUsedInActiveChangeRequests( + segmentId: number, + ): Promise { + const result = await this.db.raw( + `SELECT events.* + FROM change_request_events events + JOIN change_requests cr ON events.change_request_id = cr.id + WHERE cr.state IN ('Draft', 'In Review', 'Scheduled', 'Approved') + AND events.action IN ('updateStrategy', 'addStrategy');`, + ); + + const isUsed = result.rows.some((row) => + row.payload?.segments?.includes(segmentId), + ); + + return isUsed; + } +}