Merge lp:~edwin-grubbs/launchpad/bug-652232-person-code-action-links into lp:launchpad

[Edwin Grubbs]

Summary
-------

Moved action links for code.lp.net/~$person and
code.lp.net/~$person/$project into sidebar for consistency.

Implementation details
----------------------

Moved links into the sidebar.
    lib/lp/code/templates/person-branches.pt
    lib/lp/code/templates/person-codesummary.pt

Removed #page-summary css, since it's no longer used.
    lib/canonical/launchpad/icing/style.css

The person code pages and the person product code pages now share the
same template.
    lib/lp/code/browser/branchlisting.py
    lib/lp/code/browser/configure.zcml
    lib/lp/code/templates/personproduct-branches.pt

Fixed tests:
    lib/lp/code/stories/branches/xx-branchmergeproposal-listings.txt
    lib/lp/code/stories/branches/xx-person-branches.txt
    lib/lp/code/stories/branches/xx-personproduct-branch-listings.txt

Tests
-----

./bin/test -vv -t 'xx-branchmergeproposal-listings|xx-person-branches|xx-personproduct-branch-listings'

Demo and Q/A
------------

* Open http://code.launchpad.dev/~mark/
    * The links should be in the sidebar, and clicking on the links
      should take you to a page with the sidebar, except for the "active
      reviews" page.
    * If you log in as that user, you should see the "Register a branch"
      link.
* Open http://code.launchpad.dev/~mark/firefox
    * The links should be in the sidebar, and clicking on the links
      should take you to a page with the sidebar, except for the "active
      reviews" page.
    * Even if you are logged in as that user, you should never see the
      "Register a branch" link, since that page only registers branches
      under +junk, so it would be confusing because the list of firefox
      branches wouldn't show it.

[Guilherme Salgado (salgado)]

Hi Edwin,

This looks good, but although the sidebar is consistent with other pages, the links themselves are not. For instance, the links on bugs.lp.dev/~mark start with a verb to denote the action, as the sidebar used to hold all our action links. My impression is that we're not going to use them for actions anymore, but then I think we should change the link text on the other pages to be consistent with what we have here.

Is there a wiki page or mailing list thread where this standardization has been discussed. That'd help in making sure everybody is on the same page for future work on this front.

review: Approve (ui*)

[Curtis Hovey (sinzui)]

Links to collections and/or collection slices can begin with a count of the items instead of an action. I do not think this rule was documented. See the bugs page for a project for the example. Beuno suggested the rule because 6 links starting with "View" makes the portlet difficult to scan. eg. View active branches is awkward because we expect following a link always leads to viewing a page. The count is more most important because it sets expectations.

I think we should discuss junk. Junk is not in scope, it is not a part of bridging the gap. I deactivate about 5 projects a week because the user did not know about +junk. The user learned about personal branches in the email I sent. The project code page tells me how to pull code from the project, and has a help link to code. I would like a similar message on the person branches page.

    You can push personal branches (branches that are not related to a project) to:
    lp:~sinzui/+junk

We cannot link to +junk because there is not a view registered to show that slice of the branches collection.

review: Needs Information (ui)

[Edwin Grubbs (edwin-grubbs)]

Curtis explained where the ideas for the new format came from, but I'll send out an email so that it becomes more widely known and agreed upon. I have added the directions for pushing a personal branch that Curtis asked for.

Here is the incremental diff.

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py 2010-10-15 01:48:05 +0000
+++ lib/lp/code/browser/branchlisting.py 2010-10-18 19:57:05 +0000
@@ -977,7 +977,11 @@

     @property
     def show_action_menu(self):
- return check_permission('launchpad.Edit', self.context)
+ return self.user.inTeam(self.context)
+
+ @property
+ def show_junk_directions(self):
+ return self.user == self.context

     @property
     def initial_values(self):

=== modified file 'lib/lp/code/stories/branches/xx-person-branches.txt'
--- lib/lp/code/stories/branches/xx-person-branches.txt 2010-10-15 14:19:41 +0000
+++ lib/lp/code/stories/branches/xx-person-branches.txt 2010-10-18 20:28:35 +0000
@@ -33,6 +33,19 @@
     There are no branches related to Daniel Silverstone in Launchpad today.

