launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07380
[Merge] lp:~wallyworld/launchpad/bug-mail-information-type into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/bug-mail-information-type into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #989320 in Launchpad itself: "Bug notification emails need information type header"
https://bugs.launchpad.net/launchpad/+bug/989320
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/bug-mail-information-type/+merge/103793
== Implementation ==
Add X-Launchpad-Bug-Information-Type header to bug notification emails. Retain deprecated X-Launchpad-Bug-Privacy and X-Launchpad-Bug-Security-Related headers for backwards compatibility with scripts, email filters etc.
Simple changes to BugNotificationBuilder.
== Tests ==
Update the relevant doc tests.
Also, a previous branch to fix bug 967115 removed some tests but left unused code in the test module. This obsolete code was removed as a drive by.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/doc/bugnotification-email.txt
lib/lp/bugs/doc/bugnotification-sending.txt
lib/lp/bugs/mail/bugnotificationbuilder.py
lib/lp/bugs/model/tests/test_bug.py
./lib/lp/bugs/doc/bugnotification-email.txt
216: want has trailing whitespace.
./lib/lp/bugs/doc/bugnotification-sending.txt
286: want exceeds 78 characters.
770: source exceeds 78 characters.
--
https://code.launchpad.net/~wallyworld/launchpad/bug-mail-information-type/+merge/103793
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/bug-mail-information-type into lp:launchpad.
=== modified file 'lib/lp/bugs/doc/bugnotification-email.txt'
--- lib/lp/bugs/doc/bugnotification-email.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/bugs/doc/bugnotification-email.txt 2012-04-27 02:36:27 +0000
@@ -36,6 +36,7 @@
... IBugDelta,
... IBugSet,
... )
+ >>> from lp.registry.enums import InformationType
>>> from lp.registry.interfaces.person import IPersonSet
@@ -93,8 +94,8 @@
New security related bugs are sent with a prominent warning:
- >>> changed = bug_four.setSecurityRelated(
- ... True, getUtility(ILaunchBag).user)
+ >>> changed = bug_four.transitionToInformationType(
+ ... InformationType.UNEMBARGOEDSECURITY, getUtility(ILaunchBag).user)
>>> subject, body = generate_bug_add_email(bug_four)
>>> subject
@@ -107,9 +108,10 @@
<BLANKLINE>
...
-But of course, security related bugs are private by default:
+Security related bugs can be made embargoed:
- >>> bug_four.setPrivate(True, getUtility(ILaunchBag).user)
+ >>> bug_four.transitionToInformationType(
+ ... InformationType.EMBARGOEDSECURITY, getUtility(ILaunchBag).user)
True
>>> subject, body = generate_bug_add_email(bug_four)
@@ -224,10 +226,9 @@
>>> login("steve.alexander@xxxxxxxxxxxxxxx")
>>> edited_bug = getUtility(IBugSet).get(6)
- >>> edited_bug.setPrivate(True, getUtility(ILaunchBag).user)
+ >>> edited_bug.transitionToInformationType(
+ ... InformationType.EMBARGOEDSECURITY, getUtility(ILaunchBag).user)
True
- >>> changed = edited_bug.setSecurityRelated(
- ... True, getUtility(ILaunchBag).user)
>>> bug_delta = BugDelta(
... bug=edited_bug,
... bugurl="http://www.example.com/bugs/6";,
@@ -251,10 +252,8 @@
Now we set the bug public, and not security-related and check if the
e-mail sent changed as well.
- >>> edited_bug.setPrivate(False, getUtility(ILaunchBag).user)
- True
- >>> changed = edited_bug.setSecurityRelated(
- ... False, getUtility(ILaunchBag).user)
+ >>> changed = edited_bug.transitionToInformationType(
+ ... InformationType.PUBLIC, getUtility(ILaunchBag).user)
>>> bug_delta = BugDelta(
... bug=edited_bug,
... bugurl="http://www.example.com/bugs/6";,
@@ -532,6 +531,7 @@
Sender: bounces@xxxxxxxxxxxxx
X-Launchpad-Bug: product=firefox; ...; assignee=None;
X-Launchpad-Bug-Tags: bar foo
+ X-Launchpad-Bug-Information-Type: Embargoed Security
X-Launchpad-Bug-Private: yes
X-Launchpad-Bug-Security-Vulnerability: yes
X-Launchpad-Bug-Commenters: name12
=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt 2012-04-19 21:15:25 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt 2012-04-27 02:36:27 +0000
@@ -421,11 +421,6 @@
... title='Zero-day on Frobulator',
... information_type=InformationType.EMBARGOEDSECURITY))
- >>> sec_vuln_bug.security_related
- True
- >>> sec_vuln_bug.private
- True
-
>>> notifications = (
... getUtility(IBugNotificationSet).getNotificationsToSend())
>>> email_notifications = get_email_notifications(notifications)
@@ -730,66 +725,61 @@
... message.get_all('X-Launchpad-Bug-Tags')
-The X-Launchpad-Bug-Private header
-----------------------------------
+The X-Launchpad-Bug-Information-Type header
+-------------------------------------------
When a notification is sent out about a bug, the
-X-Launchpad-Bug-Private header shows if the bug is marked as
-private. It can have the value "yes" or "no".
+X-Launchpad-Bug-Information-Type header shows the information type value
+assigned to the bug. For backwards compatibility, the X-Launchpad-Bug-Private
+and X-Launchpad-Bug-Security-Vulnerability headers are also set. These headers
+can have the value "yes" or "no".
- >>> bug_three.private
- False
+ >>> print bug_three.information_type.title
+ Public
>>> for message in trigger_and_get_email_messages(bug_three):
- ... print message['To'], message.get_all('X-Launchpad-Bug-Private')
- test@xxxxxxxxxxxxx ['no']
+ ... print '%s %s %s %s' % (
+ ... message['To'],
+ ... message.get_all('X-Launchpad-Bug-Private'),
+ ... message.get_all('X-Launchpad-Bug-Security-Vulnerability'),
+ ... message.get_all('X-Launchpad-Bug-Information-Type'))
+ test@xxxxxxxxxxxxx ['no'] ['no'] ['Public']
Predictably, private bugs are sent with a slightly different header:
>>> with lp_dbuser():
- ... bug_three.setPrivate(True, sample_person)
- True
- >>> bug_three.private
- True
-
- >>> for message in trigger_and_get_email_messages(bug_three):
- ... print message['To'], message.get_all('X-Launchpad-Bug-Private')
- foo.bar@xxxxxxxxxxxxx ['yes']
- mark@xxxxxxxxxxx ['yes']
- test@xxxxxxxxxxxxx ['yes']
-
-The X-Launchpad-Bug-Security-Vulnerability header
--------------------------------------------------
-
-When a notification is sent out about a bug, the
-X-Launchpad-Bug-Security-Vulnerability header records if the bug is a
-security vulnerability. It can have the value "yes" or "no".
-
- >>> bug_three.security_related
- False
-
- >>> for message in trigger_and_get_email_messages(bug_three):
- ... print message['To'], (
- ... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
- foo.bar@xxxxxxxxxxxxx ['no']
- mark@xxxxxxxxxxx ['no']
- test@xxxxxxxxxxxxx ['no']
-
-The presence of the security flag on a bug is, surprise, denoted by a
-simple "yes":
+ ... bug_three.transitionToInformationType(
+ ... InformationType.USERDATA, sample_person)
+ True
+ >>> print bug_three.information_type.title
+ User Data
+
+ >>> for message in trigger_and_get_email_messages(bug_three):
+ ... print '%s %s %s %s' % (
+ ... message['To'], message.get_all('X-Launchpad-Bug-Private'),
+ ... message.get_all('X-Launchpad-Bug-Security-Vulnerability'),
+ ... message.get_all('X-Launchpad-Bug-Information-Type'))
+ foo.bar@xxxxxxxxxxxxx ['yes'] ['no'] ['User Data']
+ mark@xxxxxxxxxxx ['yes'] ['no'] ['User Data']
+ test@xxxxxxxxxxxxx ['yes'] ['no'] ['User Data']
+
+Now transition the bug to embargoed security:
>>> with lp_dbuser():
- ... bug_three.setSecurityRelated(True, getUtility(ILaunchBag).user)
- True
- >>> bug_three.security_related
- True
+ ... bug_three.transitionToInformationType(
+ ... InformationType.EMBARGOEDSECURITY, getUtility(ILaunchBag).user)
+ True
+ >>> print bug_three.information_type.title
+ Embargoed Security
>>> for message in trigger_and_get_email_messages(bug_three):
- ... print message['To'], (
- ... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
- foo.bar@xxxxxxxxxxxxx ['yes']
- mark@xxxxxxxxxxx ['yes']
- test@xxxxxxxxxxxxx ['yes']
+ ... print '%s %s %s %s' % (
+ ... message['To'], message.get_all('X-Launchpad-Bug-Private'),
+ ... message.get_all('X-Launchpad-Bug-Security-Vulnerability'),
+ ... message.get_all('X-Launchpad-Bug-Information-Type'))
+ foo.bar@xxxxxxxxxxxxx ['yes'] ['yes'] ['Embargoed Security']
+ mark@xxxxxxxxxxx ['yes'] ['yes'] ['Embargoed Security']
+ test@xxxxxxxxxxxxx ['yes'] ['yes'] ['Embargoed Security']
The X-Launchpad-Bug-Commenters header
=== modified file 'lib/lp/bugs/mail/bugnotificationbuilder.py'
--- lib/lp/bugs/mail/bugnotificationbuilder.py 2012-01-01 02:58:52 +0000
+++ lib/lp/bugs/mail/bugnotificationbuilder.py 2012-04-27 02:36:27 +0000
@@ -18,6 +18,10 @@
from zope.component import getUtility
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.registry.enums import (
+ PRIVATE_INFORMATION_TYPES,
+ SECURITY_INFORMATION_TYPES,
+ )
from lp.services.config import config
from lp.services.helpers import shortlist
from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
@@ -110,19 +114,21 @@
self.common_headers.append(
('X-Launchpad-Bug-Tags', ' '.join(bug.tags)))
- # Add the X-Launchpad-Bug-Private header. This is a simple
- # yes/no value denoting privacy for the bug.
- if bug.private:
+ self.common_headers.append(
+ ('X-Launchpad-Bug-Information-Type',
+ bug.information_type.title))
+
+ # For backwards compatibility, we still include the
+ # X-Launchpad-Bug-Private and X-Launchpad-Bug-Security-Vulnerability
+ # headers.
+ if bug.information_type in PRIVATE_INFORMATION_TYPES:
self.common_headers.append(
('X-Launchpad-Bug-Private', 'yes'))
else:
self.common_headers.append(
('X-Launchpad-Bug-Private', 'no'))
- # Add the X-Launchpad-Bug-Security-Vulnerability header to
- # denote security for this bug. This follows the same form as
- # the -Bug-Private header.
- if bug.security_related:
+ if bug.information_type in SECURITY_INFORMATION_TYPES:
self.common_headers.append(
('X-Launchpad-Bug-Security-Vulnerability', 'yes'))
else:
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2012-04-25 20:12:51 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2012-04-27 02:36:27 +0000
@@ -32,10 +32,8 @@
BugSubscriptionInfo,
)
from lp.bugs.model.bugnotification import BugNotificationRecipient
-from lp.bugs.scripts.bugnotification import get_email_notifications
from lp.registry.enums import InformationType
from lp.registry.interfaces.person import PersonVisibility
-from lp.services.database.lpstorm import IStore
from lp.testing import (
feature_flags,
login_person,
@@ -766,59 +764,6 @@
set((naked_bugtask.pillar.owner, bug_owner, who)),
subscribers)
- def _fetch_notifications(self, bug, reason_header):
- store = IStore(BugNotification)
- return store.find(
- BugNotification,
- BugNotificationRecipient.bug_notificationID == BugNotification.id,
- BugNotificationRecipient.reason_header == reason_header,
- BugNotification.bug == bug,
- BugNotification.status == BugNotificationStatus.PENDING,
- BugNotification.date_emailed == None)
-
- def _check_email_content(self, message, expected_headers,
- expected_body_text):
- # Ensure that the email header values and message body text is as
- # expected.
- headers = {}
- for header in ['X-Launchpad-Message-Rationale',
- 'X-Launchpad-Bug-Security-Vulnerability',
- 'X-Launchpad-Bug-Private']:
- if message[header]:
- headers[header] = message[header]
- self.assertEqual(expected_headers, headers)
- body_text = message.get_payload(decode=True)
- self.assertTrue(expected_body_text in body_text)
-
- def _check_notifications(self, bug, expected_recipients,
- expected_body_text, expected_reason_body,
- is_private, is_security_related, role):
- # Ensure that the content of the pending email notifications is
- # correct.
- notifications = self._fetch_notifications(bug, role)
- actual_recipients = []
- email_notifications = get_email_notifications(notifications)
- for bug_notifications, omitted, messages in email_notifications:
- for message in messages:
- expected_headers = {
- 'X-Launchpad-Bug-Private':
- 'yes' if is_private else 'no',
- 'X-Launchpad-Bug-Security-Vulnerability':
- 'yes' if is_security_related else 'no',
- 'X-Launchpad-Message-Rationale': role,
- }
- self._check_email_content(
- message, expected_headers, expected_body_text)
- expected_reason_header = role
- for notification in bug_notifications:
- for recipient in notification.recipients:
- self.assertEqual(
- expected_reason_header, recipient.reason_header)
- self.assertEqual(
- expected_reason_body, recipient.reason_body)
- actual_recipients.append(recipient.person)
- self.assertContentEqual(expected_recipients, actual_recipients)
-
def test_structural_bug_supervisor_becomes_direct_on_private(self):
# If a bug supervisor has a structural subscription to the bug, and
# the bug is marked as private, the supervisor should get a direct
Follow ups
