Skip to content

Commit

Permalink
fix: count lifecycle more accurately (#8816)
Browse files Browse the repository at this point in the history
This PR fixes three things that were wrong with the lifecycle summary
count query:

1. When counting the number of flags in each stage, it does not take
into account whether a flag has moved out of that stage. So if you have
a flag that's gone through initial -> pre-live -> live, it'll be counted
for each one of those steps, not just the last one.

2. Some flags that have been archived don't have the corresponding
archived state row in the db. This causes them to count towards their
other recorded lifecycle stages, even when they shouldn't. This is
related to the previous one, but slightly different. Cross-reference the
features table's archived_at to make sure it hasn't been archived

3. The archived number should probably be all flags ever archived in the
project, regardless of whether they were archived before or after
feature lifecycles. So we should check the feature table's archived_at
flag for the count there instead
  • Loading branch information
thomasheartman authored Nov 21, 2024
1 parent 4a769d1 commit 6d75ad7
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import dbInit, {
} from '../../../../test/e2e/helpers/database-init';
import getLogger from '../../../../test/fixtures/no-logger';
import { ProjectLifecycleSummaryReadModel } from './project-lifecycle-summary-read-model';
import type { IFeatureToggleStore, StageName } from '../../../types';
import type { StageName } from '../../../types';
import { randomId } from '../../../util';

let db: ITestDb;
Expand All @@ -14,7 +14,7 @@ beforeAll(async () => {
db = await dbInit('project_lifecycle_summary_read_model_serial', getLogger);
readModel = new ProjectLifecycleSummaryReadModel(
db.rawDatabase,
{} as unknown as IFeatureToggleStore,
db.stores.featureToggleStore,
);
});

Expand Down Expand Up @@ -211,15 +211,15 @@ describe('count current flags in each stage', () => {
const flags = [
{
name: randomId(),
stages: ['initial', 'pre-live', 'live', 'archived'],
stages: ['initial', 'live'],
},
{
name: randomId(),
stages: ['initial', 'archived'],
stages: ['initial'],
},
{
name: randomId(),
stages: ['initial', 'pre-live', 'live', 'archived'],
stages: ['initial', 'pre-live', 'live', 'completed'],
},
{ name: randomId(), stages: ['initial', 'pre-live', 'live'] },
];
Expand All @@ -230,13 +230,24 @@ describe('count current flags in each stage', () => {
createdByUserId: 1,
});

for (const stage of stages) {
const time = Date.now();
for (const [index, stage] of stages.entries()) {
await db.stores.featureLifecycleStore.insert([
{
feature: flag.name,
stage: stage as StageName,
},
]);

await db
.rawDatabase('feature_lifecycles')
.where({
feature: flag.name,
stage: stage,
})
.update({
created_at: addMinutes(time, index),
});
}
}

Expand Down Expand Up @@ -266,11 +277,60 @@ describe('count current flags in each stage', () => {
const result = await readModel.getCurrentFlagsInEachStage(project.id);

expect(result).toMatchObject({
initial: 4,
'pre-live': 3,
live: 3,
completed: 0,
archived: 3,
initial: 1,
'pre-live': 0,
live: 2,
completed: 1,
archived: 0,
});
});

test('if a flag is archived, but does not have the corresponding lifecycle stage, we still count it as archived and exclude it from other stages', async () => {
const project = await db.stores.projectStore.create({
name: 'project',
id: randomId(),
});

const flag = await db.stores.featureToggleStore.create(project.id, {
name: randomId(),
createdByUserId: 1,
});

await db.stores.featureLifecycleStore.insert([
{
feature: flag.name,
stage: 'initial',
},
]);

await db.stores.featureToggleStore.archive(flag.name);

const result = await readModel.getCurrentFlagsInEachStage(project.id);

expect(result).toMatchObject({
initial: 0,
archived: 1,
});
});

test('the archived count is based on the features table (source of truth), not the lifecycle table', async () => {
const project = await db.stores.projectStore.create({
name: 'project',
id: randomId(),
});

const flag = await db.stores.featureToggleStore.create(project.id, {
name: randomId(),
createdByUserId: 1,
});

await db.stores.featureToggleStore.archive(flag.name);

const result = await readModel.getCurrentFlagsInEachStage(project.id);

expect(result).toMatchObject({
initial: 0,
archived: 1,
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -81,16 +81,37 @@ export class ProjectLifecycleSummaryReadModel
}

async getCurrentFlagsInEachStage(projectId: string): Promise<FlagsInStage> {
const query = this.db('feature_lifecycles as fl')
const query = this.db
.with('latest_stage', (qb) => {
qb.select('fl.feature')
.max('fl.created_at as max_created_at')
.from('feature_lifecycles as fl')
.groupBy('fl.feature');
})
.from('latest_stage as ls')
.innerJoin('feature_lifecycles as fl', (qb) => {
qb.on('ls.feature', '=', 'fl.feature').andOn(
'ls.max_created_at',
'=',
'fl.created_at',
);
})
.innerJoin('features as f', 'fl.feature', 'f.name')
.where('f.project', projectId)
.whereNot('fl.stage', 'archived')
.whereNull('f.archived_at')
.select('fl.stage')
.count('fl.feature as flag_count')
.groupBy('fl.stage');

const result = await query;

return result.reduce(
const archivedCount = await this.featureToggleStore.count({
project: projectId,
archived: true,
});

const lifecycleStages = result.reduce(
(acc, row) => {
acc[row.stage] = Number(row.flag_count);
return acc;
Expand All @@ -100,9 +121,12 @@ export class ProjectLifecycleSummaryReadModel
'pre-live': 0,
live: 0,
completed: 0,
archived: 0,
},
) as FlagsInStage;
return {
...lifecycleStages,
archived: archivedCount,
};
}

async getArchivedFlagsLast30Days(projectId: string): Promise<number> {
Expand Down

0 comments on commit 6d75ad7

Please sign in to comment.