• 
      

    Enable extensible condition support.

    Review Request #14705 — Created Nov. 24, 2025 and submitted

    Information

    ReviewBot
    release-4.x

    Reviewers

    Review Board is gaining a centralized ReviewRequestConditionsField
    and a central registry that reflects any choices provided by extensions.
    This change makes use of that, providing a compatibility class for older
    versions of Review Board that behaves as before.

    This change pretty much copies /r/14281 where we enabled extensible
    condition support for RBIntegrations.

    • Ran unit tests.
    • Saw Power Pack's new owner and participant user role conditions
      in a tool config page.
    • Was able to trigger the tool using the owner condition (the
      participant condition requires more work in a separate change).
    • Tested on RB 7.0.4 dev server too.
    Summary ID
    Enable extensible condition support.
    Review Board is gaining a centralized `ReviewRequestConditionsField` and a central registry that reflects any choices provided by extensions. This change makes use of that, providing a compatibility class for older versions of Review Board that behaves as before. This change pretty much copies /r/14281 where we enabled extensible condition support for RBIntegrations.
    8ecf4f902a5de1c1d69ed1110a04e5e80cc21c64
    Description From Last Updated

    This doesn't seem appropriate. We might be running a type checker inside an RB7 tree with this code, plus lower …

    daviddavid

    Let's add one more blank line here.

    daviddavid

    too many blank lines (2) Column: 9 Error code: E303

    reviewbotreviewbot

    line too long (83 > 79 characters) Column: 80 Error code: E501

    reviewbotreviewbot
    david
    1. 
        
    2. extension/reviewbotext/compat/conditions.py (Diff revision 1)
       
       
       
      Show all issues

      This doesn't seem appropriate. We might be running a type checker inside an RB7 tree with this code, plus lower down you do if TYPE_CHECKING

      1. Type checkers get confused when there are try/excepts for imports. I spent a bunch of time on this, and the best pattern is to ensure that we're giving the type checker one path to check based on the upstream. We should be typing against the formal API and not compats, which may be stubs that proxy or something and can lead to worse code.

        The pattern most compatible with type checkers is:

        try:
            # Imports here
        except ImportError:
            if TYPE_CHECKING:
                # Avoid confusion around these types further below.
                assert False
            else:
                # Stubs here
        
    3. Show all issues

      Let's add one more blank line here.

    4. 
        
    maubin
    Review request changed
    Testing Done:
       
    • Ran unit tests.
       
    • Saw Power Pack's new owner and participant user role conditions
      in a tool config page.
       
    • Was able to trigger the tool using the owner condition (the
      participant condition requires more work in a separate change).
      +
    • Tested on RB 7.0.4 dev server too.
    Commits:
    Summary ID
    Enable extensible condition support.
    Review Board is gaining a centralized `ReviewRequestConditionsField` and a central registry that reflects any choices provided by extensions. This change makes use of that, providing a compatibility class for older versions of Review Board that behaves as before. This change pretty much copies /r/14281 where we enabled extensible condition support for RBIntegrations.
    9e4db91a047b82186c42a1888ebbea9d7e1f37ec
    Enable extensible condition support.
    Review Board is gaining a centralized `ReviewRequestConditionsField` and a central registry that reflects any choices provided by extensions. This change makes use of that, providing a compatibility class for older versions of Review Board that behaves as before. This change pretty much copies /r/14281 where we enabled extensible condition support for RBIntegrations.
    8ecf4f902a5de1c1d69ed1110a04e5e80cc21c64

    Checks run (1 failed, 1 succeeded)

    flake8 failed.
    JSHint passed.

    flake8

    maubin
    1. 
        
    2. I still had to keep this here to stop flake8 from complaining about it being undefined below.

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