Support users in multiple review groups in API responses
Review Request #8458 — Created Oct. 12, 2016 and submitted
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 amodel_parent_key
that was a many-to-
many field (and therefore the reverse relation was not unique),
resulting in aMultipleObjectsReturned
exception. This has been fixed
by customizing how we retrieve the parent object IDs.
Ran unit tests.
Description | From | Last Updated |
---|---|---|
"many-to-many" |
chipx86 | |
"have to manually" |
chipx86 | |
So as mentioned on Slack, this will result in an extra query per result (the Group.objects.get). It looks like the … |
chipx86 | |
undefined name 'parent_id_key' |
reviewbot | |
undefined name 'parent_id_key' |
reviewbot | |
This should check the response for stat=ok, item count, and the expected user. |
chipx86 |
-
-
-
-
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 inReviewGroupResource
. 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 amodel_parent_key
). Then it will populate an entry in the ID map forparent.uri_object_key
->parent_obj[parent.model_object_key]
. In the case of a Review Group, that'd begroup_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], }
-
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
-
-
-
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
- Change Summary:
-
Addressed Christian's issue
- Commit:
-
9b47c6f75af52a946e322bf3dddfbe9582454be04d6411efa2d99445865c9d738bbaf6ddf0d0791c