launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

The proposal to merge lp:~sinzui/launchpad/team-titles into lp:launchpad has been updated.

Description changed to:

Use standard page titles, bread crumbs, and headings for teams and bugs.

    Pre-implementation: No one. I largely followed the advise of mpt in
    the bug reports.

Bugs Bug #244558 Not obvious that teams can't have their own bug reports
    Page does not clearly state you are looking at a team. The
    page title, heading, and bread crumbs do not conform to Lp rules.
    Many users mistake the team for a project :(

Bug #516485 No User Name in the Title of Related Bugs Page Users/team
    The bug vhost breadcrumb adapter is not registered for IPerson. The
    adaption fails during traversal every time. When a view is careful
    to provide page_title, a few crumbs appear.

Bug #928234 Team application pages aren't obviously about a team
    Application page headings do not state you are looking at a team.
    mpt suggests that we smartquote the team and append 'team' as is
    done in the bread crumbs.

--------------------------------------------------------------------

RULES

    * All four bugs are are caused by developer confusion about how
      Lp page titles, bread crumbs and headings work. In some
      cases the views intentionally deviate from Lp rules.
    * Remove support for override_title_breadcrumbs
      * Instead check for an instances SystemErrorView.
      * Removing override_title_breadcrumbs will restore the page title
        and breadcrumbs to several bug pages and specifically team bug
        pages.
      * This partially addresses the concern that bug titles can leak
        confidential information when the user has limited view.
    * Every traversed object must have a breadcrumb adapter.
      * Register BugsVHostBreadcrumb for IPerson
    * The Person bug views must provide:
      * A terse page_title to create a proper page title and breadcrumb.
      * An informative label that explains the purpose the page as the <h1>

QA

    * Visit https://bugs.qastaging.launchpad.net/launchpad/+subscribe
    * Verify the breascrumbs read
      Launchpad itself >> Bugs >> Subscribe
    * Verify the page title is
      Subscribe : Bugs : Launchpad itself

    * Visit https://bugs.qastaging.launchpad.net/launchpad/+bug/533044
    * Verify that the page title is
      Bug #533044 <title> : Bugs : Launchpad itself

    * Visit https://bugs.qastaging.launchpad.net/launchpad/+bug/a77
    * Verify that the page title is
      Error: Page not found

    * Visit https://bugs.qastaging.launchpad.net/~launchpad
    * Verify the page title is
      Bugs : "Canonical Launchpad Engineering" team
    * Verify the breadcrumbs are
      "Canonical Launchpad Engineering" team >> Bugs
    * Verify the first heading is
      "Canonical Launchpad Engineering" team

    * Visit https://bugs.qastaging.launchpad.net/~launchpad/+assignedbugs
    * Verify the page title is
      Assigned bugs : Bugs : "Canonical Launchpad Engineering" team
    * Verify the breadcrumbs are
      "Canonical Launchpad Engineering" team >> Bugs >> Assigned bugs

    * Visit https://qastaging.launchpad.net/~launchpad
    * Verify the heading is "Canonical Launchpad Engineering" team

    * Visit https://answers.qastaging.launchpad.net/~launchpad
    * Verify the first heading is "Canonical Launchpad Engineering" team

    * Visit https://code.qastaging.launchpad.net/~launchpad
    * Verify the first heading is "Canonical Launchpad Engineering" team

    * Visit https://blueprints.qastaging.launchpad.net/~launchpad
    * Verify the first heading is "Canonical Launchpad Engineering" team

    * Visit https://translations.qastaging.launchpad.net/~launchpad
    * Verify the first heading is "Canonical Launchpad Engineering" team

LINT

    lib/lp/app/browser/tales.py
    lib/lp/bugs/browser/bugtask.py
    lib/lp/bugs/browser/structuralsubscription.py
    lib/lp/bugs/stories/bug-also-affects/xx-also-affects-new-upstream.txt
    lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
    lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt
    lib/lp/bugs/stories/bugs/xx-bug-create-question.txt
    lib/lp/bugs/stories/bugs/xx-bug-obfuscation.txt
    lib/lp/bugs/stories/bugtask-searches/xx-advanced-upstream-pending-bugwatch.txt
    lib/lp/bugs/stories/guided-filebug/xx-filebug-attachments.txt
    lib/lp/bugs/stories/guided-filebug/xx-product-guided-filebug.txt
    lib/lp/bugs/stories/guided-filebug/xx-project-guided-filebug.txt
    lib/lp/services/webapp/error.py
    lib/lp/bugs/browser/configure.zcml
    lib/lp/bugs/browser/tests/test_breadcrumbs.py
    ^ Lint is not happy with some of the stories. I can fix these after the
      review.

TEST

    ./bin/test -vvc -t xx-also-affects-new-upstream \
        -t xx-attachments-to-bug-report -t xx-bug-comments-truncated \
        -t xx-bug-create-question -t xx-bug-obfuscation \
        -t xx-advanced-upstream-pending-bugwatch \
        -t xx-filebug-attachments -t xx-product-guided-filebug \
        -t xx-project-guided-filebug lp.bugs.tests.test_doc
    ./bin/test -vvc lp.bugs.browser.tests.test_breadcrumbs
    ./bin/test -vvc -t test_title lp.registry.tests.test_person

IMPLEMENTATION

I replaced the check for override_title_breadcrumb in ObjectFormatterAPI
to instead check if the view is an instance of SystemErrorView. I
removed all override_title_breadcrumb attributes from the error views
and the two offending bugs views. I updated many tests, and it is clear
that the ellipsis in the stories was hiding the title insanity.
    lib/lp/app/browser/tales.py
    lib/lp/bugs/browser/bugtask.py
    lib/lp/bugs/browser/structuralsubscription.py
    lib/lp/bugs/stories/bug-also-affects/xx-also-affects-new-upstream.txt
    lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
    lib/lp/bugs/stories/bugs/xx-bug-comments-truncated.txt
    lib/lp/bugs/stories/bugs/xx-bug-create-question.txt
    lib/lp/bugs/stories/bugs/xx-bug-obfuscation.txt
    lib/lp/bugs/stories/bugtask-searches/xx-advanced-upstream-pending-bugwatch.txt
    lib/lp/bugs/stories/guided-filebug/xx-filebug-attachments.txt
    lib/lp/bugs/stories/guided-filebug/xx-product-guided-filebug.txt
    lib/lp/bugs/stories/guided-filebug/xx-project-guided-filebug.txt
    lib/lp/services/webapp/error.py

Bug #516485 is caused by a broken breadcrumb adaption.
BugsVHostBreadcrumb was not registered for IPerson. Adding it made
user/team page titles and breadcrumbs behave like projects and distros.
I added a test to verify the breadcrumb adapter makes the expected crumb.
    lib/lp/bugs/browser/configure.zcml
    lib/lp/bugs/browser/tests/test_breadcrumbs.py

Bug 928234 is solved by ensuring that team.title returns the smartquoted
displayname with team appended to it. I added a test, but I believe there
will be some test failures that I will need to follow up on. I may also
be able to remove numerous calls to smartquote in views now that .title
provides the definitive formatting.
    lib/lp/registry/model/person.py
    lib/lp/registry/tests/test_person.py

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/team-titles/+merge/93675
-- 
https://code.launchpad.net/~sinzui/launchpad/team-titles/+merge/93675
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/team-titles into lp:launchpad.