Skip to content

Commit 418477a

Browse files
committed
track queue moves as a decision
1 parent 031680e commit 418477a

File tree

4 files changed

+43
-52
lines changed

4 files changed

+43
-52
lines changed

src/olympia/abuse/models.py

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ def unresolved(self):
5858
Q(decisions__isnull=True)
5959
| Q(
6060
# i.e. the latest decision is a requeue
61-
decisions__action=DECISION_ACTIONS.AMO_REQUEUE,
61+
decisions__action__in=(
62+
DECISION_ACTIONS.AMO_REQUEUE, DECISION_ACTIONS.AMO_ESCALATE
63+
),
6264
decisions__overridden_by__isnull=True,
6365
)
6466
)
@@ -324,8 +326,15 @@ def process_decision(
324326
decision.send_notifications()
325327

326328
def process_queue_move(self, *, new_queue, notes):
327-
CinderQueueMove.objects.create(cinder_job=self, notes=notes, to_queue=new_queue)
328329
if new_queue == CinderAddonHandledByReviewers(self.target).queue:
330+
ContentDecision.objects.create(
331+
addon=self.target_addon,
332+
action=DECISION_ACTIONS.AMO_ESCALATE,
333+
override_of=self.decision,
334+
action_date=datetime.now(),
335+
notes=notes,
336+
cinder_job=self,
337+
)
329338
# now escalated
330339
entity_helper = CinderJob.get_entity_helper(
331340
self.target, resolved_in_reviewer_tools=True
@@ -358,27 +367,20 @@ def clear_needs_human_review_flags(self):
358367
.resolvable_in_reviewer_tools()
359368
)
360369
if (
361-
(decision_actions := tuple(self.decisions.values_list('action', flat=True)))
370+
(penultimate_action :=
371+
tuple(self.decisions.values_list('action', flat=True))[-2:-1])
362372
# i.e. was the previous decision before the current a requeue
363-
and len(decision_actions) >= 2
364-
and decision_actions[-2] == DECISION_ACTIONS.AMO_REQUEUE
373+
and penultimate_action == DECISION_ACTIONS.AMO_REQUEUE
365374
):
366375
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
367376
decisions__action=DECISION_ACTIONS.AMO_REQUEUE,
368377
decisions__overridden_by__isnull=True,
369378
).exists()
370379
reasons = {NeedsHumanReview.REASONS.SECOND_LEVEL_REQUEUE}
371-
elif self.forwarded_from_jobs.exists():
380+
elif penultimate_action == DECISION_ACTIONS.AMO_ESCALATE_ADDON:
372381
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
373-
forwarded_from_jobs__isnull=False
374-
).exists()
375-
reasons = {
376-
NeedsHumanReview.REASONS.CINDER_ESCALATION,
377-
NeedsHumanReview.REASONS.CINDER_APPEAL_ESCALATION,
378-
}
379-
elif self.queue_moves.exists():
380-
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
381-
queue_moves__id__gt=0
382+
decisions__action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
383+
decisions__overridden_by__isnull=True,
382384
).exists()
383385
reasons = {
384386
NeedsHumanReview.REASONS.CINDER_ESCALATION,
@@ -1457,11 +1459,3 @@ class CinderAppeal(ModelBase):
14571459
reporter_report = models.OneToOneField(
14581460
to=AbuseReport, on_delete=models.CASCADE, null=True
14591461
)
1460-
1461-
1462-
class CinderQueueMove(ModelBase):
1463-
cinder_job = models.ForeignKey(
1464-
to=CinderJob, on_delete=models.CASCADE, related_name='queue_moves'
1465-
)
1466-
notes = models.TextField(max_length=1000, blank=True)
1467-
to_queue = models.CharField(max_length=128)

src/olympia/abuse/tests/test_models.py

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@
6262
CinderAppeal,
6363
CinderJob,
6464
CinderPolicy,
65-
CinderQueueMove,
6665
ContentDecision,
6766
)
6867

@@ -1319,9 +1318,10 @@ def test_process_queue_move_into_reviewer_handled(self):
13191318
nhr = NeedsHumanReview.objects.get()
13201319
assert nhr.reason == NeedsHumanReview.REASONS.CINDER_ESCALATION
13211320
assert nhr.version == addon.current_version
1322-
assert CinderQueueMove.objects.filter(
1323-
cinder_job=cinder_job, to_queue='amo-env-addon-infringement', notes='notes!'
1324-
).exists()
1321+
assert cinder_job.decision.notes == 'notes!'
1322+
assert cinder_job.decision.action == DECISION_ACTIONS.AMO_ESCALATE
1323+
assert cinder_job.decision.cinder_job == cinder_job
1324+
assert cinder_job.decision.addon == cinder_job.target_addon
13251325

