Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: map project owners to projects list #6928

Merged
merged 2 commits into from
Apr 25, 2024
Merged

feat: map project owners to projects list #6928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a0f2f07
feat: map project owners to projects list
Tymek Apr 25, 2024
becc848
refactor: resolve pr comments
Tymek Apr 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
201 changes: 187 additions & 14 deletions src/lib/features/project/project-owners-read-model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,53 @@ import { type IUser, RoleName, type IGroup } from '../../types';
import { randomId } from '../../util';
import { ProjectOwnersReadModel } from './project-owners-read-model';

const mockProjectWithCounts = (name: string) => ({
name,
id: name,
description: '',
featureCount: 0,
memberCount: 0,
mode: 'open' as const,
defaultStickiness: 'default' as const,
staleFeatureCount: 0,
potentiallyStaleFeatureCount: 0,
avgTimeToProduction: 0,
});

describe('unit tests', () => {
test('maps owners to projects', () => {});
test('maps owners to projects', () => {
const projects = [{ name: 'project1' }, { name: 'project2' }] as any;

const owners = {
project1: [{ ownerType: 'user' as const, name: 'Owner Name' }],
project2: [{ ownerType: 'user' as const, name: 'Owner Name' }],
};

const projectsWithOwners = ProjectOwnersReadModel.addOwnerData(
projects,
owners,
);

expect(projectsWithOwners).toMatchObject([
{ name: 'project1', owners: [{ name: 'Owner Name' }] },
{ name: 'project2', owners: [{ name: 'Owner Name' }] },
]);
});

test('returns "system" when a project has no owners', async () => {
// this is a mapping test; not an integration test
const projects = [{ name: 'project1' }, { name: 'project2' }] as any;

const owners = {};

const projectsWithOwners = ProjectOwnersReadModel.addOwnerData(
projects,
owners,
);

expect(projectsWithOwners).toMatchObject([
{ name: 'project1', owners: [{ ownerType: 'system' }] },
{ name: 'project2', owners: [{ ownerType: 'system' }] },
]);
});
});
Comment on lines 20 to 55
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be done later, but I think we should put the unit tests in a separate file? So we'll probably have project-owners-read-model.test.ts and project-owners-read-model.integration.test.ts. Feels like it jives better with what we're doing most the time.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have 3 types that correlate better to methods under test then to any specific test type. I think we can just describe what part of the class we are testing instead of moving it to a separate file

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough 💁🏼


Expand All @@ -16,20 +59,28 @@ let readModel: ProjectOwnersReadModel;

let ownerRoleId: number;
let owner: IUser;
let owner2: IUser;
let member: IUser;
let group: IGroup;
let group2: IGroup;

