Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/olympia/abuse/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def notify_owners(self, *, log_entry_id=None, extra_context=None):
'reference_id': reference_id,
'target': self.target,
'target_url': target_url,
'type': self.decision.get_target_display(),
'type': self.decision.get_target_display().lower(),
'SITE_URL': settings.SITE_URL,
**(extra_context or {}),
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Hello,
Comment thread
denyshon marked this conversation as resolved.

{% if is_listing_rejected %}
Your {{ type }} has been approved for distribution, but the listing on Mozilla Add-ons remains unavailable until you address the violations and request a further review. You can edit it at {{ target_url }}.
Your {{ type }} has been approved for distribution, but the listing on Mozilla Add-ons remains unavailable until you address the violations and request a further review. You can edit it at {{ target_url }}.
{% elif not auto_approval %}
Your {{ type }} has been approved on Mozilla Add-ons and it is now available at {{ target_url }}.
{% else %}
Expand All @@ -11,13 +11,12 @@ Your add-on can be subject to human review at any time. Reviewers may determine
{% endif %}
{% if version_list %}Approved versions: {{ version_list }}
{% endif %}
{% if manual_reasoning_text %}Comments: {{ manual_reasoning_text }}.{% endif %}
{% if manual_reasoning_text %}Comments: {{ 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

An alternative - possibly better - place to fix this is where we're saying "Thank you!" as a comment (I've not looked for it)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I've considered that. However, in addition to "Thank you!" in the autoapproval comment, our default manual review comment is also "Thank you for your contribution." (here). So I thought it was better to remove "Thank you." here, given that it is supposed to be supplied with manual_reasoning_text

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we do have the "thank you" in the couple of places where there is a constant value for manual_reasoning_text, but it can also be any value provided by the reviewer.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

so this nails down to the question of whether reviewers are supposed to say "thank you", which I can't answer


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

Expand Down
18 changes: 9 additions & 9 deletions src/olympia/abuse/tests/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ def test_execute_action(self):
self._process_action_and_notify()
subject = f'Mozilla Add-ons: {self.addon.name}'
self._test_owner_takedown_email(subject, self.disable_snippet)
assert f'Your Extension {self.addon.name}' in mail.outbox[-1].body
assert f'Your extension {self.addon.name}' in mail.outbox[-1].body
assert len(mail.outbox) == 3
flags = self.addon.reviewerflags.reload()
assert flags.auto_approval_disabled
Expand All @@ -737,7 +737,7 @@ def test_execute_action_after_reporter_appeal(self):
self._process_action_and_notify()
subject = f'Mozilla Add-ons: {self.addon.name}'
self._test_owner_takedown_email(subject, self.disable_snippet)
assert f'Your Extension {self.addon.name}' in mail.outbox[-1].body
assert f'Your extension {self.addon.name}' in mail.outbox[-1].body
assert len(mail.outbox) == 2
self._test_reporter_appeal_takedown_email(subject)

Expand Down Expand Up @@ -1141,7 +1141,7 @@ def test_approve_override_success_but_not_approved(self):
class TestContentActionRejectVersion(TestContentActionDisableAddon):
ActionClass = ContentActionRejectVersion
activity_log_action = amo.LOG.REJECT_VERSION
disable_snippet = 'versions of your Extension have been disabled'
disable_snippet = 'versions of your extension have been disabled'
default_decision_action = DECISION_ACTIONS.AMO_REJECT_VERSION_ADDON

def setUp(self):
Expand Down Expand Up @@ -1480,7 +1480,7 @@ def test_execute_action_with_stakeholder_email(self):
assert mail.outbox[0].subject == f'Rejection issued for {self.addon.name}'
assert (
f'{self.another_version.version} will be the new current version of the '
'Extension; first approved 2025-02-03.' in mail.outbox[0].body
'extension; first approved 2025-02-03.' in mail.outbox[0].body
)

def test_execute_action_after_reporter_appeal(self):
Expand Down Expand Up @@ -1641,7 +1641,7 @@ def test_execute_action_delayed_with_stakeholder_email(self):
)
assert (
f'{self.another_version.version} will be the new current version of the '
'Extension; first approved 2025-02-03.' in mail.outbox[0].body
'extension; first approved 2025-02-03.' in mail.outbox[0].body
)

