Skip to content

Commit

Permalink
implement ContentActionRejectVersion
Browse files Browse the repository at this point in the history
  • Loading branch information
eviljeff committed Jan 24, 2025
1 parent b0f4bb1 commit 5d803c7
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 31 deletions.
104 changes: 94 additions & 10 deletions src/olympia/abuse/actions.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import random
from datetime import datetime, timedelta

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
Expand All @@ -13,12 +14,14 @@
import olympia
from olympia import amo
from olympia.activity import log_create
from olympia.addons.models import Addon
from olympia.addons.models import Addon, AddonReviewerFlags
from olympia.amo.templatetags.jinja_helpers import absolutify
from olympia.amo.utils import send_mail
from olympia.bandwagon.models import Collection
from olympia.files.models import File
from olympia.ratings.models import Rating
from olympia.users.models import UserProfile
from olympia.versions.models import VersionReviewerFlags


POLICY_DOCUMENT_URL = (
Expand Down Expand Up @@ -295,7 +298,7 @@ def log_action(self, activity_log_action, *extra_args, extra_details=None):
)
extra_details = {'human_review': human_review} | (extra_details or {})
if self.addon_version:
extra_args = (*extra_args, self.addon_version)
extra_args = (self.addon_version, *extra_args)
extra_details['version'] = self.addon_version.version
# While we still have ReviewActionReason in addition to ContentPolicy, re-add
# any instances from earlier activity logs (e.g. held action)
Expand Down Expand Up @@ -324,25 +327,106 @@ def get_owners(self):
class ContentActionRejectVersion(ContentActionDisableAddon):
description = 'Add-on version(s) have been rejected'

def should_hold_action(self):
# This action should only be used by reviewer tools, not cinder webhook
# eventually, if add-on becomes non-public do as disable
raise NotImplementedError
def __init__(self, decision):
super().__init__(decision)
self.content_review = decision.metadata.get('content_review', False)

def log_action(self, activity_log_action, *extra_args, extra_details=None):
# include target versions. addon_version will be included already
versions = tuple(
self.decision.target_versions.exclude(id=self.addon_version.id)
)
return super().log_action(
activity_log_action, *extra_args, *versions, extra_details=extra_details
)

# should_hold_action as ContentActionDisableAddon

def process_action(self):
# This action should only be used by reviewer tools, not cinder webhook
raise NotImplementedError
if not self.decision.reviewer_user:
# This action should only be used by reviewer tools, not cinder webhook
raise NotImplementedError
for version in self.decision.target_versions.all():
version.file.update(
datestatuschanged=datetime.now(),
status=amo.STATUS_DISABLED,
original_status=amo.STATUS_NULL,
status_disabled_reason=File.STATUS_DISABLED_REASONS.NONE,
)
# (Re)set pending_rejection.
VersionReviewerFlags.objects.update_or_create(
version=version,
defaults={
'pending_rejection': None,
'pending_rejection_by': None,
'pending_content_rejection': None,
},
)

self.target.update_status()
return self.log_action(
amo.LOG.REJECT_CONTENT if self.content_review else amo.LOG.REJECT_VERSION
)

def hold_action(self):
# This action should only be used by reviewer tools, not cinder webhook
raise NotImplementedError
return self.log_action(
amo.LOG.HELD_ACTION_REJECT_CONTENT
if self.content_review
else amo.LOG.HELD_ACTION_REJECT_VERSIONS
)


class ContentActionRejectVersionDelayed(ContentActionRejectVersion):
description = 'Add-on version(s) will be rejected'
reporter_template_path = 'abuse/emails/reporter_takedown_addon_delayed.txt'
reporter_appeal_template_path = 'abuse/emails/reporter_appeal_takedown_delayed.txt'

def __init__(self, decision):
super().__init__(decision)
self.days = int(self.decision.metadata.get('delayed_rejection_days', 0))

def log_action(self, activity_log_action, *extra_args, extra_details=None):
extra_details = {**(extra_details or {}), 'delayed_rejection_days': self.days}
return super().log_action(
activity_log_action, *extra_args, extra_details=extra_details
)

# should_hold_action as ContentActionDisableAddon

def process_action(self):
if not self.decision.reviewer_user:
# This action should only be used by reviewer tools, not cinder webhook
raise NotImplementedError
pending_rejection_deadline = datetime.now() + timedelta(days=self.days)

for version in self.decision.target_versions.all():
# (Re)set pending_rejection.
VersionReviewerFlags.objects.update_or_create(
version=version,
defaults={
'pending_rejection': pending_rejection_deadline,
'pending_rejection_by': self.decision.reviewer_user,
'pending_content_rejection': self.content_review,
},
)
# Developers should be notified again once the deadline is close.
AddonReviewerFlags.objects.update_or_create(
addon=self.target,
defaults={'notified_about_expiring_delayed_rejections': False},
)
return self.log_action(
amo.LOG.REJECT_CONTENT_DELAYED
if self.content_review
else amo.LOG.REJECT_VERSION_DELAYED
)