13261326
def test_process_queue_move_out_of_reviewer_handled(self):
13271327
# Not yet implemented, so just check it's silently ignored
@@ -1339,9 +1339,6 @@ def test_process_queue_move_out_of_reviewer_handled(self):
13391339
assert cinder_job.resolvable_in_reviewer_tools is True
13401340
assert len(mail.outbox) == 0
13411341
assert NeedsHumanReview.objects.count() == 1
1342-
assert CinderQueueMove.objects.filter(
1343-
cinder_job=cinder_job, to_queue='amo-env-listings', notes='out'
1344-
).exists()
13451342

13461343
def test_process_queue_move_other_queue_movement(self):
13471344
# we don't need to about these other queue moves, so just check it's silently
@@ -1354,9 +1351,6 @@ def test_process_queue_move_other_queue_movement(self):
13541351
assert not cinder_job.resolvable_in_reviewer_tools
13551352
assert len(mail.outbox) == 0
13561353
assert NeedsHumanReview.objects.count() == 0
1357-
assert CinderQueueMove.objects.filter(
1358-
cinder_job=cinder_job, to_queue='amo-env-some-other-queue', notes='?'
1359-
).exists()
13601354

13611355
def test_all_abuse_reports(self):
13621356
job = CinderJob.objects.create(job_id='fake_job_id')
@@ -1522,14 +1516,20 @@ def test_clear_needs_human_review_flags_forwarded_with_job(self):
15221516
def test_clear_needs_human_review_flags_forwarded_moved_queue(self):
15231517
job = self._setup_clear_needs_human_review_flags()
15241518
# if the job is forwarded, we make sure that there are no other forwarded jobs
1525-
CinderQueueMove.objects.create(cinder_job=job, to_queue='wherever')
1519+
job.decision.update(action=DECISION_ACTIONS.AMO_ESCALATE)
1520+
ContentDecision.objects.create(
1521+
cinder_job=job, addon=job.target_addon, action=DECISION_ACTIONS.AMO_APPROVE
1522+
)
15261523

15271524
other_forward = CinderJob.objects.create(
15281525
job_id='3',
15291526
target_addon=job.target_addon,
15301527
resolvable_in_reviewer_tools=True,
15311528
)
1532-
CinderQueueMove.objects.create(cinder_job=other_forward, to_queue='whoever')
1529+
ContentDecision.objects.create(
1530+
cinder_job=other_forward, addon=job.target_addon,
1531+
action=DECISION_ACTIONS.AMO_ESCALATE
1532+
)
15331533
job.clear_needs_human_review_flags()
15341534
assert self._nhr_exists(NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION)
15351535
assert self._nhr_exists(NeedsHumanReview.REASONS.CINDER_ESCALATION)

src/olympia/reviewers/forms.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -250,13 +250,13 @@ def create_option(
250250
# label_from_instance() on WidgetRenderedModelMultipleChoiceField returns the
251251
# full object, not a label, this is what makes this work.
252252
obj = label
253-
forwarded_notes = [
254-
*obj.forwarded_from_jobs.all().values_list('decisions__notes', flat=True),
255-
*obj.queue_moves.values_list('notes', flat=True),
256-
*obj.decisions.filter(action=DECISION_ACTIONS.AMO_REQUEUE).values_list(
253+
forwarded_notes = list(
254+
obj.decisions.filter(
255+
action__in=(DECISION_ACTIONS.AMO_REQUEUE, DECISION_ACTIONS.AMO_ESCALATE)
256+
).values_list(
257257
'notes', flat=True
258258
),
259-
]
259+
)
260260
is_appeal = obj.is_appeal
261261
reports = obj.all_abuse_reports
262262
reasons_set = {

src/olympia/reviewers/tests/test_forms.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
CinderAppeal,
1414
CinderJob,
1515
CinderPolicy,
16-
CinderQueueMove,
1716
ContentDecision,
1817
)
1918
from olympia.addons.models import Addon
@@ -1246,19 +1245,17 @@ def test_cinder_jobs_to_resolve_choices(self):
12461245
resolvable_in_reviewer_tools=True,
12471246
target_addon=self.addon,
12481247
)
1249-
CinderJob.objects.create(
1250-
job_id='forwarded_from',
1251-
forwarded_to_job=cinder_job_forwarded,
1252-
decision=ContentDecision.objects.create(
1253-
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
1254-
notes='Why o why',
1255-
addon=self.addon,
1256-
),
1248+
ContentDecision.objects.create(
1249+
action=DECISION_ACTIONS.AMO_ESCALATE,
1250+
notes='Zee de zee',
1251+
addon=self.addon,
1252+
cinder_job=cinder_job_forwarded,
12571253
)
1258-
CinderQueueMove.objects.create(
1254+
ContentDecision.objects.create(
1255+
action=DECISION_ACTIONS.AMO_REQUEUE,
1256+
notes='Why o why',
1257+
addon=self.addon,
12591258
cinder_job=cinder_job_forwarded,
1260-
notes='Zee de zee',
1261-
to_queue='amo-env-content-infringment',
12621259
)
12631260
AbuseReport.objects.create(
12641261
**{**abuse_kw, 'location': AbuseReport.LOCATION.AMO},

0 commit comments

Comments
 (0)