Merge lp:~jcsackett/launchpad/redundant-message-is-redundant into lp:launchpad

Proposed by j.c.sackett
Status: Merged
Approved by: Brad Crittenden
Approved revision: no longer in the source branch.
Merged at revision: 14448
Proposed branch: lp:~jcsackett/launchpad/redundant-message-is-redundant
Merge into: lp:launchpad
Diff against target: 50 lines (+16/-5)
2 files modified
lib/lp/bugs/browser/bugtask.py (+14/-3)
lib/lp/bugs/stories/bugs/xx-bug-activity.txt (+2/-2)
To merge this branch: bzr merge lp:~jcsackett/launchpad/redundant-message-is-redundant
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+84308@code.launchpad.net

Commit message

[r=bac][bug=898584] Updates messages for deleted bug tasks to be less redundant.

Description of the change

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

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

Nice change Jonathan.

The assignment at line 15 is unnecessary.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/bugs/browser/bugtask.py'
2--- lib/lp/bugs/browser/bugtask.py 2011-12-04 23:02:05 +0000
3+++ lib/lp/bugs/browser/bugtask.py 2011-12-05 16:40:07 +0000
4@@ -4348,11 +4348,22 @@
5 """Return a formatted summary of the change."""
6 if self.target is not None:
7 # This is a bug task. We want the attribute, as filtered out.
8- return self.attribute
9+ summary = self.attribute
10 else:
11 # Otherwise, the attribute is more normalized than what we want.
12 # Use "whatchanged," which sometimes is more descriptive.
13- return self.whatchanged
14+ summary = self.whatchanged
15+ return self.get_better_summary(summary)
16+
17+ def get_better_summary(self, summary):
18+ """For some activities, we want a different summary for the UI.
19+
20+ Some event names are more descriptive as data, but less relevant to
21+ users, who are unfamiliar with the lp code."""
22+ better_summaries = {
23+ 'bug task deleted': 'no longer affects',
24+ }
25+ return better_summaries.get(summary, summary)
26
27 @property
28 def _formatted_tags_change(self):
29@@ -4431,7 +4442,7 @@
30 return_dict[key] = cgi.escape(return_dict[key])
31
32 elif attribute == 'bug task deleted':
33- return 'no longer affects %s' % self.oldvalue
34+ return self.oldvalue
35
36 else:
37 # Our default state is to just return oldvalue and newvalue.
38
39=== modified file 'lib/lp/bugs/stories/bugs/xx-bug-activity.txt'
40--- lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2011-11-28 00:35:15 +0000
41+++ lib/lp/bugs/stories/bugs/xx-bug-activity.txt 2011-12-05 16:40:07 +0000
42@@ -319,6 +319,6 @@
43 >>> print_comments(admin_browser.contents)
44 Foo Bar (name16)
45 ... ago
46- bug task deleted:
47- no longer affects ubuntu
48+ no longer affects:
49+ ubuntu
50 --------