Use .set() when assigning multiple relations.

Review Request #11989 — Created Jan. 24, 2022 and submitted

Information

Review Board
master

Reviewers

Using the assignment operator for setting the far side of a relation
such as ManyToManyField or ForeignKey now generates an error
instructing us to use .set() instead.

In many cases, we were using .set() to set a list of relations on
newly-created objects within tests. In these cases, we can save one
query per use by using .add() instead.

Ran unit tests.

Summary ID
Use .set() when assigning multiple relations.
Using the assignment operator for setting the far side of a relation such as `ManyToManyField` or `ForeignKey` now generates an error instructing us to use `.set()` instead. In many cases, we were using `.set()` to set a list of relations on newly-created objects within tests. In these cases, we can save one query per use by using `.add()` instead. Testing Done: Ran unit tests.
4865f784cdf233789f9e9ccc96528bf589b09ac6
Description From Last Updated

Purely optional work (because I'm sure you're really hoping for more things to do), but something that's stood out for …

chipx86chipx86

E501 line too long (80 > 79 characters)

reviewbotreviewbot

E501 line too long (80 > 79 characters)

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
chipx86
  1. 
      
  2. Purely optional work (because I'm sure you're really hoping for more things to do), but something that's stood out for a long time (and this would feel like the right time to fix it) is that our unit tests are using = (or now .set()) when the probably shouldn't.

    =/set() will first delete all entries on the relation and then replace them. This is one more SQL query than we usually need in tests. We could (ever so slightly) speed up the test suite by using .add() instead in most cases.

    But I'm also fine not worrying about it at all for this change. Mostly putting that out there and on the radar.

  3. 
      
david
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to django-3.2 (8e434f5)
Loading...