Launchpad itself

Merge lp://staging/~bac/launchpad/bug-162754 into lp://staging/launchpad

bug-162754
Merge into devel

Proposed by Brad Crittenden on 2010-06-22

Status:

Merged

Approved by:

Brad Crittenden on 2010-06-23

Approved revision:

no longer in the source branch.

Merged at revision:

11049

Proposed branch:

lp://staging/~bac/launchpad/bug-162754

Merge into:

lp://staging/launchpad

Diff against target:

602 lines (+267/-61)

9 files modified

lib/canonical/launchpad/pagetests/standalone/xx-form-layout.txt (+9/-1)
lib/canonical/launchpad/templates/README (+0/-29)
lib/canonical/launchpad/templates/launchpad-form.pt (+8/-1)
lib/canonical/launchpad/webapp/tests/test_launchpadform.py (+5/-3)
lib/lp/app/browser/tests/launchpadform-view.txt (+44/-0)
lib/lp/registry/browser/product.py (+102/-19)
lib/lp/registry/browser/tests/product-edit-people-view.txt (+51/-4)
lib/lp/registry/stories/product/xx-product-add.txt (+48/-0)
lib/lp/registry/stories/product/xx-product-driver.txt (+0/-4)

To merge this branch:

bzr merge lp://staging/~bac/launchpad/bug-162754

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Curtis Hovey (community)	code		Approve on 2010-06-23
Matthew Revell (community)	text	2010-06-22	Approve on 2010-06-23
Māris Fogels (community)	code	2010-06-22	Approve on 2010-06-22
Review via email: mp+28227@code.staging.launchpad.net

Commit message

Allow registrants to disclaim maintainer role of new projects and easily set the maintainer to Registry Administrators for existing projects.

Description of the change

= Summary =

Often Launchpad users will create a new project that corresponds to an
upstream in order to file a bug or perform some other action. They may
be only marginally interested in the project but are performing a
valuable service. Unfortunately since the person registered the project
they are assumed to be the maintainer and are forced into that role even
though they don't want it.

== Proposed fix ==

Provide a checkbox on project registration that allows the project to be
created but automatically re-assigned to the ~registry team.

Also on the +edit-people page a checkbox is provided to transfer the
maintainer role to ~registry.

== Pre-implementation notes ==

Chats with Curtis.

== Implementation details ==

As above.

In order to get the layout correct on the +edit-people page some
extensions needed to be added to the launchpad-form.pt. It now looks
for a widget attribute called 'widget_class' to use as the css class for
the widget.

== Tests ==

bin/test -vvt xx-product-add.txt -t xx-product-driver.txt \
-t product-edit-people-view.txt

== Demo and Q/A ==

Create a new project and look for the new checkbox on the second page.
Go to https://launchpad.dev/firefox and click on the edit icon next to
the maintainer. Look for the new checkbox.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/registry/stories/product/xx-product-driver.txt
  lib/lp/registry/browser/product.py
  lib/lp/registry/browser/tests/product-edit-people-view.txt
  lib/canonical/launchpad/templates/launchpad-form.pt
  lib/lp/app/browser/tests/launchpadform-view.txt
  lib/canonical/launchpad/webapp/tests/test_launchpadform.py
  lib/lp/registry/stories/product/xx-product-add.txt

== Pylint notices ==

lib/canonical/launchpad/webapp/tests/test_launchpadform.py
57: [C0301] Line too long (79/78)

I'll fix this lint problem.

Revision history for this message

Māris Fogels (mars) wrote on 2010-06-22:

Hi Brad,

The code in this branch looks good. I have one question about the tests, and a few suggestions for the UI text.

For the lanchpadform-view.txt you test for presence of extra elements when the widget_class field is present. This is the positive case. Do you need to test for the absence of those same elements when the widget_field attribute is missing?

Regarding the UI text, I found that some of the long descriptions for the new options made it unclear if the flag means "I am not the maintainer of this project" or "I do not want this project to be maintained". Specifically I would reword "but you don't want to actually maintain" to be "but you do not want to be the maintainer of". This confusion appears in two of the long descriptions.

The concept and role of the "Registry Administrators" is new to the user. I think that the short and long description during project creation does a good job of introducing this concept. However, I feel that the "Assign to Registry Administrators" control does not. Both controls result in the same outcome (the admins take over), but from a user's perspective the controls are completely different, mostly because both control's primary and secondary text share no similarity. To remedy this I suggest making all of the text for the two controls the same (or very very similar): "I do not want to maintain this project".

The code looks good, so I'm marking this as "Approved", but that is conditional upon a UI review from a certified reviewer, and a look at the text by mrevell.

Maris

review: Approve (code)

Revision history for this message

Brad Crittenden (bac) wrote on 2010-06-22:

Thanks for the review Maris.

To your first point, there are two items in the form. Only one has the extra attribute and it is the only one that displays the additional CSS class. Too subtle?

Thanks for your feedback on the wording issues.

Revision history for this message

Brad Crittenden (bac) wrote on 2010-06-22:

I had one test failure running the registry suite. The fix is at http://pastebin.ubuntu.com/453606/

Revision history for this message

Matthew Revell (matthew.revell) wrote on 2010-06-23:

Thanks for this work Brad.

I agree with Mars' text suggestion. I'd refocus the descriptive text on the "I don't want to maintain this" aspect, with an explanation that registry admins will take over being secondary.

So, in the case of the first chunk of text, I'd go for something like:

"Select this if you no longer want to maintain this project in Launchpad. Launchpad's Registry Administrators team will become the project's new administrators."

Approve but I suggest changing the focus of the wording.

review: Approve (text)

Revision history for this message

Curtis Hovey (sinzui) wrote on 2010-06-23:

I really appreciate these improvements. I agree with text issue. I think both checkboxes should encapsulate the users thoughts and intent:

[X] I don't want to maintain this

I pondered something like:

[ user ] (Choose) or Register a team
or [X] I don't want to maintain this

But I can find no precedence for this. I will let you and Matthew judge if a leading "or" makes this operation clearer.

review: Approve (code)