The review_request api endpoint filters by the ID of the repository instead of the name. Should we filter by ID here too for consistency?

It might be worthwhile to add fields for "time-added-to" and "time-added-from" just so that there's more options for date filtering. Should we add these fields?

I haven't dug into the unit testing on this much yet, but making sure we have this on the checklist for this feature...
One thing to make sure of is that we have very extensive test coverage of access controls, making sure that it won't return reviews if:

It's unpublished and you are not the owner
It's on a private repository you don't have access to
It's on an invite-only review group you don't have access to
It's on a Local Site you don't have access to

this doesn't actually add the user to the local site, is there another way that i should be adding the user to the local site?

Ship It!

Many of my comments are similar to the comments API change. There's more context in there for some of the issues opened here.

Can you order these alphabetically?

Small nit, but for consistency, this is usually placed after the main resource definitions (so after added_in in this case).

Can you order these alphabetically?

We should probably stick with public instead of drafts for this, and take it as a boolean.

We can use &= for these.

We use a chaining format to keep these readable, like:

users = (
    Group.objects
    .filter(name=...)
    .values_list('users')
)

Can you update this to do one argument per line? That'll help keep this readable/maintainable going forward.

This should document *args and **kwargs.

Can you order these keys alphabetically?

Since we use a ' in the docstring, and don't use " anywhere, we can use " for the string itself and avoid escaping the '.

This should document the arguments and AssertionError.

For each of these tests, we'll want to asser that we get exactly the reviews we expect, in the order in which we expect them. You can use compare_item to help with this.
This applies to all tests.

Can you update these to use user instead of u? Same elsewhere in tests. This helps keep things consistent and greppable.

Similar, review_request instead of rr.

get_tz_aware_utcnow() is a bit legacy. We should be able to use Django's timezone.now() instead.

We're going to want to avoid using _. I have some details on this in the other API change.

Similarly, we should use review.

Add a blank line between these two.

Let's fix this to just say "Unit tests for RootReviewResource"

Most of my comments are about the test coverage. They're examples, but not the only cases, where we should probably expand our test data. We want to ensure that we never get any results we don't expect, so we should ensure there's enough test data in each test to ensure that we never unintentionally leak wrong information in the API calls.

An optimization I'll be working toward is to avoid these JOINs where possible, and since this code is new, we might as well do this now.
We can fetch the User and Repository up-front (returning self.model.objects.none() if we fail to look those up, since we know we won't get results). Just the ID (using values_list('pk', flat=True)).
We can then pass that ID in as Q(user=<id>) and Q(review_request__repository=<id>).
This will optimistically save two JOINs, improving performance of this API.

This will need to be wrapped in list(...) in order to get the values directly. Otherwise this ends up being a subquery, if we just pass it in.

The *args is off. Missing Args:.

This test should have some public and non-public reviews that aren't accessible by the user, and ensure they don't show up in results.

This test should add a mixture of accessible/inaccessible review requests and accessible/inaccessible reviews, and non-public reviews on accessible review requests, and ensure nothing inaccessible/non-public shows up in results.

This test should add a mixture of accessible/inaccessible review requests and accessible/inaccessible reviews, and public reviews on accessible review requests, and ensure nothing inaccessible/public shows up in results.

This should have some inaccessible repositories and make sure reviews on them don't show up in the results.
Same for all other repository-related tests.
We're also going to need tests that show results only if:

On the repository's user list, or
In a group on the repository's review group list

Same comments as above but for review groups.

Ship It!