6266 api endpoints for token management#6292
Conversation
pdurbin
left a comment
pdurbin
left a comment
There was a problem hiding this comment.
Overall this looks fantastic but I'm leaving a few thoughts as comments.
| au = (AuthenticatedUser) u; | ||
| } catch (ClassCastException e){ | ||
| //if we have a non-authentivated user we stop here. | ||
| return notFound("Token for " + u.getIdentifier() + " not found."); |
There was a problem hiding this comment.
Couldn't the message here be something like "The guest user cannot have an API token"?
There was a problem hiding this comment.
I modified the error messages to say that tokens for guests/private url not eligible for deletion or recreation.
| au = (AuthenticatedUser) u; | ||
| } catch (ClassCastException e){ | ||
| //if we have a non-authentivated user we stop here. | ||
| return notFound("Token for " + u.getIdentifier() + " not found."); |
There was a problem hiding this comment.
Same as above, this message could say that the Guest user can't have an API token. Or PrivateURL user, I guess. I'm just thinking that it might be nice to have a slightly different message than the one below.
| return notFound("Token for " + u.getIdentifier() + " not found."); | ||
| } | ||
|
|
||
| /* this null check is probably overkill*/ |
There was a problem hiding this comment.
Yeah, I agree this is probably overkill. 😄 The same null check appears in the DELETE code above.
There was a problem hiding this comment.
Took out the redundant null checking and fixed a copy/paste of the comment typo
and private url test
6 participants
New Contributors
Welcome! New contributors should at least glance at CONTRIBUTING.md, especially the section on pull requests where we encourage you to reach out to other developers before you start coding. Also, please note that we measure code coverage and prefer you write unit tests. Pull requests can still be reviewed without tests or completion of the checklist outlined below. Note that we use the "closes" syntax below to trigger Github's automation to close the corresponding issue once the pull request is merged.
Thanks for your contribution to Dataverse!
Related Issues
Pull Request Checklist