• 
      

    Allow Djblets extension template tags to be passed full bundle IDs.

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

    Information

    Djblets
    release-4.x

    Reviewers

    Some of the template tags for loading extension static media take an
    extension and a bundle name as arguments. It's assumed that the bundle
    name is always relative to the extension. When actually rendering the
    bundle, we construct the full bundle ID using the relative name.

    This change allows full bundle IDs to be passed to the template tags.
    The given name will be checked to see if it's a full bundle ID, and if so
    it will be used to render the media. If not, then we construct the full
    bundle ID as before.

    The motivation for this change came from a bug we found with loading review
    UI extensions' static media in Review Board. Djblets' extension template
    tags mostly copy what Django Pipeline does for rendering a package's static
    media. Except, Djblets uses the ExtensionStaticMediaNodeMixin, which allows
    us to render static media for both development setups and installed packages
    of extensions. When rendering a review UI's JavaScript and CSS bundles on the
    actual review UI page, we use Pipeline's javascript and stylesheet
    template tags directly, which means we miss out on the nice ability to handle
    installed packages.

    So for example, when running a Review Board dev server with an installed
    package of a review UI extension, the review UI extension's JavaScript and
    CSS bundles would fail to load on the review UI page. This isn't a
    problem in production servers, and isn't a problem when using a
    development install of a package.

    To fix this, we want to be able to use the ext_js_bundle and
    ext_css_bundle template tags instead of the Pipeline ones. And to be able
    to use them, we have to allow passing full bundle IDs, because that's how
    JavaScript and CSS bundle names are defined on review UI extensions.

    This change targets the 4.x branch because we need it to fix the bug for
    Power Pack 6's review UIs (PDF and Office Document review). The bug affects
    development of Power Pack 6 against Review Board 6.

    • Ran unit tests.
    • Tested using full bundle IDs with ext_js_bundle and ext_css_bundle,
      on both RB6 and RB7 dev and production servers.
    • Tested using relative bundle names with ext_js_bundle and
      ext_css_bundle, on both RB6 and RB7 dev and production servers.
    Summary ID
    Allow Djblets extension template tags to be passed full bundle IDs.
    Some of the template tags for loading extension static media take an extension and a bundle name as arguments. It's assumed that the bundle name is always relative to the extension. When actually rendering the bundle, we construct the full bundle ID using the relative name. This change allows full bundle IDs to be passed to the template tags. The given name will be checked to see if it's a full bundle ID, and if so it will be used to render the media. If not, then we construct the full bundle ID as before. The motivation for this change came from a bug we found with loading review UI extensions' static media in Review Board. Djblets' extension template tags mostly copy what Django Pipeline does for rendering a package's static media. Except, Djblets uses the `ExtensionStaticMediaNodeMixin`, which allows us to render static media for both development setups and installed packages of extensions. When rendering a review UI's JavaScript and CSS bundles on the actual review UI page, we use Pipeline's `javascript` and `stylesheet` template tags directly, which means we miss out on the nice ability to handle installed packages. So for example, when running a Review Board dev server with an installed package of a review UI extension, the review UI extension's JavaScript and CSS bundles would fail to load on the review UI page. This isn't a problem in production servers, and isn't a problem when using a development install of a package. To fix this, we want to be able to use the `ext_js_bundle` and `ext_css_bundle` template tags instead of the Pipeline ones. And to be able to use them, we have to allow passing full bundle IDs, because that's how JavaScript and CSS bundle names are defined on review UI extensions. This change targets the 4.x branch because we need it to fix the bug for Power Pack 6's review UIs (PDF and Office Document review). The bug affects development of Power Pack 6 against Review Board 6.
    1031c01207e09e4936c12b8d2573ae0a19a5ed02
    Description From Last Updated

    Can you add type hints while you're here?

    daviddavid

    As new behavior for the interface, this technically will need to be a 4.1 release. So we should bump that …

    chipx86chipx86

    Since we're adding type hints, we should add the __future__ import to this file.

    daviddavid

    Let's list 4.1, 5.3 here.

    chipx86chipx86
    david
    1. 
        
    2. Show all issues

      Can you add type hints while you're here?

      1. I was thinking about that, but type hints get added for this and the rest of the file on the 5.x branch (commit), so I thought I would just leave it since we don't really develop on Djblets 4.x anymore. That being said, should I still add them here?

      2. Ah, ok, no worries. Let's just make sure when we do the merge that everything looks good.

      3. Sounds good.

    3. Show all issues

      Since we're adding type hints, we should add the __future__ import to this file.

    4. 
        
    chipx86
    1. 
        
    2. Show all issues

      As new behavior for the interface, this technically will need to be a 4.1 release. So we should bump that here and in the version of the release.

      That'd also land in 5.3.

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

      Let's list 4.1, 5.3 here.

    3. 
        
    maubin
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.x (71c36c4)