always detect if there is an attachment for a decision #23138
Conversation
eviljeff
requested review from
a team and
KevinMind
and removed request for
a team
| def _check_notify_emails(self, decision, log_entry): | ||
| assert len(mail.outbox) == 1 | ||
| assert mail.outbox[0].to == [decision.addon.authors.first().email] | ||
| assert str(log_entry.id) in mail.outbox[0].extra_headers['Message-ID'] | ||
| assert 'days' not in mail.outbox[0].body | ||
| assert 'some review text' in mail.outbox[0].body | ||
| assert 'some policy text' not in mail.outbox[0].body | ||
| AttachmentLog.objects.create( | ||
| activity_log=log_entry, | ||
| file=ContentFile('Pseudo File', name='attachment.txt'), | ||
| ) | ||
| decision.send_notifications() | ||
| assert 'An attachment was provided.' not in mail.outbox[0].body | ||
| assert 'To respond or view the file,' not in mail.outbox[0].body | ||
| assert 'An attachment was provided.' in mail.outbox[1].body | ||
| assert 'To respond or view the file,' in mail.outbox[1].body | ||
|
|
There was a problem hiding this comment.
btw, this is just refactoring because I realised _check_notify_emails was only used in one test, so it was confusing - I initially thought I could change here to test multiple tests. code has been cut&pasted into test_execute_action_with_action_date_already directly.
|
After rejecting multiple versions and proceeding with the action I can confirm the email contains the expected text 
I can confirm the attachment is downloadable in the "review history" section for the versions 
|
There wasn't coverage for it - in the previous PR I added tests for the changes in ContentActionDisableAddon, but this fail was coming from further up the call stack, in ContentDecision. Those tests didn't test attachments with held actions. |
follow-up fix for: mozilla/addons#15412
Description
changes how we detect if there is an attachment for a decision, to correctly include the template snippet about it.
Context
It appears while fixing #15412 I fixed the attachment being available for download in developer hub, but inadvertently stopped the existence of the attachment being exposed in the email.
The more complete fix would be to refactor how
send_notificationsselects the appropriate activity log but to limit the scope of this pr, which is expected to be cherry-picked, I instead changed howhas_attachmentwas determined.Testing
Follow the testing steps from #23121
Checklist
#ISSUENUMat the top of your PR to an existing open issue in the mozilla/addons repository.