Launchpad itself

Merge lp://staging/~gary/launchpad/subscriptions_for_bug-1 into lp://staging/launchpad

subscriptions_for_bug-1
Merge into devel

Proposed by Gary Poster on 2011-02-22

Status:	Merged
Approved by:	Gary Poster on 2011-02-22
Approved revision:	no longer in the source branch.
Merged at revision:	12435
Proposed branch:	lp://staging/~gary/launchpad/subscriptions_for_bug-1
Merge into:	lp://staging/launchpad
Diff against target:	423 lines (+224/-36) 5 files modified lib/lp/bugs/browser/bugsubscription.py (+9/-4) lib/lp/bugs/browser/tests/test_bugsubscription_views.py (+17/-3) lib/lp/bugs/interfaces/bugtask.py (+19/-11) lib/lp/bugs/model/bugtask.py (+38/-18) lib/lp/bugs/model/tests/test_bugtask.py (+141/-0)
To merge this branch:	bzr merge lp://staging/~gary/launchpad/subscriptions_for_bug-1
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Leonard Richardson (community)		2011-02-22	Approve on 2011-02-22
Review via email: mp+50788@code.staging.launchpad.net

Commit message

[r=leonardr][no-qa] add an API to get all of the subscriptions for a set of bugtasks.

Description of the change

This branch adds an API to get all of the subscriptions for a set of bugtasks. I had a pre-implementation call with Graham Binns and a mid-implementation call with Danilo Segan.

I extracted a helper function from getStructuralSubscribers because my new method, getAllStructuralSubscriptions, needed some of the same logic. That logic did not appear to be tested well, so I added tests for it in addition to the functionality of the new method.

This method will be used on a view of the subscriptions for a bug. I exposed it there and added a simple test.

I agonized a bit about the location and name of the new method. In particular, the semantics of getStructuralSubscribers uses filters to determine which subscriptions are pertinent, while I wanted this new method to ignore filters. I used the word "All" in the name of the new method ("getAllStructuralSubscriptions") to try and highlight the difference.

I made several changes to placate lint. The whitespace changes for closing brackets seemed questionable to me, but it was simplest to go along with it; the rest of the lint changes were good.

Revision history for this message

Leonard Richardson (leonardr) wrote on 2011-02-22:

This looks good.

That comment starting "Either we have both distroseries..." is a monster sentence. I know you were just cleaning up the formatting, but if you can also reword it, go for it.

I don't understand why you have self-proclaimed demo content in bug-subscription-list.pt. Did you mean to remove that?

review: Approve

Revision history for this message

Gary Poster (gary) wrote on 2011-02-22:

Thank you, Leonard.

I tried to clarify the sentence you mentioned.

I actually intended to add the demo content because that template was added in a previous branch as an empty file, and I thought it would be nice to at least advance it a bit to show how to use the view attribute I added. It's unnecessary though, and maybe confusing, so I removed it.

Gary