def hold_action(self):
return self.log_action(
amo.LOG.HELD_ACTION_REJECT_CONTENT_DELAYED
if self.content_review
else amo.LOG.HELD_ACTION_REJECT_VERSIONS_DELAYED
)


class ContentActionForwardToReviewers(ContentAction):
valid_targets = (Addon,)
Expand Down
18 changes: 18 additions & 0 deletions src/olympia/abuse/migrations/0050_contentdecision_metadata.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.17 on 2025-01-24 13:42

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('abuse', '0049_contentdecision_target_versions'),
]

operations = [
migrations.AddField(
model_name='contentdecision',
name='metadata',
field=models.JSONField(default=dict),
),
]
7 changes: 4 additions & 3 deletions src/olympia/abuse/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,9 @@ class ContentDecision(ModelBase):
activities = models.ManyToManyField(
to='activity.ActivityLog', through='activity.ContentDecisionLog'
)
# Any additional metadata we need to attach to this decision that doesn't warrant a
# dedicated field
metadata = models.JSONField(default=dict)

class Meta:
db_table = 'abuse_cinderdecision'
Expand Down Expand Up @@ -1276,9 +1279,7 @@ def execute_action(self, *, release_hold=False):
if self.is_delayed:
cinder_job.pending_rejections.add(
*VersionReviewerFlags.objects.filter(
version__in=log_entry.versionlog_set.values_list(
'version', flat=True
)
version__in=self.target_versions.all()
)
)
else:
Expand Down
2 changes: 1 addition & 1 deletion src/olympia/blocklist/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def disable_versions_for_block(block, submission):
'is_addon_being_disabled': submission.disable_addon,
}
)
review.reject_multiple_versions()
review.auto_reject_multiple_versions()

for version in block.addon_versions:
# Clear active NeedsHumanReview on all blocked versions, we consider
Expand Down
44 changes: 44 additions & 0 deletions src/olympia/constants/activity.py
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,50 @@ class REQUEST_LEGAL(_LOG):
cinder_action = DECISION_ACTIONS.AMO_LEGAL_FORWARD


class HELD_ACTION_REJECT_VERSIONS(_LOG):
id = 199
action_class = 'reject'
format = _('{addon} {version} rejection held for further review.')
reviewer_format = 'Held {addon} {version} rejection by {user_responsible}.'
admin_format = reviewer_format
short = 'Held Rejection'
admin_event = True


class HELD_ACTION_REJECT_VERSIONS_DELAYED(_LOG):
id = 200
action_class = 'reject'
format = _('{addon} {version} scheduled rejection held for further review.')
reviewer_format = (
'Held {addon} {version} scheduled rejection by {user_responsible}.'
)
admin_format = reviewer_format
short = 'Held Scheduled Rejection'
admin_event = True


class HELD_ACTION_REJECT_CONTENT(_LOG):
id = 201
action_class = 'reject'
format = _('{addon} {version} content rejection held for further review.')
reviewer_format = 'Held {addon} {version} content rejection by {user_responsible}.'
admin_format = reviewer_format
short = 'Held Content Rejection'
admin_event = True


class HELD_ACTION_REJECT_CONTENT_DELAYED(_LOG):
id = 202
action_class = 'reject'
format = _('{addon} {version} scheduled content rejection held for further review.')
reviewer_format = (
'Held {addon} {version} scheduled content rejection by {user_responsible}.'
)
admin_format = reviewer_format
short = 'Held Scheduled Content Rejection'
admin_event = True


