Skip to content

Remove user ids strategy 2 #9806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: remove-user-ids-strategy
Choose a base branch
from
Open

Remove user ids strategy 2 #9806

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
adea19f
chore: remove user ids strategy
nunogois Apr 21, 2025
81f619d
chore: scouting, make events sorted in deterministic way
nunogois Apr 21, 2025
5e0e519
chore: add delete migration
nunogois Apr 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions frontend/cypress/global.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,6 @@ declare namespace Cypress {
deleteSegment_UI(segmentName: string, id: string): Chainable;

// STRATEGY
addUserIdStrategyToFeature_UI(
featureName: string,
projectName: string,
): Chainable;
addFlexibleRolloutStrategyToFeature_UI(
options: AddFlexibleRolloutStrategyOptions,
): Chainable;
Expand Down
51 changes: 0 additions & 51 deletions frontend/cypress/support/UI.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,57 +257,6 @@ export const deleteFeatureStrategy_UI = (
return cy.wait('@deleteUserStrategy');
};

export const addUserIdStrategyToFeature_UI = (
featureToggleName: string,
projectName: string,
): Chainable<any> => {
const project = projectName || 'default';
cy.visit(
`/projects/${project}/features/${featureToggleName}/strategies/create?environmentId=development&strategyName=userWithId`,
);

if (ENTERPRISE) {
cy.get('[data-testid=ADD_CONSTRAINT_ID]').click();
cy.get('[data-testid=CONSTRAINT_AUTOCOMPLETE_ID]')
.type('{downArrow}'.repeat(1))
.type('{enter}');
cy.get('[data-testid=DIALOGUE_CONFIRM_ID]').click();
}

cy.get('[data-testid=STRATEGY_INPUT_LIST]')
.type('user1')
.type('{enter}')
.type('user2')
.type('{enter}');
cy.get('[data-testid=ADD_TO_STRATEGY_INPUT_LIST]').click();

cy.intercept(
'POST',
`/api/admin/projects/default/features/${featureToggleName}/environments/*/strategies`,
(req) => {
expect(req.body.name).to.equal('userWithId');

expect(req.body.parameters.userIds.length).to.equal(11);

if (ENTERPRISE) {
expect(req.body.constraints.length).to.equal(1);
} else {
expect(req.body.constraints.length).to.equal(0);
}

req.continue((res) => {
strategyId = res.body.id;
});
},
).as('addStrategyToFeature');

// this one needs to wait until the dropdown selector of stickiness is set, that's why waitForAnimations: true
cy.get(`[data-testid=STRATEGY_FORM_SUBMIT_ID]`)
.first()
.click({ waitForAnimations: true });
return cy.wait('@addStrategyToFeature');
};

export const logout_UI = (): Chainable<any> => {
return cy.visit('/logout');
};
5 changes: 0 additions & 5 deletions frontend/cypress/support/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
deleteSegment_UI,
deleteFeatureStrategy_UI,
addFlexibleRolloutStrategyToFeature_UI,
addUserIdStrategyToFeature_UI,
updateFlexibleRolloutStrategy_UI,
do_login,
//@ts-ignore
Expand Down Expand Up @@ -47,10 +46,6 @@ Cypress.Commands.add('updateUserPassword_API', updateUserPassword_API);
Cypress.Commands.add('createFeature_UI', createFeature_UI);
Cypress.Commands.add('deleteFeatureStrategy_UI', deleteFeatureStrategy_UI);
Cypress.Commands.add('createFeature_API', createFeature_API);
Cypress.Commands.add(
'addUserIdStrategyToFeature_UI',
addUserIdStrategyToFeature_UI,
);
Cypress.Commands.add(
'addFlexibleRolloutStrategyToFeature_UI',
addFlexibleRolloutStrategyToFeature_UI,
Expand Down
39 changes: 20 additions & 19 deletions frontend/src/component/changeRequest/ChangeRequestPermissions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,20 @@ const setupOtherRoutes = (feature: string) => {
deprecated: false,
},
{
displayName: 'UserIDs',
name: 'userWithId',
displayName: 'Gradual rollout',
name: 'flexibleRollout',
editable: false,
description:
'Enable the feature for a specific set of userIds.',
'The gradual rollout strategy allows you to gradually roll out a feature to a percentage of users.',
parameters: [
{
name: 'userIds',
type: 'list',
description: '',
required: false,
name: 'rollout',
},
{
name: 'stickiness',
},
{
name: 'groupId',
},
],
deprecated: false,
Expand Down Expand Up @@ -214,12 +217,10 @@ const UnleashUiSetup: FC<{
</SWRConfig>
);

const strategiesAreDisplayed = async (
firstStrategy: string,
secondStrategy: string,
) => {
await screen.findByText(firstStrategy);
await screen.findByText(secondStrategy);
const strategiesAreDisplayed = async (strategies: string[]) => {
for (const strategy of strategies) {
await screen.findByText(strategy);
}
};

const getDeleteButtons = async () => {
Expand Down Expand Up @@ -296,7 +297,7 @@ test('open mode + non-project member can perform basic change request actions',
const featureName = 'test';
featureEnvironments(featureName, [
{ name: 'development', strategies: [] },
{ name: 'production', strategies: ['userWithId'] },
{ name: 'production', strategies: ['flexibleRollout'] },
{ name: 'custom', strategies: ['default'] },
]);
userIsMemberOfProjects([]);
Expand All @@ -316,7 +317,7 @@ test('open mode + non-project member can perform basic change request actions',

await openEnvironments(['development', 'production', 'custom']);

await strategiesAreDisplayed('UserIDs', 'Standard');
await strategiesAreDisplayed(['Gradual rollout', 'Standard']);
await deleteButtonsActiveInChangeRequestEnv();
await copyButtonsActiveInOtherEnv();
});
Expand All @@ -326,7 +327,7 @@ test('protected mode + project member can perform basic change request actions',
const featureName = 'test';
featureEnvironments(featureName, [
{ name: 'development', strategies: [] },
{ name: 'production', strategies: ['userWithId'] },
{ name: 'production', strategies: ['flexibleRollout'] },
{ name: 'custom', strategies: ['default'] },
]);
userIsMemberOfProjects([project]);
Expand All @@ -346,7 +347,7 @@ test('protected mode + project member can perform basic change request actions',

await openEnvironments(['development', 'production', 'custom']);

await strategiesAreDisplayed('UserIDs', 'Standard');
await strategiesAreDisplayed(['Gradual rollout', 'Standard']);
await deleteButtonsActiveInChangeRequestEnv();
await copyButtonsActiveInOtherEnv();
});
Expand All @@ -356,7 +357,7 @@ test.skip('protected mode + non-project member cannot perform basic change reque
const featureName = 'test';
featureEnvironments(featureName, [
{ name: 'development', strategies: [] },
{ name: 'production', strategies: ['userWithId'] },
{ name: 'production', strategies: ['flexibleRollout'] },
{ name: 'custom', strategies: ['default'] },
]);
userIsMemberOfProjects([]);
Expand All @@ -376,7 +377,7 @@ test.skip('protected mode + non-project member cannot perform basic change reque

await openEnvironments(['development', 'production', 'custom']);

await strategiesAreDisplayed('UserIDs', 'Standard');
await strategiesAreDisplayed(['Gradual rollout', 'Standard']);
await deleteButtonsInactiveInChangeRequestEnv();
await copyButtonsActiveInOtherEnv();
});
10 changes: 0 additions & 10 deletions frontend/src/component/feature/FeatureStrategy/FeatureStrategyType/FeatureStrategyType.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { IFeatureStrategy, IStrategy } from 'interfaces/strategy';
import DefaultStrategy from 'component/feature/StrategyTypes/DefaultStrategy/DefaultStrategy';
import FlexibleStrategy from 'component/feature/StrategyTypes/FlexibleStrategy/FlexibleStrategy';
import UserWithIdStrategy from 'component/feature/StrategyTypes/UserWithIdStrategy/UserWithId';
import GeneralStrategy from 'component/feature/StrategyTypes/GeneralStrategy/GeneralStrategy';
import useUnleashContext from 'hooks/api/getters/useUnleashContext/useUnleashContext';
import produce from 'immer';
Expand Down Expand Up @@ -52,15 +51,6 @@ export const FeatureStrategyType = ({
errors={errors}
/>
);
case 'userWithId':
return (
<UserWithIdStrategy
parameters={strategy.parameters ?? {}}
updateParameter={updateParameter}
editable={hasAccess}
errors={errors}
/>
);
default:
return (
<GeneralStrategy
Expand Down
32 changes: 0 additions & 32 deletions frontend/src/component/feature/StrategyTypes/UserWithIdStrategy/UserWithId.tsx
View file
Open in desktop

This file was deleted.

10 changes: 0 additions & 10 deletions ...ent/releases/ReleasePlanTemplate/TemplateForm/MilestoneStrategy/MilestoneStrategyType.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import type { IReleasePlanMilestoneStrategy } from 'interfaces/releasePlans';
import type { IStrategy } from 'interfaces/strategy';
import { MilestoneStrategyTypeFlexible } from './MilestoneStrategyTypeFlexible';
import GeneralStrategy from 'component/feature/StrategyTypes/GeneralStrategy/GeneralStrategy';
import UserWithIdStrategy from 'component/feature/StrategyTypes/UserWithIdStrategy/UserWithId';
import DefaultStrategy from 'component/feature/StrategyTypes/DefaultStrategy/DefaultStrategy';

interface IMilestoneStrategyTypeProps {
Expand Down Expand Up @@ -36,15 +35,6 @@ export const MilestoneStrategyType = ({
editable={true}
/>
);
case 'userWithId':
return (
<UserWithIdStrategy
editable={true}
parameters={strategy.parameters ?? {}}
updateParameter={updateParameter}
errors={errors}
/>
);
default:
return (
<GeneralStrategy
Expand Down
5 changes: 0 additions & 5 deletions frontend/src/utils/strategyNames.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import type { FC, SVGProps } from 'react';
import { SvgIcon, useTheme } from '@mui/material';
import LocationOnIcon from '@mui/icons-material/LocationOn';
import PeopleIcon from '@mui/icons-material/People';
import LanguageIcon from '@mui/icons-material/Language';
import PowerSettingsNewIcon from '@mui/icons-material/PowerSettingsNew';
import CodeIcon from '@mui/icons-material/Code';
Expand All @@ -28,8 +27,6 @@ export const getFeatureStrategyIcon = (strategyName?: string) => {
return LanguageIcon;
case 'flexibleRollout':
return RolloutSvgIcon;
case 'userWithId':
return PeopleIcon;
case 'applicationHostname':
return LocationOnIcon;
case 'releasePlanTemplate':
Expand All @@ -47,7 +44,6 @@ export const BuiltInStrategies = [
'gradualRolloutSessionId',
'gradualRolloutUserId',
'remoteAddress',
'userWithId',
];

export const GetFeatureStrategyIcon: FC<{ strategyName: string }> = ({
Expand All @@ -66,5 +62,4 @@ export const formattedStrategyNames: Record<string, string> = {
gradualRolloutSessionId: 'Sessions',
gradualRolloutUserId: 'Users',
remoteAddress: 'IPs',
userWithId: 'UserIDs',
};
5 changes: 4 additions & 1 deletion src/lib/features/events/event-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,10 @@ class EventStore implements IEventStore {
.select(EVENT_COLUMNS)
.from(TABLE)
.limit(100)
.orderBy('created_at', 'desc');
.orderBy([
{ column: 'created_at', order: 'desc' },
{ column: 'id', order: 'desc' },
]);
Copy link
Member Author

@nunogois nunogois Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scouting. Sometimes events are not returned in a deterministic way. E.g. if two events have the exact same created_at value, sometimes they are returned in a different order. This makes it so we still sort by created_at, but then if it's the same, we fallback to sorting by id.

Perhaps we can consider just sorting by id here and skip sorting by created_at.

if (query) {
qB = qB.where(query);
}
Expand Down
2 changes: 0 additions & 2 deletions src/lib/openapi/spec/client-application-schema.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ test('clientApplicationSchema go-sdk request', () => {
"gradualRolloutSessionId",
"gradualRolloutUserId",
"remoteAddress",
"userWithId",
"flexibleRollout"
],
"started": "2022-06-24T09:59:12.822607943+02:00",
Expand All @@ -75,7 +74,6 @@ test('clientApplicationSchema node-sdk request', () => {
"gradualRolloutRandom",
"gradualRolloutUserId",
"gradualRolloutSessionId",
"userWithId",
"remoteAddress",
"flexibleRollout"
],
Expand Down
21 changes: 21 additions & 0 deletions src/migrations/20250422070810-drop-userwithid-strategy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
'use strict';

exports.up = function (db, callback) {
db.runSql(
`
DELETE FROM strategies
WHERE name = 'userWithId' AND built_in = 1
Copy link
Member Author

@nunogois nunogois Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unlike e.g.

unleash/src/migrations/20230420125500-v5-strategy-changes.js

Line 10 in bc7856a

and not exists (select * from feature_strategies where strategy_name = name limit 1);

Here we're deleting the strategy even if it's in use. Based on #9800 it seems like this is what we want to do. However, if it's not, this is something we probably need to change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't want to delete it. The idea is:

  1. New installations will not receive it
  2. Old installations requires manual action from the user

`,
callback,
);
};

exports.down = function (db, callback) {
db.runSql(
`
INSERT INTO strategies
(name, description, parameters, built_in, deprecated, sort_order, display_name) VALUES ('userWithId', 'Enable the feature for a specific set of userIds. Prefer using "Gradual rollout" strategy with user id constraints.', '[{"name":"userIds","type":"list","description":"","required":false}]', 1, true, 2, 'UserIDs');
`,
callback,
);
};
1 change: 0 additions & 1 deletion website/docs/reference/api/legacy/unleash/admin/metrics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ This endpoint gives insight into details about application seen per feature flag
"abTest",
"default",
"betaUser",
"userWithId",
"byHostName",
"gradualRolloutWithSessionId",
"gradualRollout",
Expand Down
12 changes: 0 additions & 12 deletions website/docs/reference/api/legacy/unleash/admin/strategies.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,6 @@ Used to fetch all defined strategies and their defined parameters.
"description": "Default on/off strategy.",
"parameters": []
},
{
"name": "userWithId",
"description": "Active for userId specified in the comma seperated 'userIds' parameter.",
"parameters": [
{
"name": "userIds",
"type": "list",
"description": "List of unique userIds the feature should be active for.",
"required": true
}
]
},
{
"name": "gradualRollout",
"description": "Gradual rollout to logged in users",
Expand Down
7 changes: 0 additions & 7 deletions website/docs/reference/predefined-strategy-types.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,6 @@ title: Predefined strategy types
Predefined strategy types are a legacy implementation. Please use the [default strategy](/reference/activation-strategies) with strategy constraints to achieve your desired targeting.
:::

## UserIDs

The `userWithId` strategy is active for users with a `userId` defined in the `userIds` list.

**Parameters:**
- userIds - _List of user IDs you want the feature flag to be enabled for_

## Flexible Gradual Rollout

The `flexibleRollout` stategy has the following parameters:
Expand Down