mirror of
https://github.com/Unleash/unleash.git
synced 2025-01-25 00:07:47 +01:00
chore: Extract project read model (#7887)
Creates a new project read model exposing data to be used for the UI and for the insights module. The model contains two public methods, both based on the project store's `getProjectsWithCounts`: - `getProjectsForAdminUi` - `getProjectsForInsights` This mirrors the two places where the base query is actually in use today and adapts the query to those two explicit cases. The new `getProjectsForAdminUi` method also contains data for last flag update and last flag metric reported, as required for the new projects list screen. Additionally the read model contains a private `getMembersCount` method, which is also lifted from the project store. This method was only used in the old `getProjectsWithCounts` method, so I have also removed the method from the public interface. This PR does *not* hook up the new read model to anything or delete any existing uses of the old method. ## Why? As mentioned in the background, this query is used in two places, both to get data for the UI (directly or indirectly). This is consistent with the principles laid out in our [ADR on read vs write models](https://docs.getunleash.io/contributing/ADRs/back-end/write-model-vs-read-models). There is an argument to be made, however, that the insights module uses this as an **internal** read model, but the description of an internal model ("Internal read models are typically narrowly focused on answering one question and usually require simple queries compared to external read models") does not apply here. It's closer to the description of external read models: "View model will typically join data across a few DB tables" for display in the UI. ## Discussion points ### What about properties on the schema that are now gone? The `project-schema`, which is delivered to the UI through the `getProjects` endpoint (and nowhere else, it seems), describes properties that will no longer be sent to the front end, including `defaultStickiness`, `avgTimeToProduction`, and more. Can we just stop sending them or is that a breaking change? The schema does not define them as required properties, so in theory, not sending them isn't breaking any contracts. We can deprecate the properties and just not populate them anymore. At least that's my thought on it. I'm open to hearing other views. ### Can we add the properties in fewer lines of code? Yes! The [first commit in this PR (b7534bfa
)](b7534bfa07
) adds the two new properties in 8 lines of code. However, this comes at the cost of diluting the `getProjectsWithCounts` method further by adding more properties that are not used by the insights module. That said, that might be a worthwhile tradeoff. ## Background _(More [details in internal slack thread](https://unleash-internal.slack.com/archives/C046LV6HH6W/p1723716675436829))_ I noticed that the project store's `getProjectWithCounts` is used in exactly two places: 1. In the project service method which maps directly to the project controller (in both OSS and enterprise). 2. In the insights service in enterprise. In the case of the controller, that’s the termination point. I’d guess that when written, the store only served the purpose of showing data to the UI. In the event of the insights service, the data is mapped in getProjectStats. But I was a little surprised that they were sharing the same query, so I decided to dig a little deeper to see what we’re actually using and what we’re not (including the potential new columns). Here’s what I found. Of the 14 already existing properties, insights use only 7 and the project list UI uses only 10 (though the schema mentions all 14 (as far as I could tell from scouring the code base)). Additionally, there’s two properties that I couldn’t find any evidence of being used by either: - default stickiness - updatedAt (this is when the project was last updated; not its flags)
This commit is contained in:
parent
f2e2952c31
commit
0847a395dc
37
src/lib/features/project/project-read-model-type.ts
Normal file
37
src/lib/features/project/project-read-model-type.ts
Normal file
@ -0,0 +1,37 @@
|
||||
import type { ProjectMode } from '../../types';
|
||||
import type { IProjectQuery } from './project-store-type';
|
||||
|
||||
export type ProjectForUi = {
|
||||
id: string;
|
||||
name: string;
|
||||
description?: string;
|
||||
health: number;
|
||||
createdAt: Date;
|
||||
mode: ProjectMode;
|
||||
memberCount: number;
|
||||
favorite: boolean;
|
||||
archivedAt?: Date;
|
||||
featureCount: number;
|
||||
lastReportedFlagUsage: Date | null;
|
||||
lastFlagUpdate: Date | null;
|
||||
};
|
||||
|
||||
export type ProjectForInsights = {
|
||||
id: string;
|
||||
health: number;
|
||||
memberCount: number;
|
||||
featureCount: number;
|
||||
staleFeatureCount: number;
|
||||
potentiallyStaleFeatureCount: number;
|
||||
avgTimeToProduction: number;
|
||||
};
|
||||
|
||||
export interface IProjectReadModel {
|
||||
getProjectsForAdminUi(
|
||||
query?: IProjectQuery,
|
||||
userId?: number,
|
||||
): Promise<ProjectForUi[]>;
|
||||
getProjectsForInsights(
|
||||
query?: IProjectQuery,
|
||||
): Promise<ProjectForInsights[]>;
|
||||
}
|
239
src/lib/features/project/project-read-model.ts
Normal file
239
src/lib/features/project/project-read-model.ts
Normal file
@ -0,0 +1,239 @@
|
||||
import type { IFlagResolver } from '../../types';
|
||||
import { Knex } from 'knex';
|
||||
import type { Db } from '../../db/db';
|
||||
import type {
|
||||
IProjectReadModel,
|
||||
ProjectForInsights,
|
||||
ProjectForUi,
|
||||
} from './project-read-model-type';
|
||||
import type { IProjectQuery } from './project-store-type';
|
||||
import metricsHelper from '../../util/metrics-helper';
|
||||
import type EventEmitter from 'events';
|
||||
import type { IProjectMembersCount } from './project-store';
|
||||
import Raw = Knex.Raw;
|
||||
|
||||
const TABLE = 'projects';
|
||||
const DB_TIME = 'db_time';
|
||||
|
||||
const mapProjectForUi = (row): ProjectForUi => {
|
||||
return {
|
||||
name: row.name,
|
||||
id: row.id,
|
||||
description: row.description,
|
||||
health: row.health,
|
||||
favorite: row.favorite,
|
||||
featureCount: Number(row.number_of_features) || 0,
|
||||
memberCount: Number(row.number_of_users) || 0,
|
||||
createdAt: row.created_at,
|
||||
archivedAt: row.archived_at,
|
||||
mode: row.project_mode || 'open',
|
||||
lastReportedFlagUsage: row.last_usage,
|
||||
lastFlagUpdate: row.last_updated,
|
||||
};
|
||||
};
|
||||
|
||||
const mapProjectForInsights = (row): ProjectForInsights => {
|
||||
return {
|
||||
id: row.id,
|
||||
health: row.health,
|
||||
featureCount: Number(row.number_of_features) || 0,
|
||||
staleFeatureCount: Number(row.stale_feature_count) || 0,
|
||||
potentiallyStaleFeatureCount:
|
||||
Number(row.potentially_stale_feature_count) || 0,
|
||||
memberCount: Number(row.number_of_users) || 0,
|
||||
avgTimeToProduction: row.avg_time_to_prod_current_window || 0,
|
||||
};
|
||||
};
|
||||
|
||||
export class ProjectReadModel implements IProjectReadModel {
|
||||
private db: Db;
|
||||
|
||||
private timer: Function;
|
||||
private flagResolver: IFlagResolver;
|
||||
constructor(
|
||||
db: Db,
|
||||
|
||||
eventBus: EventEmitter,
|
||||
flagResolver: IFlagResolver,
|
||||
) {
|
||||
this.db = db;
|
||||
this.timer = (action) =>
|
||||
metricsHelper.wrapTimer(eventBus, DB_TIME, {
|
||||
store: 'project',
|
||||
action,
|
||||
});
|
||||
this.flagResolver = flagResolver;
|
||||
}
|
||||
|
||||
async getProjectsForAdminUi(
|
||||
query?: IProjectQuery,
|
||||
userId?: number,
|
||||
): Promise<ProjectForUi[]> {
|
||||
const projectTimer = this.timer('getProjectsForProjectList');
|
||||
let projects = this.db(TABLE)
|
||||
.leftJoin('features', 'features.project', 'projects.id')
|
||||
.leftJoin(
|
||||
'project_settings',
|
||||
'project_settings.project',
|
||||
'projects.id',
|
||||
)
|
||||
.leftJoin('events', 'events.feature_name', 'features.name')
|
||||
.orderBy('projects.name', 'asc');
|
||||
|
||||
if (this.flagResolver.isEnabled('archiveProjects')) {
|
||||
if (query?.archived === true) {
|
||||
projects = projects.whereNot(`${TABLE}.archived_at`, null);
|
||||
} else {
|
||||
projects = projects.where(`${TABLE}.archived_at`, null);
|
||||
}
|
||||
}
|
||||
|
||||
if (query?.id) {
|
||||
projects = projects.where(`${TABLE}.id`, query.id);
|
||||
}
|
||||
|
||||
let selectColumns = [
|
||||
this.db.raw(
|
||||
'projects.id, projects.name, projects.description, projects.health, projects.created_at, ' +
|
||||
'count(features.name) FILTER (WHERE features.archived_at is null) AS number_of_features, ' +
|
||||
'MAX(features.last_seen_at) AS last_usage,' +
|
||||
'MAX(events.created_at) AS last_updated',
|
||||
),
|
||||
'project_settings.project_mode',
|
||||
] as (string | Raw<any>)[];
|
||||
|
||||
if (this.flagResolver.isEnabled('archiveProjects')) {
|
||||
selectColumns.push(`${TABLE}.archived_at`);
|
||||
}
|
||||
|
||||
let groupByColumns = ['projects.id', 'project_settings.project_mode'];
|
||||
|
||||
if (userId) {
|
||||
projects = projects.leftJoin(`favorite_projects`, function () {
|
||||
this.on('favorite_projects.project', 'projects.id').andOnVal(
|
||||
'favorite_projects.user_id',
|
||||
'=',
|
||||
userId,
|
||||
);
|
||||
});
|
||||
selectColumns = [
|
||||
...selectColumns,
|
||||
this.db.raw(
|
||||
'favorite_projects.project is not null as favorite',
|
||||
),
|
||||
];
|
||||
groupByColumns = [...groupByColumns, 'favorite_projects.project'];
|
||||
}
|
||||
|
||||
const projectAndFeatureCount = await projects
|
||||
.select(selectColumns)
|
||||
.groupBy(groupByColumns);
|
||||
|
||||
const projectsWithFeatureCount =
|
||||
projectAndFeatureCount.map(mapProjectForUi);
|
||||
projectTimer();
|
||||
const memberTimer = this.timer('getMemberCount');
|
||||
|
||||
const memberCount = await this.getMembersCount();
|
||||
memberTimer();
|
||||
const memberMap = new Map<string, number>(
|
||||
memberCount.map((c) => [c.project, Number(c.count)]),
|
||||
);
|
||||
|
||||
return projectsWithFeatureCount.map((projectWithCount) => {
|
||||
return {
|
||||
...projectWithCount,
|
||||
memberCount: memberMap.get(projectWithCount.id) || 0,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
async getProjectsForInsights(
|
||||
query?: IProjectQuery,
|
||||
): Promise<ProjectForInsights[]> {
|
||||
const projectTimer = this.timer('getProjectsForInsights');
|
||||
let projects = this.db(TABLE)
|
||||
.leftJoin('features', 'features.project', 'projects.id')
|
||||
.leftJoin('project_stats', 'project_stats.project', 'projects.id')
|
||||
.orderBy('projects.name', 'asc');
|
||||
|
||||
if (this.flagResolver.isEnabled('archiveProjects')) {
|
||||
if (query?.archived === true) {
|
||||
projects = projects.whereNot(`${TABLE}.archived_at`, null);
|
||||
} else {
|
||||
projects = projects.where(`${TABLE}.archived_at`, null);
|
||||
}
|
||||
}
|
||||
|
||||
if (query?.id) {
|
||||
projects = projects.where(`${TABLE}.id`, query.id);
|
||||
}
|
||||
|
||||
const selectColumns = [
|
||||
this.db.raw(
|
||||
'projects.id, projects.health, ' +
|
||||
'count(features.name) FILTER (WHERE features.archived_at is null) AS number_of_features, ' +
|
||||
'count(features.name) FILTER (WHERE features.archived_at is null and features.stale IS TRUE) AS stale_feature_count, ' +
|
||||
'count(features.name) FILTER (WHERE features.archived_at is null and features.potentially_stale IS TRUE) AS potentially_stale_feature_count,',
|
||||
),
|
||||
'project_stats.avg_time_to_prod_current_window',
|
||||
] as (string | Raw<any>)[];
|
||||
|
||||
if (this.flagResolver.isEnabled('archiveProjects')) {
|
||||
selectColumns.push(`${TABLE}.archived_at`);
|
||||
}
|
||||
|
||||
const groupByColumns = [
|
||||
'projects.id',
|
||||
'project_stats.avg_time_to_prod_current_window',
|
||||
];
|
||||
|
||||
const projectAndFeatureCount = await projects
|
||||
.select(selectColumns)
|
||||
.groupBy(groupByColumns);
|
||||
|
||||
const projectsWithFeatureCount = projectAndFeatureCount.map(
|
||||
mapProjectForInsights,
|
||||
);
|
||||
projectTimer();
|
||||
const memberTimer = this.timer('getMemberCount');
|
||||
|
||||
const memberCount = await this.getMembersCount();
|
||||
memberTimer();
|
||||
const memberMap = new Map<string, number>(
|
||||
memberCount.map((c) => [c.project, Number(c.count)]),
|
||||
);
|
||||
|
||||
return projectsWithFeatureCount.map((projectWithCount) => {
|
||||
return {
|
||||
...projectWithCount,
|
||||
memberCount: memberMap.get(projectWithCount.id) || 0,
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
private async getMembersCount(): Promise<IProjectMembersCount[]> {
|
||||
const members = await this.db
|
||||
.select('project')
|
||||
.from((db) => {
|
||||
db.select('user_id', 'project')
|
||||
.from('role_user')
|
||||
.leftJoin('roles', 'role_user.role_id', 'roles.id')
|
||||
.where((builder) => builder.whereNot('type', 'root'))
|
||||
.union((queryBuilder) => {
|
||||
queryBuilder
|
||||
.select('user_id', 'project')
|
||||
.from('group_role')
|
||||
.leftJoin(
|
||||
'group_user',
|
||||
'group_user.group_id',
|
||||
'group_role.group_id',
|
||||
);
|
||||
})
|
||||
.as('query');
|
||||
})
|
||||
.groupBy('project')
|
||||
.count('user_id');
|
||||
return members;
|
||||
}
|
||||
}
|
@ -1,6 +1,5 @@
|
||||
import type {
|
||||
IEnvironmentProjectLink,
|
||||
IProjectMembersCount,
|
||||
ProjectModeCount,
|
||||
} from './project-store';
|
||||
import type {
|
||||
@ -100,8 +99,9 @@ export interface IProjectStore extends Store<IProject, string> {
|
||||
|
||||
getProjectsByUser(userId: number): Promise<string[]>;
|
||||
|
||||
getMembersCount(): Promise<IProjectMembersCount[]>;
|
||||
|
||||
/**
|
||||
* @deprecated Use the appropriate method in the project read model instead.
|
||||
*/
|
||||
getProjectsWithCounts(
|
||||
query?: IProjectQuery,
|
||||
userId?: number,
|
||||
|
@ -511,7 +511,7 @@ class ProjectStore implements IProjectStore {
|
||||
return rows.map(this.mapProjectEnvironmentRow);
|
||||
}
|
||||
|
||||
async getMembersCount(): Promise<IProjectMembersCount[]> {
|
||||
private async getMembersCount(): Promise<IProjectMembersCount[]> {
|
||||
const members = await this.db
|
||||
.select('project')
|
||||
.from((db) => {
|
||||
|
5
src/test/fixtures/fake-project-store.ts
vendored
5
src/test/fixtures/fake-project-store.ts
vendored
@ -8,7 +8,6 @@ import type {
|
||||
import NotFoundError from '../../lib/error/notfound-error';
|
||||
import type {
|
||||
IEnvironmentProjectLink,
|
||||
IProjectMembersCount,
|
||||
ProjectModeCount,
|
||||
} from '../../lib/features/project/project-store';
|
||||
import type { CreateFeatureStrategySchema } from '../../lib/openapi';
|
||||
@ -167,10 +166,6 @@ export default class FakeProjectStore implements IProjectStore {
|
||||
)!.health = healthUpdate.health;
|
||||
}
|
||||
|
||||
getMembersCount(): Promise<IProjectMembersCount[]> {
|
||||
throw new Error('Method not implemented.');
|
||||
}
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
getProjectsByUser(userId: number): Promise<string[]> {
|
||||
return Promise.resolve([]);
|
||||
|
Loading…
Reference in New Issue
Block a user