def test_execute_action_delayed_after_reporter_appeal(self):
Expand Down Expand Up @@ -1849,7 +1849,7 @@ def test_notify_stakeholders(self):

assert (
f'{self.another_version.version} will be the new current version of the '
'Extension; first approved 2025-01-02' in body
'extension; first approved 2025-01-02' in body
)

# an unlisted version should result in second link to the unlisted review page
Expand All @@ -1866,7 +1866,7 @@ def test_notify_stakeholders(self):
assert f'/review-unlisted/{self.addon.id}' in body
assert (
f'{self.another_version.version} will be the new current version of the '
'Extension; first approved 2025-01-02.' in body
'extension; first approved 2025-01-02.' in body
)

# if the listed version(s) affected are the last approved versions indicate that
Expand Down Expand Up @@ -3079,7 +3079,7 @@ def test_execute_action(self):
self._process_action_and_notify()
subject = f'Mozilla Add-ons: {self.addon.name}'
self._test_owner_takedown_email(subject, self.disable_snippet)
assert f'Your Extension {self.addon.name}' in mail.outbox[-1].body
assert f'Your extension {self.addon.name}' in mail.outbox[-1].body
assert len(mail.outbox) == 3
self._test_reporter_takedown_email(subject)
# Content-rejection doesn't affect auto-approval disabled flags.
Expand Down Expand Up @@ -3183,7 +3183,7 @@ def test_already_taken_down(self):
self._process_action_and_notify()
subject = f'Mozilla Add-ons: {self.addon.name}'
self._test_owner_takedown_email(subject, self.disable_snippet)
assert f'Your Extension {self.addon.name}' in mail.outbox[-1].body
assert f'Your extension {self.addon.name}' in mail.outbox[-1].body

