feat: add visibility when fork a project (#1272) #1617
Conversation
|
You can access the deployment of this PR at https://renku-ci-ui-1617.dev.renku.ch |
andre-code
changed the title
andre-code
changed the title
lorenzo-cavazzi
force-pushed
the
1272-add-visibility-forking-project
branch
from
acf2352
to
b42270d
Compare
…ect' into 1272-add-visibility-forking-project # Conflicts: # client/src/project/new/ProjectNew.container.js # client/src/project/new/ProjectNew.present.js
| async getVisibilities(namespace, projectVisibility = "private") { | ||
| const computeVisibilities = (options) => { | ||
| if (options.includes("private")) { | ||
| return { | ||
| visibilities: ["private"], | ||
| default: "private", | ||
| }; | ||
| } | ||
| else if (options.includes("internal")) { | ||
| return { | ||
| visibilities: ["private", "internal"], | ||
| default: "internal", | ||
| }; | ||
| } | ||
| return { | ||
| visibilities: ["private", "internal", "public"], | ||
| default: "public" | ||
| }; | ||
| }; | ||
|
|
||
| let availableVisibilities = null; | ||
| if (!namespace || !projectVisibility) | ||
| return null; | ||
|
|
||
| if (namespace?.kind === "user") { | ||
| return computeVisibilities(["public", projectVisibility]); | ||
| } | ||
| else if (namespace?.kind === "group") { | ||
| // get group visibility | ||
| const group = await this.client.getGroupByPath(namespace.full_path).then(r => r.data); | ||
| return computeVisibilities([group.visibility, projectVisibility]); | ||
| } | ||
| return availableVisibilities; |
There was a problem hiding this comment.
This is a very nice function!
We have something on the same line in ProjectNew.state.js at line 466 (async getVisibilities(namespace)). I'm wondering if it would make sense to merge the 2, or extrapolate a common function. Maybe it's not that straightforward tho...
There was a problem hiding this comment.
P.S. this is just a "nice to have". If you think it's feasible (and reasonably easy) let's include it in the PR. Otherwise, feel free to leave this out.
There was a problem hiding this comment.
At the very least, I think it would be nice to merge the computeVisibilities functions. It may be necessary to introduce a maxVisibility(projectVisibility, groupVisibility) function.
There was a problem hiding this comment.
Thanks for the suggestion, I refactor the function to be used also by the NewProjectCoordinator class.
|
This is great and works well! I tried several cases, and all were handled properly. On minor request. Not critical, but nice to have if easy. Could the message shown when updating the visibility be changed to
|
…d fork a project form
andre-code
force-pushed
the
1272-add-visibility-forking-project
branch
from
dec10ff
to
9e5bacc
Compare
|
Tearing down the temporary RenkuLab deplyoment for this PR. |
Added a Visibility option to the modal dialog. The initial value should be the same as the project being forked, and only as visible or less visible options should be available.
For a public project: public, internal, private
For an internal project: internal, private
For a private project: private
Display an informative text in the fork modal dialog when the project was forked but the custom visibility can't be added.
Successful fork
Error setting visibility
Error when setting visibility when closing the modal
closes #1272
/deploy