Skip to content

Allow Dataset DOI in check permissions API request#8995

Merged
sekmiller merged 25 commits intoIQSS:developfrom
ErykKul:8994_permissions
Apr 22, 2024
Merged

Allow Dataset DOI in check permissions API request#8995
sekmiller merged 25 commits intoIQSS:developfrom
ErykKul:8994_permissions

Conversation

@ErykKul
Copy link
Collaborator

@ErykKul ErykKul commented Sep 26, 2022

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

@coveralls
Copy link

coveralls commented Sep 26, 2022

Coverage Status

coverage: 20.047%. remained the same when pulling fcaeb27 on ErykKul:8994_permissions into 5fc7b30 on IQSS:develop.

@mreekie mreekie added the bk2211 label Nov 1, 2022
@ErykKul
Copy link
Collaborator Author

ErykKul commented Nov 21, 2022

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.

@mreekie mreekie removed the bk2211 label Jan 11, 2023
Copy link
Member

@qqmyers qqmyers left a 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.

@qqmyers qqmyers added the Size: 3 A percentage of a sprint. 2.1 hours. label Dec 13, 2023
@pdurbin pdurbin changed the title allow slash in check permissions api request Allow slash in check permissions API request Feb 28, 2024
@pdurbin pdurbin added the Type: Feature a feature request label Feb 28, 2024
@qqmyers qqmyers changed the title Allow slash in check permissions API request Allow Dataset DOI in check permissions API request Apr 4, 2024
@sekmiller
Copy link
Contributor

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!

@qqmyers
Copy link
Member

qqmyers commented Apr 4, 2024

Weird - the web editor to resolve the conflict shows 0 conflicts in that file...

@ErykKul
Copy link
Collaborator Author

ErykKul commented Apr 6, 2024

@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!

@ErykKul
Copy link
Collaborator Author

ErykKul commented Apr 17, 2024

@sekmiller @qqmyers I had to manually merge the AbstractApiBean.java, but there where no conflicts to solve. Strange. It is now merged with the develop branch.

@sekmiller
Copy link
Contributor

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.

@ErykKul
Copy link
Collaborator Author

ErykKul commented Apr 19, 2024

@sekmiller Thanks for catching that! I have just added the missing double quotes.

@sekmiller sekmiller merged commit 27d3767 into IQSS:develop Apr 22, 2024
@pdurbin pdurbin added this to the 6.3 milestone Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: 3 A percentage of a sprint. 2.1 hours. Type: Feature a feature request

Projects

Status: Medium priority

Development

Successfully merging this pull request may close these issues.

API: Slash "/" is not allowed in path parameter when checking permissions

6 participants