Don't create fake CinderJob for requeues; expose notes in log

[eviljeff]

Fixes: mozilla/addons#15652

Description

This attempts to fix the issue in a minimal way: for decisions that weren't originally connected to a job, no "fake" job is created to re-enqueue the decision; and for all cancellations, the notes entered are stored in the activity log and exposed in the reviewer tools version history.

(the notes I entered in the 2nd level review queue highlighted.

Context

Testing

• set up a 2nd level approval
  • identify an add-on that would normally result in negative actions being held for 2nd level approval, e.g. recommended (or make an add-on recommended)
  • make a decision -that doesn't resolve a job- (e.g. force disable the add-on)
• in the 2nd level approval queue, cancel & re-enqueue the add-on, with a note
• see the add-on has (re)entered the review queue
• on the review page, see the NHR in the history with the note displayed
• no job has been created for the requeue

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.

[willdurand]

fine by me, thanks

[wagnerand-moz]

make a decision -that doesn't resolve a job- (e.g. force disable the add-on)

does that mean if you make a decision that does resolve a job, we will use the flow described in the issue?

[eviljeff]

make a decision -that doesn't resolve a job- (e.g. force disable the add-on)

does that mean if you make a decision that does resolve a job, we will use the flow described in the issue?

Not sure which flow you're referencing, but the process would be same as now - the job is requeued so it can be resolved again, with the same outcomes (specific appeal response to developers; abuse report resolution response to reporters; etc)

[wagnerand-moz]

make a decision -that doesn't resolve a job- (e.g. force disable the add-on)

does that mean if you make a decision that does resolve a job, we will use the flow described in the issue?

Not sure which flow you're referencing, but the process would be same as now - the job is requeued so it can be resolved again, with the same outcomes (specific appeal response to developers; abuse report resolution response to reporters; etc)

The flow I was referring to is the flow I have described in the description of the issue, as it is happening as of now on production.

If a reviewer makes a decision that does resolve a job, and that decision is then held and subsequently canceled and the version(s) re-enqueued, will that cancel and re-enqueue activity/decision be shown as a an activity in the version history, and will that activity contain the reason the second level reviewer entered?

[eviljeff]

make a decision -that doesn't resolve a job- (e.g. force disable the add-on)

does that mean if you make a decision that does resolve a job, we will use the flow described in the issue?

Not sure which flow you're referencing, but the process would be same as now - the job is requeued so it can be resolved again, with the same outcomes (specific appeal response to developers; abuse report resolution response to reporters; etc)

The flow I was referring to is the flow I have described in the description of the issue, as it is happening as of now on production.

If a reviewer makes a decision that does resolve a job, and that decision is then held and subsequently canceled and the version(s) re-enqueued, will that cancel and re-enqueue activity/decision be shown as a an activity in the version history, and will that activity contain the reason the second level reviewer entered?

The version history would be as the screenshot shows.

[wagnerand-moz]

This looks good from a UX perspective, I didn't review the code.