Skip to content

track queue moves as a decision #23359

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.

Sign up for GitHub

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

Draft
eviljeff wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

track queue moves as a decision #23359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 21 additions & 19 deletions src/olympia/abuse/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,13 @@ def unresolved(self):
Q(decisions__isnull=True)
| Q(
# i.e. the latest decision is a requeue
decisions__action=DECISION_ACTIONS.AMO_REQUEUE,
decisions__action__in=(
DECISION_ACTIONS.AMO_REQUEUE,
DECISION_ACTIONS.AMO_ESCALATE_ADDON,
),
decisions__overridden_by__isnull=True,
)
)
).distinct()

def resolvable_in_reviewer_tools(self):
return self.filter(resolvable_in_reviewer_tools=True)
Expand Down Expand Up @@ -333,8 +336,15 @@ def process_decision(
decision.send_notifications()

def process_queue_move(self, *, new_queue, notes):
CinderQueueMove.objects.create(cinder_job=self, notes=notes, to_queue=new_queue)
if new_queue == CinderAddonHandledByReviewers(self.target).queue:
ContentDecision.objects.create(
addon=self.target_addon,
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
override_of=self.decision,
action_date=datetime.now(),
private_notes=notes,
cinder_job=self,
)
# now escalated
entity_helper = CinderJob.get_entity_helper(
self.target, resolved_in_reviewer_tools=True
Expand Down Expand Up @@ -369,20 +379,20 @@ def clear_needs_human_review_flags(self):
.unresolved()
.resolvable_in_reviewer_tools()
)
if (
(decision_actions := tuple(self.decisions.values_list('action', flat=True)))
# i.e. was the previous decision before the current a requeue
and len(decision_actions) >= 2
and decision_actions[-2] == DECISION_ACTIONS.AMO_REQUEUE
):
# the previous decision before this new decision
penultimate_action = tuple(self.decisions.values_list('action', flat=True))[
-2:-1
]
if penultimate_action == (DECISION_ACTIONS.AMO_REQUEUE,):
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
decisions__action=DECISION_ACTIONS.AMO_REQUEUE,
decisions__overridden_by__isnull=True,
).exists()
reasons = {NeedsHumanReview.REASONS.SECOND_LEVEL_REQUEUE}
elif self.queue_moves.exists():
elif penultimate_action == (DECISION_ACTIONS.AMO_ESCALATE_ADDON,):
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
queue_moves__id__gt=0
decisions__action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
decisions__overridden_by__isnull=True,
).exists()
reasons = {
NeedsHumanReview.REASONS.CINDER_ESCALATION,
Expand Down Expand Up @@ -1477,11 +1487,3 @@ class CinderAppeal(ModelBase):
reporter_report = models.OneToOneField(
to=AbuseReport, on_delete=models.CASCADE, null=True
)


class CinderQueueMove(ModelBase):
cinder_job = models.ForeignKey(
to=CinderJob, on_delete=models.CASCADE, related_name='queue_moves'
)
notes = models.TextField(max_length=1000, blank=True)
to_queue = models.CharField(max_length=128)
69 changes: 48 additions & 21 deletions src/olympia/abuse/tests/test_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
CinderAppeal,
CinderJob,
CinderPolicy,
CinderQueueMove,
ContentDecision,
)

Expand Down Expand Up @@ -777,8 +776,11 @@ def test_reviewer_handled(self):
assert list(qs) == [job, appeal_job]

