Merge lp:~gary/launchpad/refactoractivitylog into lp:launchpad

[Gary Poster]

[r=bac,benji][no-qa] move some code, which lets you look at bug activity log entires more precisely, from browser code to model code. This is a step along the path to addressing bug 164196.

This branch simply moves some code that lets you look at bug activity log entires more precisely from browser code to model code. It is a step along the path to addressing bug 164196.

pre-imp with gmb.

Lint is happy.

This is an incremental branch of internal code-organization changes, and no QA is necessary.

Thank you.

[Benji York (benji)]

This branch looks good. It's a mostly mechanical translation with a
nice addition to documentation/testing. Approved* (*Brad will have to
review my review.)

One small thing I noticed: in lib/lp/bugs/interfaces/bugactivity.py the
description arguments of the two moved attributes (target and attribute)
use different quotes (single vs. double) and indentation (just 4 spaces
for target and fancy indentation for attribute).

review: Approve (code*)

[Gary Poster (gary)]

Thank you.

I adjusted the spacing.

For single and double quotes, I generally prefer single except when I need to use double, and single are generally used in this file. I needed to use double for "attribute" because there is a single quote in the string acting as an apostrophe. I'll leave that as it is.

[Brad Crittenden (bac)]

Good branch and review.

review: Approve (code)

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-02-03 16:35:23 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-02-11 19:21:49 +0000
@@ -3801,48 +3801,10 @@
     """A decorated BugActivity."""
     delegates(IBugActivity, 'activity')
 
-    # The regular expression we use for matching bug task changes.
-    bugtask_change_re = re.compile(
-        '(?P<target>[a-z0-9][a-z0-9\+\.\-]+( \([A-Za-z0-9\s]+\))?): '
-        '(?P<attribute>assignee|importance|milestone|status)')
-
     def __init__(self, activity):
         self.activity = activity
 
     @property
-    def target(self):
-        """Return the target of this BugActivityItem.
-
-        `target` is determined based on the `whatchanged` string of the
-        original BugAcitivity.
-
-        :return: The target name of the item if `whatchanged` is of the
-        form <target_name>: <attribute>. Otherwise, return None.
-        """
-        match = self.bugtask_change_re.match(self.whatchanged)
-        if match is None:
-            return None
-        else:
-            return match.groupdict()['target']
-
-    @property
-    def attribute(self):
-        """Return the attribute changed in this BugActivityItem.
-
-        `attribute` is determined based on the `whatchanged` string of the
-        original BugAcitivity.
-
-        :return: The attribute name of the item if `whatchanged` is of
-            the form <target_name>: <attribute>. Otherwise, return the
-            original `whatchanged` string.
-        """
-        match = self.bugtask_change_re.match(self.whatchanged)
-        if match is None:
-            return self.whatchanged
-        else:
-            return match.groupdict()['attribute']
-
-    @property
     def change_summary(self):
         """Return a formatted summary of the change."""
         return self.attribute

=== modified file 'lib/lp/bugs/doc/bugactivity.txt'
--- lib/lp/bugs/doc/bugactivity.txt	2010-10-19 18:44:31 +0000
+++ lib/lp/bugs/doc/bugactivity.txt	2011-02-11 19:21:49 +0000
@@ -1,8 +1,10 @@
-= Bug Activity =
+Bug Activity
+~~~~~~~~~~~~
 
-Bugs are problems that occur in software. In Malone, various things can be
-added to, edited and removed from a bug over the course of a bug's lifetime. We
-call this stuff bug activity. This document is about bug activity.
+Bugs are problems that occur in software. In Malone, various things can
+be added to, edited and removed from a bug over the course of a bug's
+lifetime. We call this stuff bug activity. This document is about bug
+activity.
 
 Each activity can happen more than once to a bug over the course of its
 lifetime. For auditing reasons, it's useful to track when these happen, why
@@ -20,8 +22,7 @@
 are simple little functions.
 
     >>> from zope.event import notify
