Introduce new file attachment and screenshot counters for review requests.

 






 

 


  Review Request #9242
  —
  Created Oct. 3, 2017 and submitted 

 


















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  release-3.0.x
 









 
 

  
 









 
 

  
 









 
 

  fc2d939...
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
When viewing or publishing review requests, we perform a series of

lookups on the file attachments and screenshots (active and inactive) on

the review request and on the draft. This can be up to 8 queries that

we don't really want in most cases (screenshots are never used anymore,

but remain supported for old review requests), but we had no good way of

choosing whether to perform queries for them.


ReviewRequest and ReviewRequestDraft now have new

RelationCounterFields that track how many active and inactive file

attachments and screenshots there are. This allows code to simply check

the populated counters instead of performing a series of queries,

improving performance across the product and all but guaranteeing that

we won't look up screenshots anymore.

 








  


 
 

  
Unit tests pass.


Checked existing drafts and review requests and saw that the counters

were populated to the correct values.


Created a new draft and saw that it had the correct counters from the

review request.


Uploaded new file attachments and saw that the draft's counters were

updated correctly.


Deleted a file attachment and saw that the draft's counters were updated

correctly.


Published the draft. Saw that the review request's counters were updated

correctly.

 







 









 





  








 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
E501 line too long (81 > 79 characters)

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
There's not a lot of benefit to returning a generator here because sorted() has to create the list. In fact, …

      		
      
       daviddavid
      
      
       
      
     




     

      
       
       
E303 too many blank lines (2)

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
E127 continuation line over-indented for visual indent

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
E127 continuation line over-indented for visual indent

      		
      
       reviewbotreviewbot
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/reviews/models/base_review_request_details.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    There's not a lot of benefit to returning a generator here because sorted() has to create the list. In fact, if someone uses this who wants a list we'd be creating it, iterating it, and creating a second. Better to just return sorted like before.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      It was more to keep the consistency. I'd either like to use lists for both or generators for both.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      Your call but it seems silly to prioritize consistency here, where it doesn't really matter to callers.
      
 
      

      3. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to release-3.0.x (4602f42)