+Junk branches
+=============
+
+On the user's own code page, they will see directions on pushing a branch.
+
+ >>> browser.open('http://code.launchpad.dev/~name12')
+ >>> print extract_text(
+ ... find_tag_by_id(browser.contents, 'junk-branch-directions'))
+ You can push (upload) personal branches
+ (those not related to a project) with the following command:
+ bzr push lp:~name12/+junk/BRANCHNAME
+
+
 Owned Branches
 ==============

=== modified file 'lib/lp/code/templates/person-branches.pt'
--- lib/lp/code/templates/person-branches.pt 2010-10-15 01:48:05 +0000
+++ lib/lp/code/templates/person-branches.pt 2010-10-18 20:28:33 +0000
@@ -25,6 +25,14 @@
 <div metal:fill-slot="main"
      tal:define="branches view/branches">

+ <p id="junk-branch-directions" tal:condition="view/show_junk_directions">
+ You can push (upload) personal branches
+ (those not related to a project) with the following command:
+ <br/>
+ <tt>bzr push lp:~<tal:name
+ replace="view/user/name"/>/+junk/<em>BRANCHNAME</em></tt>
+ </p>
+
   <div id="no-branch-message" tal:condition="not: view/branch_count">
     <p tal:content="view/no_branch_message">
       There are no branches related to Eric the Viking today.

[Curtis Hovey (sinzui)]

thanks. I hope I will deactivate fewer projects after this lands.

review: Approve (ui)

[Guilherme Salgado (salgado)]

On Mon, 2010-10-18 at 16:31 +0000, Curtis Hovey wrote:
> Review: Needs Information ui
> Links to collections and/or collection slices can begin with a count
> of the items instead of an action. I do not think this rule was
> documented. See the bugs page for a project for the example. Beuno
> suggested the rule because 6 links starting with "View" makes the
> portlet difficult to scan. eg. View active branches is awkward because
> we expect following a link always leads to viewing a page. The count
> is more most important because it sets expectations.

That makes sense, but my point was that if this was the preferred style
now, then we should fix the pages that are still using the old one (e.g.
bugs.lp.net/~user). Should we try and file a bug for each of them or
should each team do so once Edwin sends his email clarifying the rules
to everybody?

[Curtis Hovey (sinzui)]

On Tue, 2010-10-19 at 12:28 +0000, Guilherme Salgado wrote:
> That makes sense, but my point was that if this was the preferred
> style
> now, then we should fix the pages that are still using the old one
> (e.g.
> bugs.lp.net/~user). Should we try and file a bug for each of them or
> should each team do so once Edwin sends his email clarifying the rules
> to everybody?

Thanks. That is a good example of repetition. I would happy to see
counts with these links so that I do not need to visit the empty pages.
This would help users know where they ware getting bug mail from.

I do not think we can mandate a fix because the queries that build the
counts can cause performance issues. I think Robert sent an email
regarding performance issues counting bugs on the bugs page last week.

--
__Curtis C. Hovey_________
http://launchpad.net/

[Graham Binns (gmb)]

Teeny, tiny nitpick:

141 @@ -1,4 +1,6 @@
142 -= Person / Product listings =
143 +=========================
144 +Person / Product listings
145 +=========================

I think the convention is to use '----' for the title of a doctest file. At least, that's the convention I've been adhering to, thus:

 Person / Product listings
 -------------------------

review: Approve (code)

[Edwin Grubbs (edwin-grubbs)]

According to the sphinx documentation, which I believe is why we are now using reST instead of moin syntax for our doctests, sections use "=" and subsections use "-".

http://sphinx.pocoo.org/rest.html#sections

It also seems that the vast majority of converted tests use "=" for the title of the doctest file.

$ find lib/lp/ -name '*.txt' -exec head -3 {} ';' | grep -E '^[=-]+$' | sed -re 's/(.).*/\1/' | sort | uniq -c | sort -rn
    457 =
     21 -

=== modified file 'lib/lp/code/browser/branchlisting.py'
--- lib/lp/code/browser/branchlisting.py	2010-10-20 03:44:49 +0000
+++ lib/lp/code/browser/branchlisting.py	2010-10-20 16:07:48 +0000
@@ -978,7 +978,9 @@
 
     @property
     def show_action_menu(self):
-        return self.user.inTeam(self.context)
+        if self.user is not None:
+            return self.user.inTeam(self.context)
+        return False
 
     @property
     def show_junk_directions(self):