Skip to content

Commit

Permalink
Feat/enable disable strategies (#3566)
Browse files Browse the repository at this point in the history
<!-- Thanks for creating a PR! To make it easier for reviewers and
everyone else to understand what your changes relate to, please add some
relevant content to the headings below. Feel free to ignore or delete
sections that you don't think are relevant. Thank you! ❤️ -->
Adds enabled field to feature strategies
Filter out disabled strategies when returning/evaluating

## About the changes
<!-- Describe the changes introduced. What are they and why are they
being introduced? Feel free to also add screenshots or steps to view the
changes if they're visual. -->

<!-- Does it close an issue? Multiple? -->
Closes #
[1-865](https://linear.app/unleash/issue/1-865/allow-for-enablingdisabling-strategies-in-place-backend)

<!-- (For internal contributors): Does it relate to an issue on public
roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item:
#
-->

### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally
important. Where should a reviewer start looking to get an overview of
the changes? Are any files particularly important? -->


## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged?
Got any questions or doubts? -->

---------

Signed-off-by: andreas-unleash <andreas@getunleash.ai>
  • Loading branch information
andreas-unleash committed Apr 21, 2023
1 parent 29907d8 commit 097dd8a
Show file tree
Hide file tree
Showing 20 changed files with 462 additions and 58 deletions.
9 changes: 9 additions & 0 deletions src/lib/db/feature-strategy-store.ts
Expand Up @@ -35,6 +35,7 @@ const COLUMNS = [
'parameters',
'constraints',
'created_at',
'disabled',
];
/*
const mapperToColumnNames = {
Expand Down Expand Up @@ -62,6 +63,7 @@ interface IFeatureStrategiesTable {
constraints: string;
sort_order: number;
created_at?: Date;
disabled?: boolean | null;
}

export interface ILoadFeatureToggleWithEnvsParams {
Expand All @@ -83,6 +85,7 @@ function mapRow(row: IFeatureStrategiesTable): IFeatureStrategy {
constraints: (row.constraints as unknown as IConstraint[]) || [],
createdAt: row.created_at,
sortOrder: row.sort_order,
disabled: row.disabled,
};
}

Expand All @@ -98,6 +101,7 @@ function mapInput(input: IFeatureStrategy): IFeatureStrategiesTable {
constraints: JSON.stringify(input.constraints || []),
created_at: input.createdAt,
sort_order: input.sortOrder,
disabled: input.disabled,
};
}

Expand All @@ -106,6 +110,7 @@ interface StrategyUpdate {
parameters: object;
constraints: string;
title?: string;
disabled?: boolean;
}

function mapStrategyUpdate(
Expand All @@ -121,6 +126,9 @@ function mapStrategyUpdate(
if (input.title !== null) {
update.title = input.title;
}
if (input.disabled !== null) {
update.disabled = input.disabled;
}
update.constraints = JSON.stringify(input.constraints || []);
return update;
}
Expand Down Expand Up @@ -590,6 +598,7 @@ class FeatureStrategiesStore implements IFeatureStrategiesStore {
sortOrder: r.sort_order,
id: r.strategy_id,
title: r.strategy_title || '',
disabled: r.strategy_disabled || false,
};
if (!includeId) {
delete strategy.id;
Expand Down
71 changes: 45 additions & 26 deletions src/lib/db/feature-toggle-client-store.ts
Expand Up @@ -4,36 +4,24 @@ import { DB_TIME } from '../metric-events';
import { Logger, LogProvider } from '../logger';
import {
IFeatureToggleClient,
IFeatureToggleClientStore,
IFeatureToggleQuery,
IStrategyConfig,
ITag,
} from '../types/model';
import { IFeatureToggleClientStore } from '../types/stores/feature-toggle-client-store';
import { DEFAULT_ENV } from '../util/constants';
import { PartialDeep } from '../types/partial';
PartialDeep,
} from '../types';
import { DEFAULT_ENV, ensureStringValue, mapValues } from '../util';
import EventEmitter from 'events';
import FeatureToggleStore from './feature-toggle-store';
import { ensureStringValue } from '../util/ensureStringValue';
import { mapValues } from '../util/map-values';
import Raw = Knex.Raw;
import { Db } from './db';

export interface FeaturesTable {
name: string;
description: string;
type: string;
stale: boolean;
variants: string;
project: string;
last_seen_at?: Date;
created_at?: Date;
}
import Raw = Knex.Raw;

export interface IGetAllFeatures {
featureQuery?: IFeatureToggleQuery;
archived: boolean;
isAdmin: boolean;
includeStrategyIds?: boolean;
includeDisabledStrategies?: boolean;
userId?: number;
}

Expand Down Expand Up @@ -67,6 +55,7 @@ export default class FeatureToggleClientStore
archived,
isAdmin,
includeStrategyIds,
includeDisabledStrategies,
userId,
}: IGetAllFeatures): Promise<IFeatureToggleClient[]> {
const environment = featureQuery?.environment || DEFAULT_ENV;
Expand All @@ -86,6 +75,7 @@ export default class FeatureToggleClientStore
'fe.environment as environment',
'fs.id as strategy_id',
'fs.strategy_name as strategy_name',
'fs.disabled as strategy_disabled',
'fs.parameters as parameters',
'fs.constraints as constraints',
'segments.id as segment_id',
Expand Down Expand Up @@ -180,7 +170,7 @@ export default class FeatureToggleClientStore
strategies: [],
};
if (this.isUnseenStrategyRow(feature, r)) {
feature.strategies.push(
feature.strategies?.push(
FeatureToggleClientStore.rowToStrategy(r),
);
}
Expand Down Expand Up @@ -221,13 +211,21 @@ export default class FeatureToggleClientStore
FeatureToggleClientStore.removeIdsFromStrategies(features);
}

if (!includeDisabledStrategies) {
// We should not send disabled strategies from the client API,
// as this breaks the way SDKs evaluate the status of the feature.
return this.removeDisabledStrategies(features);
}

return features;
}

private static rowToStrategy(row: Record<string, any>): IStrategyConfig {
return {
id: row.strategy_id,
name: row.strategy_name,
title: row.strategy_title,
disabled: row.strategy_disabled,
constraints: row.constraints || [],
parameters: mapValues(row.parameters || {}, ensureStringValue),
};
Expand All @@ -248,13 +246,26 @@ export default class FeatureToggleClientStore
});
}

private removeDisabledStrategies(
features: IFeatureToggleClient[],
): IFeatureToggleClient[] {
const filtered: IFeatureToggleClient[] = [];
features.forEach((feature) => {
const filteredStrategies = feature.strategies.filter(
(strategy) => !strategy.disabled,
);
filtered.push({ ...feature, strategies: filteredStrategies });
});
return filtered;
}

private isUnseenStrategyRow(
feature: PartialDeep<IFeatureToggleClient>,
row: Record<string, any>,
): boolean {
return (
row.strategy_id &&
!feature.strategies.find((s) => s.id === row.strategy_id)
!feature.strategies?.find((s) => s?.id === row.strategy_id)
);
}

Expand All @@ -276,7 +287,7 @@ export default class FeatureToggleClientStore
row.tag_value &&
!feature.tags?.some(
(tag) =>
tag.type === row.tag_type && tag.value === row.tag_value,
tag?.type === row.tag_type && tag?.value === row.tag_value,
)
);
}
Expand All @@ -286,16 +297,16 @@ export default class FeatureToggleClientStore
row: Record<string, any>,
) {
feature.strategies
.find((s) => s.id === row.strategy_id)
?.constraints.push(...row.segment_constraints);
?.find((s) => s?.id === row.strategy_id)
?.constraints?.push(...row.segment_constraints);
}

private addSegmentIdsToStrategy(
feature: PartialDeep<IFeatureToggleClient>,
row: Record<string, any>,
) {
const strategy = feature.strategies.find(
(s) => s.id === row.strategy_id,
const strategy = feature.strategies?.find(
(s) => s?.id === row.strategy_id,
);
if (!strategy) {
return;
Expand All @@ -309,12 +320,14 @@ export default class FeatureToggleClientStore
async getClient(
featureQuery?: IFeatureToggleQuery,
includeStrategyIds?: boolean,
includeDisabledStrategies?: boolean,
): Promise<IFeatureToggleClient[]> {
return this.getAll({
featureQuery,
archived: false,
isAdmin: false,
includeStrategyIds,
includeDisabledStrategies,
});
}

Expand All @@ -323,7 +336,13 @@ export default class FeatureToggleClientStore
userId,
archived,
}: IGetAdminFeatures): Promise<IFeatureToggleClient[]> {
return this.getAll({ featureQuery, archived, isAdmin: true, userId });
return this.getAll({
featureQuery,
archived: Boolean(archived),
isAdmin: true,
userId,
includeDisabledStrategies: true,
});
}
}

Expand Down
7 changes: 7 additions & 0 deletions src/lib/openapi/spec/create-feature-strategy-schema.ts
Expand Up @@ -18,6 +18,13 @@ export const createFeatureStrategySchema = {
description: 'A descriptive title for the strategy',
example: 'Gradual Rollout 25-Prod',
},
disabled: {
type: 'boolean',
description:
'A toggle to disable the strategy. defaults to false. Disabled strategies are not evaluated or returned to the SDKs',
example: false,
nullable: true,
},
sortOrder: {
type: 'number',
description: 'The order of the strategy in the list',
Expand Down
7 changes: 7 additions & 0 deletions src/lib/openapi/spec/feature-strategy-schema.ts
Expand Up @@ -26,6 +26,13 @@ export const featureStrategySchema = {
example: 'Gradual Rollout 25-Prod',
nullable: true,
},
disabled: {
type: 'boolean',
description:
'A toggle to disable the strategy. defaults to false. Disabled strategies are not evaluated or returned to the SDKs',
example: false,
nullable: true,
},
featureName: {
type: 'string',
description: 'The name or feature the strategy is attached to',
Expand Down
1 change: 1 addition & 0 deletions src/lib/openapi/spec/playground-feature-schema.test.ts
Expand Up @@ -64,6 +64,7 @@ const playgroundStrategy = (
parameters,
constraints: playgroundStrategyConstraints(),
segments: fc.array(playgroundSegment()),
disabled: fc.boolean(),
});

const playgroundStrategies = (): Arbitrary<PlaygroundStrategySchema[]> =>
Expand Down
17 changes: 16 additions & 1 deletion src/lib/openapi/spec/playground-strategy-schema.ts
Expand Up @@ -60,7 +60,15 @@ export const playgroundStrategySchema = {
$id: '#/components/schemas/playgroundStrategySchema',
type: 'object',
additionalProperties: false,
required: ['id', 'name', 'result', 'segments', 'constraints', 'parameters'],
required: [
'id',
'name',
'result',
'segments',
'constraints',
'parameters',
'disabled',
],
properties: {
name: {
description: "The strategy's name.",
Expand All @@ -79,6 +87,13 @@ export const playgroundStrategySchema = {
description: `The strategy's evaluation result. If the strategy is a custom strategy that Unleash can't evaluate, \`evaluationStatus\` will be \`${playgroundStrategyEvaluation.unknownResult}\`. Otherwise, it will be \`true\` or \`false\``,
...strategyEvaluationResults,
},
disabled: {
type: 'boolean',
description:
"The strategy's status. Disabled strategies are not evaluated",
example: false,
nullable: true,
},
segments: {
type: 'array',
description:
Expand Down
13 changes: 13 additions & 0 deletions src/lib/openapi/spec/update-feature-strategy-schema.ts
Expand Up @@ -18,6 +18,19 @@ export const updateFeatureStrategySchema = {
$ref: '#/components/schemas/constraintSchema',
},
},
title: {
type: 'string',
nullable: true,
description: 'A descriptive title for the strategy',
example: 'Gradual Rollout 25-Prod',
},
disabled: {
type: 'boolean',
description:
'A toggle to disable the strategy. defaults to true. Disabled strategies are not evaluated or returned to the SDKs',
example: false,
nullable: true,
},
parameters: {
$ref: '#/components/schemas/parametersSchema',
},
Expand Down

1 comment on commit 097dd8a

@vercel
Copy link

@vercel vercel bot commented on 097dd8a Apr 21, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.