Skip to content

Commit 5f2c2cc

Browse files
authored
Expose new current_version after rejection in stakeholder email (#23534)
1 parent f26965d commit 5f2c2cc

File tree

3 files changed

+93
-27
lines changed

3 files changed

+93
-27
lines changed

src/olympia/abuse/actions.py

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -405,30 +405,41 @@ def notify_stakeholders(self, rejection_type):
405405
template = loader.get_template(self.stakeholder_template_path)
406406
versions = list(
407407
self.target_versions.order_by('id').values_list(
408-
'version', 'channel', named=True
408+
'id', 'version', 'channel', named=True
409409
)
410410
)
411-
if any(ver.channel == amo.CHANNEL_LISTED for ver in versions):
412-
review_urls = absolutify(
413-
reverse('reviewers.review', args=['listed', self.target.id])
414-
)
415-
else:
416-
review_urls = ''
417-
if any(ver.channel == amo.CHANNEL_UNLISTED for ver in versions):
418-
if review_urls:
419-
review_urls += ' | '
420-
review_urls += absolutify(
421-
reverse('reviewers.review', args=['unlisted', self.target.id])
411+
review_urls = []
412+
for arg, channel in amo.CHANNEL_CHOICES_LOOKUP.items():
413+
if any(ver.channel == channel for ver in versions):
414+
review_urls += [
415+
absolutify(
416+
reverse('reviewers.review', args=[arg, self.target.id])
417+
)
418+
]
419+
new_current_version = (
420+
self.target.versions.filter(
421+
channel=amo.CHANNEL_LISTED, file__status=amo.STATUS_APPROVED
422422
)
423+
.exclude(id__in=(ver.id for ver in versions))
424+
.order_by('created')
425+
.last()
426+
)
423427
context_dict = {
424-
'rejection_type': rejection_type,
425-
'version_list': ', '.join(ver.version for ver in versions),
428+
'new_current_version': new_current_version,
426429
'private_notes': self.decision.private_notes,
427430
'reasoning': self.decision.reasoning,
428-
'review_urls': review_urls,
431+
'rejection_type': rejection_type,
432+
'review_urls': ' | '.join(reversed(review_urls)),
429433
'target_url': self.target.get_absolute_url()
430434
if self.target.get_url_path()
431435
else '',
436+
'type': self.decision.get_target_display(),
437+
'version_list_listed': ', '.join(
438+
vr.version for vr in versions if vr.channel == amo.CHANNEL_LISTED
439+
),
440+
'version_list_unlisted': ', '.join(
441+
vr.version for vr in versions if vr.channel == amo.CHANNEL_UNLISTED
442+
),
432443
}
433444
subject = f'{rejection_type} issued for {self.decision.get_target_name()}'
434445
message = template.render(context_dict)

src/olympia/abuse/templates/abuse/emails/stakeholder_notification.txt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{{ rejection_type }} for versions:
2-
{{ version_list }}
2+
[Listed] {{ version_list_listed }}
3+
[Unlisted] {{ version_list_unlisted }}
34

45
Reasoning:
56
{{ reasoning }}
@@ -9,5 +10,13 @@ Private notes:
910
{{ private_notes }}
1011
{% endif %}
1112

13+
{% if version_list_listed %}
14+
{% if new_current_version %}
15+
{{ new_current_version.version }} will be the new current version of the {{ type }}; first approved {{ new_current_version.file.approval_date|date:"Y-m-d" }}.
16+
{% else %}
17+
The add-on will no longer be publicly viewable on AMO.
18+
{% endif %}
19+
{% endif %}
20+
1221
{{ review_urls }}
1322
{{ target_url }}

src/olympia/abuse/tests/test_actions.py

Lines changed: 57 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,11 +1269,16 @@ def test_execute_action_with_stakeholder_email(self):
12691269
stakeholder
12701270
)
12711271
self.version.file.update(is_signed=True)
1272+
self.another_version.file.update(approval_date=datetime(2025, 2, 3))
12721273
self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED)
12731274
self._test_reject_version(content_review=False, expected_emails_from_action=1)
12741275
assert len(mail.outbox) == 4
12751276
assert mail.outbox[0].recipients() == [stakeholder.email]
12761277
assert mail.outbox[0].subject == f'Rejection issued for {self.addon.name}'
1278+
assert (
1279+
f'{self.another_version.version} will be the new current version of the '
1280+
'Extension; first approved 2025-02-03.' in mail.outbox[0].body
1281+
)
12771282

12781283
def test_execute_action_after_reporter_appeal(self):
12791284
original_job = CinderJob.objects.create(
@@ -1401,6 +1406,7 @@ def test_execute_action_delayed_with_stakeholder_email(self):
14011406
stakeholder
14021407
)
14031408
self.version.file.update(is_signed=True)
1409+
self.another_version.file.update(approval_date=datetime(2025, 2, 3))
14041410
self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED)
14051411
self._test_reject_version_delayed(
14061412
content_review=False, expected_emails_from_action=1
@@ -1411,6 +1417,10 @@ def test_execute_action_delayed_with_stakeholder_email(self):
14111417
mail.outbox[0].subject
14121418
== f'14 day delayed rejection issued for {self.addon.name}'
14131419
)
1420+
assert (
1421+
f'{self.another_version.version} will be the new current version of the '
1422+
'Extension; first approved 2025-02-03.' in mail.outbox[0].body
1423+
)
14141424

