Repository navigation
fix: filter personal comment page by target visibility before pagination - #1648
Open
ChangkeunJ wants to merge 1 commit into
Open
ChangkeunJ wants to merge 1 commit into
ChangkeunJ wants to merge 1 commit into
Conversation
This branch has not been deployed
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
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.
Fixes #1645
Proposed Changes
GetPersonalCommentPageto the comment repository. For non-admin viewers it joins the target question or answer (plus the parent question for answers) and filters out targets the viewer cannot see before pagination, mirroringschema.SimpleObjectInfo.CheckVisibility: a question target must be available and shown or owned by the viewer, an answer target must be available or owned by the viewer, and its parent question must be visible the same way.GetCommentPersonalWithPagenow uses it, so both the page content andcountreflect only the visible comments. In the issue's example, page 1 returns C withcount: 1instead of an empty list withcount: 3, and page 2 is empty withcount: 1.CommentQuery.GetOrderBycolumns with the table name so the joined query stays unambiguous; the existing callers without joins are unaffected.personal_comment_visibility_test.go) that reproduce the issue scenario against a real database, plus answer-target cases: deleted answer, hidden parent question, and the owner and admin/moderator overrides.Verified with
go test ./internal/repo/repo_test(sqlite),go build ./...,go vetandgolangci-lint runon the touched packages. The new tests fail against the old behavior (unfiltered page withtotal = 3) and pass with the fix.