not_policy_report.cinder_job.update(resolvable_in_reviewer_tools=True)
CinderQueueMove.objects.create(
cinder_job=not_policy_report.cinder_job, to_queue='?'
ContentDecision.objects.create(
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
action_date=datetime.now(),
cinder_job=not_policy_report.cinder_job,
addon=not_policy_report.target,
)
qs = CinderJob.objects.resolvable_in_reviewer_tools()
assert list(qs) == [not_policy_report.cinder_job, job, appeal_job]
Expand Down Expand Up @@ -1401,9 +1403,10 @@ def test_process_queue_move_into_reviewer_handled(self):
nhr = NeedsHumanReview.objects.get()
assert nhr.reason == NeedsHumanReview.REASONS.CINDER_ESCALATION
assert nhr.version == addon.current_version
assert CinderQueueMove.objects.filter(
cinder_job=cinder_job, to_queue='amo-env-addon-infringement', notes='notes!'
).exists()
assert cinder_job.decision.private_notes == 'notes!'
assert cinder_job.decision.action == DECISION_ACTIONS.AMO_ESCALATE_ADDON
assert cinder_job.decision.cinder_job == cinder_job
assert cinder_job.decision.addon == cinder_job.target_addon

def test_process_queue_move_out_of_reviewer_handled(self):
# Not yet implemented, so just check it's silently ignored
Expand All @@ -1421,9 +1424,7 @@ def test_process_queue_move_out_of_reviewer_handled(self):
assert cinder_job.resolvable_in_reviewer_tools is True
assert len(mail.outbox) == 0
assert NeedsHumanReview.objects.count() == 1
assert CinderQueueMove.objects.filter(
cinder_job=cinder_job, to_queue='amo-env-listings', notes='out'
).exists()
assert not cinder_job.decision

def test_process_queue_move_other_queue_movement(self):
# we don't need to about these other queue moves, so just check it's silently
Expand All @@ -1436,9 +1437,7 @@ def test_process_queue_move_other_queue_movement(self):
assert not cinder_job.resolvable_in_reviewer_tools
assert len(mail.outbox) == 0
assert NeedsHumanReview.objects.count() == 0
assert CinderQueueMove.objects.filter(
cinder_job=cinder_job, to_queue='amo-env-some-other-queue', notes='?'
).exists()
assert not cinder_job.decision

@override_switch('dsa-cinder-forwarded-review', active=True)
def test_process_queue_move_with_addon_already_moderated(self):
Expand Down Expand Up @@ -1476,7 +1475,11 @@ def test_process_queue_move_with_addon_already_moderated(self):
assert mail.outbox[0].to == ['some@email.com']
assert 'already assessed' in mail.outbox[0].body
assert ContentDecision.objects.exists()
decision = ContentDecision.objects.get()
assert ContentDecision.objects.count() == 2
assert ContentDecision.objects.first().action == (
DECISION_ACTIONS.AMO_ESCALATE_ADDON
)
decision = ContentDecision.objects.last()
assert decision.action == DECISION_ACTIONS.AMO_CLOSED_NO_ACTION
self.assertCloseToNow(decision.action_date)
assert decision.cinder_job == CinderJob.objects.get()
Expand Down Expand Up @@ -1612,14 +1615,24 @@ def test_clear_needs_human_review_flags_abuse(self):
def test_clear_needs_human_review_flags_forwarded_moved_queue(self):
job = self._setup_clear_needs_human_review_flags()
# if the job is forwarded, we make sure that there are no other forwarded jobs
CinderQueueMove.objects.create(cinder_job=job, to_queue='wherever')
job.decision.update(action=DECISION_ACTIONS.AMO_ESCALATE_ADDON)
ContentDecision.objects.create(
action=DECISION_ACTIONS.AMO_APPROVE,
addon=job.target_addon,
cinder_job=job,
override_of=job.final_decision,
)

other_forward = CinderJob.objects.create(
job_id='3',
target_addon=job.target_addon,
resolvable_in_reviewer_tools=True,
)
CinderQueueMove.objects.create(cinder_job=other_forward, to_queue='whoever')
ContentDecision.objects.create(
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
addon=job.target_addon,
cinder_job=other_forward,
)
job.clear_needs_human_review_flags()
assert self._nhr_exists(NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION)
assert self._nhr_exists(NeedsHumanReview.REASONS.CINDER_ESCALATION)
Expand All @@ -1632,6 +1645,7 @@ def test_clear_needs_human_review_flags_forwarded_moved_queue(self):
action=DECISION_ACTIONS.AMO_APPROVE,
addon=job.target_addon,
cinder_job=other_forward,
override_of=other_forward.decision,
)
job.clear_needs_human_review_flags()
assert self._nhr_exists(NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION)
Expand Down Expand Up @@ -3507,17 +3521,24 @@ def test_send_notifications_without_notifying_owners(self):
def test_resolve_job_forwarded(self):
addon_developer = user_factory()
addon = addon_factory(users=[addon_developer])
cinder_job = CinderJob.objects.create(job_id='999')
ContentDecision.objects.create(
addon=addon,
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
action_date=datetime.now(),
private_notes='why moved',
cinder_job=cinder_job,
)
decision = ContentDecision.objects.create(
addon=addon,
action=DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
action_date=datetime.now(),
reasoning='some review text',
reviewer_user=self.reviewer_user,
cinder_job=cinder_job,
)
policies = [CinderPolicy.objects.create(name='policy', uuid='12345678')]
decision.policies.set(policies)
cinder_job = CinderJob.objects.create(job_id='999', decision=decision)
CinderQueueMove.objects.create(cinder_job=cinder_job, to_queue='wherever')
NeedsHumanReview.objects.create(
version=addon.current_version,
reason=NeedsHumanReview.REASONS.CINDER_ESCALATION,
Expand Down Expand Up @@ -3563,14 +3584,20 @@ def test_resolve_job_forwarded(self):
def test_execute_action_forwarded_to_legal(self):
addon_developer = user_factory()
addon = addon_factory(users=[addon_developer])
cinder_job = CinderJob.objects.create(job_id='999')
ContentDecision.objects.create(
addon=addon,
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
private_notes='why moved',
cinder_job=cinder_job,
)
decision = ContentDecision.objects.create(
addon=addon,
action=DECISION_ACTIONS.AMO_LEGAL_FORWARD,
reasoning='some reasoning',
reviewer_user=self.reviewer_user,
cinder_job=cinder_job,
)
cinder_job = CinderJob.objects.create(job_id='999', decision=decision)
CinderQueueMove.objects.create(cinder_job=cinder_job, to_queue='wherever')
NeedsHumanReview.objects.create(
version=addon.current_version,
reason=NeedsHumanReview.REASONS.CINDER_ESCALATION,
Expand Down Expand Up @@ -3598,8 +3625,8 @@ def test_execute_action_forwarded_to_legal(self):
decision.execute_action()

cinder_job.reload()
assert cinder_job.decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD
self.assertCloseToNow(cinder_job.decision.action_date)
assert cinder_job.final_decision.action == DECISION_ACTIONS.AMO_LEGAL_FORWARD
self.assertCloseToNow(cinder_job.final_decision.action_date)
assert not NeedsHumanReview.objects.filter(
is_active=True, reason=NeedsHumanReview.REASONS.CINDER_ESCALATION
).exists()
Expand Down
2 changes: 1 addition & 1 deletion src/olympia/constants/abuse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
('AMO_BAN_USER', 1, 'User ban'),
('AMO_DISABLE_ADDON', 2, 'Add-on disable'),
# Used to indicate the job has been forwarded to AMO
('AMO_ESCALATE_ADDON', 3, '(Obsolete) Forward add-on to reviewers'),
('AMO_ESCALATE_ADDON', 3, 'Forward add-on to reviewers'),
# 4 is unused
('AMO_DELETE_RATING', 5, 'Rating delete'),
('AMO_DELETE_COLLECTION', 6, 'Collection delete'),
Expand Down
48 changes: 33 additions & 15 deletions src/olympia/reviewers/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,34 @@ def create_option(
# label_from_instance() on WidgetRenderedModelMultipleChoiceField returns the
# full object, not a label, this is what makes this work.
obj = label

forwarded_or_requeued_decisions = list(
obj.decisions.filter(
action__in=(
DECISION_ACTIONS.AMO_REQUEUE,
DECISION_ACTIONS.AMO_ESCALATE_ADDON,
)
).order_by('-created')
)

is_appeal = obj.is_appeal
queue_moves = list(obj.queue_moves.order_by('-created'))
requeued_decisions = list(
obj.decisions.filter(action=DECISION_ACTIONS.AMO_REQUEUE).order_by(
'-created'
)
forwarded = next(
(
decision
for decision in forwarded_or_requeued_decisions
if decision.action == DECISION_ACTIONS.AMO_ESCALATE_ADDON
),
None,
)
forwarded = queue_moves[0].created if queue_moves else None
requeued = requeued_decisions[0].created if requeued_decisions else None
requeued = next(
(
decision
for decision in forwarded_or_requeued_decisions
if decision.action == DECISION_ACTIONS.AMO_REQUEUE
),
None,
)

reports = obj.all_abuse_reports
reasons_set = {
(report.REASONS.for_value(report.reason).display,) for report in reports
Expand All @@ -266,13 +285,12 @@ def create_option(
)
for report in reports
)
forwarded_or_requeued_notes = [
*(move.notes for move in queue_moves),
*(decision.private_notes for decision in requeued_decisions),
]
internal_notes_gen = (
decision.private_notes for decision in forwarded_or_requeued_decisions
)
internal_notes = (
((f'Reasoning: {"; ".join(forwarded_or_requeued_notes)}',),)
if forwarded_or_requeued_notes
((f'Reasoning: {"; ".join(internal_notes_gen)}',),)
if forwarded_or_requeued_decisions
else ()
)
appeals = (
Expand All @@ -294,10 +312,10 @@ def create_option(
'</details>',
format_datetime(obj.created),
'[Appeal] ' if is_appeal else '',
format_html('[Forwarded on {}] ', format_datetime(forwarded))
format_html('[Forwarded on {}] ', format_datetime(forwarded.created))
if forwarded
else '',
format_html('[Requeued on {}] ', format_datetime(requeued))
format_html('[Requeued on {}] ', format_datetime(requeued.created))
if requeued
else '',
format_html_join(', ', '"{}"', reasons_set),
Expand Down
10 changes: 5 additions & 5 deletions src/olympia/reviewers/tests/test_forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
CinderAppeal,
CinderJob,
CinderPolicy,
CinderQueueMove,
ContentDecision,
)
from olympia.addons.models import Addon
Expand Down Expand Up @@ -1338,11 +1337,12 @@ def test_cinder_jobs_to_resolve_choices(self):
addon=self.addon,
cinder_job=cinder_job_forwarded,
)
CinderQueueMove.objects.create(
ContentDecision.objects.create(
created=datetime(2025, 5, 22, 11, 42, 5, 541216),
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
private_notes='Zee de zee',
addon=self.addon,
cinder_job=cinder_job_forwarded,
notes='Zee de zee',
to_queue='amo-env-content-infringment',
)
AbuseReport.objects.create(
**{**abuse_kw, 'location': AbuseReport.LOCATION.AMO},
Expand Down Expand Up @@ -1391,7 +1391,7 @@ def test_cinder_jobs_to_resolve_choices(self):
'[Forwarded on May 22, 2025, 11:42 a.m.] '
'[Requeued on May 23, 2025, 10:54 p.m.] '
'"DSA: It violates Mozilla\'s Add-on Policies"\n'
'Reasoning: Zee de zee; Why o why\n\n'
'Reasoning: Why o why; Zee de zee\n\n'
'Show detail on 1 reports\n'
'v[<script>alert()</script>]: ddd'
)
Expand Down
Loading