Skip to content

Commit

Permalink
chore: remove deprecated strategies and allow deprecate default (#3575)
Browse files Browse the repository at this point in the history
## About the changes
Improvements around strategies to steer users toward using [strategy
constraints](https://docs.getunleash.io/reference/strategy-constraints)
instead of [custom
strategies](https://docs.getunleash.io/reference/custom-activation-strategies)


Related to #1265 

Important changes:
1.[ Ability to deprecate default
strategy](https://github.com/Unleash/unleash/pull/3575/files#diff-3b49e566565b01580ff7a15f662d2cb824547fdfb3935d0a0baa7fae326d28d6L232-L235)
2. [Remove strategies that were deprecated in v4 only if they're not
used](https://github.com/Unleash/unleash/pull/3575/files#diff-a04de7adf327c48d54b030bed70d9b1d62ff14df3efc668eb6897a210124ebe0R7-R10)
3. [Fresh Unleash installations will not include deprecated strategies
from
v4](https://github.com/Unleash/unleash/pull/3575/files#diff-46265b0d51725a5acbf251589b98a8970e45d3a1c6f3d7ea77bdcc3714e672d8L31-L78)
4. [Deprecate userWithId
strategy](https://github.com/Unleash/unleash/pull/3575/files#diff-a04de7adf327c48d54b030bed70d9b1d62ff14df3efc668eb6897a210124ebe0R13)
(note: if in use it will continue to work, it'll be not listed when
selecting strategies for a feature toggle).
5. [Switch the order of strategies to make gradual rollout first and
change the comment to suggest using gradual rollout instead of other
existing
strategies](https://github.com/Unleash/unleash/pull/3575/files#diff-a04de7adf327c48d54b030bed70d9b1d62ff14df3efc668eb6897a210124ebe0R16-R18)
  • Loading branch information
gastonfournier committed Apr 21, 2023
1 parent 097dd8a commit 4dd4d43
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 77 deletions.
Expand Up @@ -216,7 +216,6 @@ export const StrategiesList = () => {
<StrategySwitch
deprecated={original.deprecated}
onToggle={onToggle(original)}
disabled={original.name === 'default'}
/>
<ActionCell.Divider />
<StrategyEditButton
Expand Down
8 changes: 0 additions & 8 deletions src/lib/routes/admin-api/strategy.test.ts
Expand Up @@ -207,11 +207,3 @@ test('reactivating a non-existent strategy yields 404', async () => {
.set('Content-Type', 'application/json')
.expect(404);
});
test("deprecating 'default' strategy will yield 403", async () => {
jest.spyOn(global.console, 'error').mockImplementation(() => jest.fn());
const { request, base } = await getSetup();
return request
.post(`${base}/api/admin/strategies/default/deprecate`)
.set('Content-Type', 'application/json')
.expect(403);
});
5 changes: 0 additions & 5 deletions src/lib/routes/admin-api/strategy.ts
Expand Up @@ -229,11 +229,6 @@ class StrategyController extends Controller {
const userName = extractUsername(req);
const { strategyName } = req.params;

if (strategyName === 'default') {
res.status(403).end();
return;
}

await this.strategyService.deprecateStrategy(strategyName, userName);
res.status(200).end();
}
Expand Down
72 changes: 72 additions & 0 deletions src/migrations/20230420125500-v5-strategy-changes.js
@@ -0,0 +1,72 @@
'use strict';

exports.up = function (db, callback) {
db.runSql(
`
-- delete deprecated strategies still present in v4
delete from strategies
where name in ('gradualRolloutUserId', 'gradualRolloutRandom', 'gradualRolloutSessionId')
and deprecated
and not exists (select * from feature_strategies where strategy_name = name limit 1);
-- deprecate strategies on v5
update strategies set deprecated = true where name in ('userWithId');
-- update strategy descriptions and sort order
update strategies set sort_order = 1, description = 'This strategy turns on / off for your entire userbase. Prefer using "Gradual rollout" strategy (100%=on, 0%=off).' WHERE name = 'default';
update strategies set sort_order = 0 WHERE name = 'flexibleRollout';
update strategies set description = 'Enable the feature for a specific set of userIds. Prefer using "Gradual rollout" strategy with user id constraints.' WHERE name = 'userWithId';
`,
callback,
);
};

exports.down = function (db, callback) {
db.runSql(
`
-- restore deleted strategies
insert into strategies (name, description, parameters, deprecated, sort_order) values
('gradualRolloutRandom', 'Randomly activate the feature toggle. No stickiness.', [
{
"name": "percentage",
"type": "percentage",
"description": "",
"required": false
}
], true, 3),
('gradualRolloutSessionId', 'Gradually activate feature toggle. Stickiness based on session id.', [
{
"name": "percentage",
"type": "percentage",
"description": "",
"required": false
},
{
"name": "groupId",
"type": "string",
"description": "Used to define a activation groups, which allows you to correlate across feature toggles.",
"required": true
}
], true, 4),
('gradualRolloutUserId', 'Gradually activate feature toggle for logged in users. Stickiness based on user id.', [
{
"name": "percentage",
"type": "percentage",
"description": "",
"required": false
},
{
"name": "groupId",
"type": "string",
"description": "Used to define a activation groups, which allows you to correlate across feature toggles.",
"required": true
}
], true, 5);
-- revert sort order
update strategies set sort_order = 0 WHERE name = 'default';
update strategies set sort_order = 1 WHERE name = 'flexibleRollout';
`,
callback,
);
};
48 changes: 0 additions & 48 deletions src/migrations/default-strategies.json
Expand Up @@ -28,54 +28,6 @@
}
]
},
{
"name": "gradualRolloutRandom",
"description": "Randomly activate the feature toggle. No stickiness.",
"parameters": [
{
"name": "percentage",
"type": "percentage",
"description": "",
"required": false
}
]
},
{
"name": "gradualRolloutSessionId",
"description": "Gradually activate feature toggle. Stickiness based on session id.",
"parameters": [
{
"name": "percentage",
"type": "percentage",
"description": "",
"required": false
},
{
"name": "groupId",
"type": "string",
"description": "Used to define a activation groups, which allows you to correlate across feature toggles.",
"required": true
}
]
},
{
"name": "gradualRolloutUserId",
"description": "Gradually activate feature toggle for logged in users. Stickiness based on user id.",
"parameters": [
{
"name": "percentage",
"type": "percentage",
"description": "",
"required": false
},
{
"name": "groupId",
"type": "string",
"description": "Used to define a activation groups, which allows you to correlate across feature toggles.",
"required": true
}
]
},
{
"name": "remoteAddress",
"description": "Active for remote addresses defined in the IPs list.",
Expand Down
9 changes: 0 additions & 9 deletions src/test/e2e/api/admin/strategy.e2e.test.ts
Expand Up @@ -171,15 +171,6 @@ test('can reactivate a deprecated strategy', async () => {
.expect((res) => expect(res.body.deprecated).toBe(false));
});

test('cannot deprecate default strategy', async () => {
expect.assertions(0);

await app.request
.post('/api/admin/strategies/default/deprecate')
.set('Content-Type', 'application/json')
.expect(403);
});

test('can update a exiting strategy with deprecated', async () => {
await app.request
.post('/api/admin/strategies')
Expand Down
12 changes: 6 additions & 6 deletions src/test/e2e/api/client/feature.optimal304.e2e.test.ts
Expand Up @@ -122,14 +122,14 @@ test('returns calculated hash', async () => {
.get('/api/client/features')
.expect('Content-Type', /json/)
.expect(200);
expect(res.headers.etag).toBe('"ae443048:19"');
expect(res.body.meta.etag).toBe('"ae443048:19"');
expect(res.headers.etag).toBe('"ae443048:16"');
expect(res.body.meta.etag).toBe('"ae443048:16"');
});

test('returns 304 for pre-calculated hash', async () => {
return app.request
.get('/api/client/features')
.set('if-none-match', '"ae443048:19"')
.set('if-none-match', '"ae443048:16"')
.expect(304);
});

Expand All @@ -146,9 +146,9 @@ test('returns 200 when content updates and hash does not match anymore', async (

const res = await app.request
.get('/api/client/features')
.set('if-none-match', 'ae443048:19')
.set('if-none-match', 'ae443048:16')
.expect(200);

expect(res.headers.etag).toBe('"ae443048:20"');
expect(res.body.meta.etag).toBe('"ae443048:20"');
expect(res.headers.etag).toBe('"ae443048:17"');
expect(res.body.meta.etag).toBe('"ae443048:17"');
});

0 comments on commit 4dd4d43

Please sign in to comment.