-
Notifications
You must be signed in to change notification settings - Fork 1
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
add the privilege entry #53
base: main
Are you sure you want to change the base?
Conversation
const person = await projectService.addPersonToProject(params); | ||
|
||
await addUserPrivilege( | ||
person.principalUri, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @evert, this is giving me an error.
Also, wondering if I was on the right track. My intuition is, I should also be adding the project.href
instead of the person.href
on line 27.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this section should go into the projectService
instead, because it's all part of 'adding a person to a project'. There you should also not have this issue because you always have a principalUrl.
params.role, | ||
new URL(project.href, ctx.request.origin), | ||
); | ||
await projectService.addPersonToProject(params, projectId, origin); |
There was a problem hiding this comment.
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.
return person; | ||
project = await findById(projectId); | ||
|
||
} finally { |
There was a problem hiding this comment.
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.
Changes
addPersonToProject()
now returns aPerson
addUserPrivilege
in theperson-collection
, we are adding the new Person as a with the designated role