launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07386
[Merge] lp:~stevenk/launchpad/bugdelta-information_type into lp:launchpad
Steve Kowalik has proposed merging lp:~stevenk/launchpad/bugdelta-information_type into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #933766 in Launchpad itself: "Update bug to use information_visibility_policy"
https://bugs.launchpad.net/launchpad/+bug/933766
For more details, see:
https://code.launchpad.net/~stevenk/launchpad/bugdelta-information_type/+merge/103802
Add support for changing information_type to BugDelta and BugChange.
I have left BugVisibilityChange and BugSecurityChange alone (and actually added the latter to __all__), and left the current tests alone, and added a few tests for the new behaviour.
I have gotten annoyed and ripped out the Bug{Visibility,Security}Change tests from bug-change.txt, since as far as I can tell it is duplicated between test_bugchange.py and bug-change.txt. I'm happy to completly rip out bug-change.txt in this branch if asked.
--
https://code.launchpad.net/~stevenk/launchpad/bugdelta-information_type/+merge/103802
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/bugdelta-information_type into lp:launchpad.
=== modified file 'lib/lp/bugs/adapters/bugchange.py'
--- lib/lp/bugs/adapters/bugchange.py 2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/adapters/bugchange.py 2012-04-27 06:02:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Implementations for bug changes."""
@@ -23,6 +23,8 @@
'BugConvertedToQuestion',
'BugDescriptionChange',
'BugDuplicateChange',
+ 'BugInformationTypeChange',
+ 'BugSecurityChange',
'BugTagsChange',
'BugTaskAdded',
'BugTaskAssigneeChange',
@@ -56,7 +58,9 @@
RESOLVED_BUGTASK_STATUSES,
UNRESOLVED_BUGTASK_STATUSES,
)
+from lp.registry.enums import InformationType
from lp.registry.interfaces.product import IProduct
+from lp.services.features import getFeatureFlag
from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.webapp.publisher import canonical_url
@@ -102,10 +106,13 @@
# The order of the field names in this list is important; this is
# the order in which changes will appear both in the bug activity
# log and in notification emails.
- bug_change_field_names = [
- 'duplicateof', 'title', 'description', 'private', 'security_related',
- 'tags', 'attachment',
- ]
+ bug_change_field_names = ['duplicateof', 'title', 'description']
+ if bool(getFeatureFlag(
+ 'disclosure.show_information_type_in_ui.enabled')):
+ bug_change_field_names.append('information_type')
+ else:
+ bug_change_field_names.extend(('private', 'security_related'))
+ bug_change_field_names.extend(('tags', 'attachment'))
for field_name in bug_change_field_names:
field_delta = getattr(bug_delta, field_name)
if field_delta is not None:
@@ -540,6 +547,32 @@
return {'text': notification_text}
+class BugInformationTypeChange(AttributeChange):
+ """Used to represent a change to the information_type of an `IBug`."""
+
+ def title(self, value):
+ # This function is unnecessary when display_userdata_as_private is
+ # removed.
+ show_userdata_as_private = bool(getFeatureFlag(
+ 'disclosure.display_userdata_as_private.enabled'))
+ title = value.title
+ if value == InformationType.USERDATA and show_userdata_as_private:
+ title = 'Private'
+ return title
+
+ def getBugActivity(self):
+ return {
+ 'newvalue': self.title(self.new_value),
+ 'oldvalue': self.title(self.old_value),
+ 'whatchanged': 'information type'
+ }
+
+ def getBugNotification(self):
+ return {
+ 'text': "** Information type changed from %s to %s" % (
+ self.title(self.old_value), self.title(self.new_value))}
+
+
class BugVisibilityChange(AttributeChange):
"""Describes a change to a bug's visibility."""
@@ -918,6 +951,7 @@
'description': BugDescriptionChange,
'private': BugVisibilityChange,
'security_related': BugSecurityChange,
+ 'information_type': BugInformationTypeChange,
'tags': BugTagsChange,
'title': BugTitleChange,
'attachment': BugAttachmentChange,
=== modified file 'lib/lp/bugs/adapters/bugdelta.py'
--- lib/lp/bugs/adapters/bugdelta.py 2011-12-22 17:24:23 +0000
+++ lib/lp/bugs/adapters/bugdelta.py 2012-04-27 06:02:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Components related to bugs."""
@@ -16,9 +16,9 @@
def __init__(self, bug, bugurl, user,
title=None, description=None, name=None,
- private=None, security_related=None, duplicateof=None,
- external_reference=None, bugwatch=None, cve=None,
- attachment=None, tags=None,
+ private=None, security_related=None, information_type=None,
+ duplicateof=None, external_reference=None, bugwatch=None,
+ cve=None, attachment=None, tags=None,
added_bugtasks=None, bugtask_deltas=None,
bug_before_modification=None):
self.bug = bug
@@ -30,6 +30,7 @@
self.name = name
self.private = private
self.security_related = security_related
+ self.information_type = information_type
self.duplicateof = duplicateof
self.external_reference = external_reference
self.bugwatch = bugwatch
=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py 2012-04-18 03:56:54 +0000
+++ lib/lp/bugs/browser/bugtask.py 2012-04-27 06:02:29 +0000
@@ -789,7 +789,7 @@
else:
activity = self.context.bug.activity
bug_change_re = (
- 'affects|description|security vulnerability|'
+ 'affects|description|security vulnerability|information type|'
'summary|tags|visibility|bug task deleted')
bugtask_change_re = (
'[a-z0-9][a-z0-9\+\.\-]+( \([A-Za-z0-9\s]+\))?: '
=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py 2012-04-19 20:56:13 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py 2012-04-27 06:02:29 +0000
@@ -538,3 +538,25 @@
view = create_initialized_view(
bug.default_bugtask, '+addcomment', form=form)
self.assertEqual(0, len(view.errors))
+
+
+class TestBugTaskInterestingActivity(BrowserTestCase):
+ """Tests for which activities are interesting enough to show on
+ Bug:+index."""
+
+ layer = LaunchpadFunctionalLayer
+
+ def test_information_type_is_interesting(self):
+ # information_type changes are interesting enough for Bug:+index.
+ bug = self.factory.makeBug()
+ feature_flag = {
+ 'disclosure.show_information_type_in_ui.enabled': 'on'}
+ with FeatureFixture(feature_flag):
+ browser = self.getViewBrowser(
+ bug.default_bugtask, rootsite="bugs", view_name='+secrecy')
+ browser.getControl("User Data").selected = True
+ browser.getControl('Change').click()
+ browser = self.getViewBrowser(bug, rootsite="bugs")
+ soup = BeautifulSoup(browser.contents)
+ information_type = soup.find(text='information type')
+ self.assertIsNot(None, information_type)
=== modified file 'lib/lp/bugs/doc/bug-change.txt'
--- lib/lp/bugs/doc/bug-change.txt 2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/doc/bug-change.txt 2012-04-27 06:02:29 +0000
@@ -311,51 +311,6 @@
{'newvalue': '...',
'whatchanged': 'marked as duplicate'}
-
-=== BugVisibilityChange ===
-
-BugVisibilityChange is used to represent a change in a Bug's `private`
-attribute.
-
- >>> from lp.bugs.adapters.bugchange import (
- ... BugVisibilityChange)
-
- >>> bug_visibility_change = BugVisibilityChange(
- ... when=nowish, person=example_person,
- ... what_changed='private', old_value=example_bug.private,
- ... new_value=True)
-
-IBug.private is a boolean but to make it more readable we express it in
-activity and notification records as a string, where True = 'Private'
-and False = 'Public'. We also refer to it as "visibility" rather than
-privacy.
-
- >>> print pretty(bug_visibility_change.getBugActivity())
- {'newvalue': 'private',
- 'oldvalue': 'public',
- 'whatchanged': 'visibility'}
-
-We also use the 'Private', 'Public' and 'Visibility' terms in the
-notification text.
-
- >>> print bug_visibility_change.getBugNotification()['text']
- ** Visibility changed to: Private
-
-If we reverse the changes we'll see the opposite values in the
-notification and activity entries.
-
- >>> bug_visibility_change = BugVisibilityChange(
- ... when=nowish, person=example_person,
- ... what_changed='private', old_value=True, new_value=False)
- >>> print pretty(bug_visibility_change.getBugActivity())
- {'newvalue': 'public',
- 'oldvalue': 'private',
- 'whatchanged': 'visibility'}
-
- >>> print bug_visibility_change.getBugNotification()['text']
- ** Visibility changed to: Public
-
-
== BugTagsChange ==
BugTagsChange is used to represent a change in a Bug's tag list.
@@ -385,49 +340,6 @@
** Tags added: zillionth-tag
-=== BugSecurityChange ===
-
-BugSecurityChange is used to represent a change in a Bug's
-`security_related` attribute.
-
- >>> from lp.bugs.adapters.bugchange import (
- ... BugSecurityChange)
-
- >>> bug_security_change = BugSecurityChange(
- ... when=nowish, person=example_person,
- ... what_changed='security_related',
- ... old_value=False, new_value=True)
-
-IBug.security_related is a boolean but to make it more readable we
-express it in activity and notification records as a short phrase.
-
-Marking a bug as security related causes one set of terms/phrases to
-be used.
-
- >>> print pretty(bug_security_change.getBugActivity())
- {'newvalue': 'yes',
- 'oldvalue': 'no',
- 'whatchanged': 'security vulnerability'}
-
- >>> print bug_security_change.getBugNotification()['text']
- ** This bug has been flagged as a security vulnerability
-
-Going the other way the phrases are similar.
-
- >>> bug_security_change = BugSecurityChange(
- ... when=nowish, person=example_person,
- ... what_changed='security_related',
- ... old_value=True, new_value=False)
-
- >>> print pretty(bug_security_change.getBugActivity())
- {'newvalue': 'no',
- 'oldvalue': 'yes',
- 'whatchanged': 'security vulnerability'}
-
- >>> print bug_security_change.getBugNotification()['text']
- ** This bug is no longer flagged as a security vulnerability
-
-
=== CveLinkedToBug / CveUnlinkedFromBug ===
These describe the linking or unlinking of a CVE to a bug.
=== modified file 'lib/lp/bugs/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py 2012-04-18 05:52:41 +0000
+++ lib/lp/bugs/interfaces/bug.py 2012-04-27 06:02:29 +0000
@@ -1075,18 +1075,20 @@
bugurl = Attribute("The absolute URL to the bug.")
user = Attribute("The IPerson that did the editing.")
- # fields on the bug itself
+ # Fields on the bug itself.
title = Attribute("A dict with two keys, 'old' and 'new', or None.")
description = Attribute("A dict with two keys, 'old' and 'new', or None.")
private = Attribute("A dict with two keys, 'old' and 'new', or None.")
security_related = Attribute(
"A dict with two keys, 'old' and 'new', or None.")
+ information_type = Attribute(
+ "A dict with two keys, 'old' and 'new', or None.")
name = Attribute("A dict with two keys, 'old' and 'new', or None.")
duplicateof = Attribute(
"A dict with two keys, 'old' and 'new', or None. Key values are "
"IBug's")
- # other things linked to the bug
+ # Other things linked to the bug.
bugwatch = Attribute(
"A dict with two keys, 'old' and 'new', or None. Key values are "
"IBugWatch's.")
=== modified file 'lib/lp/bugs/subscribers/bug.py'
--- lib/lp/bugs/subscribers/bug.py 2012-03-13 00:45:33 +0000
+++ lib/lp/bugs/subscribers/bug.py 2012-04-27 06:02:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009, 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -32,6 +32,7 @@
from lp.registry.interfaces.product import IProduct
from lp.services.config import config
from lp.services.database.sqlbase import block_implicit_flushes
+from lp.services.features import getFeatureFlag
from lp.services.mail.helpers import get_contact_email_addresses
from lp.services.mail.sendmail import (
format_address,
@@ -115,9 +116,14 @@
IBugDelta if there are changes, or None if there were no changes.
"""
changes = {}
-
- for field_name in ("title", "description", "name", "private",
- "security_related", "duplicateof", "tags"):
+ fields = ["title", "description", "name"]
+ if bool(getFeatureFlag(
+ 'disclosure.show_information_type_in_ui.enabled')):
+ fields.append('information_type')
+ else:
+ fields.extend(('private', 'security_related'))
+ fields.extend(("duplicateof", "tags"))
+ for field_name in fields:
# fields for which we show old => new when their values change
old_val = getattr(old_bug, field_name)
new_val = getattr(new_bug, field_name)
=== modified file 'lib/lp/bugs/tests/test_bugchanges.py'
--- lib/lp/bugs/tests/test_bugchanges.py 2012-04-19 21:15:25 +0000
+++ lib/lp/bugs/tests/test_bugchanges.py 2012-04-27 06:02:29 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for recording changes done to a bug."""
@@ -8,7 +8,10 @@
ObjectModifiedEvent,
)
from lazr.lifecycle.snapshot import Snapshot
-from testtools.matchers import StartsWith
+from testtools.matchers import (
+ MatchesStructure,
+ StartsWith,
+ )
from zope.component import getUtility
from zope.event import notify
from zope.interface import providedBy
@@ -21,6 +24,8 @@
from lp.bugs.interfaces.cve import ICveSet
from lp.bugs.model.bugnotification import BugNotification
from lp.bugs.scripts.bugnotification import construct_email_notifications
+from lp.registry.enums import InformationType
+from lp.services.features.testing import FeatureFixture
from lp.services.librarian.browser import ProxiedLibraryFileAlias
from lp.services.webapp.interfaces import ILaunchBag
from lp.services.webapp.publisher import canonical_url
@@ -139,19 +144,12 @@
self.assertEqual(len(expected_activities), len(new_activities))
for expected_activity in expected_activities:
added_activity = new_activities.pop(0)
- self.assertEqual(
- expected_activity['person'], added_activity.person)
- self.assertEqual(
- expected_activity['whatchanged'],
- added_activity.whatchanged)
- self.assertEqual(
- expected_activity.get('oldvalue'),
- added_activity.oldvalue)
- self.assertEqual(
- expected_activity.get('newvalue'),
- added_activity.newvalue)
- self.assertEqual(
- expected_activity.get('message'), added_activity.message)
+ self.assertThat(added_activity, MatchesStructure.byEquality(
+ person=expected_activity['person'],
+ whatchanged=expected_activity['whatchanged'],
+ oldvalue=expected_activity.get('oldvalue'),
+ newvalue=expected_activity.get('newvalue'),
+ message=expected_activity.get('message')))
if expected_notification is None:
self.assertEqual(0, len(new_notifications))
@@ -607,6 +605,61 @@
expected_notification=visibility_change_notification,
bug=private_bug)
+ def test_change_information_type(self):
+ # Changing the information type of a bug adds items to the activity
+ # log and notifications.
+ bug = self.factory.makeBug()
+ self.saveOldChanges(bug=bug)
+ feature_flag = {
+ 'disclosure.show_information_type_in_ui.enabled': 'on'}
+ with FeatureFixture(feature_flag):
+ bug.transitionToInformationType(
+ InformationType.EMBARGOEDSECURITY, self.user)
+
+ information_type_change_activity = {
+ 'person': self.user,
+ 'whatchanged': 'information type',
+ 'oldvalue': 'Public',
+ 'newvalue': 'Embargoed Security',
+ }
+ information_type_change_notification = {
+ 'text': '** Information type changed from Public to Embargoed '
+ 'Security',
+ 'person': self.user,
+ }
+ self.assertRecordedChange(
+ expected_activity=information_type_change_activity,
+ expected_notification=information_type_change_notification,
+ bug=bug)
+
+ def test_change_information_type_userdata_as_private(self):
+ # Changing the information type of a bug to User Data with the
+ # display_userdata_as_private flag enabled adds the change as
+ # 'Private' to the activity log and notifications.
+ bug = self.factory.makeBug()
+ self.saveOldChanges(bug=bug)
+ feature_flags = {
+ 'disclosure.show_information_type_in_ui.enabled': 'on',
+ 'disclosure.display_userdata_as_private.enabled': 'on'}
+ with FeatureFixture(feature_flags):
+ bug.transitionToInformationType(
+ InformationType.USERDATA, self.user)
+
+ information_type_change_activity = {
+ 'person': self.user,
+ 'whatchanged': 'information type',
+ 'oldvalue': 'Public',
+ 'newvalue': 'Private',
+ }
+ information_type_change_notification = {
+ 'text': '** Information type changed from Public to Private',
+ 'person': self.user,
+ }
+ self.assertRecordedChange(
+ expected_activity=information_type_change_activity,
+ expected_notification=information_type_change_notification,
+ bug=bug)
+
def test_tags_added(self):
# Adding tags to a bug will add BugActivity and BugNotification
# entries.
Follow ups
