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

Jump to bottom

add the privilege entry #53

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

add the privilege entry #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
37762c1
add the privilege entry
syedfarhankabir Oct 31, 2022
6c572f0
add the privilege entry
syedfarhankabir Oct 31, 2022
0b8bed6
add the privilege entry
syedfarhankabir Oct 31, 2022
533ed2e
add the privilege entry
syedfarhankabir Oct 31, 2022
eccd9de
add the privilege entry
syedfarhankabir Oct 31, 2022
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
6 changes: 4 additions & 2 deletions src/project/controller/person-collection.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ class ProjectPersonCollection extends Controller {
const params = {
role: ctx.request.body.role,
name: ctx.request.body.name,
href: ctx.request.body.href
href: ctx.request.body.href,
};
const projectId = +ctx.params.projectId;
const origin = ctx.request.origin;

await projectService.addPersonToProject(params);
await projectService.addPersonToProject(params, projectId, origin);
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this object is now starting to look really weird. What are params, what aren't params? Why are some things in params? Decide on a path, either a parameters object, or do every argument separately.

Also generally avoid passing something like a projectId, you always want to pass the full object.


ctx.status = 201;

Expand Down
15 changes: 14 additions & 1 deletion src/project/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as clientService from '../client/service';
import * as personService from '../person/service';
import { ProjectsRecord } from 'knex/types/tables';
import ketting from '../ketting';
import { addUserPrivilege } from '../a12n';

export async function findAll(): Promise<Project[]> {

Expand Down Expand Up @@ -79,12 +80,14 @@ function mapRecord(input: ProjectsRecord, client: Client): Project {

}

export async function addPersonToProject(params: PersonProjectForm): Promise<void> {
export async function addPersonToProject(params: PersonProjectForm, projectId: number, origin: string): Promise<void> {

const principalUri = await findOrCreatePrincipal(params.href, params.name);

let project : Project;
try {
await personService.findByPrincipalUrl(principalUri);
project = await findById(projectId);
} catch(error) {

if(!(error instanceof NotFound)){
Expand All @@ -96,6 +99,16 @@ export async function addPersonToProject(params: PersonProjectForm): Promise<vo
name: params.name,
principalUri,
});
project = await findById(projectId);

} finally {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not a good use of finally. Just leave this outside of this block. It now runs even if there's other errors happening, and we really don't want that.

project = await findById(projectId);
await addUserPrivilege(
principalUri,
params.role,
new URL(project.href, origin),
);

}

}
Expand Down