def test_target_appeal_decline(self):
self.addon.update(status=amo.STATUS_REJECTED)
Expand Down
4 changes: 2 additions & 2 deletions src/olympia/abuse/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1411,7 +1411,7 @@ def test_create_and_execute_decision_triggers_emails_when_disable_confirmed(self
process_mock.assert_called()
assert len(mail.outbox) == 1
assert mail.outbox[0].to == [author.email]
assert 'will not reinstate your Extension' in mail.outbox[0].body
assert 'will not reinstate your extension' in mail.outbox[0].body

def test_create_and_execute_decision_triggers_emails_when_disable_reverted(self):
data = self.get_data(filename='target_appeal_change_to_approve.json')
Expand All @@ -1437,7 +1437,7 @@ def test_create_and_execute_decision_triggers_emails_when_disable_reverted(self)
process_mock.assert_called()
assert len(mail.outbox) == 1
assert mail.outbox[0].to == [author.email]
assert 'we have restored your Extension' in mail.outbox[0].body
assert 'we have restored your extension' in mail.outbox[0].body

def test_create_and_execute_decision_triggers_emails_for_reporter_appeal_disable(
self,
Expand Down
6 changes: 3 additions & 3 deletions src/olympia/reviewers/tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -1746,7 +1746,7 @@ def test_reject_versions_with_resolved_cinder_job(self):
# We notify the addon developer (only) while resolving abuse reports
assert len(mail.outbox) == 1
assert 'Some cômments' in mail.outbox[0].body
assert 'your Extension have been disabled'
assert 'your extension have been disabled'
assert 'right to appeal' in mail.outbox[0].body

def test_reject_versions_with_resolved_cinder_job_no_third_party(self):
Expand Down Expand Up @@ -1788,7 +1788,7 @@ def test_reject_versions_with_resolved_cinder_job_no_third_party(self):
# We notify the addon developer while resolving cinder jobs
assert len(mail.outbox) == 1
assert 'Some cômments' in mail.outbox[0].body
assert 'your Extension have been disabled' in mail.outbox[0].body
assert 'your extension have been disabled' in mail.outbox[0].body
assert 'in an assessment performed on our own initiative' in mail.outbox[0].body

def test_reject_versions_with_multiple_delayed_rejections(self):
Expand Down Expand Up @@ -1850,7 +1850,7 @@ def test_reject_versions_with_multiple_delayed_rejections(self):
# Only the latest comment should be used.
assert 'Some cômments' in mail.outbox[0].body
assert 'Some old cômments' not in mail.outbox[0].body
assert 'your Extension have been disabled' in mail.outbox[0].body
assert 'your extension have been disabled' in mail.outbox[0].body
assert 'in an assessment performed on our own initiative' in mail.outbox[0].body

def test_reject_versions_different_user(self):
Expand Down
22 changes: 11 additions & 11 deletions src/olympia/reviewers/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2780,7 +2780,7 @@ def test_reject_multiple_versions(self):
self._test_reject_multiple_versions({})
message = mail.outbox[0]
self.check_subject(message)
assert 'versions of your Extension have been disabled' in message.body
assert 'versions of your extension have been disabled' in message.body
assert 'received from a third party' not in message.body

def test_reject_multiple_versions_non_human(self):
Expand All @@ -2801,8 +2801,8 @@ def test_reject_multiple_versions_resolving_abuse_report(self):
self._test_reject_multiple_versions({'cinder_jobs_to_resolve': [cinder_job]})
message = mail.outbox[0]
self.check_subject(message)
assert 'Extension Delicious Bookmarks was manually reviewed' in message.body
assert 'those versions of your Extension have been disabled' in message.body
assert 'extension Delicious Bookmarks was manually reviewed' in message.body
assert 'those versions of your extension have been disabled' in message.body
assert 'received from a third party' in message.body
assert not NeedsHumanReview.objects.filter(is_active=True).exists()

Expand Down Expand Up @@ -2883,7 +2883,7 @@ def test_reject_multiple_versions_with_delay(self):
self._test_reject_multiple_versions_with_delay({})
message = mail.outbox[0]
self.check_subject(message)
assert 'Your Extension Delicious Bookmarks was manually' in message.body
assert 'Your extension Delicious Bookmarks was manually' in message.body
assert 'will be disabled' in message.body

def test_reject_multiple_versions_with_delay_resolving_abuse_reports(self):
Expand All @@ -2903,7 +2903,7 @@ def test_reject_multiple_versions_with_delay_resolving_abuse_reports(self):
)
message = mail.outbox[0]
self.check_subject(message)
assert 'Your Extension Delicious Bookmarks was manually' in message.body
assert 'Your extension Delicious Bookmarks was manually' in message.body
assert 'will be disabled' in message.body
log = (
ActivityLog.objects.for_addons(self.addon)
Expand Down Expand Up @@ -2955,8 +2955,8 @@ def test_reject_multiple_versions_except_latest(self):
message = mail.outbox[0]
assert message.to == [self.addon.authors.all()[0].email]
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 'Your extension Delicious Bookmarks was manually' in message.body
assert 'versions of your extension have been disabled' 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 @@ -3021,7 +3021,7 @@ def test_reject_multiple_versions_content_review(self):
message = mail.outbox[0]
assert message.to == [self.addon.authors.all()[0].email]
self.check_subject(message)
assert 'Your Extension Delicious Bookmarks was manually' in message.body
assert 'Your extension Delicious Bookmarks was manually' in message.body
assert 'have been disabled' in message.body
log_token = ActivityLogToken.objects.get()
assert log_token.uuid.hex in message.reply_to[0]
Expand Down Expand Up @@ -3077,7 +3077,7 @@ def test_reject_multiple_versions_content_review_with_delay(self):
message = mail.outbox[0]
assert message.to == [self.addon.authors.all()[0].email]
self.check_subject(message)
assert 'Your Extension Delicious Bookmarks was manually' in message.body
assert 'Your extension Delicious Bookmarks was manually' in message.body
assert 'will be disabled' in message.body
log_token = ActivityLogToken.objects.get()
assert log_token.uuid.hex in message.reply_to[0]
Expand Down Expand Up @@ -3345,7 +3345,7 @@ def test_reject_multiple_versions_unlisted(self):
message = mail.outbox[0]
assert message.to == [self.addon.authors.all()[0].email]
self.check_subject(message)
assert 'versions of your Extension have been disabled' in message.body
assert 'versions of your extension have been disabled' in message.body
log_token = ActivityLogToken.objects.get()
assert log_token.uuid.hex in message.reply_to[0]

Expand Down Expand Up @@ -3574,7 +3574,7 @@ def test_approve_rejected_listing_content_review(self):
assert len(mail.outbox) == 1
message = mail.outbox[0]
assert message.to == [self.addon.authors.all()[0].email]
assert 'have restored your Extension' in message.body
assert 'have restored your extension' in message.body

@override_switch('enable-content-rejection', active=True)
def test_reject_listing_content_review(self):
Expand Down