Don't inherit due date and/or NeedsHumanReview for ABUSE_ADDON_VIOLATION/CINDER_ESCALATION

[diox]

Only for those reasons: as soon as a version has more reasons for needing human review its due date (and NHR itself) can be inherited from again.

Fixes mozilla/addons#15535

Testing

Scenario 1:

• Pick a recommended add-on with a listed version already public
• Report that add-on for policy violation inside the add-on. That should trigger a Needs Human Review for the current version of the add-on, for ABUSE_ADDON_VIOLATION reason. You should see the add-on in the manual review queue with a due date.
• Submit a new version for that add-on. Run auto_approve command, and you should see that the version is not auto-approved (because the add-on is recommended).
• If you refresh the manual review queue and filter out Reported for abuse within the add-on you should still see the add-on, now that it has a new version. Its due date should not be the same as the one it had before.
• You can check the review page for the add-on, and both versions should have a different due date.

Scenario 2:

• Pick a non promoted add-on with a listed version already public
• Report that add-on for policy violation inside the add-on. That should trigger a Needs Human Review for the current version of the add-on, for ABUSE_ADDON_VIOLATION reason. You should see the add-on in the manual review queue with a due date.
• Submit a new version for that add-on. Run auto_approve command, and you should see that the version is auto-approved.
• If you refresh the manual review queue and filter out Reported for abuse within the add-on you should not see the add-on. If you check that filter you should see it.
• You can check the review page for the add-on, and only the old version should have a due date.

Scenario 3 (unchanged from before):

• Pick a non promoted add-on with a listed version already public
• In review page for this add-on, set Needs Human Review for the latest version of that add-on. The version should get a due date.
• Submit a new version for that add-on. Run auto_approve command, and you should see that the version is auto-approved.
• You can check the review page for the add-on, and both versions should have a due date. The new version should have been flagged for Previous version in channel had needs human review set

[KevinMind]

Query I used to select an addon

addon = Addon.objects.public().filter(
    promotedaddonpromotion__promoted_group__group_id=PROMOTED_GROUP_CHOICES.RECOMMENDED,
    versions__channel=amo.CHANNEL_LISTED,
    versions__file__status=amo.STATUS_APPROVED,
).distinct().first()

Scenario 1 failed

Both reviews have the same due date

Scenario 2 passed

Scenario 3 passed

Though it's not clear if I expect the same or different due dates

[diox]

Scenario 3 should be the same due date yes (inheritance working just as it did before my patch in that case, since the initial reason is not abuse related).

Scenario 1 works for me locally. If you check in a shell, are the due_date for those versions really identical ? Also, what's the reason displayed for needing human review in the second version ? And are there any others ? (your screenshot cuts just before that)

[eviljeff]

scenario 1:
The versions show different due dates:

but the manual review queue has the due date for the abuse report version, not the new upload.

(I edit the due date of the older version so the ordering would change)

scenario 2:

If you refresh the manual review queue and filter out Reported for abuse within the add-on you should still see the add-on

I don't see the add-on, and I think that's correct? It's only NHR is for abuse, so if that's filtered out, you shouldn't see it?

scenario 3:
works.

Though I noticed if you try this with a promoted add-on that would be flagged anyway the version gets two NHRs - one because the previous version was flagged for NHR (because it's promoted); one because it's promoted. 🤷‍♂️

[diox]

The queue not showing the right due date in scenario 1 is interesting: that's because the query we made to find the due date just blindly looks for the earliest due date for each add-on, it doesn't care about filtering.

Previously this behavior was hidden because we were inheriting from the due date, but now that we aren't this is more visible. Going to need to refactor the queryset that deals with the queue to fix that.

[diox]

@eviljeff Version and due date being displayed in the queue for scenario 1 should have been fixed by a53d2e9 ; you were right about scenario 2, I edited the description.