launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/redundant-message-is-redundant into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/redundant-message-is-redundant/+merge/84308

Summary
=======
Recently the ability to delete bugtasks was introduced; the bugactivity
message for this event was flagged by mpt as redundant.

This branch updates the message displayed in comment activity for deleted
bugtasks.

Ordinarily the easiest approach would be to change the "whatchanged" value
stored for deleted bug tasks in the DB. However, in this instance there is a
mismatch between the value stored and what we want to show. "bug task deleted"
is the most logical thing to store in the DB, as that *is* the event and is
useful for lp developers. But our UI talks about "affecting" and "no longer
affects". To get around this, the branch introduces a "better_summary" option
for bug changes that have this sort of mismatch.

Preimp
======
None

Implementation
==============
* bugchanges have a new method, better_summary, which takes a summary and
  better summary if available. For now, this is only used for
  'bug task deleted', but can easily be used for other bug activity events.

* the change details return value for 'bug task deleted' is now just the id of
  the deleted task.

Test
====
bin/test -vvct bug-activity
bin/test -vvct bugchanges

QA
==
Find a bug with a deleted bug task (or set one up). The activity logged in
comments should be

{{{ no longer affects:$name-of-project }}}

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/stories/bugs/xx-bug-activity.txt
-- 
https://code.launchpad.net/~jcsackett/launchpad/redundant-message-is-redundant/+merge/84308
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/redundant-message-is-redundant into lp:launchpad.

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-11-28 19:20:17 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-12-02 17:57:28 +0000
@@ -4332,11 +4332,23 @@
         """Return a formatted summary of the change."""
         if self.target is not None:
             # This is a bug task.  We want the attribute, as filtered out.
-            return self.attribute
+            summary = self.attribute
         else:
             # Otherwise, the attribute is more normalized than what we want.
             # Use "whatchanged," which sometimes is more descriptive.
-            return self.whatchanged
+            summary = self.whatchanged
+        better_summary = self.get_better_summary(summary)
+        return better_summary
+
+    def get_better_summary(self, summary):
+        """For some activities, we want a different summary for the UI.
+
+        Some event names are more descriptive as data, but less relevant to
+        users, who are unfamiliar with the lp code."""
+        better_summaries = {
+            'bug task deleted': 'no longer affects',
+            }
+        return better_summaries.get(summary, summary)
 
     @property
     def _formatted_tags_change(self):
@@ -4415,7 +4427,7 @@
                     return_dict[key] = cgi.escape(return_dict[key])
 
         elif attribute == 'bug task deleted':
-            return 'no longer affects %s' % self.oldvalue
+            return self.oldvalue
 
         else:
             # Our default state is to just return oldvalue and newvalue.

=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-activity.txt'
--- lib/lp/bugs/stories/bugs/xx-bug-activity.txt	2011-11-28 00:35:15 +0000
+++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt	2011-12-02 17:57:28 +0000
@@ -319,6 +319,6 @@
     >>> print_comments(admin_browser.contents)
     Foo Bar (name16)
     ... ago
-    bug task deleted:
-    no longer affects ubuntu
+    no longer affects:
+    ubuntu
     --------