fix: filter personal comment page by target visibility before pagination

internal/repo/comment/comment_repo.go

@@ -159,6 +159,48 @@ func (cr *commentRepo) GetCommentPage(ctx context.Context, commentQuery *comment
 	return
 }
 
+// GetPersonalCommentPage get personal comment page
+func (cr *commentRepo) GetPersonalCommentPage(ctx context.Context, commentQuery *comment.CommentQuery) (
+	commentList []*entity.Comment, total int64, err error,
+) {
+	commentList = make([]*entity.Comment, 0)
+
+	session := cr.data.DB.Context(ctx)
+	session.OrderBy(commentQuery.GetOrderBy())
+	session.Where("comment.status = ?", entity.CommentStatusAvailable)
+	session.Where("comment.user_id = ?", commentQuery.UserID)
+
+	if !commentQuery.IsAdminModerator {
+		// Only list comments whose target the viewer can see, following
+		// schema.SimpleObjectInfo.CheckVisibility: a target question must be
+		// available and shown, or owned by the viewer; a target answer must be
+		// available or owned by the viewer, and its parent question must be
+		// visible the same way. Filtering here keeps both the page content and
+		// the total on the visible comments only.
+		session.Join("LEFT", "question AS target_question", "comment.object_id = target_question.id")
+		session.Join("LEFT", "answer AS target_answer", "comment.object_id = target_answer.id")
+		session.Join("LEFT", "question AS parent_question", "target_answer.question_id = parent_question.id")
+		session.Where(
+			"(target_question.id IS NOT NULL AND"+
+				" ((target_question.status NOT IN (?, ?) AND target_question.show != ?) OR target_question.user_id = ?))"+
+				" OR (target_answer.id IS NOT NULL"+
+				" AND (target_answer.status NOT IN (?, ?) OR target_answer.user_id = ?)"+
+				" AND ((parent_question.status NOT IN (?, ?) AND parent_question.show != ?) OR parent_question.user_id = ?))",
+			entity.QuestionStatusDeleted, entity.QuestionStatusPending, entity.QuestionHide,
+			commentQuery.LoginUserID,
+			entity.AnswerStatusDeleted, entity.AnswerStatusPending, commentQuery.LoginUserID,
+			entity.QuestionStatusDeleted, entity.QuestionStatusPending, entity.QuestionHide,
+			commentQuery.LoginUserID,
+		)
+	}
+
+	total, err = pager.Help(commentQuery.Page, commentQuery.PageSize, &commentList, &entity.Comment{}, session)
+	if err != nil {
+		err = errors.InternalServer(reason.DatabaseError).WithError(err).WithStack()
+	}
+	return
+}
+
 // RemoveAllUserComment remove all user comment
 func (cr *commentRepo) RemoveAllUserComment(ctx context.Context, userID string) (err error) {
 	session := cr.data.DB.Context(ctx).Where("user_id = ?", userID)

internal/repo/repo_test/personal_comment_visibility_test.go

@@ -0,0 +1,237 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package repo_test
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/apache/answer/internal/base/pager"
+	"github.com/apache/answer/internal/entity"
+	"github.com/apache/answer/internal/repo/comment"
+	"github.com/apache/answer/internal/repo/question"
+	"github.com/apache/answer/internal/repo/unique"
+	commentService "github.com/apache/answer/internal/service/comment"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// The personal comment list must paginate only the comments whose target is
+// visible to the viewer; filtering after pagination leaves empty pages and an
+// unfiltered count (https://github.com/apache/answer/issues/1645).
+func TestPersonalCommentPageFiltersHiddenQuestions(t *testing.T) {
+	ctx := context.Background()
+	questionRepo := question.NewQuestionRepo(testDataSource, unique.NewUniqueIDRepo(testDataSource))
+	commentRepo := comment.NewCommentRepo(testDataSource, unique.NewUniqueIDRepo(testDataSource))
+
+	const (
+		author        = "8101"
+		stranger      = "8102"
+		questionOwner = "8103"
+	)
+
+	newQuestion := func(title string, show int) *entity.Question {
+		q := &entity.Question{
+			UserID:       questionOwner,
+			Title:        title,
+			OriginalText: title,
+			ParsedText:   title,
+			Status:       entity.QuestionStatusAvailable,
+			Show:         show,
+		}
+		require.NoError(t, questionRepo.AddQuestion(ctx, q))
+		return q
+	}
+
+	// Same scenario as the issue: the author comments in the order C, B, A, so
+	// the newest-first order is A, B, C. Only C's question stays public.
+	hiddenQuestionA := newQuestion("hidden question A", entity.QuestionHide)
+	hiddenQuestionB := newQuestion("hidden question B", entity.QuestionHide)
+	visibleQuestionC := newQuestion("visible question C", entity.QuestionShow)
+
+	baseTime := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC)
+	addComment := func(objectID, questionID string, createdAt time.Time) *entity.Comment {
+		c := &entity.Comment{
+			UserID:       author,
+			ObjectID:     objectID,
+			QuestionID:   questionID,
+			Status:       entity.CommentStatusAvailable,
+			OriginalText: "comment",
+			ParsedText:   "comment",
+		}
+		require.NoError(t, commentRepo.AddComment(ctx, c))
+		// created_at is set by xorm on insert; pin it so the newest-first order
+		// does not depend on insert timing.
+		_, err := testDataSource.DB.Context(ctx).Table(&entity.Comment{}).ID(c.ID).
+			Update(map[string]any{"created_at": createdAt})
+		require.NoError(t, err)
+		return c
+	}
+
+	commentC := addComment(visibleQuestionC.ID, visibleQuestionC.ID, baseTime)
+	commentB := addComment(hiddenQuestionB.ID, hiddenQuestionB.ID, baseTime.Add(time.Hour))
+	commentA := addComment(hiddenQuestionA.ID, hiddenQuestionA.ID, baseTime.Add(2*time.Hour))
+
+	query := func(viewer string, isAdminModerator bool, page int) ([]*entity.Comment, int64) {
+		list, total, err := commentRepo.GetPersonalCommentPage(ctx, &commentService.CommentQuery{
+			PageCond:         pager.PageCond{Page: page, PageSize: 2},
+			UserID:           author,
+			QueryCond:        "created_at",
+			LoginUserID:      viewer,
+			IsAdminModerator: isAdminModerator,
+		})
+		require.NoError(t, err)
+		return list, total
+	}
+
+	// A stranger only sees the comment on the visible question: the first page
+	// contains C and the count is 1, instead of an empty page with count 3.
+	page1, total := query(stranger, false, 1)
+	assert.Equal(t, int64(1), total)
+	require.Len(t, page1, 1)
+	assert.Equal(t, commentC.ID, page1[0].ID)
+
+	page2, total := query(stranger, false, 2)
+	assert.Equal(t, int64(1), total)
+	assert.Empty(t, page2)
+
+	// The owner of the hidden questions sees all three comments.
+	page1, total = query(questionOwner, false, 1)
+	assert.Equal(t, int64(3), total)
+	require.Len(t, page1, 2)
+	assert.Equal(t, commentA.ID, page1[0].ID)
+	assert.Equal(t, commentB.ID, page1[1].ID)
+
+	// So does an admin or moderator.
+	page1, total = query(stranger, true, 1)
+	assert.Equal(t, int64(3), total)
+	require.Len(t, page1, 2)
+	assert.Equal(t, commentA.ID, page1[0].ID)
+	assert.Equal(t, commentB.ID, page1[1].ID)
+}
+
+func TestPersonalCommentPageFiltersRestrictedAnswers(t *testing.T) {
+	ctx := context.Background()
+	questionRepo := question.NewQuestionRepo(testDataSource, unique.NewUniqueIDRepo(testDataSource))
+	commentRepo := comment.NewCommentRepo(testDataSource, unique.NewUniqueIDRepo(testDataSource))
+
+	const (
+		author        = "8201"
+		stranger      = "8202"
+		answerAuthor  = "8204"
+		questionOwner = "8203"
+	)
+
+	newQuestion := func(title string, show int, status int) *entity.Question {
+		q := &entity.Question{
+			UserID:       questionOwner,
+			Title:        title,
+			OriginalText: title,
+			ParsedText:   title,
+			Status:       status,
+			Show:         show,
+		}
+		require.NoError(t, questionRepo.AddQuestion(ctx, q))
+		return q
+	}
+
+	visibleQuestion := newQuestion("visible question", entity.QuestionShow, entity.QuestionStatusAvailable)
+	hiddenQuestion := newQuestion("hidden question", entity.QuestionHide, entity.QuestionStatusAvailable)
+
+	newAnswer := func(q *entity.Question, status int) *entity.Answer {
+		a := &entity.Answer{
+			QuestionID:   q.ID,
+			UserID:       answerAuthor,
+			OriginalText: "answer",
+			ParsedText:   "answer",
+			Status:       status,
+		}
+		_, err := testDataSource.DB.Context(ctx).Insert(a)
+		require.NoError(t, err)
+		return a
+	}
+
+	visibleAnswer := newAnswer(visibleQuestion, entity.AnswerStatusAvailable)
+	answerOnHiddenQuestion := newAnswer(hiddenQuestion, entity.AnswerStatusAvailable)
+	deletedAnswer := newAnswer(visibleQuestion, entity.AnswerStatusDeleted)
+
+	baseTime := time.Date(2024, 2, 1, 0, 0, 0, 0, time.UTC)
+	addComment := func(objectID, questionID string, createdAt time.Time) *entity.Comment {
+		c := &entity.Comment{
+			UserID:       author,
+			ObjectID:     objectID,
+			QuestionID:   questionID,
+			Status:       entity.CommentStatusAvailable,
+			OriginalText: "comment",
+			ParsedText:   "comment",
+		}
+		require.NoError(t, commentRepo.AddComment(ctx, c))
+		_, err := testDataSource.DB.Context(ctx).Table(&entity.Comment{}).ID(c.ID).
+			Update(map[string]any{"created_at": createdAt})
+		require.NoError(t, err)
+		return c
+	}
+
+	commentOnVisible := addComment(visibleAnswer.ID, visibleQuestion.ID, baseTime)
+	commentOnHiddenParent := addComment(answerOnHiddenQuestion.ID, hiddenQuestion.ID, baseTime.Add(time.Hour))
+	commentOnDeleted := addComment(deletedAnswer.ID, visibleQuestion.ID, baseTime.Add(2*time.Hour))
+
+	query := func(viewer string, isAdminModerator bool, page int) ([]*entity.Comment, int64) {
+		list, total, err := commentRepo.GetPersonalCommentPage(ctx, &commentService.CommentQuery{
+			PageCond:         pager.PageCond{Page: page, PageSize: 2},
+			UserID:           author,
+			QueryCond:        "created_at",
+			LoginUserID:      viewer,
+			IsAdminModerator: isAdminModerator,
+		})
+		require.NoError(t, err)
+		return list, total
+	}
+
+	// A stranger only sees the comment on the available answer under a visible
+	// question.
+	page1, total := query(stranger, false, 1)
+	assert.Equal(t, int64(1), total)
+	require.Len(t, page1, 1)
+	assert.Equal(t, commentOnVisible.ID, page1[0].ID)
+
+	// The answer author also sees their comment on the deleted answer, but not
+	// the one under the hidden question they don't own.
+	page1, total = query(answerAuthor, false, 1)
+	assert.Equal(t, int64(2), total)
+	require.Len(t, page1, 2)
+	assert.Equal(t, commentOnDeleted.ID, page1[0].ID)
+	assert.Equal(t, commentOnVisible.ID, page1[1].ID)
+
+	// The owner of the hidden question sees the comment under it instead.
+	page1, total = query(questionOwner, false, 1)
+	assert.Equal(t, int64(2), total)
+	require.Len(t, page1, 2)
+	assert.Equal(t, commentOnHiddenParent.ID, page1[0].ID)
+	assert.Equal(t, commentOnVisible.ID, page1[1].ID)
+
+	// Admins and moderators see all three.
+	page1, total = query(stranger, true, 1)
+	assert.Equal(t, int64(3), total)
+	require.Len(t, page1, 2)
+	assert.Equal(t, commentOnDeleted.ID, page1[0].ID)
+	assert.Equal(t, commentOnHiddenParent.ID, page1[1].ID)
+}

