Add a link to the latest diff revision in the review request resource
Review Request #8270 — Created July 6, 2016 and submitted
The review request resource now links to the latest diff (as
latest_diffin thelinkssection of the response) when the
associated review request has diffs.
Ran unit tests.
| Description | From | Last Updated |
|---|---|---|
|
Col: 80 E501 line too long (95 > 79 characters) |
 reviewbot
|
|
|
A useful method, but this is going to hit the database per item. Instead, I'd suggest updating get_queryset to do … |
 chipx86
|
|
|
We should also do a self.assertNumQueries with this. I'd expect the value to be (old value) + 1, with the … |
chipx86
|
|
|
Too many blank lines. |
chipx86
|
|
|
Col: 80 E501 line too long (95 > 79 characters) |
reviewbot
|
|
|
Just to verify, this isn't doing any extra queryies, because of the prefetch_related, right? Can we add a comment here … |
chipx86
|
|
|
Can we put request on its own line? In this particular case, it helps the argument from getting a bit … |
chipx86
|
|
|
Col: 80 E501 line too long (95 > 79 characters) |
reviewbot
|
-
-
A useful method, but this is going to hit the database per item. Instead, I'd suggest updating
get_querysetto do a prefetch_related for DiffSet. (I wish we had the newerPrefetchstuff to limit fields and be smarter about queries, but alas). I think this would have to look something like:queryset = queryset.select_related('diffset_history').prefetch_related('diffsets')Then instead of
get_latest_diffset, we'd need to grab the last diff inreview_request.diffset_history.diffsets.all().Something like that.
-
We should also do a
self.assertNumQuerieswith this. I'd expect the value to be (old value) + 1, with the above query. -
- Change Summary:
-
Added a unit test to assert number of queries.
Rewrote db interaction to use fewer queries. - Commit:
-
2193cbc15af4a52682093de6b034d92c8c02334c084eb4c34f7372104392c2dd1666f0cd8584f806
-
Tool: Pyflakes Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py Tool: PEP8 Style Checker Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py -
-
-
Just to verify, this isn't doing any extra queryies, because of the
prefetch_related, right? Can we add a comment here just mentioning that we already have this data and it's not a new query?Reason being that otherwise,
diffsets.latest().only('revision')would be a better query, if we didn't already have the data. -
Can we put
requeston its own line? In this particular case, it helps the argument from getting a bit lost in the other noise.
- Change Summary:
-
Address Christian's issues.
- Commit:
-
084eb4c34f7372104392c2dd1666f0cd8584f8069c5f21c3579451480ab7199884cb723d4b4907d9
Tool: Pyflakes Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py Tool: PEP8 Style Checker Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py