Merge lp:~dev-nigelj/launchpad/bug837290 into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Graham Binns |
Approved revision: | no longer in the source branch. |
Merged at revision: | 13839 |
Proposed branch: | lp:~dev-nigelj/launchpad/bug837290 |
Merge into: | lp:launchpad |
Diff against target: |
72 lines (+51/-0) 2 files modified
lib/lp/app/javascript/subscribers/subscribers_list.js (+1/-0) lib/lp/app/javascript/subscribers/tests/test_subscribers_list.js (+50/-0) |
To merge this branch: | bzr merge lp:~dev-nigelj/launchpad/bug837290 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | code | Approve | |
Review via email: mp+73369@code.launchpad.net |
Commit message
[r=gmb][bug=837290] SubscribersList() now handles default configuration options properly so as to prevent objects from being undefined.
Description of the change
Summary
Bug 837290 is an edge case, where if the only subscriber to a bug is logged-in and viewing the bug, the message 'No other undefined' appears instead of 'No other subscribers'. Further testing showed this was also the case for the answer tracker system.
Proposed Fix
The issue appears to be that within lib/lp/
My proposed fix is to merge 'config' with the defaults, to both 'this' and 'config' so that if not specified by callers (both bugs & answers don't pass all details and merging is required) of SubscribersLoader() then correct defaults will be passed to SubscribersList() and the sub-function that produces the 'No other subscribers' message.
Pre-implementation notes
I discussed the issue with Graham (gmb) on #launchpad-dev who agreed on IRC that it seems that this way is likely the best way to go.
Implementation details
I implemented the Proposed Fix to lib/lp/
Tests
I have repeated two unit tests to run from within SubscribersLoader() (instead of SubscribersList()) to test that configuration merging works as intended.
Testing against the devel branch, the two tests failed, against my patch, the all tests passed.
Demo & Q/A
On a clean Launchpad setup, I logged in with the default <email address hidden>/test user/pass combination created a new project, configured the bug tracker and created a bug against the new project.
Once the bug is created, the right sidebar displays "No other subscribers" on an un-patched Launchpad, this will say "No other undefined"
Lint
njones:
bzr: ERROR: unknown command "pipes"
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/
lib/lp/
(Not sure why 'bzr pipes' is missing but other than that, looks clean)
Hi Nigel,
Nice branch, thanks for doing the hard work! I'm happy for this to land and will run it through EC2 for you shortly.