diff --git a/src/lib/features/client-feature-toggles/client-feature-toggle-store.ts b/src/lib/features/client-feature-toggles/client-feature-toggle-store.ts index 435dae0d74..d6c28cffc7 100644 --- a/src/lib/features/client-feature-toggles/client-feature-toggle-store.ts +++ b/src/lib/features/client-feature-toggles/client-feature-toggle-store.ts @@ -22,6 +22,7 @@ import type EventEmitter from 'events'; import FeatureToggleStore from '../feature-toggle/feature-toggle-store'; import type { Db } from '../../db/db'; import Raw = Knex.Raw; +import { sortStrategies } from '../../util/sortStrategies'; export interface IGetAllFeatures { featureQuery?: IFeatureToggleQuery; @@ -95,6 +96,7 @@ export default class FeatureToggleClientStore 'fs.parameters as parameters', 'fs.constraints as constraints', 'fs.sort_order as sort_order', + 'fs.milestone_id as milestone_id', 'fs.variants as strategy_variants', 'segments.id as segment_id', 'segments.constraints as segment_constraints', @@ -244,16 +246,8 @@ export default class FeatureToggleClientStore const cleanedFeatures = features.map(({ strategies, ...rest }) => ({ ...rest, strategies: strategies - ?.sort((strategy1, strategy2) => { - if ( - typeof strategy1.sortOrder === 'number' && - typeof strategy2.sortOrder === 'number' - ) { - return strategy1.sortOrder - strategy2.sortOrder; - } - return 0; - }) - .map(({ id, title, sortOrder, ...strategy }) => ({ + ?.sort(sortStrategies) + .map(({ id, title, sortOrder, milestoneId, ...strategy }) => ({ ...strategy, ...(isPlayground && title ? { title } : {}), @@ -275,6 +269,7 @@ export default class FeatureToggleClientStore constraints: row.constraints || [], parameters: mapValues(row.parameters || {}, ensureStringValue), sortOrder: row.sort_order, + milestoneId: row.milestone_id, }; strategy.variants = row.strategy_variants || []; return strategy; diff --git a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model.ts b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model.ts index 969e21d7f8..e0a7c76a4f 100644 --- a/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model.ts +++ b/src/lib/features/client-feature-toggles/delta/client-feature-toggle-delta-read-model.ts @@ -17,6 +17,7 @@ import { } from '../../../internals'; import metricsHelper from '../../../util/metrics-helper'; import FeatureToggleStore from '../../feature-toggle/feature-toggle-store'; +import { sortStrategies } from '../../../util/sortStrategies'; export default class ClientFeatureToggleDeltaReadModel implements IClientFeatureToggleDeltaReadModel @@ -58,6 +59,7 @@ export default class ClientFeatureToggleDeltaReadModel 'fs.parameters as parameters', 'fs.constraints as constraints', 'fs.sort_order as sort_order', + 'fs.milestone_id as milestone_id', 'fs.variants as strategy_variants', 'segments.id as segment_id', 'segments.constraints as segment_constraints', @@ -175,16 +177,8 @@ export default class ClientFeatureToggleDeltaReadModel const cleanedFeatures = features.map(({ strategies, ...rest }) => ({ ...rest, strategies: strategies - ?.sort((strategy1, strategy2) => { - if ( - typeof strategy1.sortOrder === 'number' && - typeof strategy2.sortOrder === 'number' - ) { - return strategy1.sortOrder - strategy2.sortOrder; - } - return 0; - }) - .map(({ id, title, sortOrder, ...strategy }) => ({ + ?.sort(sortStrategies) + .map(({ id, title, sortOrder, milestoneId, ...strategy }) => ({ ...strategy, })), })); @@ -216,6 +210,7 @@ export default class ClientFeatureToggleDeltaReadModel constraints: row.constraints || [], parameters: mapValues(row.parameters || {}, ensureStringValue), sortOrder: row.sort_order, + milestoneId: row.milestone_id, }; strategy.variants = row.strategy_variants || []; return strategy; diff --git a/src/lib/features/export-import-toggles/export-import-service.ts b/src/lib/features/export-import-toggles/export-import-service.ts index f55c7cec89..7558007a09 100644 --- a/src/lib/features/export-import-toggles/export-import-service.ts +++ b/src/lib/features/export-import-toggles/export-import-service.ts @@ -949,6 +949,7 @@ export default class ExportImportService projectId, environment, strategyName, + milestoneId, ...rest } = item; return { diff --git a/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts b/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts index 0ea0a5d5ff..d6d1b26869 100644 --- a/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts +++ b/src/lib/features/feature-toggle/converters/feature-toggle-row-converter.ts @@ -9,6 +9,7 @@ import type { } from '../../../types'; import { mapValues, ensureStringValue } from '../../../util'; +import { sortStrategies } from '../../../util/sortStrategies'; import type { FeatureConfigurationClient } from '../types/feature-toggle-strategies-store-type'; export class FeatureToggleRowConverter { @@ -106,6 +107,7 @@ export class FeatureToggleRowConverter { constraints: row.constraints || [], parameters: mapValues(row.parameters || {}, ensureStringValue), sortOrder: row.sort_order, + milestoneId: row.milestone_id, disabled: row.strategy_disabled, variants: row.strategy_variants || [], }; @@ -128,16 +130,8 @@ export class FeatureToggleRowConverter { Object.values(result).map(({ strategies, ...rest }) => ({ ...rest, strategies: strategies - ?.sort((strategy1, strategy2) => { - if ( - typeof strategy1.sortOrder === 'number' && - typeof strategy2.sortOrder === 'number' - ) { - return strategy1.sortOrder - strategy2.sortOrder; - } - return 0; - }) - .map(({ title, sortOrder, ...strategy }) => ({ + ?.sort(sortStrategies) + .map(({ title, sortOrder, milestoneId, ...strategy }) => ({ ...strategy, ...(title ? { title } : {}), })), diff --git a/src/lib/features/feature-toggle/feature-toggle-controller.ts b/src/lib/features/feature-toggle/feature-toggle-controller.ts index 2a30b16a6e..f0932d47c3 100644 --- a/src/lib/features/feature-toggle/feature-toggle-controller.ts +++ b/src/lib/features/feature-toggle/feature-toggle-controller.ts @@ -856,6 +856,7 @@ export default class ProjectFeaturesController extends Controller { projectId: project, environment: environmentId, createdAt, + milestoneId, ...rest } = strategy; return { ...rest, name: strategyName }; diff --git a/src/lib/features/feature-toggle/feature-toggle-service.ts b/src/lib/features/feature-toggle/feature-toggle-service.ts index 14a4b6680f..75d9b80c1b 100644 --- a/src/lib/features/feature-toggle/feature-toggle-service.ts +++ b/src/lib/features/feature-toggle/feature-toggle-service.ts @@ -112,6 +112,7 @@ import type { IFeatureLifecycleReadModel } from '../feature-lifecycle/feature-li import type { ResourceLimitsSchema } from '../../openapi'; import { throwExceedsLimitError } from '../../error/exceeds-limit-error'; import type { Collaborator } from './types/feature-collaborators-read-model-type'; +import { sortStrategies } from '../../util/sortStrategies'; interface IFeatureContext { featureName: string; @@ -596,15 +597,7 @@ class FeatureToggleService { environment, ) ) - .sort((strategy1, strategy2) => { - if ( - typeof strategy1.sortOrder === 'number' && - typeof strategy2.sortOrder === 'number' - ) { - return strategy1.sortOrder - strategy2.sortOrder; - } - return 0; - }) + .sort(sortStrategies) .map((strategy) => strategy.id); const eventPreData: StrategyIds = { strategyIds: existingOrder }; @@ -624,15 +617,7 @@ class FeatureToggleService { environment, ) ) - .sort((strategy1, strategy2) => { - if ( - typeof strategy1.sortOrder === 'number' && - typeof strategy2.sortOrder === 'number' - ) { - return strategy1.sortOrder - strategy2.sortOrder; - } - return 0; - }) + .sort(sortStrategies) .map((strategy) => strategy.id); const eventData: StrategyIds = { strategyIds: newOrder }; @@ -1042,6 +1027,7 @@ class FeatureToggleService { title: strat.title, disabled: strat.disabled, sortOrder: strat.sortOrder, + milestoneId: strat.milestoneId, segments, }); } diff --git a/src/lib/features/feature-toggle/feature-toggle-store.ts b/src/lib/features/feature-toggle/feature-toggle-store.ts index fe412fb998..4084024119 100644 --- a/src/lib/features/feature-toggle/feature-toggle-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-store.ts @@ -152,6 +152,7 @@ export default class FeatureToggleStore implements IFeatureToggleStore { 'fs.parameters as parameters', 'fs.constraints as constraints', 'fs.sort_order as sort_order', + 'fs.milestone_id as milestone_id', 'fs.variants as strategy_variants', 'segments.id as segment_id', 'segments.constraints as segment_constraints', diff --git a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts index 05ed24720f..68e380412e 100644 --- a/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts +++ b/src/lib/features/feature-toggle/feature-toggle-strategies-store.ts @@ -62,6 +62,7 @@ interface IFeatureStrategiesTable { constraints: string; variants: string; sort_order: number; + milestone_id?: string; created_at?: Date; disabled?: boolean | null; } @@ -86,6 +87,7 @@ function mapRow(row: IFeatureStrategiesTable): IFeatureStrategy { variants: (row.variants as unknown as IStrategyVariant[]) || [], createdAt: row.created_at, sortOrder: row.sort_order, + milestoneId: row.milestone_id, disabled: row.disabled, }; } @@ -326,6 +328,9 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore { feature_name: featureName, environment, }) + .orderByRaw( + 'CASE WHEN milestone_id IS NOT NULL THEN 0 ELSE 1 END ASC', + ) .orderBy([ { column: 'sort_order', diff --git a/src/lib/types/model.ts b/src/lib/types/model.ts index cd97dd91b4..cfcf0fdeb6 100644 --- a/src/lib/types/model.ts +++ b/src/lib/types/model.ts @@ -38,6 +38,7 @@ export interface IStrategyConfig { segments?: number[]; parameters?: { [key: string]: string }; sortOrder?: number; + milestoneId?: string; title?: string | null; disabled?: boolean | null; } @@ -49,6 +50,7 @@ export interface IFeatureStrategy { strategyName: string; parameters: { [key: string]: string }; sortOrder?: number; + milestoneId?: string; constraints: IConstraint[]; variants?: IStrategyVariant[]; createdAt?: Date; diff --git a/src/lib/util/sortStrategies.test.ts b/src/lib/util/sortStrategies.test.ts new file mode 100644 index 0000000000..57b8dd5b78 --- /dev/null +++ b/src/lib/util/sortStrategies.test.ts @@ -0,0 +1,68 @@ +import { sortStrategies } from './sortStrategies'; + +describe('sortStrategies', () => { + it('should prioritize strategies with milestoneId over those without', () => { + const strategies = [ + { sortOrder: 2 }, + { milestoneId: 'm1', sortOrder: 1 }, + { sortOrder: 1 }, + { milestoneId: 'm2', sortOrder: 2 }, + ]; + + const sorted = strategies.sort(sortStrategies); + + expect(sorted).toEqual([ + { milestoneId: 'm1', sortOrder: 1 }, + { milestoneId: 'm2', sortOrder: 2 }, + { sortOrder: 1 }, + { sortOrder: 2 }, + ]); + }); + + it('should sort by sortOrder when both have milestoneId', () => { + const strategies = [ + { milestoneId: 'm1', sortOrder: 2 }, + { milestoneId: 'm2', sortOrder: 1 }, + ]; + + const sorted = strategies.sort(sortStrategies); + + expect(sorted).toEqual([ + { milestoneId: 'm2', sortOrder: 1 }, + { milestoneId: 'm1', sortOrder: 2 }, + ]); + }); + + it('should sort by sortOrder when neither has milestoneId', () => { + const strategies = [ + { sortOrder: 3 }, + { sortOrder: 1 }, + { sortOrder: 2 }, + ]; + + const sorted = strategies.sort(sortStrategies); + + expect(sorted).toEqual([ + { sortOrder: 1 }, + { sortOrder: 2 }, + { sortOrder: 3 }, + ]); + }); + + it('should return 0 if both milestoneId and sortOrder are equal', () => { + const strategy1 = { milestoneId: 'm1', sortOrder: 1 }; + const strategy2 = { milestoneId: 'm1', sortOrder: 1 }; + + const result = sortStrategies(strategy1, strategy2); + + expect(result).toBe(0); + }); + + it('should handle cases where sortOrder is not a number', () => { + const strategies = [{ sortOrder: undefined }, { sortOrder: 1 }]; + + const sorted = strategies.sort(sortStrategies); + + expect(sorted).toEqual([{ sortOrder: undefined }, { sortOrder: 1 }]); + }); +}); diff --git a/src/lib/util/sortStrategies.ts b/src/lib/util/sortStrategies.ts new file mode 100644 index 0000000000..fe9b5da345 --- /dev/null +++ b/src/lib/util/sortStrategies.ts @@ -0,0 +1,23 @@ +import type { IStrategyConfig } from '../types'; + +type SortableStrategy = Pick; + +export const sortStrategies = ( + strategy1: SortableStrategy, + strategy2: SortableStrategy, +): number => { + if (strategy1.milestoneId && !strategy2.milestoneId) { + return -1; + } + if (!strategy1.milestoneId && strategy2.milestoneId) { + return 1; + } + + if ( + typeof strategy1.sortOrder === 'number' && + typeof strategy2.sortOrder === 'number' + ) { + return strategy1.sortOrder - strategy2.sortOrder; + } + return 0; +};