Make review request page entries pluggable and page data more dynamic.

Review Request #9041 — Created July 19, 2017 and submitted

Information

Review Board
release-3.0.x
b11a099...

Reviewers

The separation of the calculation of the review request page data from
the view provided an opportunity for making more of the page dynamic,
allowing additional content to appear on the review request page. This
change realizes that by introducing a registry that provides the entries
for a page, and making the page data calculation more dynamic based on
the needs of the entry.

ReviewRequestPageEntryRegistry tracks all the entries on the page
(the review request, initial status updates, reviews, and change
descriptions). Extensions could add additional entry types to the
registry, allowing them to show up on the page.

BaseReviewRequestPageEntry subclasse must now specify a unique
entry_type_id, and can specify their data requirements (things like
reviews, file attachments, drafts, and more), along with specifying
whether they should appear before the reviews section ("initial"
entries) or within the main reviews section (the default). These classes
are also now responsible for generating the instances of the entries,
based on page data (including data they compute while building the
entries).

The requirements specified by the entries are used when computing the
data in ReviewRequestPageData. Only the data needed by the entries in
use will be computed. Callers can also specify the specific entry types
they care about when constructing ReviewRequestPageData, which will
help when dynamically reloading parts of the review request page in a
future change.

Unit tests have been added to sanity-check the new entry support and the
dynamic state for the page.

Tested several of the review requests I had here, making sure that the
states and ordering all seemed correct. We'll need more real-world
testing, just to be sure.

Unit tests pass.

Description From Last Updated

F401 'djblets.registries.errors.ItemLookupError' imported but unused

reviewbotreviewbot

I think it would make slightly more intuitive sense to check these conditions in the opposite order (are they enabled …

daviddavid

F401 'djblets.registries.errors.ItemLookupError' imported but unused

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

chipx86
Review request changed
Change Summary:
  • Renamed entry_id to entry_type_id.
  • Renamed entry_type to entry_pos.
Description:
   

The separation of the calculation of the review request page data from

    the view provided an opportunity for making more of the page dynamic,
    allowing additional content to appear on the review request page. This
    change realizes that by introducing a registry that provides the entries
    for a page, and making the page data calculation more dynamic based on
    the needs of the entry.

   
   

ReviewRequestPageEntryRegistry tracks all the entries on the page

    (the review request, initial status updates, reviews, and change
    descriptions). Extensions could add additional entry types to the
    registry, allowing them to show up on the page.

   
   

BaseReviewRequestPageEntry subclasse must now specify a unique

~   entry_id, and can specify their data requirements (things like
  ~ entry_type_id, and can specify their data requirements (things like
    reviews, file attachments, drafts, and more), along with specifying
    whether they should appear before the reviews section ("initial"
    entries) or within the main reviews section (the default). These classes
    are also now responsible for generating the instances of the entries,
    based on page data (including data they compute while building the
    entries).

   
   

The requirements specified by the entries are used when computing the

    data in ReviewRequestPageData. Only the data needed by the entries in
    use will be computed. Callers can also specify the specific entry types
    they care about when constructing ReviewRequestPageData, which will
    help when dynamically reloading parts of the review request page in a
    future change.

   
   

Unit tests have been added to sanity-check the new entry support and the

    dynamic state for the page.

Commit:
25b8e3c1cc2b6ce8a80b5cced534d0d2deb4f4a3
b11a0993db15179f55f2869b6d8f4284fbe5ab0f

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
  1. Love it!

    1. :D

      More goodness coming.

  2. reviewboard/reviews/detail.py (Diff revision 2)
     
     
    Show all issues

    I think it would make slightly more intuitive sense to check these conditions in the opposite order (are they enabled at all? do I need them?)

  3. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-3.0.x (5f698e7)