Only register action JS models if the action is being rendered.

 






 

 


  Review Request #14800
  —
  Created Feb. 5, 2026 and updated 

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-7.1.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
With the new action framework changes that separate out action model

registration from rendering, we were now registering every single action

on every page, whether or not they were being used. This meant that any

user would end up seeing actions for other pages, including the admin

page, which certainly wouldn't be correct. While there are cases where

an action may be worth registering when not rendered, in most cases we

want to avoid this behavior.


This change tackles this in two ways:


    

  1. 

    All rendered action IDs are tracked in the PageState, making it
    
   easy to determine which actions have been placed on the page in some
    
   way.
    

    2. 

  2. 

    A new BaseAction.should_register() has been added that returns
    
   whether registration of the model should take place. By default,
    
   this will only return True if the action has been rendered (which
    
   has the benefit of inheriting the result of should_render()).
    
   Subclasses can override this to always register, or to limit to
    
   certain URLs or other criteria (but should always respect
    
should_render()).
    

    3. 



With this, actions are now only registered on the pages in which they're

used, by default, without limiting more advanced use cases.

 








  


 
 

  
Unit tests pass.


Verified that admin actions weren't showing up on other pages, but were

showing up in the admin UI.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Only register action JS models if the action is being rendered.
      
With the new action framework changes that separate out action model
registration from rendering, we were now registering every single action
on every page, whether or not they were being used. This meant that any
user would end up seeing actions for other pages, including the admin
page, which certainly wouldn't be correct. While there are cases where
an action may be worth registering when not rendered, in most cases we
want to avoid this behavior.

This change tackles this in two ways:

1. All rendered action IDs are tracked in the `PageState`, making it
   easy to determine which actions have been placed on the page in some
   way.

2. A new `BaseAction.should_register()` has been added that returns
   whether registration of the model should take place. By default,
   this will only return `True` if the action has been rendered (which
   has the benefit of inheriting the result of `should_render()`).
   Subclasses can override this to always register, or to limit to
   certain URLs or other criteria (but should always respect
   `should_render()`).

With this, actions are now only registered on the pages in which they're
used, by default, without limiting more advanced use cases.

     


    		
    118414adf2ae171e631d0b8d5053ea89b41f83e2

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Can you add a test for an action that overrides should_register to always register the action?

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
do not use bare 'except' Column: 9 Error code: E722

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
Perhaps this instead? You use setdefault just below in templatetags/actions.py rendered_action_ids: set[str] = page_state.extra_data.setdefault( 'rb-actions-rendered', set())

      		
      
       daviddavid
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
Fixed a bare except.

   

  








  

   

    Commits:
   

   

    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Only register action JS models if the action is being rendered.
      
With the new action framework changes that separate out action model
registration from rendering, we were now registering every single action
on every page, whether or not they were being used. This meant that any
user would end up seeing actions for other pages, including the admin
page, which certainly wouldn't be correct. While there are cases where
an action may be worth registering when not rendered, in most cases we
want to avoid this behavior.

This change tackles this in two ways:

1. All rendered action IDs are tracked in the `PageState`, making it
   easy to determine which actions have been placed on the page in some
   way.

2. A new `BaseAction.should_register()` has been added that returns
   whether registration of the model should take place. By default,
   this will only return `True` if the action has been rendered (which
   has the benefit of inheriting the result of `should_render()`).
   Subclasses can override this to always register, or to limit to
   certain URLs or other criteria (but should always respect
   `should_render()`).

With this, actions are now only registered on the pages in which they're
used, by default, without limiting more advanced use cases.

     


    		
    335dc50fa6f11ccd1c320574f04c6d28acbe4954

   




   


    

    

     

      Only register action JS models if the action is being rendered.
      
With the new action framework changes that separate out action model
registration from rendering, we were now registering every single action
on every page, whether or not they were being used. This meant that any
user would end up seeing actions for other pages, including the admin
page, which certainly wouldn't be correct. While there are cases where
an action may be worth registering when not rendered, in most cases we
want to avoid this behavior.

This change tackles this in two ways:

1. All rendered action IDs are tracked in the `PageState`, making it
   easy to determine which actions have been placed on the page in some
   way.

2. A new `BaseAction.should_register()` has been added that returns
   whether registration of the model should take place. By default,
   this will only return `True` if the action has been rendered (which
   has the benefit of inheriting the result of `should_render()`).
   Subclasses can override this to always register, or to limit to
   certain URLs or other criteria (but should always respect
   `should_render()`).

With this, actions are now only registered on the pages in which they're
used, by default, without limiting more advanced use cases.

     


    		
    118414adf2ae171e631d0b8d5053ea89b41f83e2

   



  
 







   

  





  

   

    Diff:
   

   

    
 Revision 2 (+306 -2)
Show changes
 
    reviewboard/actions/base.py 
    reviewboard/actions/renderers.py 
    reviewboard/actions/templatetags/actions.py 
    reviewboard/actions/tests/test_base_action.py 
 

   

  











 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.
 





 















  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david



    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 







  2. 
 
    
  
  
    
  
   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Can you add a test for an action that overrides should_register to always register the action?
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 















  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/actions/renderers.py
       


        (Diff revision 2)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Perhaps this instead? You use setdefault just below in templatetags/actions.py
    

    

    rendered_action_ids: set[str] = page_state.extra_data.setdefault(
  'rb-actions-rendered', set())

    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5.