Merge lp://staging/~danilo/launchpad/bug-772754-other-subscribers-loading into lp://staging/launchpad

Proposed by Данило Шеган
Status: Merged
Approved by: Данило Шеган
Approved revision: no longer in the source branch.
Merged at revision: 13243
Proposed branch: lp://staging/~danilo/launchpad/bug-772754-other-subscribers-loading
Merge into: lp://staging/launchpad
Prerequisite: lp://staging/~danilo/launchpad/bug-772754-other-subscribers-activity
Diff against target: 933 lines (+781/-2)
11 files modified
lib/lp/bugs/browser/bugsubscription.py (+54/-1)
lib/lp/bugs/browser/configure.zcml (+6/-0)
lib/lp/bugs/browser/tests/test_bugsubscription_views.py (+147/-0)
lib/lp/bugs/configure.zcml (+1/-0)
lib/lp/bugs/interfaces/bug.py (+9/-0)
lib/lp/bugs/javascript/subscribers_list.js (+162/-0)
lib/lp/bugs/javascript/tests/test_subscribers_list.js (+352/-0)
lib/lp/bugs/model/bug.py (+11/-0)
lib/lp/bugs/model/tests/test_bug.py (+31/-0)
lib/lp/bugs/templates/bug-portlet-subscribers.pt (+1/-0)
lib/lp/bugs/templates/bugtask-index.pt (+7/-1)
To merge this branch: bzr merge lp://staging/~danilo/launchpad/bug-772754-other-subscribers-loading
Reviewer Review Type Date Requested Status
Graham Binns (community) code Approve
Review via email: mp+64186@code.staging.launchpad.net

Description of the change

= Bug 772754: Other subscribers list, part 5 =

Warning: oversized branch. Perhaps should have split it up into server-side and JS branches.

This is part of ongoing work for providing the "other subscribers" list as indicated in mockup https://launchpadlibrarian.net/71552495/all-in-one.png attached to bug 772754 by Gary.

This branch builds on top of the previous branches and provides finally ties in all the SubscribersList JS code with the actual subscribers loading.

To do the loading, method getDirectSubscribersWithDetails is added to provide both subscriber Person records and BugSubscription records in one go (for the sake of bug_notification_level, which is needed to determine which section a subscriber goes to).

For 'Maybe notified' subscribers, we are using existing APIs (getIndirectSubscribers).

Output of these methods is further hooked up into view +bug-portlet-subscriber-details which provides JSON suitable for direct use by the loading JS.

Note how this doesn't really remove any of the existing subscribers loading, so you end up with two subscribers lists after this.

It is comprehensively tested (I believe). An integration test would be nice, but definitely for a later branch.

Considering the branch is over-sized already, I didn't bother looking into model/bug.py lint issues (except that I made sure I introduced no new ones).

Another thing I am unsure about is if I should move BugSubscribersLoader into a separate JS module. SubscribersList can be useful for more than just bug subscriptions (fwiw, it could work well on the MP page like this one).

== Tests ==

lp/bugs/javascript/tests/test_subscribers_list.html

bin/test -cvvt BugPortletSubscribersWithDetailsTests -t test_get_direct_subscribers_with_details

== Demo and Q/A ==

N/A

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/configure.zcml
  lib/lp/bugs/browser/bugsubscription.py
  lib/lp/bugs/browser/configure.zcml
  lib/lp/bugs/browser/tests/test_bugsubscription_views.py
  lib/lp/bugs/interfaces/bug.py
  lib/lp/bugs/javascript/subscribers_list.js
  lib/lp/bugs/javascript/tests/test_subscribers_list.js
  lib/lp/bugs/model/bug.py
  lib/lp/bugs/model/tests/test_bug.py
  lib/lp/bugs/templates/bug-portlet-subscribers.pt
  lib/lp/bugs/templates/bugtask-index.pt

./lib/lp/bugs/model/bug.py
      52: 'SQLRaw' imported but unused
      52: 'Join' imported but unused
      52: 'Exists' imported but unused
     171: 'get_bug_privacy_filter' imported but unused
      52: 'Count' imported but unused
     303: E301 expected 1 blank line, found 0
    2607: E225 missing whitespace around operator
make: *** [lint] Error 7

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.