launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

The proposal to merge lp:~sinzui/launchpad/pageids into lp:launchpad has been updated.

Description changed to:

The page performance report cannot handle Page-Ids with random/changing information like revisionids in URLs. Launchpad's
standard views always provide sensible __name__ and __class__.__name__ attrs
to construct them. Two cases where views are dynamically generated for
templates and resources do not provide the expected attributes, or the
code does not know how to find the data.

--------------------------------------------------------------------
RULES

    Pre-implementation: no one
    * HelpFolders are missing the __name__ and that is trivial to register
      with the dynamic view class.
    * The ++model++ example is a case were the context is also a view, and
      it is dynamically generated from a page template. Either the
      registration can modified, or constructPageID() must learn how to
      handle the recursive issue.
      * Handling at least one-level of recursion will prevent future breaks
        for future dynamic view classes from happening.

QA

    * Visit https://blueprints.qastaging.launchpad.net/+help-blueprints/workitems-help.html/++profile++show
    * Verify the Page-Id is just "RootObject:+help-blueprints", not dir
      path
    * Visit https://bugs.qastaging.launchpad.net/ubuntu/+bugs/++model++
    * Expect a timeout, view the oops.
    * Verify the Page-id is Distribution:+bugs:JsonModelNamespaceView,
      no path to the template.

LINT

    lib/lp/services/inlinehelp/README.txt
    lib/lp/services/inlinehelp/zcml.py
    lib/lp/services/webapp/publication.py
    lib/lp/services/webapp/doc/webapp-publication.txt
    lib/lp/services/webapp/tests/test_pageid.py

LoC:
    This add about 40 lines of code, but I have a 3000+ line credit.

TEST

    ./bin/test -vvc -t README lp.services.inlinehelp
    ./bin/test -vvc -t PageID lp.services.webapp.tests.test_pageid
    ./bin/test -vvc -t webapp-publication.txt lp.services.webapp.tests.test_doc

IMPLEMENTATION

Use the name from the zcml as the help folder's __name__ attr.
    lib/lp/services/inlinehelp/README.txt
    lib/lp/services/inlinehelp/zcml.py

Recuse through constructPageID() when the context is a dynamic view
created from a page template. I converted from doctests to unittests.
    lib/lp/services/webapp/publication.py
    lib/lp/services/webapp/doc/webapp-publication.txt
    lib/lp/services/webapp/tests/test_pageid.py

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