From 5e45ec25e933f54a7ae44d75892ebfd76f3cf89f Mon Sep 17 00:00:00 2001 From: andreas-unleash Date: Fri, 14 Jul 2023 12:11:32 +0300 Subject: [PATCH] Revert "Feat/add strategy update event on strategy ordering (#4234)" (#4243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 16e3799b9a399d79b2a025eaa4b2d5957a012349. ## About the changes Closes # ### Important files ## Discussion points --- src/lib/openapi/spec/event-schema.ts | 16 +--- .../admin-api/project/project-features.ts | 18 ++--- src/lib/services/feature-toggle-service.ts | 75 ++++--------------- .../api/admin/project/features.e2e.test.ts | 67 ----------------- 4 files changed, 21 insertions(+), 155 deletions(-) diff --git a/src/lib/openapi/spec/event-schema.ts b/src/lib/openapi/spec/event-schema.ts index 5af07bde2d..9dd6cac18e 100644 --- a/src/lib/openapi/spec/event-schema.ts +++ b/src/lib/openapi/spec/event-schema.ts @@ -12,35 +12,29 @@ const eventDataSchema = { description: 'Name of the feature toggle/strategy/environment that this event relates to', example: 'my.first.toggle', - nullable: true, }, description: { type: 'string', description: 'The description of the object this event relates to', example: 'Toggle description', - nullable: true, }, type: { type: 'string', - nullable: true, description: 'If this event relates to a feature toggle, the type of feature toggle.', example: 'release', }, project: { - nullable: true, type: 'string', description: 'The project this event relates to', example: 'default', }, stale: { - nullable: true, description: 'Is the feature toggle this event relates to stale', type: 'boolean', example: true, }, variants: { - nullable: true, description: 'Variants configured for this toggle', type: 'array', items: { @@ -48,7 +42,6 @@ const eventDataSchema = { }, }, createdAt: { - nullable: true, type: 'string', format: 'date-time', description: @@ -56,11 +49,11 @@ const eventDataSchema = { example: '2023-07-05T12:56:00.000Z', }, lastSeenAt: { - nullable: true, type: 'string', format: 'date-time', description: 'The time the feature was last seen', example: '2023-07-05T12:56:00.000Z', + nullable: true, }, impressionData: { description: @@ -134,12 +127,7 @@ export const eventSchema = { 'The name of the feature toggle the event relates to, if applicable.', example: 'my.first.feature', }, - data: { - ...eventDataSchema, - description: - "Data relating to the current state of the event's subject.", - nullable: true, - }, + data: eventDataSchema, preData: { ...eventDataSchema, description: diff --git a/src/lib/routes/admin-api/project/project-features.ts b/src/lib/routes/admin-api/project/project-features.ts index 0d933dcfe1..f342330225 100644 --- a/src/lib/routes/admin-api/project/project-features.ts +++ b/src/lib/routes/admin-api/project/project-features.ts @@ -936,7 +936,7 @@ export default class ProjectFeaturesController extends Controller { } async setStrategiesSortOrder( - req: IAuthRequest< + req: Request< FeatureStrategyParams, any, SetStrategySortOrderSchema, @@ -944,18 +944,10 @@ export default class ProjectFeaturesController extends Controller { >, res: Response, ): Promise { - const { featureName, projectId: project, environment } = req.params; - const createdBy = extractUsername(req); - await this.startTransaction(async (tx) => - this.transactionalFeatureToggleService( - tx, - ).updateStrategiesSortOrder( - featureName, - environment, - project, - createdBy, - req.body, - ), + const { featureName } = req.params; + await this.featureService.updateStrategiesSortOrder( + featureName, + req.body, ); res.status(200).send(); diff --git a/src/lib/services/feature-toggle-service.ts b/src/lib/services/feature-toggle-service.ts index 905ebfca75..987542c13b 100644 --- a/src/lib/services/feature-toggle-service.ts +++ b/src/lib/services/feature-toggle-service.ts @@ -369,7 +369,7 @@ class FeatureToggleService { featureStrategy: IFeatureStrategy, segments: ISegment[] = [], ): Saved { - const result: Saved = { + return { id: featureStrategy.id, name: featureStrategy.strategyName, title: featureStrategy.title, @@ -378,56 +378,16 @@ class FeatureToggleService { parameters: featureStrategy.parameters, segments: segments.map((segment) => segment.id) ?? [], }; - - if (this.flagResolver.isEnabled('strategyVariant')) { - result.sortOrder = featureStrategy.sortOrder; - } - return result; } async updateStrategiesSortOrder( featureName: string, - environment: string, - project: string, - createdBy: string, sortOrders: SetStrategySortOrderSchema, ): Promise> { await Promise.all( - sortOrders.map(async ({ id, sortOrder }) => { - const strategyToUpdate = - await this.featureStrategiesStore.getStrategyById(id); - await this.featureStrategiesStore.updateSortOrder( - id, - sortOrder, - ); - const updatedStrategy = - await this.featureStrategiesStore.getStrategyById(id); - - const tags = await this.tagStore.getAllTagsForFeature( - featureName, - ); - const segments = await this.segmentService.getByStrategy( - strategyToUpdate.id, - ); - const strategy = this.featureStrategyToPublic( - updatedStrategy, - segments, - ); - await this.eventStore.store( - new FeatureStrategyUpdateEvent({ - featureName, - environment, - project, - createdBy, - preData: this.featureStrategyToPublic( - strategyToUpdate, - segments, - ), - data: strategy, - tags: tags, - }), - ); - }), + sortOrders.map(async ({ id, sortOrder }) => + this.featureStrategiesStore.updateSortOrder(id, sortOrder), + ), ); } @@ -512,31 +472,24 @@ class FeatureToggleService { ); } + const tags = await this.tagStore.getAllTagsForFeature(featureName); const segments = await this.segmentService.getByStrategy( newFeatureStrategy.id, ); - const strategy = this.featureStrategyToPublic( newFeatureStrategy, segments, ); - - if (this.flagResolver.isEnabled('strategyVariant')) { - const tags = await this.tagStore.getAllTagsForFeature( + await this.eventStore.store( + new FeatureStrategyAddEvent({ + project: projectId, featureName, - ); - - await this.eventStore.store( - new FeatureStrategyAddEvent({ - project: projectId, - featureName, - createdBy, - environment, - data: strategy, - tags, - }), - ); - } + createdBy, + environment, + data: strategy, + tags, + }), + ); return strategy; } catch (e) { if (e.code === FOREIGN_KEY_VIOLATION) { diff --git a/src/test/e2e/api/admin/project/features.e2e.test.ts b/src/test/e2e/api/admin/project/features.e2e.test.ts index 7ef392ae88..441c138a30 100644 --- a/src/test/e2e/api/admin/project/features.e2e.test.ts +++ b/src/test/e2e/api/admin/project/features.e2e.test.ts @@ -26,10 +26,6 @@ import { v4 as uuidv4 } from 'uuid'; import supertest from 'supertest'; import { randomId } from '../../../../../lib/util/random-id'; import { DEFAULT_PROJECT } from '../../../../../lib/types'; -import { - FeatureStrategySchema, - SetStrategySortOrderSchema, -} from '../../../../../lib/openapi'; let app: IUnleashTest; let db: ITestDb; @@ -3231,66 +3227,3 @@ test('Enabling a feature environment should add the default strategy when only d expect(res.body.strategies[1].disabled).toBeFalsy(); }); }); - -test('Updating feature strategy sort-order should trigger a FeatureStrategyUpdatedEvent when strategyVariant is true', async () => { - app = await setupAppWithCustomConfig( - db.stores, - { - experimental: { - flags: { - strictSchemaValidation: true, - strategyVariant: true, - }, - }, - }, - db.rawDatabase, - ); - - const envName = 'sort-order-within-environment-strategyVariant'; - const featureName = 'feature.sort.order.event.list'; - - await db.stores.environmentStore.create({ - name: envName, - type: 'test', - }); - - await app.request - .post('/api/admin/projects/default/environments') - .send({ - environment: envName, - }) - .expect(200); - - await app.request - .post('/api/admin/projects/default/features') - .send({ name: featureName }) - .expect(201); - - await addStrategies(featureName, envName); - const { body } = await app.request.get( - `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies`, - ); - - const strategies: FeatureStrategySchema[] = body; - let order = 1; - const sortOrders: SetStrategySortOrderSchema = []; - - strategies.forEach((strategy) => { - sortOrders.push({ id: strategy.id!, sortOrder: order++ }); - }); - - await app.request - .post( - `/api/admin/projects/default/features/${featureName}/environments/${envName}/strategies/set-sort-order`, - ) - .send(sortOrders) - .expect(200); - - const response = await app.request.get(`/api/admin/events`); - const { body: eventsBody } = response; - let { events } = eventsBody; - - expect(events[0].type).toBe('feature-strategy-update'); - expect(events[1].type).toBe('feature-strategy-update'); - expect(events[2].type).toBe('feature-strategy-update'); -});