Skip to content

add the ability to delete versions via the API#20143

Merged
eviljeff merged 5 commits into
mozilla:masterfrom
eviljeff:19784-delete-versions
Jan 5, 2023
Merged

add the ability to delete versions via the API#20143
eviljeff merged 5 commits into
mozilla:masterfrom
eviljeff:19784-delete-versions

Conversation

@eviljeff

@eviljeff eviljeff commented Jan 4, 2023

Copy link
Copy Markdown
Member

@eviljeff
eviljeff requested review from a team and diox and removed request for a team January 4, 2023 14:17
response = self.client.delete(self.url)
assert response.status_code == 401
assert not self.version.reload().deleted

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a couple more tests

  • Logged in user, not author (403)
  • Logged in author, not owner (204, we allow any author just like devhub)

@diox diox Jan 4, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also... a regular reviewer (without Addons:Edit but with listed review permission) shouldn't be allowed to delete versions. As this stands it looks like they would ? That's worth testing to be sure.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@eviljeff
eviljeff requested a review from diox January 4, 2023 22:16
@eviljeff
eviljeff force-pushed the 19784-delete-versions branch from d258847 to 9c29ebb Compare January 5, 2023 14:28
@eviljeff
eviljeff force-pushed the 19784-delete-versions branch from 9c29ebb to 26b212f Compare January 5, 2023 14:29
Comment on lines +3977 to +3978
# even if they are a reviewer
self.grant_permission(another_user, ':'.join(amo.permissions.ADDONS_EDIT))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's nice to test ADDONS_EDIT but we should also test with something that satisfies acl.is_reviewer()

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@eviljeff
eviljeff merged commit 9035c69 into mozilla:master Jan 5, 2023
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.

Add the ability to delete versions through the API

2 participants