Merge lp:~danilo/launchpad/bug728370-direct-subs into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Данило Шеган |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12826 |
Proposed branch: | lp:~danilo/launchpad/bug728370-direct-subs |
Merge into: | lp:launchpad |
Prerequisite: | lp:~danilo/launchpad/bug728370-nondirect-subs |
Diff against target: |
899 lines (+779/-13) 6 files modified
lib/lp/bugs/javascript/subscription.js (+212/-9) lib/lp/bugs/javascript/tests/test_subscription.html (+1/-0) lib/lp/bugs/javascript/tests/test_subscription.js (+560/-1) lib/lp/bugs/model/personsubscriptioninfo.py (+2/-1) lib/lp/bugs/model/tests/test_personsubscriptioninfo.py (+1/-0) lib/lp/bugs/templates/bug-subscription-list.pt (+3/-2) |
To merge this branch: | bzr merge lp:~danilo/launchpad/bug728370-direct-subs |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+57464@code.launchpad.net |
Commit message
[r=bac][bug=728370][incr] Provide rendering of descriptions for all non-structural subscriptions on a bug:+subscriptions page.
Description of the change
= Bug 728370: show subscription descriptions =
This is part of work on bug 728370: displaying user-readable descriptions of all the subscriptions a person has either directly or implicitely to a particular bug. Main goal of the page is for people to be able to unsubscribe quickly from bug mail. This branch does not worry about structural subscriptions.
I'll fix lint issues after the review (so diff doesn't grow even further). Diff is slightly over-sized, but majority of the changes are tests (about 2/3 of the diff).
== Proposed fix ==
Continuing the work on the branch this depends on (getting landed), we introduce a direct subscription handler and tests for it. It directly returns the reason (unlike "other" subscriptions which also returned potential variable replacements).
Also, this includes actual Y.Node construction for in-page elements containing these descriptions, but it is ugly on-purpose. Prettyfying should be the next, independent step (branch). Since this is protected by the feature flag, no harm is done by landing this as-is.
In future steps (not part of 728370), we'll also introduce actions for each of these subscriptions. The goal is to get this landed and allow people to independently work on prettyfying and adding suitable actions for unsubscribing.
== Tests ==
lib/lp/
bin/test -cvvt PersonSubscript
== Demo and Q/A ==
Look at:
https:/
(note, add "malone.
To facilitate the demo, add a few subscriptions:
- directly subscribe to bug 1
- subscribe one of your teams you are a member of
- subscribe one of your teams you are an admin of
- make bugs 2 and 3 duplicates of bug 1
- subscribe personally/
duplicates
- make yourself/your team the assignee on one or several bug tasks
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
./lib/lp/
119: E501 line too long (80 characters)
259: E301 expected 1 blank line, found 0
269: E301 expected 1 blank line, found 0
276: E301 expected 1 blank line, found 0
307: E202 whitespace before ')'
315: E202 whitespace before ')'
119: Line exceeds 78 characters.
./lib/lp/
12: 'searchbuilder' imported but unused
29: 'anonymous_
8: 'Store' imported but unused
16: 'BugTaskStatus' imported but unused
10: 'ProxyFactory' imported but unused
9: 'Unauthorized' imported but unused
16: 'BugTaskImportance' imported but unused
29: 'login_person' imported but unused
26: 'BugSubscriptio
13: 'IStore' imported but unused
37: E302 expected 2 blank lines, found 1
Hi Danilo,
Two small issues I found are:
* dedent the line 'one direct personal subscription.');
* for bug.private the situation may be more complicated. the person may be the product owner and not in the bug supervisor team. it is a corner case but one where we should not give false information.
The elaborate substitution framework you've created looks to be very well thought out and tested. I hope that the usage of it justifies the work you've done...but I don't see the larger design here.
I'm very pleased with the level of documentation and the testing you've done is quite thorough.