Support users in multiple review groups in API responses

Review Request #8458 - Created Oct. 12, 2016 and submitted

Barret Rennie
Review Board
release-2.0.x
4382
8449
8457
4d6411e...
reviewboard

The patch that fixed the review group user resource links broke when a
user was a member of more than one group (which is a likely occurance).
This was because we were using a model_parent_key that was a many-to-
many field (and therefore the reverse relation was not unique),
resulting in a MultipleObjectsReturned exception. This has been fixed
by customizing how we retrieve the parent object IDs.

Ran unit tests.

  • 0
  • 0
  • 6
  • 0
  • 6
Description From Last Updated
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
  2. 
      
Christian Hammond
  1. 
      
  2. "have to manually"

  3. reviewboard/webapi/resources/review_group_user.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    So as mentioned on Slack, this will result in an extra query per result (the Group.objects.get). It looks like the reason this is being done is so it can chain up a list of parents, which is fine in a general sense but not necessary for groups (which won't have a parent -- Local Sites don't count, since they're not part of the resource tree).

    We have a few things going for us that the default get_href_parent_ids doesn't have. We know the value used in the URL, and we know the key needed for looking up that value and returning it. We also know there isn't a parent in ReviewGroupResource. So we can short-circuit a lot of the default implementation and do our own thing.

    The default implementation doesn't know which ID we need, and it needs to handle any number of parents and corner cases involving the IDs. So it's going to do more work. It's going to check if the parent resource has a model_parent_key, and if set, it will fetch the object (much like you're doing here) and try to gets its parents (ReviewGroupResource will be a no-op here, since it doesn't have a model_parent_key). Then it will populate an entry in the ID map for parent.uri_object_key -> parent_obj[parent.model_object_key]. In the case of a Review Group, that'd be group_name: child.group.name.

    But since we already know that group name, and are building a specific implementation (as opposed to that general implementation), we don't need to do any of that work.

    I think this whole method can become:

    parent_id_key = self._parent_resource.uri_object_key
    
    return {
        self._parent_resource.uri_object_key: kwargs[parent_id_key],
    }
    
  4. 
      
Barret Rennie
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
  2.  undefined name 'parent_id_key'
    
  3.  undefined name 'parent_id_key'
    
  4. 
      
Barret Rennie
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
  2. 
      
Christian Hammond
  1. 
      
  2. This should check the response for stat=ok, item count, and the expected user.

  3. 
      
Barret Rennie
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/webapi/resources/review_group_user.py
        reviewboard/webapi/tests/test_review_group_user.py
    
    
  2. 
      
Christian Hammond
  1. Ship It!
  2. 
      
Barret Rennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (a516969)
Loading...