Skip to content

Commit

Permalink
Revert "Feat/add strategy update event on strategy ordering (#4234)" (#…
Browse files Browse the repository at this point in the history 
…4243)

This reverts commit 16e3799.


<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->

## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->

<!-- Does it close an issue? Multiple? -->
Closes #

<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->

### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->


## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->
  • Loading branch information
@andreas-unleash
andreas-unleash committed Jul 14, 2023
1 parent 383e522 commit 5e45ec2
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 155 deletions.
16 changes: 2 additions & 14 deletions src/lib/openapi/spec/event-schema.ts
View file
Expand Up @@ -12,55 +12,48 @@ 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: {
$ref: '#/components/schemas/variantSchema',
},
},
createdAt: {
nullable: true,
type: 'string',
format: 'date-time',
description:
'The time the event happened as a RFC 3339-conformant timestamp.',
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:
Expand Down Expand Up @@ -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:
Expand Down
18 changes: 5 additions & 13 deletions src/lib/routes/admin-api/project/project-features.ts
View file
Expand Up @@ -936,26 +936,18 @@ export default class ProjectFeaturesController extends Controller {
}

async setStrategiesSortOrder(
req: IAuthRequest<
req: Request<
FeatureStrategyParams,
any,
SetStrategySortOrderSchema,
any
>,
res: Response,
): Promise<void> {
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();
Expand Down
75 changes: 14 additions & 61 deletions src/lib/services/feature-toggle-service.ts
View file
Expand Up @@ -369,7 +369,7 @@ class FeatureToggleService {
featureStrategy: IFeatureStrategy,
segments: ISegment[] = [],
): Saved<IStrategyConfig> {
const result: Saved<IStrategyConfig> = {
return {
id: featureStrategy.id,
name: featureStrategy.strategyName,
title: featureStrategy.title,
Expand All @@ -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<Saved<any>> {
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),
),
);
}

Expand Down Expand Up @@ -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) {
Expand Down
67 changes: 0 additions & 67 deletions src/test/e2e/api/admin/project/features.e2e.test.ts
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
});

0 comments on commit 5e45ec2

Please sign in to comment.