Merge lp:~wallyworld/launchpad/blueprint-subscription-forms into lp:launchpad
Status: | Merged |
---|---|
Approved by: | Ian Booth |
Approved revision: | no longer in the source branch. |
Merged at revision: | 13273 |
Proposed branch: | lp:~wallyworld/launchpad/blueprint-subscription-forms |
Merge into: | lp:launchpad |
Prerequisite: | lp:~wallyworld/launchpad/blueprint-subscriptions-tales-refactor |
Diff against target: |
596 lines (+188/-155) 8 files modified
lib/lp/blueprints/browser/configure.zcml (+8/-2) lib/lp/blueprints/browser/specification.py (+12/-49) lib/lp/blueprints/browser/specificationsubscription.py (+79/-16) lib/lp/blueprints/stories/standalone/subscribing.txt (+60/-19) lib/lp/blueprints/templates/specification-subscriber-row.pt (+2/-2) lib/lp/blueprints/templates/specification-subscription-delete.pt (+18/-0) lib/lp/blueprints/templates/specification-subscription.pt (+2/-64) lib/lp/blueprints/tests/test_webservice.py (+7/-3) |
To merge this branch: | bzr merge lp:~wallyworld/launchpad/blueprint-subscription-forms |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Benji York (community) | code | Approve | |
Review via email: mp+65184@code.launchpad.net |
Commit message
[r=benji][bug=50875] Allow teams to be unsubscribed from blueprints and fix (modernise) form infrastructure.
Description of the change
This branch builds on the blueprint refactoring work in the previous branch to add support for unsubscribing teams from blueprints. It also replaces old blueprint form code with the preferred LaunchpadFormView based infrastructure.
== Implementation ==
Blueprint subscription forms were still implemented using the old form infrastructure which relied on processing html posts in the view initialise() method. The code was modernised to use @action and LaunchpadFormView for managing subscriptions. A few artifacts of the implementation:
- replace two different forms and code used to modify a subscription with a single implementation
- split the "all in one" form used for managing one's own subscriptions into separate add/modify/delete forms
- use the same forms as above to handle subscribing someone else
- consistent use of "Subscribe", "Unsubscribe" etc for the form buttons as opposed to "Subscribe"
- better informational messages - the name of the "someone else" is included in the info message when a different user of subscribed or unsubscribed
The above html changes will make it easier to do the ajax implementation consistent with the rest of lp. To reiterate - this branch does the work to provide the html forms based implementation.
For both the current logged in user, and any allowed teams, unsubscribing is done by clicking the red "remove" icon to the right of the user name. Editing a subscription is done by clicking the icon to the left of the user name.
== Demo ==
http://
The screenshot shows the result of subscribing the Launchpad Administrators team. See the info message and the entry in the portal. The admins team can be unsubscribed hence the remove icon, but the other team and user shown in the portal cannot be subscribed so there's no option to do it.
== Tests ==
Blueprint subscriptions are tested by doc tests in subscribing.txt. These were modified to account for the changed button names as well as adding a test for unsubscribing teams and tests for the better informational messages.
== 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/
./lib/lp/
203: E251 no spaces around keyword / parameter equals
./lib/lp/
128: source exceeds 78 characters.
246: source exceeds 78 characters.
This branch looks good. I had a couple of thoughts while reading over
it, but nothing that would keep us from landing it.
In specificationsu bscription. py (line 221 of the diff), would it be
better to say "Modify subscription to %s" (adding the "to")?
I also noticed that the elements of __all__ in bscription. py aren't quite in alphabetical order.
specificationsu
This test confuses me:
Or we can click the icon next to their name to get to the subscription edit
page.
>>> browser. getLink( url='/+ subscription/ stub'). click() getControl( name='field. essential' ).value = 'no' getControl( 'Change' ).click( )
>>> browser.
>>> browser.
I don't see any assertions. Maybe instead of the second and third lines
there should be a check to be sure the current URL is what is expected.