Make review request page entries pluggable and page data more dynamic.
Review Request #9041 — Created July 19, 2017 and submitted
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 inReviewRequestPageData
. Only the data needed by the entries in
use will be computed. Callers can also specify the specific entry types
they care about when constructingReviewRequestPageData
, 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 |
reviewbot | |
I think it would make slightly more intuitive sense to check these conditions in the opposite order (are they enabled … |
david | |
F401 'djblets.registries.errors.ItemLookupError' imported but unused |
reviewbot |
- Change Summary:
-
- Renamed
entry_id
toentry_type_id
. - Renamed
entry_type
toentry_pos
.
- Renamed
- 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 likereviews, 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 inuse will be computed. Callers can also specify the specific entry types they care about when constructing ReviewRequestPageData
, which willhelp 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:
-
25b8e3c1cc2b6ce8a80b5cced534d0d2deb4f4a3b11a0993db15179f55f2869b6d8f4284fbe5ab0f
- Diff:
-
Revision 2 (+1373 -372)