-
Notifications
You must be signed in to change notification settings - Fork 5
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/marxan 1445 grant project owner role on imported cloned project to user #967
Feat/marxan 1445 grant project owner role on imported cloned project to user #967
Conversation
This pull request is being automatically deployed with Vercel (learn more). marxan – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan/HRum44tqomE8XKbG3pMvPxuJH9DM marxan-storybook – ./app🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/778vhYHJAyRdcG9FhhzEJtzUGPdn |
0472181
to
7139a51
Compare
// directly on elements of Api code, so there were two options: | ||
// - Move ProjectRoles enum to libs package | ||
// - Harcode the rol | ||
// We took the second approach because we are only referencing values from that enum |
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.
👍🏼
await em | ||
.createQueryBuilder() | ||
.insert() | ||
.into(`users_scenarios`) | ||
.values({ | ||
user_id: ownerId, | ||
scenario_id: scenarioId, | ||
// It would be great to use ScenarioRoles enum instead of having | ||
// the role hardcoded. The thing is that Geoprocessing code shouldn't depend | ||
// directly on elements of Api code, so there were two options: | ||
// - Move ScenarioRoles enum to libs package | ||
// - Harcode the rol | ||
// We took the second approach because we are only referencing values from that enum | ||
// here | ||
role_id: 'scenario_owner', | ||
}) | ||
.execute(); |
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.
on second thought, you are right (as always 😅) on this - we could skip it and the project owner would get an implicit scenario_owner
role by the time any scenarios are created (by importing them, in this case) within the project, but the behaviour you implemented here directly maps to the one we have in place when creating scenarios "manually", where the user who creates the scenario is made explicitly a "scenario_owner".
This PR adds logic for granting project/scenario owner role to the user that schedules an import operation
Feature relevant tickets
grant project_owner role on new (cloned/imported) project to the user who requested the import/clone