Merge lp://staging/~sinzui/launchpad/restore-revid into lp://staging/launchpad

Proposed by Curtis Hovey
Status: Merged
Merged at revision: 11026
Proposed branch: lp://staging/~sinzui/launchpad/restore-revid
Merge into: lp://staging/launchpad
Diff against target: 280 lines (+93/-40)
9 files modified
lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt (+2/-9)
lib/canonical/launchpad/pagetests/basics/user-requested-oops.txt (+2/-2)
lib/lp/app/browser/tests/base-layout.txt (+1/-1)
lib/lp/app/templates/base-layout-macros.pt (+1/-0)
lib/lp/app/templates/base-layout.pt (+1/-1)
lib/lp/registry/browser/person.py (+14/-3)
lib/lp/registry/browser/tests/people-views.txt (+32/-0)
lib/lp/registry/stories/person/xx-people-search.txt (+30/-21)
lib/lp/registry/templates/people-index.pt (+10/-3)
To merge this branch: bzr merge lp://staging/~sinzui/launchpad/restore-revid
Reviewer Review Type Date Requested Status
Michael Nelson (community) code ui Approve
Review via email: mp+27781@code.staging.launchpad.net

Description of the change

This is my branch to fix trivial mistakes and oversights.

    lp:~sinzui/launchpad/restore-revid
    Diff size: 261
    Launchpad bug:
          https://bugs.launchpad.net/bugs/595218
          https://bugs.launchpad.net/bugs/28612
          https://bugs.launchpad.net/bugs/111251
    Test command: ./bin/test -vv \
          -t demo-and-lpnet -t base-layout -t people-view -t xx-people-search
    Pre-implementation: no one
    Target release: 10.06

fix trivial mistakes and oversights
------------------------------------

Bug #595218 [restore the bzr revision number to the != lpnet footer]
    The change to remove the link to the release number (10.05) wrongly
    removed the revision number (r10101)

Bug #28612 ["People and teams" list uses columns but isn't sortable]
    Search results for people and teams, such as
    <https://launchpad.net/people/?name=abc&searchfor=peopleonly>, are shown
    as a table with columns for display name, Launchpad ID, and karma value.
    However, it is not possible toa sort the table by clicking the columns.

Bug #111251 [do not display karma for teams on /people]
    Sometimes the list on that page will include only teams and thus the Karma
    column could be omitted. On the other hand, sometimes we display people
    and teams and thus would have to use "n/a" (or something similar) as the
    value of the Karma column for teams.

Rules
-----

Bug #595218 [restore the bzr revision number to the != lpnet footer]
    * Use revno where version was used in base-layout.

Bug #28612 ["People and teams" list uses columns but isn't sortable]
    * Add "sortable" to the table class and an id for the script hooks.
      Yes, it is that simple to make any listing table sortable.
      We wanted this for the 3.0 release, but I was told it was too hard.

Bug #111251 [Do not display karma for teams]
    * Use an mdash instead of karma id the row is a team.
    * Do not display the karma column if the search is teams only.
    * We wanted this for the 3.0 release, but I was told it was too complex.

QA
--

Bug #595218 [restore the bzr revision number to the != lpnet footer]
    * Visit edge and verify you can see r110nn before Beta site.

Bug #28612 ["People and teams" list uses columns but isn't sortable]
    * Visit https://launchpad.net/people and do a search
    * Verify you can sort the columns

Bug #111251 [do not display karma for teams]
    * Visit https://launchpad.net/people and do a search
    * Verify the teams have m-dashes instead of 0 karma
    * Search for teams only.
    * Verify the karma column is not shown.

Lint
----

Linting changed files:
  lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt
  lib/lp/app/browser/tests/base-layout.txt
  lib/lp/app/templates/base-layout-macros.pt
  lib/lp/app/templates/base-layout.pt
  lib/lp/registry/browser/person.py
  lib/lp/registry/browser/tests/people-views.txt
  lib/lp/registry/stories/person/xx-people-search.txt
  lib/lp/registry/templates/people-index.pt

Test
----

    * lib/canonical/launchpad/pagetests/basics/demo-and-lpnet.txt
      * Updated the test to verify the revno in the footer.
      * Removed duplicate test.
    * lib/lp/app/browser/tests/base-layout.txt
      * Updated the comment test to verify the revno.
    * lib/lp/registry/browser/tests/people-views.txt
      * Updated the test to verify that is_teams_only and is_people_only
        properties.
    * lib/lp/registry/stories/person/xx-people-search.txt
      * Removed data tests that hid the problem in the page and did not
        explain what users saw.
      * Added the header and karma column to the tests.

Implementation
--------------

    * lib/lp/app/templates/base-layout-macros.pt
      * Added revno to the footer on != lpnet pages.
    * lib/lp/app/templates/base-layout.pt
      * Replaced the version with the revno in the footer.
    * lib/lp/registry/browser/person.py
      * Added is_teams_only and is_people_only to the view for the template
        to know when to show columns.
    * lib/lp/registry/templates/people-index.pt
      * Added an id to the listing table and added the sortable class.
      * Added a rule to use a m-dash when the row is a team.
      * Added a rule to remove the karma column when the search is teamsonly.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

Great Curtis.

Only one tiny comment:

> === modified file 'lib/lp/registry/templates/people-index.pt'
> --- lib/lp/registry/templates/people-index.pt 2009-08-24 20:22:29 +0000
> +++ lib/lp/registry/templates/people-index.pt 2010-06-17 10:19:35 +0000
> @@ -159,7 +159,14 @@
> tal:attributes="href person/fmt:url"/>
> </td>
> <td tal:content="person/name">foobar</td>
> - <td class="amount" tal:content="person/karma">34</td>
> + <td class="amount"
> + tal:condition="not: view/is_teams_only">
> + <tal:user
> + condition="not: person/teamowner"

s/teamowner/is_team

> + content="person/karma">34</tal:user>
> + <tal:team
> + condition="person/teamowner">&mdash;</tal:team>

and here too.

> + </td>
> </tr>
> </tbody>
> </table>

Thanks!

review: Approve (code)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

I also ran it locally and checked the sorting, and the column... can't see any obvious improvements that could be made.

review: Approve (code ui)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.