Skip to content

Commit

Permalink
feat: add potentiallyStale filter (#8784)
Browse files Browse the repository at this point in the history
This PR adds support for the `potentiallyStale` value in the feature
search API. The value is added as a third option for `state` (in
addition to `stale` and `active`). Potentially stale is a subset of
active flags, so stale flags are never considered potentially stale,
even if they have the flag set in the db.

Because potentially stale is a separate column in the db, this
complicates the query a bit. As such, I've created a specialized
handling function in the feature search store: if the query doesn't
include `potentiallyStale`, handle it as we did before (the mapping has
just been moved). If the query *does* contain potentially stale, though,
the handling is quite a bit more involved because we need to check
multiple different columns against each other.

In essence, it's based on this logic:

when you’re searching for potentially stale flags, you should only get flags that are active and marked as potentially stale. You should not get stale flags.

This can cause some confusion, because in the db, we don’t clear the potentially stale status when we mark a flag as stale, so we can get flags that are both stale and potentially stale.

However, as a user, if you’re looking for potentially stale flags, I’d be surprised to also get (only some) stale flags, because if a flag is stale, it’s definitely stale, not potentially stale.

This leads us to these six different outcomes we need to handle when your search includes potentially stale and stale or active:

1. You filter for “potentially stale” flags only. The API will give you only flags that are active and marked as potentially stale. You will not get stale flags.
2. You filter only for flags that are not potentially stale. You will get all flags that are active and not potentially stale and all stale flags.
3. You search for “is any of stale, potentially stale”. This is our “unhealthy flags” metric. You get all stale flags and all flags that are active and potentially stale
4. You search for “is none of stale, potentially stale”: This gives you all flags that are active and not potentially stale. Healthy flags, if you will.
5. “is any of active, potentially stale”: you get all active flags. Because we treat potentially stale as a subset of active, this is the same as “is active”
6. “is none of active, potentially stale”: you get all stale flags. As in the previous point, this is the same as “is not active”
  • Loading branch information
thomasheartman authored Nov 19, 2024
1 parent 8935a01 commit 8da201a
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 18 deletions.
3 changes: 0 additions & 3 deletions src/lib/features/feature-search/feature-search-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ export class FeatureSearchService {
if (params.state) {
const parsedState = parseSearchOperatorValue('stale', params.state);
if (parsedState) {
parsedState.values = parsedState.values.map((value) =>
value === 'active' ? 'false' : 'true',
);
queryParams.push(parsedState);
}
}
Expand Down
102 changes: 101 additions & 1 deletion src/lib/features/feature-search/feature-search-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,19 +569,119 @@ class FeatureSearchStore implements IFeatureSearchStore {
}
}

const applyStaleConditions = (
query: Knex.QueryBuilder,
staleConditions?: IQueryParam,
): void => {
if (!staleConditions) return;

const { values, operator } = staleConditions;

if (!values.includes('potentiallyStale')) {
applyGenericQueryParams(query, [
{
...staleConditions,
values: values.map((value) =>
value === 'active' ? 'false' : 'true',
),
},
]);
return;
}

const valueSet = new Set(
values.filter((value) =>
['stale', 'active', 'potentiallyStale'].includes(value || ''),
),
);
const allSelected = valueSet.size === 3;
const onlyPotentiallyStale = valueSet.size === 1;
const staleAndPotentiallyStale =
valueSet.has('stale') && valueSet.size === 2;

if (allSelected) {
switch (operator) {
case 'IS':
case 'IS_ANY_OF':
// All flags included; no action needed
break;
case 'IS_NOT':
case 'IS_NONE_OF':
// All flags excluded
query.whereNotIn('features.stale', [false, true]);
break;
}
return;
}

if (onlyPotentiallyStale) {
switch (operator) {
case 'IS':
case 'IS_ANY_OF':
query
.where('features.stale', false)
.where('features.potentially_stale', true);
break;
case 'IS_NOT':
case 'IS_NONE_OF':
query.where((qb) =>
qb
.where('features.stale', true)
.orWhere('features.potentially_stale', false),
);
break;
}
return;
}

if (staleAndPotentiallyStale) {
switch (operator) {
case 'IS':
case 'IS_ANY_OF':
query.where((qb) =>
qb
.where('features.stale', true)
.orWhere('features.potentially_stale', true),
);
break;
case 'IS_NOT':
case 'IS_NONE_OF':
query
.where('features.stale', false)
.where('features.potentially_stale', false);
break;
}
} else {
switch (operator) {
case 'IS':
case 'IS_ANY_OF':
query.where('features.stale', false);
break;
case 'IS_NOT':
case 'IS_NONE_OF':
query.where('features.stale', true);
break;
}
}
};
const applyQueryParams = (
query: Knex.QueryBuilder,
queryParams: IQueryParam[],
): void => {
const tagConditions = queryParams.filter((param) => param.field === 'tag');
const staleConditions = queryParams.find(
(param) => param.field === 'stale',
);
const segmentConditions = queryParams.filter(
(param) => param.field === 'segment',
);
const genericConditions = queryParams.filter(
(param) => param.field !== 'tag',
(param) => !['tag', 'stale'].includes(param.field),
);
applyGenericQueryParams(query, genericConditions);

applyStaleConditions(query, staleConditions);

applyMultiQueryParams(
query,
tagConditions,
Expand Down
73 changes: 60 additions & 13 deletions src/lib/features/feature-search/feature.search.e2e.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -968,27 +968,74 @@ test('should search features by state with operators', async () => {
});
});

test('should search features by created date with operators', async () => {
test('should search features by potentially stale', async () => {
await app.createFeature({
name: 'my_feature_a',
createdAt: '2023-01-27T15:21:39.975Z',
stale: false,
});
await app.createFeature({
name: 'my_feature_b',
createdAt: '2023-01-29T15:21:39.975Z',
stale: true,
});

const { body } = await filterFeaturesByCreated('IS_BEFORE:2023-01-28');
expect(body).toMatchObject({
features: [{ name: 'my_feature_a' }],
await app.createFeature({
name: 'my_feature_c',
stale: false,
});

const { body: afterBody } = await filterFeaturesByCreated(
'IS_ON_OR_AFTER:2023-01-28',
);
expect(afterBody).toMatchObject({
features: [{ name: 'my_feature_b' }],
await app.createFeature({
name: 'my_feature_d',
stale: true,
});

// this is all done on a schedule, so there's no imperative way to mark something as potentially stale today.
await db
.rawDatabase('features')
.update('potentially_stale', true)
.whereIn('name', ['my_feature_c', 'my_feature_d']);

const check = async (filter: string, expectedFlags: string[]) => {
const { body } = await filterFeaturesByState(filter);
expect(body).toMatchObject({
features: expectedFlags.map((flag) => ({ name: flag })),
});
};

// single filters work
await check('IS:potentiallyStale', ['my_feature_c']);
// (stale or !potentiallyStale)
await check('IS_NOT:potentiallyStale', [
'my_feature_a',
'my_feature_b',
'my_feature_d',
]);

// combo filters work
await check('IS_ANY_OF:active,potentiallyStale', [
'my_feature_a',
'my_feature_c',
]);

// (potentiallyStale OR stale)
await check('IS_ANY_OF:potentiallyStale,stale', [
'my_feature_b',
'my_feature_c',
'my_feature_d',
]);

await check('IS_ANY_OF:active,potentiallyStale,stale', [
'my_feature_a',
'my_feature_b',
'my_feature_c',
'my_feature_d',
]);

await check('IS_NONE_OF:active,potentiallyStale,stale', []);

await check('IS_NONE_OF:active,potentiallyStale', [
'my_feature_b',
'my_feature_d',
]);

await check('IS_NONE_OF:potentiallyStale,stale', ['my_feature_a']);
});

test('should filter features by combined operators', async () => {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/features/feature-search/search-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export const parseSearchOperatorValue = (
return {
field,
operator: match[1] as IQueryOperator,
values: match[2].split(','),
values: match[2].split(',').map((value) => value.trim()),
};
}

Expand Down

0 comments on commit 8da201a

Please sign in to comment.