internal/service/comment/comment_service.go

@@ -58,6 +58,8 @@ type CommentRepo interface {
 	GetComment(ctx context.Context, commentID string) (comment *entity.Comment, exist bool, err error)
 	GetCommentPage(ctx context.Context, commentQuery *CommentQuery) (
 		comments []*entity.Comment, total int64, err error)
+	GetPersonalCommentPage(ctx context.Context, commentQuery *CommentQuery) (
+		comments []*entity.Comment, total int64, err error)
 }
 
 type CommentQuery struct {
@@ -68,16 +70,20 @@ type CommentQuery struct {
 	QueryCond string
 	// user id
 	UserID string
+	// the viewer who requests the personal page; targets that this user cannot
+	// see are filtered out before pagination
+	LoginUserID      string
+	IsAdminModerator bool
 }
 
 func (c *CommentQuery) GetOrderBy() string {
 	if c.QueryCond == "vote" {
-		return "vote_count DESC,created_at ASC"
+		return "comment.vote_count DESC,comment.created_at ASC"
 	}
 	if c.QueryCond == "created_at" {
-		return "created_at DESC"
+		return "comment.created_at DESC"
 	}
-	return "created_at ASC"
+	return "comment.created_at ASC"
 }
 
 // CommentService user service
@@ -532,11 +538,13 @@ func (cs *CommentService) GetCommentPersonalWithPage(ctx context.Context, req *s
 	}
 
 	dto := &CommentQuery{
-		PageCond:  pager.PageCond{Page: req.Page, PageSize: req.PageSize},
-		UserID:    req.UserID,
-		QueryCond: "created_at",
+		PageCond:         pager.PageCond{Page: req.Page, PageSize: req.PageSize},
+		UserID:           req.UserID,
+		QueryCond:        "created_at",
+		LoginUserID:      req.LoginUserID,
+		IsAdminModerator: req.IsAdminModerator,
 	}
-	commentList, total, err := cs.commentRepo.GetCommentPage(ctx, dto)
+	commentList, total, err := cs.commentRepo.GetPersonalCommentPage(ctx, dto)
 	if err != nil {
 		return nil, err
 	}