Code review comment for lp:~sinzui/launchpad/milestone-table-bug-490648

Revision history for this message
Curtis Hovey (sinzui) wrote :

This is my branch to improve the performance of milestone table rows.
Remove the duplicate queries and do not query objects that will not be used.

    lp:~sinzui/launchpad/milestone-table-bug-490648
    Diff size: 112
    Launchpad bug: https://bugs.launchpad.net/bugs/490648
    Test command: ./bin/test -vv -t "reg.*milestone"
    Pre-implementation: no one
    Target release: 3.1.12

= Improve the performance of the milestone table rows =

For each milestone that is shown in the milestone table, the release is looked
up three times and the structuralsubscription is looked up once, but never
used. The MilestoneView provides the release as view/release. The other two
uses must use the view's copy. The structuralsubscription is gotten because
the navigation menu is used to get the edit link. A new menu is needed that
only provides the edit link to ensure the extra db call is made.

== Rules ==

    * Create a navigation menu for the MilestoneView (used by portlets and
      tables) to provide only th links needed.
    * Update the template to use the view's copy of the release.

== QA ==

    Comparing https://edge.launchpad.net/launchpad-registry/3.1 with
    https://launchpad.net/launchpad-registry/3.1, there should be 12
    fewer queries.

== Lint ==

Linting changed files:
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/milestone.py
  lib/lp/registry/browser/tests/milestone-views.txt
  lib/lp/registry/templates/productseries-milestone-table-row.pt

== Test ==

    * lib/lp/registry/browser/tests/milestone-views.txt
      * Added a test to verify the link in the new menu is legitimate.

== Implementation ==

    * lib/lp/registry/browser/configure.zcml
      * Registered the new menu.
    * lib/lp/registry/browser/milestone.py
      * Added a new navigation menu and updated MilestoneView to use it.
    * lib/lp/registry/templates/productseries-milestone-table-row.pt
      * Updated the template to use the new menu.
      * Replaced the 2 calls to product_release to use the view's release.

« Back to merge proposal