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

feat: return project roles #8314

Merged
merged 3 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import { AccountStore } from '../../db/account-store';
import { FakeAccountStore } from '../../../test/fixtures/fake-account-store';
import { OnboardingReadModel } from '../onboarding/onboarding-read-model';
import { FakeOnboardingReadModel } from '../onboarding/fake-onboarding-read-model';
import { AccessStore } from '../../db/access-store';
import FakeAccessStore from '../../../test/fixtures/fake-access-store';

export const createPersonalDashboardService = (
db: Db,
Expand All @@ -34,6 +36,7 @@ export const createPersonalDashboardService = (
}),
new PrivateProjectChecker(stores, config),
new AccountStore(db, config.getLogger),
new AccessStore(db, config.eventBus, config.getLogger),
);
};

Expand All @@ -50,5 +53,6 @@ export const createFakePersonalDashboardService = (config: IUnleashConfig) => {
}),
new FakePrivateProjectChecker(),
new FakeAccountStore(),
new FakeAccessStore(),
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ export default class PersonalDashboardController extends Controller {

const projectDetails =
await this.personalDashboardService.getPersonalProjectDetails(
user.id,
req.params.projectId,
);

Expand All @@ -115,8 +116,6 @@ export default class PersonalDashboardController extends Controller {
personalDashboardProjectDetailsSchema.$id,
{
...projectDetails,
owners: [{ ownerType: 'user', name: 'placeholder' }],
roles: [{ name: 'placeholder', id: 0, type: 'project' }],
},
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
import type { IProjectReadModel } from '../project/project-read-model-type';
import type { IPrivateProjectChecker } from '../private-project/privateProjectCheckerType';
import type {
IAccessStore,
IAccountStore,
IEventStore,
IOnboardingReadModel,
Expand All @@ -36,6 +37,8 @@ export class PersonalDashboardService {

private onboardingReadModel: IOnboardingReadModel;

private accessStore: IAccessStore;

constructor(
personalDashboardReadModel: IPersonalDashboardReadModel,
projectOwnersReadModel: IProjectOwnersReadModel,
Expand All @@ -45,6 +48,7 @@ export class PersonalDashboardService {
featureEventFormatter: FeatureEventFormatter,
privateProjectChecker: IPrivateProjectChecker,
accountStore: IAccountStore,
accessStore: IAccessStore,
) {
this.personalDashboardReadModel = personalDashboardReadModel;
this.projectOwnersReadModel = projectOwnersReadModel;
Expand All @@ -54,6 +58,7 @@ export class PersonalDashboardService {
this.featureEventFormatter = featureEventFormatter;
this.privateProjectChecker = privateProjectChecker;
this.accountStore = accountStore;
this.accessStore = accessStore;
}

getPersonalFeatures(userId: number): Promise<PersonalFeature[]> {
Expand Down Expand Up @@ -95,6 +100,7 @@ export class PersonalDashboardService {
}

async getPersonalProjectDetails(
userId: number,
projectId: string,
): Promise<PersonalDashboardProjectDetailsSchema> {
const recentEvents = await this.eventStore.searchEvents(
Expand All @@ -117,11 +123,24 @@ export class PersonalDashboardService {
const owners =
await this.projectOwnersReadModel.getProjectOwners(projectId);

const allRoles = await this.accessStore.getAllProjectRolesForUser(
Copy link
Contributor

Choose a reason for hiding this comment

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

in the next PR we can Promise.all all those async operations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yeah. Actually lemme do it right away, while I'm in that context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nah, I'll do it in a follow-up after all. Think it's better served that way

userId,
projectId,
);

const projectRoles = allRoles
.filter((role) => ['project', 'custom'].includes(role.type))
.map((role) => ({
id: role.id,
name: role.name,
type: role.type as PersonalDashboardProjectDetailsSchema['roles'][number]['type'],
}));

return {
latestEvents: formattedEvents,
onboardingStatus,
owners,
roles: [],
roles: projectRoles,
};
}

Expand Down
4 changes: 2 additions & 2 deletions src/lib/services/access-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import FakeGroupStore from '../../test/fixtures/fake-group-store';
import { FakeAccountStore } from '../../test/fixtures/fake-account-store';
import FakeRoleStore from '../../test/fixtures/fake-role-store';
import FakeEnvironmentStore from '../features/project-environments/fake-environment-store';
import AccessStoreMock from '../../test/fixtures/fake-access-store';
import FakeAccessStore from '../../test/fixtures/fake-access-store';
import { GroupService } from '../services/group-service';
import type { IRole } from '../../lib/types/stores/access-store';
import {
Expand Down Expand Up @@ -271,7 +271,7 @@ test('throws error when trying to delete a project role in use by group', async
const accountStore = new FakeAccountStore();
const roleStore = new FakeRoleStore();
const environmentStore = new FakeEnvironmentStore();
const accessStore = new AccessStoreMock();
const accessStore = new FakeAccessStore();
accessStore.getGroupIdsForRole = groupIdResultOverride;
accessStore.getUserIdsForRole = async (): Promise<number[]> => {
return [];
Expand Down
6 changes: 3 additions & 3 deletions src/test/fixtures/fake-access-store.ts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file doesn't live where I expected it to. It also didn't expose the class as a named export (so the fact that it was mismatched didn't really matter much). Should we move it or leave it where it is?

Copy link
Contributor

Choose a reason for hiding this comment

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

Access subdomain has not been fully moved to feature oriented architecture yet. I remember moving only the composition root because this was all I needed at that time to finish some other big migration. We can finish the migration (depending how much you're inclined to do it) to full feature oriented architecture but not in this PR :)

Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {
import FakeRoleStore from './fake-role-store';
import type { PermissionRef } from '../../lib/services/access-service';

class AccessStoreMock implements IAccessStore {
export class FakeAccessStore implements IAccessStore {
fakeRolesStore: IRoleStore;

userToRoleMap: Map<number, number> = new Map();
Expand Down Expand Up @@ -327,6 +327,6 @@ class AccessStoreMock implements IAccessStore {
}
}

module.exports = AccessStoreMock;
module.exports = FakeAccessStore;

export default AccessStoreMock;
export default FakeAccessStore;
Loading