Set renderedInline on the AbstractReviewable model.

 






 

 


  Review Request #14627
  —
  Created Oct. 2, 2025 and updated 

 




















 

  

   Information

  





 
 

  maubin
 









 
 

  Review Board
 









 
 

  release-7.1.x
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
We have a renderedInline attribute on both the AbstractReviewable model

and AbstractReviewableView JS objects for our review UIs. This

attribute says whether the review UI is being rendered inline or not. We

were properly setting this on AbstractReviewableView, however we

weren't setting it on AbstractReviewable. This change fixes that.


We also add a ReviewUI._inline attribute that gets set along with

the request attribute. This way any method on the review UI can

access the inline information.

 








  


 
 

  
    

  • Confirmed that the attribute is set on the AbstractReviewable,
    
  viewed an inline review UI and a standalone one.
    • 

  • Confirmed that the attribute is still properly set on
    
AbstractReviewableView.
    • 

  • Ran unit tests.
    • 


 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Set renderedInline on the AbstractReviewable model.
      
We have a `renderedInline` attribute on both the `AbstractReviewable` model
and `AbstractReviewableView` JS objects for our review UIs. This
attribute says whether the review UI is being rendered inline or not. We
were properly setting this on `AbstractReviewableView`, however we
weren't setting it on `AbstractReviewable`. This change fixes that.

     


    		
    f34d7a1102570694065c118cc497d99f56fa1ca0

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
I don't think we need a version changed comment here. This is a bug fix rather than an API change.

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
Seems like a better fix would be to include this in the data returned from ReviewUI.get_js_model_data

      		
      
       daviddavid
      
      
       
      
     


    
   

  

 







 






  

 



















 




 




 














 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david



    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/static/rb/js/reviews/models/abstractReviewableModel.ts
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    I don't think we need a version changed comment here. This is a bug fix rather than an API change.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/templates/reviews/ui/default.html
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Seems like a better fix would be to include this in the data returned from ReviewUI.get_js_model_data
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I prefered this fix because we have the inline information readily available in ReviewUI.build_render_context, and we do the same thing when initializing the JS view class a few lines above this.
      

      If I wanted to include it in ReviewUI.get_js_model_data, I'd have to pull the inline information out of the request again in there, or set a ReviewUI._inline variable when we pull the info out of the request in  ReviewUI.render_to_response. I suppose it would be a good idea to have a ReviewUI._inline variable, so that any methods on the review UI can easily access the inline information. Thoughts?
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      It's probably best to have that state stored, although it does feel slightly icky because whether we're rendering inline is theoretically only known for the duration of the render_to_* method. In practice, a file attachment is either a regular attachment or it's connected to a diff, so there's no actual case where we'd have that state change, even if the ReviewUI lives across multiple requests.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      Right. Well even if the inline state changed across multiple requests, we'd update _inline with the latest value from the request anyways (like how we store the request on the Review UI too). I'll go with that and update this.
      
 
      

      4. 


 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 



 

  

   




maubin




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
    

  • Created a ReviewUI._inline attribute.
    • 

  • Sets inline information through ReviewUI.get_js_view_data and get_js_model_data.
    • 


   

  








  

   

    Description:
   

   

    
     
We have a renderedInline attribute on both the AbstractReviewable model
     and AbstractReviewableView JS objects for our review UIs. This
     attribute says whether the review UI is being rendered inline or not. We
     were properly setting this on AbstractReviewableView, however we
     weren't setting it on AbstractReviewable. This change fixes that.
   + 

   + 
We also add a ReviewUI._inline attribute that gets set along with
   + the request attribute. This way any method on the review UI can
   + access the inline information.

   

  





  

   

    Testing Done:
   

   

    
 ~   
Confirmed that the attribute is set on the AbstractReviewable,
 ~   viewed an inline review UI and a standalone one.
   ~ 
  • Confirmed that the attribute is set on the AbstractReviewable,
    
  viewed an inline review UI and a standalone one.
   ~ 
  • Confirmed that the attribute is still properly set on
    
AbstractReviewableView.
   + 
  • Ran unit tests.

   

  





  

   

    Commits:
   

   

    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Set renderedInline on the AbstractReviewable model.
      
We have a `renderedInline` attribute on both the `AbstractReviewable` model
and `AbstractReviewableView` JS objects for our review UIs. This
attribute says whether the review UI is being rendered inline or not. We
were properly setting this on `AbstractReviewableView`, however we
weren't setting it on `AbstractReviewable`. This change fixes that.

     


    		
    467fac07bcea53144fea3c6f8516a61defdbe4cf

   




   


    

    

     

      Set renderedInline on the AbstractReviewable model.
      
We have a `renderedInline` attribute on both the `AbstractReviewable` model
and `AbstractReviewableView` JS objects for our review UIs. This
attribute says whether the review UI is being rendered inline or not. We
were properly setting this on `AbstractReviewableView`, however we
weren't setting it on `AbstractReviewable`. This change fixes that.

     


    		
    f34d7a1102570694065c118cc497d99f56fa1ca0

   



  
 







   

  





  

   

    Diff:
   

   

    
 Revision 2 (+54 -44)
Show changes
 
    reviewboard/reviews/tests/test_review_ui.py 
    reviewboard/reviews/ui/base.py 
    reviewboard/static/rb/js/reviews/models/abstractReviewableModel.ts 
    reviewboard/templates/reviews/ui/default.html 
 

   

  











 

  
Checks run (2 succeeded)



  

 flake8
 passed.
 





  

 JSHint
 passed.
 





 















  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david



    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3.