Skip to content

add support for Approve marking listing content as approved #24366

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

Merged
eviljeff merged 1 commit into from
Jan 28, 2026
+140 −45
Merged

add support for Approve marking listing content as approved #24366

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
45 changes: 37 additions & 8 deletions src/olympia/abuse/actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -882,7 +882,7 @@ def process_action(self, release_hold=False):
and target.status == amo.STATUS_REJECTED
):
AddonApprovalsCounter.objects.update_or_create(
addon=self.target,
addon=target,
defaults={
'last_content_review': datetime.now(),
'last_content_review_pass': True,
Expand Down Expand Up @@ -919,10 +919,10 @@ def process_action(self, release_hold=False):
amo.LOG.UNDELETE_RATING,
self.target.addon,
extra_details={
'body': str(self.target.body),
'addon_id': self.target.addon.pk,
'addon_title': str(self.target.addon.name),
'is_flagged': self.target.ratingflag_set.exists(),
'body': str(target.body),
'addon_id': target.addon.pk,
'addon_title': str(target.addon.name),
'is_flagged': target.ratingflag_set.exists(),
},
)

Expand All @@ -941,11 +941,40 @@ def previous_decisions(self):
return self.decision.__class__.objects.filter(pk=self.decision.override_of_id)


class ContentActionApproveNoAction(AnyTargetMixin, NoActionMixin, ContentAction):
description = 'Reported content is within policy, initial decision, so no action'
class ContentActionApproveListingContent(
AnyTargetMixin, AnyOwnerEmailMixin, ContentAction
):
description = 'Reported content is within policy'
reporter_template_path = 'abuse/emails/reporter_content_approve.txt'
reporter_appeal_template_path = 'abuse/emails/reporter_appeal_approve.txt'

def __init__(self, decision):
super().__init__(decision)
self.status = self.target.status if isinstance(self.target, Addon) else None

def get_owners(self):
if self.status == amo.STATUS_REJECTED:
# If we're approving listing content that was rejected, we need to
# notify the authors.
return self.target.authors.all()
return ()

def process_action(self, release_hold=False):
if isinstance(self.target, Addon):
AddonApprovalsCounter.objects.update_or_create(
addon=self.target,
defaults={
'last_content_review': datetime.now(),
'last_content_review_pass': True,
},
)
if self.status == amo.STATUS_REJECTED:
# Call the function to correct it the status
self.target.update_status()
log_entry = self.log_action(amo.LOG.APPROVE_LISTING_CONTENT)
return log_entry
return None


class ContentActionApproveInitialDecision(
AnyTargetMixin, NoActionMixin, AnyOwnerEmailMixin, ContentAction
Expand Down Expand Up @@ -991,7 +1020,7 @@ class ContentActionNotImplemented(AnyTargetMixin, NoActionMixin, ContentAction):
DECISION_ACTIONS.AMO_BLOCK_ADDON: ContentActionBlockAddon,
DECISION_ACTIONS.AMO_DELETE_COLLECTION: ContentActionDeleteCollection,
DECISION_ACTIONS.AMO_DELETE_RATING: ContentActionDeleteRating,
DECISION_ACTIONS.AMO_APPROVE: ContentActionApproveNoAction,
DECISION_ACTIONS.AMO_APPROVE: ContentActionApproveListingContent,
DECISION_ACTIONS.AMO_APPROVE_VERSION: ContentActionApproveInitialDecision,
DECISION_ACTIONS.AMO_IGNORE: ContentActionIgnore,
DECISION_ACTIONS.AMO_CLOSED_NO_ACTION: ContentActionAlreadyModerated,
Expand Down
19 changes: 19 additions & 0 deletions src/olympia/abuse/templates/abuse/emails/ContentActionApproveListingContent.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Hello,

Previously, your {{ type }} was suspended/removed from Mozilla Add-ons, based on a finding that you had violated Mozilla's policies.

We have now determined that your content is within policy, and based on that determination, we have restored your {{ type }}. It is now available at {{ target_url }}.
{% if manual_reasoning_text %}{{ manual_reasoning_text }}. {% endif %}

{% if has_attachment %}
An attachment was provided. {% if dev_url %}To respond or view the file, visit {{ dev_url }}.{% endif %}

{% endif %}
Thank you.

More information about Mozilla's add-on policies can be found at {{ policy_document_url }}.

[{{ reference_id }}]
--
Mozilla Add-ons Team
{{ SITE_URL }}
117 changes: 82 additions & 35 deletions src/olympia/abuse/tests/test_actions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
from ..actions import (
ContentAction,
ContentActionApproveInitialDecision,
ContentActionApproveNoAction,
ContentActionApproveListingContent,
ContentActionBanUser,
ContentActionBlockAddon,
ContentActionDeleteCollection,
Expand Down Expand Up @@ -279,16 +279,18 @@ def test_approve_override_success(self):
self._test_approve_appeal_or_override(ContentActionOverrideApprove)
assert 'After reviewing your appeal' not in mail.outbox[0].body

def _test_reporter_no_action_taken(
self,
*,
ActionClass=ContentActionApproveNoAction,
action=DECISION_ACTIONS.AMO_APPROVE,
):
def _test_reporter_no_action_taken(self, *, ActionClass, action):
raise NotImplementedError

def _test_reporter_content_approved_action_taken(self):
# For most ActionClasses, there is no action taken.
return self._test_reporter_no_action_taken(
ActionClass=ContentActionApproveListingContent,
action=DECISION_ACTIONS.AMO_APPROVE,
)

def test_reporter_content_approve_report(self):
subject = self._test_reporter_no_action_taken()
subject = self._test_reporter_content_approved_action_taken()
assert len(mail.outbox) == 2
self._test_reporter_content_approve_email(subject)

Expand All @@ -310,14 +312,15 @@ def test_reporter_appeal_approve(self):
decision=original_job.decision, reporter_report=self.abuse_report_auth
)
self.cinder_job.reload()
subject = self._test_reporter_no_action_taken()
subject = self._test_reporter_content_approved_action_taken()
assert len(mail.outbox) == 1 # only abuse_report_auth reporter
self._test_reporter_appeal_approve_email(subject)

def test_owner_content_approve_report_email(self):
# This isn't called by cinder actions, but is triggered by reviewer actions
subject = self._test_reporter_no_action_taken(
ActionClass=ContentActionApproveInitialDecision
ActionClass=ContentActionApproveInitialDecision,
action=DECISION_ACTIONS.AMO_APPROVE,
)
assert len(mail.outbox) == 3
self._test_reporter_content_approve_email(subject)
Expand Down Expand Up @@ -365,7 +368,7 @@ def test_email_content_not_escaped(self):
action.notify_owners()
assert unsafe_str in mail.outbox[0].body

action = ContentActionApproveNoAction(self.decision)
action = ContentActionApproveListingContent(self.decision)
mail.outbox.clear()
action.notify_reporters(
reporter_abuse_reports=[self.abuse_report_auth], is_appeal=True
Expand Down Expand Up @@ -508,12 +511,7 @@ def test_ban_user_after_reporter_appeal(self):
assert len(mail.outbox) == 2
self._test_reporter_appeal_takedown_email(subject)

def _test_reporter_no_action_taken(
self,
*,
ActionClass=ContentActionApproveNoAction,
action=DECISION_ACTIONS.AMO_APPROVE,
):
def _test_reporter_no_action_taken(self, *, ActionClass, action):
self.decision.update(action=action)
action = ActionClass(self.decision)
assert action.process_action() is None
Expand Down Expand Up @@ -755,12 +753,7 @@ def _test_approve_appeal_or_override(self, ContentActionClass):
action.notify_owners()
self._test_owner_restore_email(f'Mozilla Add-ons: {self.addon.name}')

def _test_reporter_no_action_taken(
self,
*,
ActionClass=ContentActionApproveNoAction,
action=DECISION_ACTIONS.AMO_APPROVE,
):
def _test_reporter_no_action_taken(self, *, ActionClass, action):
self.decision.update(action=action)
action = ActionClass(self.decision)
assert action.process_action() is None
Expand All @@ -772,6 +765,70 @@ def _test_reporter_no_action_taken(
action.notify_owners()
return f'Mozilla Add-ons: {self.addon.name}'

def _test_reporter_content_approved_action_taken(self):
# override because Addon's get content reviewed if marked as Approve
action = DECISION_ACTIONS.AMO_APPROVE
self.decision.update(action=action)
action = ContentActionApproveListingContent(self.decision)
activity = action.process_action()
assert activity.log == amo.LOG.APPROVE_LISTING_CONTENT
assert activity.arguments == [self.addon, self.decision, self.policy]
assert activity.user == self.task_user

assert self.addon.reload().status == amo.STATUS_APPROVED
assert ActivityLog.objects.count() == 2
second_activity = ActivityLog.objects.exclude(pk=activity.pk).get()
assert second_activity.log == amo.LOG.REVIEWER_PRIVATE_COMMENT
assert second_activity.arguments == [self.addon, self.decision]
assert second_activity.user == self.task_user
assert second_activity.details == {'comments': self.decision.private_notes}

counter = AddonApprovalsCounter.objects.get(addon=self.addon)
assert counter.last_content_review_pass is True
self.assertCloseToNow(counter.last_content_review)

assert len(mail.outbox) == 0
self.cinder_job.notify_reporters(action)
action.notify_owners()
return f'Mozilla Add-ons: {self.addon.name}'

def test_content_approve_rejected_listing_content(self):
AddonApprovalsCounter.objects.create(
addon=self.addon, last_content_review_pass=False
)
self.addon.update(status=amo.STATUS_REJECTED)
action = DECISION_ACTIONS.AMO_APPROVE
self.decision.update(action=action)
action = ContentActionApproveListingContent(self.decision)
activity = action.process_action()
assert activity.log == amo.LOG.APPROVE_LISTING_CONTENT
assert activity.arguments == [self.addon, self.decision, self.policy]
assert activity.user == self.task_user

assert self.addon.reload().status == amo.STATUS_APPROVED
assert ActivityLog.objects.count() == 3
second_activity, third_activity = ActivityLog.objects.exclude(
pk=activity.pk
).filter()
assert second_activity.log == amo.LOG.REVIEWER_PRIVATE_COMMENT
assert second_activity.arguments == [self.addon, self.decision]
assert second_activity.user == self.task_user
assert second_activity.details == {'comments': self.decision.private_notes}
assert third_activity.log == amo.LOG.CHANGE_STATUS

counter = AddonApprovalsCounter.objects.get(addon=self.addon)
assert counter.last_content_review_pass is True
self.assertCloseToNow(counter.last_content_review)

assert len(mail.outbox) == 0
self.cinder_job.notify_reporters(action)
action.notify_owners()
subject = f'Mozilla Add-ons: {self.addon.name}'

assert len(mail.outbox) == 3
self._test_reporter_content_approve_email(subject)
assert 'within policy, and based on that determination' in mail.outbox[-1].body

def test_target_appeal_decline(self):
self.addon.update(status=amo.STATUS_DISABLED)
ActivityLog.objects.all().delete()
Expand Down Expand Up @@ -2159,12 +2216,7 @@ def test_delete_collection_after_reporter_appeal(self):
assert len(mail.outbox) == 2
self._test_reporter_appeal_takedown_email(subject)

def _test_reporter_no_action_taken(
self,
*,
ActionClass=ContentActionApproveNoAction,
action=DECISION_ACTIONS.AMO_APPROVE,
):
def _test_reporter_no_action_taken(self, *, ActionClass, action):
self.decision.update(action=action)
action = ActionClass(self.decision)
assert action.process_action() is None
Expand Down Expand Up @@ -2338,12 +2390,7 @@ def test_delete_rating_after_reporter_appeal(self):
assert len(mail.outbox) == 2
self._test_reporter_appeal_takedown_email(subject)

def _test_reporter_no_action_taken(
self,
*,
ActionClass=ContentActionApproveNoAction,
action=DECISION_ACTIONS.AMO_APPROVE,
):
def _test_reporter_no_action_taken(self, *, ActionClass, action):
self.decision.update(action=action)
action = ActionClass(self.decision)
assert action.process_action() is None
Expand Down
4 changes: 2 additions & 2 deletions src/olympia/abuse/tests/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
from olympia.ratings.models import Rating

from ..actions import (
ContentActionApproveNoAction,
ContentActionApproveListingContent,
ContentActionDisableAddon,
ContentActionTargetAppealApprove,
ContentActionTargetAppealRemovalAffirmation,
Expand Down Expand Up @@ -1508,7 +1508,7 @@ def test_process_decision_triggers_no_target_email_for_reporter_approve(self):
)
req = self.get_request(data=data)
with mock.patch.object(
ContentActionApproveNoAction, 'process_action'
ContentActionApproveListingContent, 'process_action'
) as process_mock:
cinder_webhook(req)
process_mock.assert_called()
Expand Down
Loading