Skip to content

Conversation

eviljeff
Copy link
Member

@eviljeff eviljeff commented May 22, 2025

Fixes: mozilla/addons#15296

Description

For Cinder decisions, redefines target versions for appeals on VERSION_REJECT_ADDON; corrects the action; and allows the action class to unreject the versions rejected with the appealed decision(s).

Context

The issue identified two problems we had when legal was forwarded an appeal on a version rejection - if we denied the appeal we didn't see it as a the same action and offered another appeal; and if we accepted the appeal there was no way to reverse the rejections.

Testing

follow the STR from the issue basically,

  • reject one or more versions in the reviewer tools
  • appeal those rejections
  • in the reviewer tools forward the appeal job to legal
  • in cinder, in the legal queue, deny the appeal by selecting the same policy the original job was rejected for (or any policy that takes down content, really)
  • replay the payload for that decision locally
  • see the developer gets an email saying we affirmed the original decision

repeat but:

  • at the cinder legal queue step, select Approve (or one of the Ignore policies) to accept the appeal
  • replay the payload for that decision locally
  • see the developer gets an email saying we accepted their appeal
  • we have unrejected the versions

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

Comment on lines -1662 to +1687
self.addon.update_status(self.user)
self.addon.update_status()
Copy link
Member Author

Choose a reason for hiding this comment

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

this (and below) were drive-by fixes when I was looking into update_status - the arg is supposed to be a Version instance to ignore, so passing self.user was always useless.

file.original_status = amo.STATUS_NULL
file.status_disabled_reason = File.STATUS_DISABLED_REASONS.NONE
file.status_disabled_reason = File.STATUS_DISABLED_REASONS.NONE
Copy link
Member Author

Choose a reason for hiding this comment

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

I figured any time we're updating the status of a file we want to clear status_disabled_reason (?)

Comment on lines -445 to +452
original_status=amo.STATUS_NULL,
original_status=version.file.status,
Copy link
Member Author

Choose a reason for hiding this comment

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

this (and the change in reviewers/utils.py) are so we save the status before it's rejected, so we're able to unreject it. I checked and we don't appear to rely on original_status being a particular value unless status_disabled_reason is also set to a none-None value so I can't (immediately) think of a problem with this.

@eviljeff eviljeff force-pushed the 15296-rejection-appeals-via-cinder branch from 64182f1 to 4b8157f Compare May 22, 2025 15:59
@eviljeff eviljeff force-pushed the 15296-rejection-appeals-via-cinder branch from 4b8157f to 70bae5f Compare May 23, 2025 10:51
@eviljeff eviljeff marked this pull request as ready for review May 23, 2025 11:04
@eviljeff eviljeff requested review from a team and diox and removed request for a team May 23, 2025 11:05
@eviljeff eviljeff merged commit 55f7613 into mozilla:master May 27, 2025
42 checks passed
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.

[Bug]: Problems resolving appeals escalated to Cinder's Legal queue for rejected versions in rev tools
2 participants