-
Notifications
You must be signed in to change notification settings - Fork 531
feat: "Link Dataset/Dataverse" permission #11534
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
Conversation
|
tests are passing, looks good to me - merging. |
|
We need to revert this. Here's the PR to do so: |
|
@vera as mentioned above, we had to revert this PR. As written, it breaks existing databases. I assume you're still interested in this functionality. Would you be able to create a new PR (and an issue first, if you don't mind) that includes a Flyway script like the one in #8174 that updates the database with new permission bits. That way, existing databases will get the updated permission bits, thanks to the Flyway script. We can help with the Flyway script if you need it! |
|
@vera created a follow up issue here (thanks!): |
|
I've resubmitted the PR with a Flyway migration script here: #11691 Sorry, I don't know how I missed the big comment in |
|
@vera thanks! Don't worry, a number of us missed it! Thanks for the new PR! ❤️ |
What this PR does / why we need it:
Linking or unlinking a dataset or dataverse now requires the new "Link Dataset/Dataverse" permission.
Previously, this action was covered by the "Publish Dataset/Dataverse" permission.
Linking and publishing permissions can now be granted separately, allowing for more fine-grained access control.
Which issue(s) this PR closes:
/
Special notes for your reviewer:
/
Suggestions on how to test this:
I've extended an existing test which can be run with
mvn test -Dtest="DatasetsIT#testCreateDeleteDatasetLink"to check that datasets cannot be linked/unlinked unless you have the curator role, which has been altered to include the new link permissions.Does this PR introduce a user interface change? If mockups are available, please link/include them here:
/
Is there a release notes update needed for this change?:
I've added a short release note.
Additional documentation:
/