Mirrors/unleash.unleash
1
0
Fork 0
mirror of https://github.com/Unleash/unleash.git synced 2024-12-22 19:07:54 +01:00
Code Issues Projects Releases Wiki Activity

fix: if a strategy both uses a segment actively and in CRs, list it twice (#5390)

Browse Source 
This PR changes the behavior of the API a little bit. Instead of
removing any strategies from `changeRequestStrategies` that are also
in `strategies`, we keep them in instead.

The reason for this is that the overview of where a segment is used is
incomplete if it shows only strategies but not CRs. Imagine this:

You want to delete a segment, but you're told it's only used in strategy
S.
So you go and remove it from strategy S, but then you're told it's
suddenly used in CRs A, B, and C. This is now a two-step operation
with a bad surprise. Instead, we could show you immediately that this
segment is used in strategy S and CRs A, B, and C.
This commit is contained in:
Thomas Heartman 2023-11-22 14:13:18 +01:00 committed by GitHub
parent 432aed3034
commit b211345a44
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 4 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
src/lib/services/segment-service.ts
View File

@ -119,15 +119,10 @@ export class SegmentService implements ISegmentService {
const strategies =
await this.featureStrategiesStore.getStrategiesBySegment(id);
const strategyIds = new Set(strategies.map((s) => s.id));
const changeRequestStrategies = (
const changeRequestStrategies =
await this.changeRequestSegmentUsageReadModel.getStrategiesUsedInActiveChangeRequests(
id,
)
).filter(
(strategy) => !('id' in strategy && strategyIds.has(strategy.id)),
);
);
return { strategies, changeRequestStrategies };
}

4
src/test/e2e/api/admin/segment.e2e.test.ts
View File

@ -583,7 +583,7 @@ describe('detect strategy usage in change requests', () => {
expect(strategies).toStrictEqual([]);
});
test('If a segment is used in an existing strategy and in a CR for the same strategy, the strategy should only be listed once', async () => {
test('If a segment is used in an existing strategy and in a CR for the same strategy, the strategy should be listed both places', async () => {
await createSegment({ name: 'a', constraints: [] });
const toggle = mockFeatureToggle();
await createFeatureToggle(app, toggle);
@ -628,6 +628,6 @@ describe('detect strategy usage in change requests', () => {
expect(strategies).toMatchObject([{ id: strategyId }]);
expect(changeRequestStrategies).toStrictEqual([]);
expect(changeRequestStrategies).toMatchObject([{ id: strategyId }]);
});
});