• 
      

    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)