Colin Watson has proposed merging ~cjwatson/launchpad:dmarc-for-merges-and-code
into launchpad:master.
Commit message:
Use DMARC-compliant From addresses in code review emails
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1879740 in Launchpad itself: "Make Launchpad Code Review / Merge Review
/ Code Notices DMARC Compliant"
https://bugs.launchpad.net/launchpad/+bug/1879740
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/406528
Most of this work is by Thomas Ward; I just tidied a few things up and fixed
the tests.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~cjwatson/launchpad:dmarc-for-merges-and-code into launchpad:master.
diff --git a/lib/lp/code/doc/branch-merge-proposal-notifications.txt b/lib/lp/code/doc/branch-merge-proposal-notifications.txt
index 70d15f0..e7134c2 100644
--- a/lib/lp/code/doc/branch-merge-proposal-notifications.txt
+++ b/lib/lp/code/doc/branch-merge-proposal-notifications.txt
@@ -129,7 +129,7 @@ An email is sent to subscribers of either branch and the default reviewer.
>>> notification = notifications[0]
>>> print(notification['From'])
- Eric <e...@example.com>
+ Eric <mp+...@code.launchpad.test>
>>> print(notification['Subject'])
[Merge] lp://dev/~person-name... into lp://dev/~person-name...
>>> print(notification['X-Launchpad-Project'])
diff --git a/lib/lp/code/doc/codereviewcomment.txt b/lib/lp/code/doc/codereviewcomment.txt
index 8023afd..263f73e 100644
--- a/lib/lp/code/doc/codereviewcomment.txt
+++ b/lib/lp/code/doc/codereviewcomment.txt
@@ -86,7 +86,7 @@ Now run the pending job to send the email.
>>> notifications = [email for email in notifications if
... email['X-Launchpad-Message-Rationale'] == 'Owner']
>>> print_emails(include_reply_to=True, notifications=notifications)
- From: Sender Person <sen...@example.com>
+ From: Sender Person <mp+...@code.launchpad.test>
To: ...
Reply-To: mp+...@code.launchpad.test
Subject: Please merge
diff --git a/lib/lp/code/mail/branchmergeproposal.py b/lib/lp/code/mail/branchmergeproposal.py
index 9566ae8..d876ad5 100644
--- a/lib/lp/code/mail/branchmergeproposal.py
+++ b/lib/lp/code/mail/branchmergeproposal.py
@@ -8,9 +8,9 @@ __metaclass__ = type
from lp.code.enums import CodeReviewNotificationLevel
from lp.code.mail.branch import BranchMailer
-from lp.services.config import config
from lp.services.mail.basemailer import BaseMailer
from lp.services.mail.sendmail import (
+ format_address,
format_address_for_person,
get_msgid,
)
@@ -52,9 +52,8 @@ class BMPMailer(BranchMailer):
recipients = merge_proposal.getNotificationRecipients(
CodeReviewNotificationLevel.STATUS)
- assert from_user.preferredemail is not None, (
- 'The sender must have an email address.')
- from_address = format_address_for_person(from_user)
+ from_address = format_address(
+ from_user.display_name, merge_proposal.address)
return cls(
'%(proposal_title)s',
@@ -74,11 +73,10 @@ class BMPMailer(BranchMailer):
recipients = merge_proposal.getNotificationRecipients(
CodeReviewNotificationLevel.STATUS)
if from_user is not None:
- assert from_user.preferredemail is not None, (
- 'The sender must have an email address.')
- from_address = format_address_for_person(from_user)
+ from_address = format_address(
+ from_user.display_name, merge_proposal.address)
else:
- from_address = config.canonical.noreply_from_address
+ from_address = merge_proposal.address
return cls(
'%(proposal_title)s',
'branch-merge-proposal-updated.txt', recipients,
@@ -88,7 +86,8 @@ class BMPMailer(BranchMailer):
@classmethod
def forReviewRequest(cls, reason, merge_proposal, from_user):
"""Return a mailer for a request to review a BranchMergeProposal."""
- from_address = format_address_for_person(from_user)
+ from_address = format_address(
+ from_user.display_name, merge_proposal.address)
recipients = {reason.subscriber: reason}
return cls(
'%(proposal_title)s',
diff --git a/lib/lp/code/mail/codereviewcomment.py b/lib/lp/code/mail/codereviewcomment.py
index 0a885d2..f9e882f 100644
--- a/lib/lp/code/mail/codereviewcomment.py
+++ b/lib/lp/code/mail/codereviewcomment.py
@@ -44,7 +44,8 @@ class CodeReviewCommentMailer(BMPMailer):
self.message = code_review_comment.message
from_person = self.message.owner
from_address = format_address(
- from_person.displayname, from_person.preferredemail.email)
+ from_person.display_name,
+ code_review_comment.branch_merge_proposal.address)
merge_proposal = code_review_comment.branch_merge_proposal
BMPMailer.__init__(
self, self.message.subject, None, recipients, merge_proposal,
diff --git a/lib/lp/code/mail/tests/test_branchmergeproposal.py b/lib/lp/code/mail/tests/test_branchmergeproposal.py
index f121fa7..06bc638 100644
--- a/lib/lp/code/mail/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/mail/tests/test_branchmergeproposal.py
@@ -143,7 +143,9 @@ class TestMergeProposalMailing(TestCaseWithFactory):
'Reply-To': bmp.address,
'Message-Id': '<foobar-example-com>'},
ctrl.headers)
- self.assertEqual('Baz Qux <baz....@example.com>', ctrl.from_addr)
+ self.assertEqual(
+ 'Baz Qux <mp+%d@%s>' % (bmp.id, config.launchpad.code_domain),
+ ctrl.from_addr)
reviewer_id = format_address_for_person(reviewer)
self.assertEqual(set([reviewer_id, bmp.address]), set(ctrl.to_addrs))
mailer.sendAll()
@@ -337,7 +339,9 @@ class TestMergeProposalMailing(TestCaseWithFactory):
mailer.message_id = '<foobar-example-com>'
ctrl = mailer.generateEmail('baz.q...@example.com', subscriber)
self.assertEqual('<foobar-example-com>', ctrl.headers['Message-Id'])
- self.assertEqual('Baz Qux <baz....@example.com>', ctrl.from_addr)
+ self.assertEqual(
+ 'Baz Qux <mp+%d@%s>' % (bmp.id, config.launchpad.code_domain),
+ ctrl.from_addr)
bmp.root_message_id = None
pop_notifications()
mailer.sendAll()
@@ -576,7 +580,9 @@ class TestMergeProposalMailing(TestCaseWithFactory):
mailer = BMPMailer.forReviewRequest(
request, request.merge_proposal, requester)
self.assertEqual(
- 'Requester <reques...@example.com>', mailer.from_address)
+ 'Requester <mp+%d@%s>' % (
+ request.merge_proposal.id, config.launchpad.code_domain),
+ mailer.from_address)
self.assertEqual(
request.merge_proposal.preview_diff,
mailer.preview_diff)
diff --git a/lib/lp/code/mail/tests/test_codereviewcomment.py b/lib/lp/code/mail/tests/test_codereviewcomment.py
index 7d4ec13..8c65a49 100644
--- a/lib/lp/code/mail/tests/test_codereviewcomment.py
+++ b/lib/lp/code/mail/tests/test_codereviewcomment.py
@@ -99,8 +99,9 @@ class TestCodeReviewComment(TestCaseWithFactory):
self.assertEqual(
comment.branch_merge_proposal, mailer.merge_proposal)
sender = comment.message.owner
- sender_address = format_address(sender.displayname,
- sender.preferredemail.email)
+ sender_address = format_address(
+ sender.displayname,
+ "mp+%d@%s" % (bmp.id, config.launchpad.code_domain))
self.assertEqual(sender_address, mailer.from_address)
self.assertEqual(comment, mailer.code_review_comment)
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index e8e7205..69d0595 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -2924,8 +2924,7 @@ class TestGitRepositoryDetectMerges(TestCaseWithFactory):
self.assertIn(
"Work in progress => Merged",
notifications[0].get_payload(decode=True).decode("UTF-8"))
- self.assertEqual(
- config.canonical.noreply_from_address, notifications[0]["From"])
+ self.assertEqual(proposal.address, notifications[0]["From"])
recipients = set(msg["x-envelope-to"] for msg in notifications)
expected = set(
[proposal.source_git_repository.registrant.preferredemail.email,
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
