fix(notes): visible_notes handles a paginated list, not just a queryset - #15593
Merged
Conversation
NoteSerializer.Meta.list_serializer_class = VisibleNotesSerializer, and under
DRF pagination the list serializer's to_representation receives the page as a
plain list (the queryset is evaluated before serialization). VisibleNotesSerializer
only special-cases a Manager, so it passed the list straight to visible_notes(),
which then called list.filter():
AttributeError: 'list' object has no attribute 'filter' -> HTTP 500
Superusers skip the .filter() branch, so this only surfaced for non-superusers
(and the no-user report path), on any paginated notes read.
Apply the same visibility rule whether visible_notes() is given a queryset or an
already-evaluated iterable: keep the ORM filter for querysets, filter in Python
for a list. visible_notes is the single choke-point every read path shares, so
there are no call-site changes.
Regression test: NoteVisibilityTest.test_helper_accepts_an_already_evaluated_list
passes a list (as pagination does) for author / colleague / superuser / no-user
and asserts the same rule the queryset path already enforces.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Move the multi-line docstring summary to the second line so ruff's D213 (multi-line-summary-second-line) passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012CyqgbQCF7dVVMxMXaoEaY
…andles-paginated-list # Conflicts: # dojo/notes/helper.py
Contributor
|
Conflicts have been resolved. A maintainer will review the pull request shortly. |
blakeaowens
approved these changes
Aug 10, 2026
Maffooch
approved these changes
Aug 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
dojo/notes/helper.py::visible_notes()assumed its input is always a queryset and called.filter()on it. ButNoteSerializer.Meta.list_serializer_class = VisibleNotesSerializer, and under DRF pagination the list serializer'sto_representationreceives the page as a plainlist— the queryset is evaluated into a page before serialization.VisibleNotesSerializeronly special-cases aManager, so it passed that list straight tovisible_notes(), which then didlist.filter(...):Superusers skip the
.filter()branch (they receive everything), so this only surfaced for non-superusers and the no-user report-rendering path — on any paginated notes read.Fix
Make
visible_notes()apply the same visibility rule whether it receives a queryset or an already-evaluated iterable:.filter(...),list) → apply the identical rule in Python.visible_notesis the single choke-point every read path (API + UI) shares, so this needs no changes at any call site, and the UI callers (which pass querysets) are unaffected.Tests
Added
unittests/test_apiv2_note_visibility.py::NoteVisibilityTest::test_helper_accepts_an_already_evaluated_list, which passes alist(exactly what pagination hands the serializer) for author / colleague / superuser / no-user and asserts the same visibility the queryset path already enforces. It fails before this change withAttributeError: 'list' object has no attribute 'filter'and passes after.Regression introduced with the private-note visibility work (
VisibleNotesSerializer/visible_notes); the existing helper tests only exercised the queryset form, so the paginated-list path went uncovered.