New Datagrid Columns: People and Groups

 






 

 


  Review Request #2290
  —
  Created April 15, 2011 and submitted

 


















 

  

   Information

  





 
 

  philipp
 









 
 

  Review Board
 









 
 

  
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
I extended datagrid.py and realized two new columns for Dashboard: 
Review Groups and Review People. In our case all requests are assigned 
to a team group. Therefore the People column helps my team to see 
which requests are not yet processed. 

Both columns are sortable. In order to allow sorting I had to disable optimization. This will be only done if People or Group is actually used for sorting.
It turned out the current implementation relies on optimization. I had to make sure that columns are always added - not only with the query is optimized.



 








  


 
 

  

 







 









 





  








 









 






  

 








 














 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      So, I'm actually thinking this needs to be simplified to just not allow sorting on these columns. I'm guessing that sorting is something you guys want, but we can't go this route. Turning off optimizations will actually cause the dashboard to totally swamp a server, with a large enough install. We've seen it happen before, and I really don't want to deal with those bugs again :)

If we were to have any sort of advanced sorting, we'd need it to be optimization-friendly, cross-database, and it'd need to live entirely in the djblets datagrids backend code.
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      First off, thanks for your feedback!

I fully agree to you. While implementing the "sortable" feature I discovered some weaknesses / missing features in djblets. However, my intention was to keep changes local and not to change djblets. That's something for more experienced djblets users.

One question about using my implementation: Sorting groups/people columns is used only occasionally by users. Might this already have a heavy impact on performance and stability? What do you think? 

I updated a highly reduced diff. No raw SQL, no sorting, just the two new columns. 
      
 
      

      3. 


 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/reviews/datagrids.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    I'd need to see some testing done about this working with MySQL, PostgreSQL, and sqlite. Though I will say, I'd much prefer to minimize raw SQL here.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/reviews/datagrids.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    This must match the rest of our style.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      OK. I will use something like enable_sorting_on_virtual_columns in future and maybe a shorter version as well ;-)
      
 
      

      2. 


 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/reviews/datagrids.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Make sure all lines are < 80 columns.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      The new diff does not contain such long lines.
      
 
      

      2. 


 
    


    

 
    

    5. 




  5. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 




  

 
















 



 

  

   





 PH





   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
simplified, non-sortable solution

   

  








  

   

    Diff:
   

   

    
 Revision 2 (+28)
Show changes
 
    reviewboard/reviews/datagrids.py 
 

   

  












  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Pushed to master d88d045. Thanks!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3.