Summary

Clean up the code for a lot of our centralized query logic.

Review Request #12339 — Created June 3, 2022 and submitted June 10, 2022, 7:07 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

release-5.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

This is a basic cleanup change that improves the readability of several
of the larger query-building functions in our model managers. This
mainly centers around:

Consistent naming (q vs. queryset instead of query being used
for either or qs sometimes being used).
|= and &= instead of q = q | or q = q &.
Doing as much with Q() objects as possible before calling methods
like filter(...).
Starting with a Q() object before any conditional logic, instead of
starting fresh inside of conditionals, to avoid potential issues with
logic being overridden unintentionally.
General formatting improvements to help readability of some logic
when combining Q() objects.
Using our modern chaining-like formatting for multi-line queryset
conditionals (chaining filter() and order_by() across lines, for
instance).

This helps ensure more consistent code going forward, and lays a
foundation for some query optimization work that is coming.

Testing Done

All unit tests pass.

Commits

Summary	ID
Clean up the code for a lot of our centralized query logic. This is a basic cleanup change that improves the readability of several of the larger query-building functions in our model managers. This mainly centers around: 1. Consistent naming (`q` vs. `queryset` instead of `query` being used for either or `qs` sometimes being used). 2. `\|=` and `&=` instead of `q = q \|` or `q = q &`. 3. Doing as much with `Q()` objects as possible before calling methods like `filter(...)`. 4. Starting with a `Q()` object before any conditional logic, instead of starting fresh inside of conditionals, to avoid potential issues with logic being overridden unintentionally. 5. General formatting improvements to help readability of some logic when combining `Q()` objects. 6. Using our modern chaining-like formatting for multi-line queryset conditionals (chaining `filter()` and `order_by()` across lines, for instance). This helps ensure more consistent code going forward, and lays a foundation for some query optimization work that is coming.	dded5eb906a36ae94de036e04bc95fbc7251ba22

Summary

Clean up the code for a lot of our centralized query logic.

This is a basic cleanup change that improves the readability of several of the larger query-building functions in our model managers. This mainly centers around: 1. Consistent naming (`q` vs. `queryset` instead of `query` being used for either or `qs` sometimes being used). 2. `|=` and `&=` instead of `q = q |` or `q = q &`. 3. Doing as much with `Q()` objects as possible before calling methods like `filter(...)`. 4. Starting with a `Q()` object before any conditional logic, instead of starting fresh inside of conditionals, to avoid potential issues with logic being overridden unintentionally. 5. General formatting improvements to help readability of some logic when combining `Q()` objects. 6. Using our modern chaining-like formatting for multi-line queryset conditionals (chaining `filter()` and `order_by()` across lines, for instance). This helps ensure more consistent code going forward, and lays a foundation for some query optimization work that is coming.

dded5eb906a36ae94de036e04bc95fbc7251ba22

Issues

Description	From	Last Updated
F821 undefined name 'query'	reviewbot	June 3, 2022, 5:07 p.m.
F841 local variable 'query' is assigned to but never used	reviewbot	June 3, 2022, 5:07 p.m.
can get rid of this since we create q on line 122	maubin	June 8, 2022, 3:05 p.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/reviews/managers.py (Diff revision 1)
Show all issues
```
F821 undefined name 'query'
```

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

Show all issues

F841 local variable 'query' is assigned to but never used

Change Summary:

Added a missing unstaged line.

Commits:

	Summary	ID
	Clean up the code for a lot of our centralized query logic. This is a basic cleanup change that improves the readability of several of the larger query-building functions in our model managers. This mainly centers around: 1. Consistent naming (`q` vs. `queryset` instead of `query` being used for either or `qs` sometimes being used). 2. `\|=` and `&=` instead of `q = q \|` or `q = q &`. 3. Doing as much with `Q()` objects as possible before calling methods like `filter(...)`. 4. Starting with a `Q()` object before any conditional logic, instead of starting fresh inside of conditionals, to avoid potential issues with logic being overridden unintentionally. 5. General formatting improvements to help readability of some logic when combining `Q()` objects. 6. Using our modern chaining-like formatting for multi-line queryset conditionals (chaining `filter()` and `order_by()` across lines, for instance). This helps ensure more consistent code going forward, and lays a foundation for some query optimization work that is coming.	267347a48d77308e92cc0893dffe1696b20a8262
	Clean up the code for a lot of our centralized query logic. This is a basic cleanup change that improves the readability of several of the larger query-building functions in our model managers. This mainly centers around: 1. Consistent naming (`q` vs. `queryset` instead of `query` being used for either or `qs` sometimes being used). 2. `\|=` and `&=` instead of `q = q \|` or `q = q &`. 3. Doing as much with `Q()` objects as possible before calling methods like `filter(...)`. 4. Starting with a `Q()` object before any conditional logic, instead of starting fresh inside of conditionals, to avoid potential issues with logic being overridden unintentionally. 5. General formatting improvements to help readability of some logic when combining `Q()` objects. 6. Using our modern chaining-like formatting for multi-line queryset conditionals (chaining `filter()` and `order_by()` across lines, for instance). This helps ensure more consistent code going forward, and lays a foundation for some query optimization work that is coming.	46fc50c76f7e5c38380e9ef68036b049cb167284

Diff:

Revision 2 (+184 -184)

Show changes

	reviewboard/hostingsvcs/managers.py
	reviewboard/reviews/managers.py
	reviewboard/scmtools/managers.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/reviews/managers.py (Diff revision 2)
Show all issues
```
can get rid of this since we create q on line 122
```

Change Summary:

Removed a now-redundant q = Q().

Commits:

	Summary	ID
	Clean up the code for a lot of our centralized query logic. This is a basic cleanup change that improves the readability of several of the larger query-building functions in our model managers. This mainly centers around: 1. Consistent naming (`q` vs. `queryset` instead of `query` being used for either or `qs` sometimes being used). 2. `\|=` and `&=` instead of `q = q \|` or `q = q &`. 3. Doing as much with `Q()` objects as possible before calling methods like `filter(...)`. 4. Starting with a `Q()` object before any conditional logic, instead of starting fresh inside of conditionals, to avoid potential issues with logic being overridden unintentionally. 5. General formatting improvements to help readability of some logic when combining `Q()` objects. 6. Using our modern chaining-like formatting for multi-line queryset conditionals (chaining `filter()` and `order_by()` across lines, for instance). This helps ensure more consistent code going forward, and lays a foundation for some query optimization work that is coming.	46fc50c76f7e5c38380e9ef68036b049cb167284
	Clean up the code for a lot of our centralized query logic. This is a basic cleanup change that improves the readability of several of the larger query-building functions in our model managers. This mainly centers around: 1. Consistent naming (`q` vs. `queryset` instead of `query` being used for either or `qs` sometimes being used). 2. `\|=` and `&=` instead of `q = q \|` or `q = q &`. 3. Doing as much with `Q()` objects as possible before calling methods like `filter(...)`. 4. Starting with a `Q()` object before any conditional logic, instead of starting fresh inside of conditionals, to avoid potential issues with logic being overridden unintentionally. 5. General formatting improvements to help readability of some logic when combining `Q()` objects. 6. Using our modern chaining-like formatting for multi-line queryset conditionals (chaining `filter()` and `order_by()` across lines, for instance). This helps ensure more consistent code going forward, and lays a foundation for some query optimization work that is coming.	dded5eb906a36ae94de036e04bc95fbc7251ba22

Diff:

Revision 3 (+184 -188)

Show changes

	reviewboard/hostingsvcs/managers.py
	reviewboard/reviews/managers.py
	reviewboard/scmtools/managers.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (6fbfc20)