• 
      

    Fix rendering Review UI extension static media on dev servers.

    Review Request #14391 — Created March 24, 2025 and submitted

    Information

    Review Board
    release-6.x

    Reviewers

    While developing Power Pack 6 against Review Board 6, a bug was found
    where the JavaScript and CSS bundles for the PDF and Office Document review
    UIs failed to load on the review UI pages. This only happened on the dev
    server when using an installed package of Power Pack 6. This bug didn't
    affect production servers. It also didn't affect Review Board 7 dev servers,
    which can run with a development install of the Power Pack package.

    To fix this, we now use Djblets' ext_js_bundle and ext_css_bundle
    template tags instead of Django Pipeline's javascript and stylesheet
    tags. The Djblets tags essentially do the same thing as Pipeline's and
    call on the Pipeline code in their implementation. The difference is that
    the Djblets tags use the ExtensionStaticMediaNodeMixin, which allows them
    to render static media for both development setups and installed packages
    of extensions on dev servers.

    This fix relies on a Djblets fix (/r/14390) which will land in 4.0.1.
    However, this is backwards compatible with older versions of Djblets in
    the sense that nothing will break if Review Board runs with an older version
    of Djblets that doesn't include the fix there. The Djblets fix enables
    ext_css_bundle and ext_js_bundle to do the right when passing full
    bundle IDs from a review UI's js_bundle_names and css_bundle_names
    attributes. When a server is running with a version of Djblets that
    doesn't include the fix, the Djblets tags will return nothing and then
    this will fall back on using Pipeline's javascript and stylesheet
    tags, which will do the right thing on production servers. One annoying
    thing with the backwards compatibility is that when the Djblets tags
    return nothing, they'll also output a log warning complaning that the
    bundles names don't exist, since the bundle names are the full IDs
    instead of relative names which older versions of Djblets can't handle
    properly.

    • Ran unit tests.
    • Ran JS unit tests.

    Tested this patch on the following:
    - RB6 production and dev servers that contain the Djblets fix.
    - RB6 production and dev servers that don't contain the Djblets fix.
    - RB7 production and dev servers that contain the Djblets fix.
    - RB7 production and dev servers that don't contain the Djblets fix.

    Summary ID
    Fix rendering Review UI extension static media on dev servers.
    While developing Power Pack 6 against Review Board 6, a bug was found where the JavaScript and CSS bundles for the PDF and Office Document review UIs failed to load on the review UI pages. This only happened on the dev server when using an installed package of Power Pack 6. This bug didn't affect production servers. It also didn't affect Review Board 7 dev servers, which can run with a development install of the Power Pack package. To fix this, we now use Djblets' `ext_js_bundle` and `ext_css_bundle` template tags instead of Django Pipeline's `javascript` and `stylesheet` tags. The Djblets tags essentially do the same thing as Pipeline's and call on the Pipeline code in their implementation. The difference is that the Djblets tags use the `ExtensionStaticMediaNodeMixin`, which allows them to render static media for both development setups and installed packages of extensions on dev servers. This fix relies on a Djblets fix (/r/14390) which will land in 4.0.1. However, this is backwards compatible with older versions of Djblets in the sense that nothing will break if Review Board runs with an older version of Djblets that doesn't include the fix there. The Djblets fix enables `ext_css_bundle` and `ext_js_bundle` to do the right when passing full bundle IDs from a review UI's `js_bundle_names` and `css_bundle_names` attributes. When a server is running with a version of Djblets that doesn't include the fix, the Djblets tags will return nothing and then this will fall back on using Pipeline's `javascript` and `stylesheet` tags, which will do the right thing on production servers. One annoying thing with the backwards compatibility is that when the Djblets tags return nothing, they'll also output a log warning complaning that the bundles names don't exist, since the bundle names are the full IDs instead of relative names which older versions of Djblets can't handle properly.
    642cd040ee9fa5ccc69cb14be1528745f854dd5d
    Description From Last Updated

    This tag is indented 1 space too many.

    daviddavid

    Missing space after endif.

    chipx86chipx86

    This is indented too many levels.

    chipx86chipx86

    Missing space after endif.

    chipx86chipx86
    david
    1. 
        
    2. Probably not a real issue with your commit message, but I notice the code block markers are escaped in your description.

    3. Show all issues

      This tag is indented 1 space too many.

    4. 
        
    maubin
    david
    1. Ship It!
    2. 
        
    chipx86
    1. 
        
    2. Show all issues

      Missing space after endif.

    3. Show all issues

      This is indented too many levels.

    4. Show all issues

      Missing space after endif.

    5. 
        
    maubin
    david
    1. Ship It!
    2. 
        
    chipx86
    1. Ship It!
    2. 
        
    maubin
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-6.x (c8b927c)