LOGS = [x for x in vars().values() if isclass(x) and issubclass(x, _LOG) and x != _LOG]
# Make sure there's no duplicate IDs.
assert len(LOGS) == len({log.id for log in LOGS})
Expand Down
2 changes: 1 addition & 1 deletion src/olympia/reviewers/management/commands/auto_reject.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ def reject_versions(self, *, addon, versions, latest_version):
'cinder_jobs_to_resolve': cinder_jobs,
'versions': versions,
}
helper.handler.reject_multiple_versions()
helper.handler.auto_reject_multiple_versions()
VersionReviewerFlags.objects.filter(version__in=list(versions)).update(
pending_rejection=None,
pending_rejection_by=None,
Expand Down
24 changes: 16 additions & 8 deletions src/olympia/reviewers/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2345,7 +2345,10 @@ def _test_reject_multiple_versions(self, extra_data, human_review=True):
self.helper.set_data(
{**self.get_data(), 'versions': self.addon.versions.all(), **extra_data}
)
self.helper.handler.reject_multiple_versions()
if human_review:
self.helper.handler.reject_multiple_versions()
else:
self.helper.handler.auto_reject_multiple_versions()

self.addon.reload()
self.file.reload()
Expand Down Expand Up @@ -2382,9 +2385,9 @@ def _test_reject_multiple_versions(self, extra_data, human_review=True):
decision = ContentDecision.objects.get()
assert log.arguments == [
self.addon,
decision,
self.review_version,
old_version,
decision,
]

# listed auto approvals should be disabled until the next manual
Expand Down Expand Up @@ -2489,7 +2492,7 @@ def _test_reject_multiple_versions_with_delay(self, extra_data):
.get()
)
decision = ContentDecision.objects.get()
assert log.arguments == [self.addon, self.review_version, old_version, decision]
assert log.arguments == [self.addon, decision, self.review_version, old_version]

# The flag to prevent the authors from being notified several times
# about pending rejections should have been reset, and auto approvals
Expand Down Expand Up @@ -2577,7 +2580,7 @@ def test_reject_multiple_versions_except_latest(self):
self.check_subject(message)
assert 'Your Extension Delicious Bookmarks was manually' in message.body
assert 'versions of your Extension have been disabled' in message.body
assert 'Affected versions: 3.1, 2.1.072' in message.body
assert 'Affected versions: 2.1.072, 3.1' in message.body
log_token = ActivityLogToken.objects.filter(version=extra_version).get()
assert log_token.uuid.hex in message.reply_to[0]

Expand Down Expand Up @@ -2709,7 +2712,7 @@ def test_reject_multiple_versions_content_review_with_delay(self):
.get()
)
decision = ContentDecision.objects.get()
assert log.arguments == [self.addon, self.review_version, old_version, decision]
assert log.arguments == [self.addon, decision, self.review_version, old_version]

def test_unreject_latest_version_approved_addon(self):
first_version = self.review_version
Expand Down Expand Up @@ -2914,7 +2917,7 @@ def test_reject_multiple_versions_unlisted(self):
.get()
)
decision = ContentDecision.objects.get()
assert log.arguments == [self.addon, self.review_version, old_version, decision]
assert log.arguments == [self.addon, decision, self.review_version, old_version]

def _setup_reject_multiple_versions_delayed(self, content_review):
# Do a rejection with delay.
Expand Down Expand Up @@ -2965,7 +2968,7 @@ def _setup_reject_multiple_versions_delayed(self, content_review):
.get()
)
decision = ContentDecision.objects.get()
assert log.arguments == [self.addon, self.review_version, old_version, decision]
assert log.arguments == [self.addon, decision, self.review_version, old_version]
# The request user is recorded as scheduling the rejection.
assert log.user == original_user

Expand All @@ -2982,7 +2985,7 @@ def _test_reject_multiple_versions_delayed(self, content_review):
# Clear our the ActivityLogs.
ActivityLog.objects.all().delete()

self.helper.handler.reject_multiple_versions()
self.helper.handler.auto_reject_multiple_versions()

self.addon.reload()
assert self.addon.status == amo.STATUS_NULL
Expand Down Expand Up @@ -3550,6 +3553,7 @@ def test_record_decision_called_everywhere_checkbox_shown_listed(self):
},
'reject': {
'should_email': True,
'uses_content_action': True,
'cinder_action': DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
},
'confirm_auto_approved': {
Expand All @@ -3558,6 +3562,7 @@ def test_record_decision_called_everywhere_checkbox_shown_listed(self):
},
'reject_multiple_versions': {
'should_email': True,
'uses_content_action': True,
'cinder_action': DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
},
'disable_addon': {
Expand All @@ -3583,6 +3588,7 @@ def test_record_decision_called_everywhere_checkbox_shown_listed(self):
},
'reject_multiple_versions': {
'should_email': True,
'uses_content_action': True,
'cinder_action': DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
},
'disable_addon': {
Expand Down Expand Up @@ -3641,6 +3647,7 @@ def test_record_decision_called_everywhere_checkbox_shown_unlisted(self):
},
'reject_multiple_versions': {
'should_email': True,
'uses_content_action': True,
'cinder_action': DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
},
'confirm_multiple_versions': {
Expand Down Expand Up @@ -3669,6 +3676,7 @@ def test_record_decision_called_everywhere_checkbox_shown_unlisted(self):
},
'reject_multiple_versions': {
'should_email': True,
'uses_content_action': True,
'cinder_action': DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON,
},
'confirm_multiple_versions': {
Expand Down
Loading

0 comments on commit 5d803c7

Please sign in to comment.