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.

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

Project owners read model - db read #6916

Merged
merged 18 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions src/lib/features/project/project-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,10 @@ export default class ProjectController extends Controller {
user.id,
);

// if (this.flagResolver.isEnabled('projectsListNewCards')) {
// TODO: get project owners and add to response
// }

this.openApiService.respondWithValidation(
200,
res,
Expand Down
175 changes: 175 additions & 0 deletions src/lib/features/project/project-owners-read-model.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
import dbInit, { type ITestDb } from '../../../test/e2e/helpers/database-init';
import getLogger from '../../../test/fixtures/no-logger';
import { type IUser, RoleName, type IGroup } from '../../types';
import { randomId } from '../../util';
import { ProjectOwnersReadModel } from './project-owners-read-model';

describe('unit tests', () => {
test('maps owners to projects', () => {});
test('returns "system" when a project has no owners', async () => {
// this is a mapping test; not an integration test
});
});

let db: ITestDb;
let readModel: ProjectOwnersReadModel;

let ownerRoleId: number;
let owner: IUser;
let member: IUser;
let group: 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',
Tymek marked this conversation as resolved.
Show resolved Hide resolved
username: 'owner',
email: '[email protected]',
imageUrl: 'image-url-1',
};
const memberData = {
name: 'Member Name',
username: 'member',
email: '[email protected]',
imageUrl: 'image-url-2',
};

// create users
owner = await db.stores.userStore.insert(ownerData);
member = await db.stores.userStore.insert(memberData);

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

afterAll(async () => {
if (db) {
await db.destroy();
}
});

afterEach(async () => {
if (db) {
const projects = await db.stores.projectStore.getAll();
for (const project of projects) {
// Clean only project roles, not all roles
await db.stores.roleStore.removeRolesForProject(project.id);
}
await db.stores.projectStore.deleteAll();
}
});

