-
Notifications
You must be signed in to change notification settings - Fork 490
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
Allow Dataset DOI in check permissions API request #8995
Conversation
The workaround here is to first get the dataset with the API, the response contains the technical (DB) ID that can be used (it does not contain slashes, it is an integer) to check the permissions. |
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.
The fix looks like it works for the case where the dataset PID exists, but it will return 500 errors instead of 404 when it isn't found or 400 for a missing ":persistentId" query param.
Hey @ErykKul, I'd like to pick this up but there's a merge conflict at the moment - so if you could resolve it I'll be able to test. Thanks! |
Weird - the web editor to resolve the conflict shows 0 conflicts in that file... |
@sekmiller @qqmyers I see that my PRs moved, but I am on vacation right now for another week, I will be back on 15th. I will re-merge the develop branch to them, do the necessary changes and retest once that I will be back at work. Some of them are open for quite some time, so they need some extra attention from me before merging, Thanks! |
@sekmiller @qqmyers I had to manually merge the |
Hey @ErykKul This looks great. Thanks! The only thing I need you to do before I can merge it is for you to update the doc so that the url is in double quotes as in curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/admin/permissions/:persistentId?persistentId=$PERSISTENT_IDENTIFIER" - I would do it myself but I don't think I have permission. |
@sekmiller Thanks for catching that! I have just added the missing double quotes. |
What this PR does / why we need it:
It allows using a global identifier, e.g., doi, when checking permissions.
Which issue(s) this PR closes:
Closes #8994