14151425
def test_execute_action_delayed_after_reporter_appeal(self):
14161426
original_job = CinderJob.objects.create(
@@ -1527,6 +1537,7 @@ def test_notify_stakeholders(self):
15271537
stakeholder
15281538
)
15291539
self.decision.update(private_notes='', reasoning='Bad things!')
1540+
self.another_version.file.update(approval_date=datetime(2025, 1, 2))
15301541

15311542
# the addon is not promoted
15321543
assert self.addon.publicly_promoted_groups == []
@@ -1537,33 +1548,68 @@ def test_notify_stakeholders(self):
15371548
self.make_addon_promoted(self.addon, PROMOTED_GROUP_CHOICES.RECOMMENDED)
15381549
action.notify_stakeholders('teh reason')
15391550
assert len(mail.outbox) == 1
1551+
body = mail.outbox[0].body
15401552
assert mail.outbox[0].recipients() == [stakeholder.email]
15411553
assert mail.outbox[0].subject == f'teh reason issued for {self.addon.name}'
1542-
assert 'Bad things!' in mail.outbox[0].body
1543-
assert 'Private notes:' not in mail.outbox[0].body
1554+
assert 'Bad things!' in body
1555+
assert 'Private notes:' not in body
15441556
assert (
1545-
f'teh reason for versions:\n{listed_version.version}' in mail.outbox[0].body
1557+
f'teh reason for versions:\n'
1558+
f'[Listed] {listed_version.version}\n'
1559+
'[Unlisted] \n' in body
1560+
)
1561+
assert f'/review-listed/{self.addon.id}' in body
1562+
assert f'/review-unlisted/{self.addon.id}' not in body
1563+
assert self.addon.get_absolute_url() in body
1564+
1565+
assert (
1566+
f'{self.another_version.version} will be the new current version of the '
1567+
'Extension; first approved 2025-01-02' in body
15461568
)
1547-
assert f'/review-listed/{self.addon.id}' in mail.outbox[0].body
1548-
assert f'/review-unlisted/{self.addon.id}' not in mail.outbox[0].body
1549-
assert self.addon.get_absolute_url() in mail.outbox[0].body
15501569

15511570
# an unlisted version should result in second link to the unlisted review page
15521571
self.decision.target_versions.add(unlisted_version)
15531572
action.notify_stakeholders('teh reason')
15541573
assert len(mail.outbox) == 2 # another email
1574+
body = mail.outbox[1].body
1575+
assert (
1576+
'teh reason for versions:\n'
1577+
f'[Listed] {listed_version.version}\n'
1578+
f'[Unlisted] {unlisted_version.version}\n'
1579+
) in body
1580+
assert f'/review-listed/{self.addon.id} | ' in body
1581+
assert f'/review-unlisted/{self.addon.id}' in body
1582+
assert (
1583+
f'{self.another_version.version} will be the new current version of the '
1584+
'Extension; first approved 2025-01-02.' in body
1585+
)
1586+
1587+
# if the listed version(s) affected are the last approved versions indicate that
1588+
self.another_version.file.update(status=amo.STATUS_DISABLED)
1589+
self.old_version.file.update(status=amo.STATUS_AWAITING_REVIEW)
1590+
action.notify_stakeholders('teh reason')
1591+
assert len(mail.outbox) == 3 # another email
1592+
body = mail.outbox[2].body
15551593
assert (
15561594
'teh reason for versions:\n'
1557-
f'{listed_version.version}, {unlisted_version.version}'
1558-
) in mail.outbox[1].body
1559-
assert f'/review-listed/{self.addon.id} | ' in mail.outbox[1].body
1560-
assert f'/review-unlisted/{self.addon.id}' in mail.outbox[1].body
1595+
f'[Listed] {listed_version.version}\n'
1596+
f'[Unlisted] {unlisted_version.version}\n'
1597+
) in body
1598+
assert 'The add-on will no longer be publicly viewable on AMO.' in body
1599+
1600+
# if no listed versions are affected we don't mention about the current version
1601+
self.decision.target_versions.set([unlisted_version])
1602+
action.notify_stakeholders('teh reason')
1603+
assert len(mail.outbox) == 4 # another email
1604+
body = mail.outbox[3].body
1605+
assert 'will be the current' not in body
1606+
assert 'no longer' not in body
15611607

15621608
# And check that if no versions were signed we don't send an email
15631609
listed_version.file.update(is_signed=False)
15641610
unlisted_version.file.update(is_signed=False)
15651611
action.notify_stakeholders('teh reason')
1566-
assert len(mail.outbox) == 2 # still two
1612+
assert len(mail.outbox) == 4
15671613

15681614
def test_notify_stakeholders_with_private_notes(self):
15691615
stakeholder = user_factory()

0 commit comments

Comments
 (0)