Summary:

Flesh out the documentation for ReviewRequestManager query functions.

Review Request #12340 — Created June 8, 2022 and submitted June 16, 2022, 7:08 p.m.

Information
Owner:	chipx86
Repository:	Review Board
Branch:	release-5.0.x
Bugs:
Depends On:
Reviewers
Groups:	reviewboard
People:

Description

The query functions in ReviewRequestManager are very old, and most
either lacked documentation or didn't meet current documentation
conventions.

As some work is being done in these functions, and given the importance
of these functions and their behavior, it's time to ensure they meet
documentation standards.

This updates the docstrings for all the query functions, making
behaviors and usage explicit.

No logic has changed.

Testing Done

Built the docs and checked through them. Didn't see anything
obvious or any build errors/warnings related to this file.

Commits

Summary
	Flesh out the documentation for ReviewRequestManager query functions.

Screenshots

Issues

Description	From	Last Updated
Could shorten this to just "The User instance or username."	maubin	June 10, 2022, 7:09 p.m.
Change to "The User instance or username."	maubin	June 10, 2022, 7:09 p.m.
Can change this to "The query object." and move the information about the query into the description	maubin	June 10, 2022, 7:10 p.m.
Should we mention that if a local site is given, it is assumed that the user (if given) has access …	maubin	June 10, 2022, 7:10 p.m.
Add a Raises: section for the django.contrib.auth.models.User.DoesNotExist error that could happen in in self.get_to_or_from_user_query	maubin	June 10, 2022, 7:11 p.m.

flake8 passed.

JSHint passed.

reviewboard/reviews/managers.py (Diff revision 1)

Show all issues

Could shorten this to just "The User instance or username."

chipx86 June 10, 2022, 7:14 p.m.

Think I actually need to elaborate more. In the queries, there's a couple references to users. The viewing user and the owner. I'm going to change these to be explicit about what's expected where.

reviewboard/reviews/managers.py (Diff revision 1)

Show all issues

Change to "The User instance or username."

chipx86 June 10, 2022, 7:14 p.m.

There's an example of one that differs in meaning from the previous.

reviewboard/reviews/managers.py (Diff revision 1)

Show all issues

Can change this to "The query object." and move the information about the query into the description

reviewboard/reviews/managers.py (Diff revision 1)

Show all issues

Should we mention that if a local site is given, it is assumed that the user (if given) has access to that local site (i.e. we don't check whether a user has access to that local site). If so we could include that here under the arg description (and every where else where a local_site arg is used) or maybe under the description for the method where you talk about filtering based on access

reviewboard/reviews/managers.py (Diff revision 1)

Show all issues

Add a Raises: section for the django.contrib.auth.models.User.DoesNotExist error that could happen in in self.get_to_or_from_user_query

Change Summary:


Clarified each user-related argument.
Simplified some redundant information in Returns.
Added a missing Raises.
Updated local_site arguments to mention that the caller should first validate user access.

Commits:

	Summary
-		Flesh out the documentation for ReviewRequestManager query functions.
+		Flesh out the documentation for ReviewRequestManager query functions.

Diff:

Revision 2 (+614 -56)

Show changes

reviewboard/reviews/managers.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

Good call on being more explicit about the user args

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (bbb7b44)