Make accessibility checks more robust for Reviews and Comments queries
Review Request #12424 — Created June 29, 2022 and submitted — Latest diff uploaded
Recently, it was discovered that when querying for reviews there may be
duplicateQ(public=True)
's that appear in the queries to fetch the reviews.
The cause of this duplication lies inReviewManager
's_query
method, which
handles the logic for querying for reviews. If an unauthenticated or anonymous
user was querying for the reviews, thepublic
query field would be set twice,
once to thepublic
value that was passed and once toTrue
, with the latter
ensuring that only public reviews get returned to unauthenticated/anonymous
users. This however makes the query less efficient and can lead to the wrong
results being included in the returned queryset if aFalse
value forpublic
was passed in.To fix this, we ensure that the
public
query field is only set once, for any
type of user, inReviewManager._query
. This fix was applied to
CommentManager._query
as well.This change also adds a
public
parameter toReviewManager.accessible
to
ensure that thepublic
parameter inReviewManager._query
handles all of
the logic surrounding public and unpublished reviews. Previously, you would
have to set thepublic
query field using theextra_query
parameter in
ReviewManager.accessible
, which could also lead to duplicate or
conflictingQ(public=
queries.Finally, this change also improves the tests for the
ReviewManager
and
CommentManager
. The tests are more clear, organized and well documented,
they useassertQueries
, and have more comprehensive test coverage surrounding
access control.
- Added new unit tests for
ReviewManager.accessible
with setting the
public
parameter. - Ran all unit tests with success.