Skip to content
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

[DC-1142] Add delete permission to snapshot stewards on requests #1476

Merged
merged 2 commits into from
Jul 8, 2024

Conversation

s-rubenstein
Copy link
Contributor

Ticket: https://broadworkbench.atlassian.net/browse/DC-1142

What:
This adds delete to the approver role, so that snapshot stewards have the ability to delete snapshot access requests.

Why:

This allows cleanup of requests to unblock deletion of snapshots. Mostly important for tests and local development.

How:

<For your reviewers' sake, please describe in ~1 paragraph how this PR accomplishes its goal.>

<If the PR is big, please indicate where a reviewer should start reading it (i.e. which file or function).>


PR checklist

  • I've followed the instructions if I've made any changes to the API, especially if they're breaking changes
  • I've filled out the Security Risk Assessment (requires Broad Internal network access) and attached the result to the JIRA ticket

Copy link
Contributor

@rjohanek rjohanek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want the approver to be able to delete snapshot requests? I know it is so that the underlying snapshot can be deleted, but what is the case that the underlying snapshot needs to be deleted? Is this just for us/ease of testing, if so does this have any repercussions that we need to consider? Like if approvers are deleting requests instead of rejecting them, it leaves little information for researchers. Is that something we need to worry about?

@s-rubenstein
Copy link
Contributor Author

@rjohanek I think most of the time we won't want them deleting requests, and we probably don't really want to encourage deleting of requests in general. However, for the time being we need to enable test cleanup, and this is the best way to do that. I think the risk of approvers deleting requests is low (if they do, it may be confusing to researchers, but it is functionally equivalent to rejecting.)

What we really want here is a cascade functionality for deleting Sam resources, but that would be a much greater change/feature for Sam, so we're punting on that.

Copy link

sonarcloud bot commented Jul 8, 2024

@s-rubenstein s-rubenstein merged commit 201d5a9 into develop Jul 8, 2024
24 checks passed
@s-rubenstein s-rubenstein deleted the sr/dc-1142-delete-permission-on-requests branch July 8, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants