Merge lp:~stevenk/launchpad/bugdelta-information_type into lp:launchpad

[Steve Kowalik]

Add support for information_type to BugDelta and BugChange, controlled with a feature flag.

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.

[Ian Booth (wallyworld)]

Looks mighty fine. As discussed on irc, best to add TestBugTaskInterestingActivity as a doc test so everything is kept together and the tests don't become fragmented. Add add XXX for the removal of BugVisibilityChange and BugSecurityChange when the time is right.

I am a little concerned with the bug-change.txt deletions since the visibility and security changes which are deleted don't appear to be tested anywhere else. Adding a ff check to the doc test as is done elsewhere may be more appropriate so that we have full test coverage.

review: Approve

=== 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-30 02:03:28 +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,33 @@
         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))}
+
+
+# XXX: This can be deleted when show_information_type_in_ui is removed.
 class BugVisibilityChange(AttributeChange):
     """Describes a change to a bug's visibility."""
 
@@ -571,6 +605,7 @@
         return {'text': "** Visibility changed to: %s" % visibility_string}
 
 
+# XXX: This can be deleted when show_information_type_in_ui is removed.
 class BugSecurityChange(AttributeChange):
     """Describes a change to a bug's security setting."""
 
@@ -918,6 +953,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-30 02:03:28 +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-30 02:03:28 +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/interfaces/bug.py'
--- lib/lp/bugs/interfaces/bug.py	2012-04-18 05:52:41 +0000
+++ lib/lp/bugs/interfaces/bug.py	2012-04-30 02:03:28 +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/stories/bugs/xx-bug-activity.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-activity.txt	2012-04-04 05:46:26 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt	2012-04-30 02:03:28 +0000
@@ -318,3 +318,21 @@
     no longer affects:
     ubuntu
     --------
+
+Changes to information_type are shown when the feature flag is set.
+
+    >>> from lp.services.features.testing import FeatureFixture
+    >>> feature_flag = {
+    ...     'disclosure.show_information_type_in_ui.enabled': 'on'}
+    >>> with FeatureFixture(feature_flag):
+    ...     admin_browser.open(
+    ...         "http://bugs.launchpad.dev/jokosher/+bug/14/+secrecy")
+    ...     admin_browser.getControl("User Data").selected = True
+    ...     admin_browser.getControl('Change').click()
+    ...     admin_browser.open("http://bugs.launchpad.dev/jokosher/+bug/14")
+    ...     print_comments(admin_browser.contents)
+    Foo Bar (name16)
+    ... ago
+    information type:
+    Embargoed Security => User Data
+    --------

=== 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-30 02:03:28 +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-30 02:03:28 +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.