beforeAll(async () => {
db = await dbInit('project_owners_read_model_serial', getLogger);
readModel = new ProjectOwnersReadModel(db.rawDatabase, db.stores.roleStore);
ownerRoleId = (await db.stores.roleStore.getRoleByName(RoleName.OWNER)).id;

const ownerData = {
name: 'Owner User',
name: 'Owner Name',
username: 'owner',
email: '[email protected]',
imageUrl: 'image-url-1',
};
const ownerData2 = {
name: 'Second Owner Name',
username: 'owner2',
email: '[email protected]',
imageUrl: 'image-url-3',
};
const memberData = {
name: 'Member Name',
username: 'member',
Expand All @@ -40,10 +91,13 @@ beforeAll(async () => {
// create users
owner = await db.stores.userStore.insert(ownerData);
member = await db.stores.userStore.insert(memberData);
owner2 = await db.stores.userStore.insert(ownerData2);

// create groups
group = await db.stores.groupStore.create({ name: 'Group Name' });
await db.stores.groupStore.addUserToGroups(owner.id, [group.id]);
group2 = await db.stores.groupStore.create({ name: 'Second Group Name' });
await db.stores.groupStore.addUserToGroups(member.id, [group.id]);
});

afterAll(async () => {
Expand All @@ -64,6 +118,12 @@ afterEach(async () => {
});

describe('integration tests', () => {
test('returns an empty object if there are no projects', async () => {
const owners = await readModel.getAllProjectOwners();

expect(owners).toStrictEqual({});
});

test('name takes precedence over username', async () => {
const projectId = randomId();
await db.stores.projectStore.create({ id: projectId, name: projectId });
Expand All @@ -77,7 +137,7 @@ describe('integration tests', () => {
const owners = await readModel.getAllProjectOwners();
expect(owners).toMatchObject({
[projectId]: expect.arrayContaining([
expect.objectContaining({ name: 'Owner User' }),
expect.objectContaining({ name: 'Owner Name' }),
]),
});
});
Expand All @@ -98,7 +158,7 @@ describe('integration tests', () => {
[projectId]: [
{
ownerType: 'user',
name: 'Owner User',
name: 'Owner Name',
email: '[email protected]',
imageUrl: 'image-url-1',
},
Expand Down Expand Up @@ -128,7 +188,7 @@ describe('integration tests', () => {
const owners = await readModel.getAllProjectOwners();

expect(owners).toMatchObject({
[projectId]: [{ name: 'Owner User' }],
[projectId]: [{ name: 'Owner Name' }],
});
});

Expand All @@ -155,21 +215,134 @@ describe('integration tests', () => {
});
});

test('users are listed before groups', async () => {});
test('users are listed before groups', async () => {
const projectId = randomId();
await db.stores.projectStore.create({ id: projectId, name: projectId });

test('owners (users and groups) are sorted by when they were added; oldest first', async () => {});
await db.stores.accessStore.addGroupToRole(
group.id,
ownerRoleId,
'',
projectId,
);

test('returns the system owner for the default project', async () => {});
await db.stores.accessStore.addUserToRole(
owner.id,
ownerRoleId,
projectId,
);

test('returns an empty list if there are no projects', async () => {
const owners = await readModel.getAllProjectOwners();

expect(owners).toStrictEqual({});
expect(owners).toMatchObject({
[projectId]: [
{
email: '[email protected]',
imageUrl: 'image-url-1',
name: 'Owner Name',
ownerType: 'user',
},
{
name: 'Group Name',
ownerType: 'group',
},
],
});
});

test('enriches fully', async () => {
const owners = await readModel.enrichWithOwners([]);
test('owners (users and groups) are sorted by when they were added; oldest first', async () => {
const projectId = randomId();
await db.stores.projectStore.create({ id: projectId, name: projectId });

// Raw query in order to set the created_at date
await db.rawDatabase('role_user').insert({
user_id: owner2.id,
role_id: ownerRoleId,
project: projectId,
created_at: new Date('2024-01-01T00:00:00.000Z'),
});

// Raw query in order to set the created_at date
await db.rawDatabase('group_role').insert({
group_id: group2.id,
role_id: ownerRoleId,
project: projectId,
created_at: new Date('2024-01-01T00:00:00.000Z'),
});

await db.stores.accessStore.addGroupToRole(
group.id,
ownerRoleId,
'',
projectId,
);

await db.stores.accessStore.addUserToRole(
owner.id,
ownerRoleId,
projectId,
);

const owners = await readModel.getAllProjectOwners();

expect(owners).toMatchObject({
[projectId]: [
{
email: '[email protected]',
imageUrl: 'image-url-3',
name: 'Second Owner Name',
ownerType: 'user',
},
{
email: '[email protected]',
imageUrl: 'image-url-1',
name: 'Owner Name',
ownerType: 'user',
},
{
name: 'Second Group Name',
ownerType: 'group',
},
{
name: 'Group Name',
ownerType: 'group',
},
],
});
});

test('does not modify an empty array', async () => {
const projectsWithOwners = await readModel.addOwners([]);

expect(projectsWithOwners).toStrictEqual([]);
});

test('adds system owner when no owners are found', async () => {
const projectIdA = randomId();
const projectIdB = randomId();
await db.stores.projectStore.create({
id: projectIdA,
name: projectIdA,
});
await db.stores.projectStore.create({
id: projectIdB,
name: projectIdB,
});

await db.stores.accessStore.addUserToRole(
owner.id,
ownerRoleId,
projectIdB,
);

const projectsWithOwners = await readModel.addOwners([
mockProjectWithCounts(projectIdA),
mockProjectWithCounts(projectIdB),
]);

expect(owners).toStrictEqual([]);
expect(projectsWithOwners).toMatchObject([
{ name: projectIdA, owners: [{ ownerType: 'system' }] },
{ name: projectIdB, owners: [{ ownerType: 'user' }] },
]);
});
});
44 changes: 21 additions & 23 deletions src/lib/features/project/project-owners-read-model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,14 @@ export class ProjectOwnersReadModel {
this.roleStore = roleStore;
}

addOwnerData(
static addOwnerData(
projects: IProjectWithCount[],
owners: ProjectOwnersDictionary,
): IProjectWithCountAndOwners[] {
// const projectsWithOwners = projects.map((p) => ({
// ...p,
// owners: projectOwners[p.id] || [],
// }));
return [];
return projects.map((project) => ({
...project,
owners: owners[project.name] || [{ ownerType: 'system' }],
}));
}

private async getAllProjectUsersByRole(
Expand All @@ -62,6 +61,7 @@ export class ProjectOwnersReadModel {
'ru.project',
)
.from(`${T.ROLE_USER} as ru`)
.orderBy('ru.created_at', 'asc')
.join(`${T.ROLES} as r`, 'ru.role_id', 'r.id')
.where('r.id', roleId)
.join(`${T.USERS} as user`, 'ru.user_id', 'user.id');
Expand Down Expand Up @@ -93,6 +93,7 @@ export class ProjectOwnersReadModel {
const groupsResult = await this.db
.select('groups.name', 'gr.created_at', 'gr.project')
.from(`${T.GROUP_ROLE} as gr`)
.orderBy('gr.created_at', 'asc')
.join(`${T.ROLES} as r`, 'gr.role_id', 'r.id')
.where('r.id', roleId)
.join('groups', 'gr.group_id', 'groups.id');
Expand Down Expand Up @@ -122,30 +123,27 @@ export class ProjectOwnersReadModel {
const usersDict = await this.getAllProjectUsersByRole(ownerRole.id);
const groupsDict = await this.getAllProjectGroupsByRole(ownerRole.id);

const projects = [
...new Set([...Object.keys(usersDict), ...Object.keys(groupsDict)]),
];

const dict = Object.fromEntries(
projects.map((project) => {
return [
project,
[
...(usersDict[project] || []),
...(groupsDict[project] || []),
],
];
}),
);
const dict: Record<
string,
Array<UserProjectOwner | GroupProjectOwner>
> = usersDict;

Object.keys(groupsDict).forEach((project) => {
if (project in dict) {
dict[project] = dict[project].concat(groupsDict[project]);
} else {
dict[project] = groupsDict[project];
}
});

return dict;
}

async enrichWithOwners(
async addOwners(
projects: IProjectWithCount[],
): Promise<IProjectWithCountAndOwners[]> {
const owners = await this.getAllProjectOwners();

return this.addOwnerData(projects, owners);
return ProjectOwnersReadModel.addOwnerData(projects, owners);
}
}
Loading