mirror of
https://github.com/Unleash/unleash.git
synced 2025-07-12 13:48:35 +02:00
fix: do not allow creating cr for same environment (#10010)
If there are two concurrent requests to create or edit change requests, two separate ones may be created in parallel. The UI does not currently handle this scenario, and if additional changes are made, they might be added to a random existing change request—resulting in a messy and unpredictable state. This PR adds a unique index to the `change_requests` table ``` on (created_by, project, environment) WHERE state NOT IN ('Applied', 'Cancelled', 'Rejected', 'Scheduled'). ``` In the extremely rare case where such conflicting data already exists in a database, the migration will automatically cancel one of the conflicting change requests.
This commit is contained in:
parent
eef32b7cf5
commit
d8c83fb824
@ -9,6 +9,7 @@ import { randomId } from '../../util/index.js';
|
||||
|
||||
let db: ITestDb;
|
||||
let user: IUser;
|
||||
let user2: IUser;
|
||||
|
||||
const CR_ID = 123456;
|
||||
const CR_ID_2 = 234567;
|
||||
@ -26,6 +27,10 @@ beforeAll(async () => {
|
||||
username: 'cr-creator',
|
||||
});
|
||||
|
||||
user2 = await db.stores.userStore.insert({
|
||||
username: 'cr-creator-2',
|
||||
});
|
||||
|
||||
readModel = createChangeRequestSegmentUsageReadModel(db.rawDatabase);
|
||||
|
||||
await db.stores.featureToggleStore.create('default', {
|
||||
@ -56,13 +61,14 @@ const createCR = async (
|
||||
state,
|
||||
changeRequestId = CR_ID,
|
||||
changeRequestTitle: string | null = CR_TITLE,
|
||||
userId = user.id,
|
||||
) => {
|
||||
await db.rawDatabase.table('change_requests').insert({
|
||||
id: changeRequestId,
|
||||
environment: 'default',
|
||||
state,
|
||||
project: 'default',
|
||||
created_by: user.id,
|
||||
created_by: userId,
|
||||
created_at: '2023-01-01 00:00:00',
|
||||
min_approvals: 1,
|
||||
title: changeRequestTitle,
|
||||
@ -212,7 +218,7 @@ test.each([
|
||||
|
||||
test(`If the same strategy appears in multiple CRs with the same segment, each segment should be listed as its own entry`, async () => {
|
||||
await createCR('In review', CR_ID, CR_TITLE);
|
||||
await createCR('In review', CR_ID_2, null);
|
||||
await createCR('In review', CR_ID_2, null, user2.id);
|
||||
|
||||
const segmentId = 3;
|
||||
const strategyId = randomId();
|
||||
|
@ -12,6 +12,7 @@ import {
|
||||
CREATE_FEATURE_STRATEGY,
|
||||
DEFAULT_PROJECT,
|
||||
type IUnleashStores,
|
||||
TEST_AUDIT_USER,
|
||||
UPDATE_FEATURE_ENVIRONMENT,
|
||||
} from '../../types/index.js';
|
||||
import { DEFAULT_ENV } from '../../util/index.js';
|
||||
@ -76,6 +77,22 @@ beforeAll(async () => {
|
||||
],
|
||||
'production',
|
||||
);
|
||||
|
||||
await app.services.userService.createUser(
|
||||
{
|
||||
username: 'admin@test.com',
|
||||
rootRole: 1,
|
||||
},
|
||||
TEST_AUDIT_USER,
|
||||
);
|
||||
|
||||
await app.services.userService.createUser(
|
||||
{
|
||||
username: 'admin2@test.com',
|
||||
rootRole: 1,
|
||||
},
|
||||
TEST_AUDIT_USER,
|
||||
);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
@ -1339,15 +1356,26 @@ const createChangeRequest = async ({
|
||||
feature,
|
||||
environment,
|
||||
state,
|
||||
}: { id: number; feature: string; environment: string; state: string }) => {
|
||||
await db
|
||||
.rawDatabase('change_requests')
|
||||
.insert({ id, environment, state, project: 'default', created_by: 1 });
|
||||
createdBy,
|
||||
}: {
|
||||
id: number;
|
||||
feature: string;
|
||||
environment: string;
|
||||
state: string;
|
||||
createdBy: number;
|
||||
}) => {
|
||||
await db.rawDatabase('change_requests').insert({
|
||||
id,
|
||||
environment,
|
||||
state,
|
||||
project: 'default',
|
||||
created_by: createdBy,
|
||||
});
|
||||
await db.rawDatabase('change_request_events').insert({
|
||||
id,
|
||||
feature,
|
||||
action: 'updateEnabled',
|
||||
created_by: 1,
|
||||
created_by: createdBy,
|
||||
change_request_id: id,
|
||||
});
|
||||
};
|
||||
@ -1361,48 +1389,56 @@ test('should return change request ids per environment', async () => {
|
||||
feature: 'my_feature_a',
|
||||
environment: 'production',
|
||||
state: 'In review',
|
||||
createdBy: 1,
|
||||
});
|
||||
await createChangeRequest({
|
||||
id: 2,
|
||||
feature: 'my_feature_a',
|
||||
environment: 'production',
|
||||
state: 'Applied',
|
||||
createdBy: 1,
|
||||
});
|
||||
await createChangeRequest({
|
||||
id: 3,
|
||||
feature: 'my_feature_a',
|
||||
environment: 'production',
|
||||
state: 'Cancelled',
|
||||
createdBy: 1,
|
||||
});
|
||||
await createChangeRequest({
|
||||
id: 4,
|
||||
feature: 'my_feature_a',
|
||||
environment: 'production',
|
||||
state: 'Rejected',
|
||||
createdBy: 1,
|
||||
});
|
||||
await createChangeRequest({
|
||||
id: 5,
|
||||
feature: 'my_feature_a',
|
||||
environment: 'development',
|
||||
state: 'Draft',
|
||||
createdBy: 1,
|
||||
});
|
||||
await createChangeRequest({
|
||||
id: 6,
|
||||
feature: 'my_feature_a',
|
||||
environment: 'development',
|
||||
state: 'Scheduled',
|
||||
createdBy: 1,
|
||||
});
|
||||
await createChangeRequest({
|
||||
id: 7,
|
||||
feature: 'my_feature_a',
|
||||
environment: 'development',
|
||||
state: 'Approved',
|
||||
createdBy: 2,
|
||||
});
|
||||
await createChangeRequest({
|
||||
id: 8,
|
||||
feature: 'my_feature_b',
|
||||
environment: 'development',
|
||||
state: 'Approved',
|
||||
createdBy: 3,
|
||||
});
|
||||
|
||||
const { body } = await searchFeatures({});
|
||||
|
32
src/migrations/20250529153326-cr-uniqueness.js
Normal file
32
src/migrations/20250529153326-cr-uniqueness.js
Normal file
@ -0,0 +1,32 @@
|
||||
exports.up = (db, callback) => {
|
||||
db.runSql(
|
||||
`
|
||||
WITH ranked AS (
|
||||
SELECT id,
|
||||
ROW_NUMBER() OVER (
|
||||
PARTITION BY created_by, project, environment
|
||||
ORDER BY created_at DESC
|
||||
) AS rn
|
||||
FROM change_requests
|
||||
WHERE state NOT IN ('Applied', 'Cancelled', 'Rejected', 'Scheduled')
|
||||
)
|
||||
UPDATE change_requests
|
||||
SET state = 'Cancelled'
|
||||
WHERE id IN (
|
||||
SELECT id FROM ranked WHERE rn > 1
|
||||
);
|
||||
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS unique_pending_request_per_user_project_env
|
||||
ON change_requests (created_by, project, environment)
|
||||
WHERE state NOT IN ('Applied', 'Cancelled', 'Rejected', 'Scheduled');
|
||||
`,
|
||||
callback,
|
||||
);
|
||||
};
|
||||
|
||||
exports.down = (db, callback) => {
|
||||
db.runSql(
|
||||
` DROP INDEX IF EXISTS unique_pending_request_per_user_project_env;`,
|
||||
callback,
|
||||
);
|
||||
};
|
Loading…
Reference in New Issue
Block a user