Skip to content

Commit

Permalink
Re-implement cinder forwards (escalations) as a property of the job
Browse files Browse the repository at this point in the history
  • Loading branch information
eviljeff committed Aug 19, 2024
1 parent 4a368ca commit e95695a
Show file tree
Hide file tree
Showing 11 changed files with 392 additions and 363 deletions.
92 changes: 67 additions & 25 deletions src/olympia/abuse/cinder.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ def post_report(self, *, job):
a keyword argument."""
pass

def job_moved_queue(self, *, job):
"""Handle an existing job moving queue."""
raise NotImplementedError


class CinderUser(CinderEntity):
type = 'amo_user'
Expand Down Expand Up @@ -294,9 +298,9 @@ def appeal(self, *args, **kwargs):
class CinderAddon(CinderEntity):
type = 'amo_addon'

def __init__(self, addon, version=None):
def __init__(self, addon, version_string=None):
self.addon = addon
self.version = version
self.version_string = version_string
self.related_users = self.addon.authors.all()

@property
Expand Down Expand Up @@ -470,63 +474,101 @@ class CinderAddonHandledByReviewers(CinderAddon):
def queue(cls):
return f'{settings.CINDER_QUEUE_PREFIX}{cls.queue_suffix}'

def flag_for_human_review(self, appeal=False):
def flag_for_human_review(
self, *, reported_versions, appeal=False, forwarded=False
):
from olympia.reviewers.models import NeedsHumanReview

waffle_switch_name = (
'dsa-appeals-review' if appeal else 'dsa-abuse-reports-review'
'dsa-appeals-review'
if appeal
else 'dsa-cinder-escalations-review'
if forwarded
else 'dsa-abuse-reports-review'
)
if not waffle.switch_is_active(waffle_switch_name):
log.info(
'Not adding %s to review queue despite %s because %s switch is off',
self.addon,
'appeal' if appeal else 'report',
'appeal' if appeal else 'forward' if forwarded else 'report',
waffle_switch_name,
)
return

reason = (
NeedsHumanReview.REASONS.ADDON_REVIEW_APPEAL
if appeal
else NeedsHumanReview.REASONS.CINDER_ESCALATION
if forwarded
else NeedsHumanReview.REASONS.ABUSE_ADDON_VIOLATION
)
nhr_object = NeedsHumanReview(
version=self.version, reason=reason, is_active=True

version_objs = (
set(
self.addon.versions(manager='unfiltered_for_relations')
.filter(version__in=reported_versions)
.exclude(
needshumanreview__reason=reason,
needshumanreview__is_active=True,
)
.no_transforms()
)
if reported_versions
else set()
)
if self.version:
if not NeedsHumanReview.objects.filter(
version=self.version, reason=reason, is_active=True
).exists():
nhr_object.save(_no_automatic_activity_log=True)
versions_to_log = [self.version]
else:
versions_to_log = []
else:
versions_to_log = self.addon.set_needs_human_review_on_latest_versions(
nhr_object = None
# We need custom save() and post_save to be triggered, so we can't
# optimize this via bulk_create().
for version in version_objs:
nhr_object = NeedsHumanReview(
version=version, reason=reason, is_active=True
)
nhr_object.save(_no_automatic_activity_log=True)
# If we have more versions specified than versions we flagged, flag latest
# to be safe. (Either because there was an unknown version, or a None)
if len(version_objs) != len(reported_versions) or len(reported_versions) == 0:
version_objs = version_objs.union(
self.addon.set_needs_human_review_on_latest_versions(
reason=reason,
ignore_reviewed=False,
unique_reason=True,
skip_activity_log=True,
)
)
if version_objs:
version_objs = sorted(version_objs, key=lambda v: v.id)
# we just need this for get_reason_display
nhr_object = nhr_object or NeedsHumanReview(
version=version_objs[-1],
reason=reason,
ignore_reviewed=False,
unique_reason=True,
skip_activity_log=True,
is_active=True,
)
if versions_to_log:
activity.log_create(
amo.LOG.NEEDS_HUMAN_REVIEW_CINDER,
*versions_to_log,
*version_objs,
details={'comments': nhr_object.get_reason_display()},
user=core.get_user() or get_task_user(),
)

def post_report(self, job):
if not job.is_appeal:
self.flag_for_human_review(appeal=False)
reported_version = self.version_string
self.flag_for_human_review(
reported_versions={reported_version}, appeal=False
)
# If our report was added to an appeal job (i.e. an appeal was ongoing,
# and a report was made against the add-on), don't flag the add-on for
# human review again: we should already have one because of the appeal.

def appeal(self, *args, **kwargs):
self.flag_for_human_review(appeal=True)
self.flag_for_human_review(reported_versions=set(), appeal=True)
return super().appeal(*args, **kwargs)

def job_moved_queue(self, job):
reported_versions = set(
job.abusereport_set.values_list('addon_version', flat=True)
)
self.flag_for_human_review(reported_versions=reported_versions, forwarded=True)


class CinderReport(CinderEntity):
type = 'amo_report'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Generated by Django 4.2.15 on 2024-08-19 12:58

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('abuse', '0036_alter_abusereport_appellant_job_cinderappealtext'),
]

operations = [
migrations.AddField(
model_name='cinderjob',
name='is_forwarded',
field=models.BooleanField(default=False),
),
migrations.AddField(
model_name='cinderjob',
name='notes',
field=models.CharField(max_length=255, null=True),
),
migrations.AlterField(
model_name='cinderdecision',
name='action',
field=models.PositiveSmallIntegerField(choices=[(1, 'User ban'), (2, 'Add-on disable'), (3, 'Escalate add-on to reviewers'), (5, 'Rating delete'), (6, 'Collection delete'), (7, 'Approved (no action)'), (8, 'Add-on version reject'), (9, 'Add-on version delayed reject warning'), (10, 'Approved (new version approval)'), (11, 'Invalid report, so ignored'), (12, 'Content already removed (no action)')]),
),
migrations.AlterField(
model_name='cinderpolicy',
name='default_cinder_action',
field=models.PositiveSmallIntegerField(blank=True, choices=[(1, 'User ban'), (2, 'Add-on disable'), (3, 'Escalate add-on to reviewers'), (5, 'Rating delete'), (6, 'Collection delete'), (7, 'Approved (no action)'), (8, 'Add-on version reject'), (9, 'Add-on version delayed reject warning'), (10, 'Approved (new version approval)'), (11, 'Invalid report, so ignored'), (12, 'Content already removed (no action)')], null=True),
),
]
46 changes: 46 additions & 0 deletions src/olympia/abuse/migrations/0038_auto_20240819_1343.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Generated by Django 4.2.15 on 2024-08-19 13:43

from django.db import migrations, models

from olympia.constants.abuse import DECISION_ACTIONS


def migrate_escalate_action_to_is_forwarded(apps, schema_editor):
CinderDecision = apps.get_model('abuse', 'CinderDecision')
CinderJob = apps.get_model('abuse', 'CinderJob')

CinderJob.objects.filter(
decision__action=DECISION_ACTIONS.AMO_ESCALATE_ADDON
).update(
is_forwarded=True,
notes=models.Subquery(
CinderDecision.objects.filter(pk=models.OuterRef('decision')).values('notes')[:1]
)
)
CinderDecision.objects.filter(action=DECISION_ACTIONS.AMO_ESCALATE_ADDON).delete()


def recreate_escalate_decisions(apps, schema_editor):
CinderDecision = apps.get_model('abuse', 'CinderDecision')
CinderJob = apps.get_model('abuse', 'CinderJob')

CinderDecision.objects.bulk_create(
CinderDecision(
cinder_job=job,
action=DECISION_ACTIONS.AMO_ESCALATE_ADDON,
notes=job.notes,
addon=job.target_addon
)
for job in CinderJob.objects.filter(is_forwarded=True)
)


class Migration(migrations.Migration):

dependencies = [
('abuse', '0037_cinderjob_is_forwarded_cinderjob_notes_and_more'),
]

operations = [
migrations.RunPython(migrate_escalate_action_to_is_forwarded, recreate_escalate_decisions)
]
55 changes: 27 additions & 28 deletions src/olympia/abuse/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,7 @@ def for_addon(self, addon):
return self.filter(target_addon=addon).order_by('-pk')

def unresolved(self):
return self.filter(
models.Q(decision__isnull=True)
| models.Q(decision__action__in=tuple(DECISION_ACTIONS.UNRESOLVED.values))
)
return self.filter(decision__isnull=True)

def resolvable_in_reviewer_tools(self):
return self.filter(resolvable_in_reviewer_tools=True)
Expand Down Expand Up @@ -89,6 +86,8 @@ class CinderJob(ModelBase):
related_name='cinder_job',
)
resolvable_in_reviewer_tools = models.BooleanField(default=None, null=True)
is_forwarded = models.BooleanField(default=False)
notes = models.CharField(max_length=255, null=True)

objects = CinderJobManager()

Expand Down Expand Up @@ -129,17 +128,10 @@ def get_entity_helper(
cls, target, *, resolved_in_reviewer_tools, addon_version_string=None
):
if isinstance(target, Addon):
version = (
addon_version_string
and target.versions(manager='unfiltered_for_relations')
.filter(version=addon_version_string)
.no_transforms()
.first()
)
if resolved_in_reviewer_tools:
return CinderAddonHandledByReviewers(target, version)
return CinderAddonHandledByReviewers(target, addon_version_string)
else:
return CinderAddon(target, version)
return CinderAddon(target, addon_version_string)
elif isinstance(target, UserProfile):
return CinderUser(target)
elif isinstance(target, Rating):
Expand Down Expand Up @@ -235,6 +227,15 @@ def notify_reporters(self, action_helper):
is_appeal=True,
)

def process_job_action(self, *, notes):
self.update(notes=notes, resolvable_in_reviewer_tools=True, is_forwarded=True)
entity_helper = self.get_entity_helper(
self.target,
addon_version_string=None,
resolved_in_reviewer_tools=True,
)
entity_helper.job_moved_queue(self)

def process_decision(
self,
*,
Expand All @@ -246,6 +247,11 @@ def process_decision(
):
"""This is called for cinder originated decisions.
See resolve_job for reviewer tools originated decisions."""
if decision_action == DECISION_ACTIONS.AMO_ESCALATE_ADDON:
# temporary redirect while we still handle AMO_ESCALATE_ADDON action rather
# than moving queue, which triggers job action webhook
return self.process_job_action(notes=decision_notes)

overriden_action = getattr(self.decision, 'action', None)
# We need either an AbuseReport or CinderDecision for the target props
abuse_report_or_decision = (
Expand All @@ -270,11 +276,7 @@ def process_decision(
],
},
)
self.update(
decision=cinder_decision,
resolvable_in_reviewer_tools=self.resolvable_in_reviewer_tools
or decision_action == DECISION_ACTIONS.AMO_ESCALATE_ADDON,
)
self.update(decision=cinder_decision)
policies = CinderPolicy.objects.filter(
uuid__in=policy_ids
).without_parents_if_their_children_are_present()
Expand Down Expand Up @@ -302,10 +304,6 @@ def resolve_job(self, *, log_entry):
abuse_report_or_decision.target,
resolved_in_reviewer_tools=self.resolvable_in_reviewer_tools,
)
was_escalated = (
self.decision
and self.decision.action == DECISION_ACTIONS.AMO_ESCALATE_ADDON
)

cinder_decision = self.decision or CinderDecision(
addon=abuse_report_or_decision.addon,
Expand Down Expand Up @@ -337,11 +335,9 @@ def resolve_job(self, *, log_entry):
.unresolved()
.resolvable_in_reviewer_tools()
)
if was_escalated:
if self.is_forwarded:
has_unresolved_jobs_with_similar_reason = (
base_unresolved_jobs_qs.filter(
decision__action=DECISION_ACTIONS.AMO_ESCALATE_ADDON
).exists()
base_unresolved_jobs_qs.filter(is_forwarded=True).exists()
)
reason = NeedsHumanReview.REASONS.CINDER_ESCALATION
elif self.is_appeal:
Expand Down Expand Up @@ -1147,8 +1143,11 @@ def notify_reviewer_decision(
'reasoning': self.notes,
'policy_uuids': [policy.uuid for policy in policies],
}
if not overriden_action and (
cinder_job := getattr(self, 'cinder_job', None)
if (
not overriden_action
and (cinder_job := getattr(self, 'cinder_job', None))
# TODO: drop this condition once we've removed AMO_ESCALATE_ADDON
and not cinder_job.is_forwarded
):
decision_cinder_id = entity_helper.create_job_decision(
job_id=cinder_job.job_id, **create_decision_kw
Expand Down
Loading

0 comments on commit e95695a

Please sign in to comment.