Fix REST API for RepositoryInfoResource

Review Request #1631 — Created June 1, 2010 and submitted

Information

Review Board

Reviewers

Accessing a repository's info through the REST API was resulting in an exception due to a missing argument to get_object(). 
RepositoryInfoResource was also missing some required variables that get_object() needed.
Added a unit test for RepositoryInfoResource
mike_conley
  1. Nice work - just one note; see below.
  2. reviewboard/webapi/resources.py (Diff revision 1)
     
     
    I could be wrong here (still new myself to the WebAPI code), but since Repository is not actually a child of another resource, I think we can omit this line.  
    
    I made the change on my end, still seems to work alright.
    1. I added this based on this line in the djblets documentation under "Parents and URLs" in djblets/webapi/resources.py:
      
      "Child objects should set the ``model_parent_key`` variable to the field name of the object's parent in the resource hierarchy."
      
      RepositoryResource has repository_info_resource in its item_child_resources array, so I tried to do what other item_child_resource objects did with their model_parent_key value. The value here probably doesn't matter because get_parent_object( ) is never called. Honestly, it was just a stab in the dark, so hopefully chipx86 can clear things up here.
    2. Gah- after re-reading my comment and your comment it makes sense. This is talking about the Model hierarchy and not the Resource hierarchy. 
      You're right: omitting this line should be fine.
  3. 
      
chipx86
  1. 
      
  2. reviewboard/webapi/resources.py (Diff revision 1)
     
     
    Yeah, this probably isn't needed.
  3. reviewboard/webapi/resources.py (Diff revision 1)
     
     
    Actually, this should probably do:
    
    repository = repository_resource.get_object(request, *args, **kwargs)
    
    Then we don't need the model above or the uri_object_key. This file would only have a single line.
  4. reviewboard/webapi/tests.py (Diff revision 1)
     
     
    This should use the same format as other unit tests for dynamic parts of the path: "<id>"
  5. reviewboard/webapi/tests.py (Diff revision 1)
     
     
    I'd prefer not hard-coding the number for this test. Instead, do a query first for a Repository entry using the SVNTool. Then plug that ID in and reuse that repository for the check below. That way, if the fixtures change and the Repository we can test with has a new ID, we're not broken by it.
  6. reviewboard/webapi/tests.py (Diff revision 1)
     
     
     
    The parameters should be aligned. They can both be on their own lines, indented 3 spaces from "self.assertEqual"
  7. 
      
FA
mike_conley
  1. Jacob:
    
    Just one last small thing from me - see below,
    
    -Mike
  2. reviewboard/webapi/tests.py (Diff revision 2)
     
     
    Actually, since RepositoryResourceTests inherits from BaseWebAPITestCase, I think you get the repo for free with self.repository. (See reviewboard/webapi/tests.py lines 36 - 39).
    
    So you can omit this line, and below:
    
    rsp = self.apiGet(... % self.repository.pk)
    ...
    self.assertEqual(rsp['info'], self.repository.get_scmtool().get_repository_info())
    
    Tested it locally, seems to work.
  3. 
      
FA
Review request changed
Change Summary:
Integrating Mike's feedback.
mike_conley
  1. Looks good to me.  Nice work.
  2. 
      
chipx86
  1. Looks good. Committed to master (431d025)
  2.