describe('integration tests', () => {
test('name takes precedence over username', async () => {
const projectId = randomId();
await db.stores.projectStore.create({ id: projectId, name: projectId });

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

const owners = await readModel.getAllProjectOwners();
expect(owners).toMatchObject({
[projectId]: expect.arrayContaining([
expect.objectContaining({ name: 'Owner User' }),
]),
});
});
Tymek marked this conversation as resolved.
Show resolved Hide resolved

test('gets project user owners', async () => {
const projectId = randomId();
await db.stores.projectStore.create({ id: projectId, name: projectId });

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

const owners = await readModel.getAllProjectOwners();

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

test('does not get regular project members', async () => {
const projectId = randomId();
await db.stores.projectStore.create({ id: projectId, name: projectId });

const memberRole = await db.stores.roleStore.getRoleByName(
RoleName.MEMBER,
);
await db.stores.accessStore.addUserToRole(
owner.id,
ownerRoleId,
projectId,
);

await db.stores.accessStore.addUserToRole(
member.id,
memberRole.id,
projectId,
);

const owners = await readModel.getAllProjectOwners();

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

test('gets project group owners', async () => {
const projectId = randomId();
await db.stores.projectStore.create({ id: projectId, name: projectId });

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

const owners = await readModel.getAllProjectOwners();

expect(owners).toMatchObject({
[projectId]: [
{
ownerType: 'group',
name: 'Group Name',
},
],
});
});

test('users are listed before groups', async () => {});

test('owners (users and groups) are sorted by when they were added; oldest first', async () => {});

test('returns the system owner for the default project', async () => {});

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

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

test('enriches fully', async () => {
const owners = await readModel.enrichWithOwners([]);

expect(owners).toStrictEqual([]);
});
});
151 changes: 151 additions & 0 deletions src/lib/features/project/project-owners-read-model.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import type { Db } from '../../db/db';
import { RoleName, type IProjectWithCount, type IRoleStore } from '../../types';

const T = {
ROLE_USER: 'role_user',
GROUP_ROLE: 'group_role',
ROLES: 'roles',
USERS: 'users',
};

type SystemOwner = { ownerType: 'system' };
type UserProjectOwner = {
ownerType: 'user';
name: string;
email?: string;
imageUrl?: string;
};
type GroupProjectOwner = {
ownerType: 'group';
name: string;
};
type ProjectOwners =
| [SystemOwner]
| Array<UserProjectOwner | GroupProjectOwner>;

export type ProjectOwnersDictionary = Record<string, ProjectOwners>;

type IProjectWithCountAndOwners = IProjectWithCount & {
owners: ProjectOwners;
};

export class ProjectOwnersReadModel {
private db: Db;
roleStore: IRoleStore;

constructor(db: Db, roleStore: IRoleStore) {
this.db = db;
this.roleStore = roleStore;
}

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

private async getAllProjectUsersByRole(
roleId: number,
): Promise<Record<string, UserProjectOwner[]>> {
const usersResult = await this.db
.select(
'user.username',
'user.name',
'user.email',
'user.image_url',
'ru.created_at',
'ru.project',
)
.from(`${T.ROLE_USER} as ru`)
.join(`${T.ROLES} as r`, 'ru.role_id', 'r.id')
.where('r.id', roleId)
.join(`${T.USERS} as user`, 'ru.user_id', 'user.id');
const usersDict: Record<string, UserProjectOwner[]> = {};

usersResult.forEach((user) => {
const project = user.project as string;

const data: UserProjectOwner = {
ownerType: 'user',
name: user?.name || user?.username,
email: user?.email,
imageUrl: user?.image_url,
};

if (project in usersDict) {
usersDict[project] = [...usersDict[project], data];
} else {
usersDict[project] = [data];
}
});

return usersDict;
}

private async getAllProjectGroupsByRole(
roleId: number,
): Promise<Record<string, GroupProjectOwner[]>> {
const groupsResult = await this.db
.select('groups.name', 'gr.created_at', 'gr.project')
.from(`${T.GROUP_ROLE} as gr`)
.join(`${T.ROLES} as r`, 'gr.role_id', 'r.id')
.where('r.id', roleId)
.join('groups', 'gr.group_id', 'groups.id');

const groupsDict: Record<string, GroupProjectOwner[]> = {};

groupsResult.forEach((group) => {
const project = group.project as string;

const data: GroupProjectOwner = {
ownerType: 'group',
name: group?.name,
};

if (project in groupsDict) {
groupsDict[project] = [...groupsDict[project], data];
} else {
groupsDict[project] = [data];
}
});

return groupsDict;
}

async getAllProjectOwners(): Promise<ProjectOwnersDictionary> {
Copy link
Member Author

Choose a reason for hiding this comment

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

Splitting it in 2 private methods makes the main one more readable to me, so that's what I did

const ownerRole = await this.roleStore.getRoleByName(RoleName.OWNER);
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] || []),
],
];
}),
);
Comment on lines +125 to +139
Copy link
Contributor

Choose a reason for hiding this comment

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

This is optional, but there's a lot of spreading here. We're not necessarily doing it in a loop, but it's a lot more iterations than we need. How about doing something like this instead (coded in github, so might not work directly 😅 ):

Suggested change
const projects = [
...new Set([...Object.keys(usersDict), ...Object.keys(groupsDict)]),
];
const dict = Object.fromEntries(
projects.map((project) => {
return [
project,
[
...(usersDict[project] || []),
...(groupsDict[project] || []),
],
];
}),
);
for (const [project, groups] of Object.entries(groupsDict) {
if (project in usersDict) {
usersDict[project] = usersDict[project].concat(groups)
} else {
usersDict[project] = groups
}
}

and then return the usersDict instead? That'll save us a lot of iteration and feels more readable to me 🤷🏼

Copy link
Member Author

Choose a reason for hiding this comment

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

done in next PR


return dict;
}

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

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