Do a lot of preparatory work for adding local-site rooting to the webapi.

 






 

 


  Review Request #1950
  —
  Created Nov. 27, 2010 and submitted

 


















 

  

   Information

  





 
 

  david
 









 
 

  Review Board
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Do a lot of preparatory work for adding local-site rooting to the webapi.

This change adds a bunch of infrastructure to prepare for my next changes which
actually add knowledge of local-site infrastructure and URLs to the API. This
includes:

- New test fixtures.

- Local site checking in manager.accessible() and model.is_accessible_by() for
  Group, LocalSite, Repository, and ReviewRequest.

- A fix for a circular import in webapi/encoder.py (this import was only
  circular when running -only- the webapi tests, for some reason).

- A fix for the to_group_deprecated mess (this actually adds local-site
  knowledge to a bit of the deprecated json API, which wasn't planned yet but
  it's nice to clean it up).

- Add a check_local_site decorator which will be used to check for access to the
  local site before returning data or links.

- Add an override of WebAPIResource.get_href which knows how to serialize links
  including the local-site name.

 








  


 
 

  
Ran unit tests.

 







 









 





  








 









 






  

 








 














 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Overall, this looks good.

One additional thing that I'd like you to do, which is easy and can be done separately. Since we're adding new unit test fixtures, can you update the codebase/unit-tests/fixtures docs and add this one to the list of things to load/dump?
    
  
    
 
    

 
    
  



 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/site/models.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    Potentially, you could have several sites but have them all anonymous. So we should check if anonymous access is enabled for the site before requiring an authenticated user.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      LocalSite has no concept of anonymous access. We can maybe add that later but I don't want to deal with it now.
      
 
      

      2. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/site/tests.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    The assert_ functions are being deprecated in Python 2.7 and I believe in Django. The docs say to just use assertTrue, which should do the right thing.
    
  
    
 
    

 
    
  



 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/webapi/encoder.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    Can we maybe move this import into the class definition? Or will that continue to break things? I think we end up taking a small performance hit every time encode() is called, and that's called quite a lot.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I'm going to change this by moving status_to_string and string_to_status into encoder and importing it from the other places. This unifies the two versions of them that we had, too.
      
 
      

      2. 


 
    


    

 
    

    5. 






  5. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/webapi/resources.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    This should probably go in webapi/decorators.py
    
  
    
 
    

 
    
  



 
    

    6. 






  6. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/webapi/resources.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    If a name is specified but it doesn't exist in the database, this will return None and we'll check if the user has access on the default site. Is that what we want? Feels like we should instead error out with a 404.
    
  
    
 
    

 
    
  



 
    

    7. 






  7. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      reviewboard/webapi/resources.py
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

    
     
    

    
     
    

    
     
    

       
  
    
 
    

   
    
    
    





    
    Will that work for subdirectory installs?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Uh, I really don't know. How does reverse() work with subdirectory installs?
      
 
      

      2. 


 
    


    

 
    

    8. 




  8. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    9. 




  

 
















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  








 
Change Summary:

 
- Fix imports for decorator (found when trying to actually use it)
- Fix get_href to take into account the SITE_ROOT.





 



  

   
Diff:

   
    
 Revision 3 (+220 -82)
Show changes
 
    docs/codebase/unit-tests/fixtures.txt 
    reviewboard/urls.py 
    reviewboard/reviews/managers.py 
    reviewboard/reviews/models.py 
    reviewboard/reviews/tests.py 
    reviewboard/reviews/views.py 
    reviewboard/reviews/fixtures/test_reviewrequests.json 
    reviewboard/scmtools/models.py 
  10 more
 

   		
  



 










  

 
















 





 

  

   




chipx86




   

   







  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Awesome :)
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 









     

    

   

  

  

   
   Loading...