-    >>> from lazr.lifecycle.event import (
-    ...     ObjectCreatedEvent, ObjectModifiedEvent)
+    >>> from lazr.lifecycle.event import ObjectModifiedEvent
     >>> from lazr.lifecycle.snapshot import Snapshot
     >>> from lp.bugs.interfaces.bugtask import (
     ...     IBugTaskSet,
@@ -32,7 +33,8 @@
     >>> user = getUtility(ILaunchBag).user
 
 
-== User files a bug ==
+User files a bug
+================
 
     >>> from lp.bugs.interfaces.bug import CreateBugParams
     >>> firefox = getUtility(IProductSet)['firefox']
@@ -50,7 +52,8 @@
     u'added bug'
 
 
-== Bug title edited ==
+Bug title edited
+================
 
     >>> from lp.bugs.interfaces.bug import IBug
     >>> old_state = Snapshot(bug, providing=IBug)
@@ -67,12 +70,14 @@
     u'new bug title'
 
 
-== Source package assignment edited ==
+Source package assignment edited
+================================
 
     >>> from lazr.lifecycle.snapshot import Snapshot
     >>> from lp.bugs.interfaces.bugtask import BugTaskStatus
     >>> from lp.registry.interfaces.distribution import IDistributionSet
-    >>> from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
+    >>> from lp.registry.interfaces.sourcepackagename import (
+    ...     ISourcePackageNameSet)
     >>> mozilla_firefox = getUtility(ISourcePackageNameSet)['mozilla-firefox']
     >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
     >>> source_package_assignment = getUtility(IBugTaskSet).createTask(
@@ -97,8 +102,27 @@
     >>> latest_activity.newvalue == BugTaskStatus.CONFIRMED.title
     True
 
-
-== Upstream product assignment edited ==
+You will notice that the `whatchanged` attribute in this case specifies
+a project and an attribute.  This happens when the change is to a bugtask.
+The activity object object provides a couple of simple attributes to separate
+out these values: `target` and `attribute`.
+
+    >>> latest_activity.target
+    u'mozilla-firefox (Ubuntu)'
+    >>> latest_activity.attribute
+    u'status'
+
+If the activity is not for a bug task, `target` is None, and `attribute` is
+the same as `whatchanged`.  For instance, look at the attributes on the
+previous activity.
+
+    >>> print bug.activity[-2].target
+    None
+    >>> bug.activity[-2].attribute
+    u'summary'
+
+Upstream product assignment edited
+==================================
 
     >>> product_assignment = getUtility(IBugTaskSet).createTask(
     ...   bug=bug, product=getUtility(IProductSet)['thunderbird'], owner=user)
@@ -115,13 +139,18 @@
     >>> latest_activity = bug.activity[-1]
     >>> latest_activity.whatchanged
     u'thunderbird: status'
+    >>> latest_activity.target
+    u'thunderbird'
+    >>> latest_activity.attribute
+    u'status'
     >>> latest_activity.oldvalue == BugTaskStatus.NEW.title
     True
     >>> latest_activity.newvalue == BugTaskStatus.INVALID.title
     True
 
 
-== Bug report is marked as a duplicate of another bug report ==
+Bug report is marked as a duplicate of another bug report
+=========================================================
 
     >>> edit_fields = [
     ...     "id", "title", "description", "name",
@@ -140,7 +169,8 @@
     True
 
 
-== Bug report has its duplicate marker changed to another bug report ==
+Bug report has its duplicate marker changed to another bug report
+=================================================================
 
     >>> edit_fields = [
     ...     "id", "title", "description", "name", "private", "duplicateof",
@@ -159,7 +189,8 @@
     True
 
 
-== The bug report is un-duplicated ==
+The bug report is un-duplicated
+===============================
 
     >>> edit_fields = [
     ...     "id", "title", "description", "name", "private", "duplicateof",
@@ -177,7 +208,8 @@
     True
 
 
-==  A bug with multiple duplicates ==
+A bug with multiple duplicates
+==============================
 
 When a bug has multiple duplicates and is itself marked a duplicate,
 the duplicates are automatically duped to the same master bug.  These changes
@@ -217,7 +249,8 @@
     True
 
 
-== BugActivityItem ==
+BugActivityItem
+===============
 
 BugActivityItem implements the stuff that BugActivity doesn't need to
 know about.
@@ -299,8 +332,8 @@
     added: tag3<br />removed: tag2
 
 For changes to bug tasks, BugActivityItem returns the name of the attribute
-that was changed (see "Determining the target and attribute of a
-change", below).
+that was changed (using the `attribute` property on the bug activity
+discussed above).
 
     >>> activity = getUtility(IBugActivitySet).new(
     ...     bug=bug_one, whatchanged='malone: status', oldvalue='New',
@@ -344,40 +377,3 @@
     >>> print "%s: %s" % (
     ...     activity_item.change_summary, activity_item.change_details)
     description: updated
-
-
-=== Determining the target and attribute of a change ===
-
-Some changes - specifically changes to a BugTask - are listed in
-BugActivity as targeted changes, i.e. their `whatchanged` string will be
-of the form "<target>: <attribute>". BugActivityItem offers two
-properties by which to extract these.
-
-BugActivityItem.target will return the <target> part of the whatchanged
-string or None if the BugActivity is not targeted to a BugTask.
-
-    >>> activity = getUtility(IBugActivitySet).new(
-    ...     bug=bug_one, whatchanged='malone: assignee',
-    ...     oldvalue='nobody', newvalue='New Assignee',
-    ...     person=user, datechanged=nowish)
-    >>> targeted_activity_item = BugActivityItem(activity)
-    >>> print targeted_activity_item.target
-    malone
-
-    >>> activity = getUtility(IBugActivitySet).new(
-    ...     bug=bug_one, whatchanged='description',
-    ...     oldvalue='Old description', newvalue='New description',
-    ...     person=user, datechanged=nowish)
-    >>> untargeted_activity_item = BugActivityItem(activity)
-    >>> print untargeted_activity_item.target
-    None
-
-BugActivityItem.attribute will return the <attribute> part of the
-whatchanged string if the activity is targeted to a BugTask. Otherwise
-it will simply return the whole whatchanged string.
-
-    >>> print targeted_activity_item.attribute
-    assignee
-
-    >>> print untargeted_activity_item.attribute
-    description

=== modified file 'lib/lp/bugs/interfaces/bugactivity.py'
--- lib/lp/bugs/interfaces/bugactivity.py	2010-09-20 19:23:08 +0000
+++ lib/lp/bugs/interfaces/bugactivity.py	2011-02-11 19:21:49 +0000
@@ -54,6 +54,20 @@
                  description=_("The property of the bug that changed."),
                  readonly=True))
 
+    target = TextLine(
+        title=_('Change Target'), required=False, readonly=True,
+        description=_(
+            'The target of what changed, if the change occurred on a '
+            'bugtask.'))
+
+    attribute = TextLine(
+        title=_('Changed Attribute'), required=True, readonly=True,
+        description=_(
+            "The attribute that changed.  If the change occurred on a "
+            "bugtask, this will be the bugtask's attribute; otherwise "
+            "it will be the bug attribute, and the same as 'what "
+            "changed'."))
+
     oldvalue = exported(
         TextLine(title=_('Old Value'),
                  description=_("The value before the change."),

=== modified file 'lib/lp/bugs/model/bugactivity.py'
--- lib/lp/bugs/model/bugactivity.py	2010-08-20 20:31:18 +0000
+++ lib/lp/bugs/model/bugactivity.py	2011-02-11 19:21:49 +0000
@@ -6,6 +6,8 @@
 __metaclass__ = type
 __all__ = ['BugActivity', 'BugActivitySet']
 
+import re
+
 from sqlobject import (
     ForeignKey,
     StringCol,
@@ -39,6 +41,42 @@
     newvalue = StringCol(default=None)
     message = StringCol(default=None)
 
+    # The regular expression we use for matching bug task changes.
+    bugtask_change_re = re.compile(
+        '(?P<target>[a-z0-9][a-z0-9\+\.\-]+( \([A-Za-z0-9\s]+\))?): '
+        '(?P<attribute>assignee|importance|milestone|status)')
+
+    @property
+    def target(self):
+        """Return the target of this BugActivityItem.
+
+        `target` is determined based on the `whatchanged` string.
+
+        :return: The target name of the item if `whatchanged` is of the
+        form <target_name>: <attribute>. Otherwise, return None.
+        """
+        match = self.bugtask_change_re.match(self.whatchanged)
+        if match is None:
+            return None
+        else:
+            return match.groupdict()['target']
+
+    @property
+    def attribute(self):
+        """Return the attribute changed in this BugActivityItem.
+
+        `attribute` is determined based on the `whatchanged` string.
+
+        :return: The attribute name of the item if `whatchanged` is of
+            the form <target_name>: <attribute>. Otherwise, return the
+            original `whatchanged` string.
+        """
+        match = self.bugtask_change_re.match(self.whatchanged)
+        if match is None:
+            return self.whatchanged
+        else:
+            return match.groupdict()['attribute']
+
 
 class BugActivitySet:
     """See IBugActivitySet."""