Skip to content
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.

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

Change Wording of Escalated Appeals in Version History #22972

Merged
merged 14 commits into from
Jan 10, 2025
14 changes: 9 additions & 5 deletions src/olympia/abuse/cinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,10 +515,10 @@ def flag_for_human_review(self, *, related_versions, appeal=False, forwarded=Fal
from olympia.reviewers.models import NeedsHumanReview

waffle_switch_name = (
'dsa-appeals-review'
if appeal
else 'dsa-cinder-forwarded-review'
'dsa-cinder-forwarded-review'
if forwarded
else 'dsa-appeals-review'
if appeal
Copy link
Member

Choose a reason for hiding this comment

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

mmm, was this change necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If CINDER_APPEAL_ESCALATION is meant to behave virtually identical to CINDER_ESCALATION (beyond the history), it would make sense for this waffle switch name to be the same, no? If it's the other way around, CINDER_APPEAL_ESCALATION would have 'dsa-appeals-review' as the name instead.

Copy link
Member

Choose a reason for hiding this comment

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

I can see the logic (and don't disagree), but one of reasons we have these series of waffles switches it the rollout is being phased, and reviewers handling appeals is a later phase than handling forwarded reports. So for now I we're going to have to stick with the inconsistency.

else 'dsa-abuse-reports-review'
)
if not waffle.switch_is_active(waffle_switch_name):
Expand All @@ -530,7 +530,9 @@ def flag_for_human_review(self, *, related_versions, appeal=False, forwarded=Fal
)
return
reason = (
NeedsHumanReview.REASONS.ADDON_REVIEW_APPEAL
NeedsHumanReview.REASONS.CINDER_APPEAL_ESCALATION
if appeal and forwarded
else NeedsHumanReview.REASONS.ADDON_REVIEW_APPEAL
if appeal
else NeedsHumanReview.REASONS.CINDER_ESCALATION
if forwarded
Expand Down Expand Up @@ -611,7 +613,9 @@ def post_queue_move(self, *, job):
reported_versions = set(
job.abusereport_set.values_list('addon_version', flat=True)
)
self.flag_for_human_review(related_versions=reported_versions, forwarded=True)
self.flag_for_human_review(
related_versions=reported_versions, appeal=job.is_appeal, forwarded=True
)


class CinderAddonHandledByLegal(CinderAddon):
Expand Down
16 changes: 11 additions & 5 deletions src/olympia/abuse/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,25 +412,31 @@ def clear_needs_human_review_flags(self):
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
forwarded_from_jobs__isnull=False
).exists()
reason = NeedsHumanReview.REASONS.CINDER_ESCALATION
reasons = {
NeedsHumanReview.REASONS.CINDER_ESCALATION,
NeedsHumanReview.REASONS.CINDER_APPEAL_ESCALATION,
}
elif self.queue_moves.exists():
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
queue_moves__id__gt=0
).exists()
reason = NeedsHumanReview.REASONS.CINDER_ESCALATION
reasons = {
NeedsHumanReview.REASONS.CINDER_ESCALATION,
NeedsHumanReview.REASONS.CINDER_APPEAL_ESCALATION,
}
elif self.is_appeal:
has_unresolved_jobs_with_similar_reason = base_unresolved_jobs_qs.filter(
appealed_decisions__isnull=False
).exists()
reason = NeedsHumanReview.REASONS.ADDON_REVIEW_APPEAL
reasons = {NeedsHumanReview.REASONS.ADDON_REVIEW_APPEAL}
else:
# If the job we're resolving was not an appeal or escalation
# then all abuse reports are considered dealt with.
has_unresolved_jobs_with_similar_reason = None
reason = NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION
reasons = {NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION}
if not has_unresolved_jobs_with_similar_reason:
NeedsHumanReview.objects.filter(
version__addon_id=addon.id, is_active=True, reason=reason
version__addon_id=addon.id, is_active=True, reason__in=reasons
).update(is_active=False)
addon.update_all_due_dates()

Expand Down
18 changes: 18 additions & 0 deletions src/olympia/abuse/tests/test_cinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -1454,6 +1454,24 @@ def test_post_queue_move(self):

self._check_post_queue_move_test(listed_version, unlisted_version)

def test_post_queue_move_appeal(self):
cinder_instance, cinder_job, listed_version, _ = (
self._setup_post_queue_move_test()
)
appeal = CinderJob.objects.create(job_id='an appeal job')
cinder_job.update(
decision=ContentDecision.objects.create(
action=DECISION_ACTIONS.AMO_DISABLE_ADDON,
addon=cinder_instance.addon,
appeal_job=appeal,
)
)
cinder_instance.post_queue_move(job=appeal)
assert (
listed_version.reload().needshumanreview_set.get().reason
== NeedsHumanReview.REASONS.CINDER_APPEAL_ESCALATION
)

def test_post_queue_move_specific_version(self):
# but if we have a version specified, we flag that version
cinder_instance, cinder_job, listed_version, unlisted_version = (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.17 on 2025-01-08 14:33

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('reviewers', '0041_reviewqueuehistory'),
]

operations = [
migrations.AlterField(
model_name='needshumanreview',
name='reason',
field=models.SmallIntegerField(choices=[(0, 'Unknown'), (1, 'Hit scanner rule'), (2, 'Was added to a promoted group'), (3, 'Over growth threshold for usage tier'), (4, 'Previous version in channel had needs human review set'), (5, 'Sources provided while pending rejection'), (6, 'Developer replied'), (7, 'Manually set as needing human review by a reviewer'), (8, 'Auto-approved but still had an approval delay set in the past'), (9, 'Over abuse reports threshold for usage tier'), (10, 'Escalated for an abuse report, via cinder'), (11, 'Reported for abuse within the add-on'), (12, "Appeal of a reviewer's decision about a policy violation"), (13, 'Has auto-approval disabled'), (14, 'Belongs to a promoted group'), (15, 'Escalated appeal via cinder')], default=0, editable=False),
),
]
8 changes: 7 additions & 1 deletion src/olympia/reviewers/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -892,10 +892,16 @@ class NeedsHumanReview(ModelBase):
),
('AUTO_APPROVAL_DISABLED', 13, 'Has auto-approval disabled'),
('BELONGS_TO_PROMOTED_GROUP', 14, 'Belongs to a promoted group'),
('CINDER_APPEAL_ESCALATION', 15, 'Escalated appeal via cinder'),
)
REASONS.add_subset(
'ABUSE_OR_APPEAL_RELATED',
('CINDER_ESCALATION', 'ABUSE_ADDON_VIOLATION', 'ADDON_REVIEW_APPEAL'),
(
'CINDER_ESCALATION',
'ABUSE_ADDON_VIOLATION',
'ADDON_REVIEW_APPEAL',
'CINDER_APPEAL_ESCALATION',
),
)

reason = models.SmallIntegerField(
Expand Down
Loading