Fix issues publishing when there are multiple groups with ACLs.

Review Request #13399 — Created Nov. 3, 2023 and submitted — Latest diff uploaded

Information

Review Board
release-6.x

Reviewers

We've had a few reports of users who had problems publishing reviews or
replies, and it seems like the cause is duplicate entries being returned
by our accessible object queries used by the batch endpoint. We'd then
do a length comparison to make sure that the length of the fetched
result was the same as the length of the requested set of objects to
publish, and fail with a very unhelpful message. This is most easily
triggered by having multiple invite-only review groups assigned to a
review request.

This change fixes the problem by using Django's in_bulk() method to
aggregate the results by PK. This handles deduplication without having
to add DISTINCT to the query, which can cause performance problems in
big databases on MySQL.

A new unit test has been added which failed with the reported error
before the change, and passes after.

  • Manually tested on both SQLite and MySQL. Saw that it failed before
    the change and succeeded afterward.
  • Ran unit tests.

Diff Revision 3

This is not the most recent revision of the diff. The latest diff is revision 4. See what's changed.

orig
1
2
3
4

Commits

First Last Summary ID Author
Fix issues publishing when there are multiple groups with ACLs.
We've had a few reports of users who had problems publishing reviews or replies, and it seems like the cause is duplicate entries being returned by our accessible object queries used by the batch endpoint. We'd then do a length comparison to make sure that the length of the fetched result was the same as the length of the requested set of objects to publish, and fail with a very unhelpful message. This is most easily triggered by having multiple invite-only review groups assigned to a review request. This change fixes the problem by using Django's `in_bulk()` method to aggregate the results by PK. This handles deduplication without having to add DISTINCT to the query, which can cause performance problems in big databases on MySQL. A new unit test has been added which failed with the reported error before the change, and passes after. Testing Done: - Manually tested on both SQLite and MySQL. Saw that it failed before the change and succeeded afterward. - Ran unit tests.
c852304c3bdf99cb1ed33a96a521187cbdeab754 David Trowbridge
reviewboard/reviews/tests/test_batch_view.py
reviewboard/reviews/views/batch.py
Loading...