omit deleted comment threads (#901)

app/api/controller/pullreq/activity_list.go

@@ -31,37 +31,78 @@ func (c *Controller) ActivityList(
 	repoRef string,
 	prNum int64,
 	filter *types.PullReqActivityFilter,
-) ([]*types.PullReqActivity, int64, error) {
+) ([]*types.PullReqActivity, error) {
 	repo, err := c.getRepoCheckAccess(ctx, session, repoRef, enum.PermissionRepoView)
 	if err != nil {
-		return nil, 0, fmt.Errorf("failed to acquire access to repo: %w", err)
+		return nil, fmt.Errorf("failed to acquire access to repo: %w", err)
 	}
 
 	pr, err := c.pullreqStore.FindByNumber(ctx, repo.ID, prNum)
 	if err != nil {
-		return nil, 0, fmt.Errorf("failed to find pull request by number: %w", err)
+		return nil, fmt.Errorf("failed to find pull request by number: %w", err)
 	}
 
 	list, err := c.activityStore.List(ctx, pr.ID, filter)
 	if err != nil {
-		return nil, 0, fmt.Errorf("failed to list pull requests activities: %w", err)
+		return nil, fmt.Errorf("failed to list pull requests activities: %w", err)
 	}
 
-	// the function returns deleted comments, but it removes their content
-	for _, act := range list {
-		if act.Deleted != nil {
-			act.Text = ""
+	list = removeDeletedComments(list)
+
+	return list, nil
+}
+
+func allCommentsDeleted(comments []*types.PullReqActivity) bool {
+	for _, comment := range comments {
+		if comment.Deleted == nil {
+			return false
 		}
 	}
-
-	if filter.Limit == 0 {
-		return list, int64(len(list)), nil
-	}
-
-	count, err := c.activityStore.Count(ctx, pr.ID, filter)
-	if err != nil {
-		return nil, 0, fmt.Errorf("failed to count pull request activities: %w", err)
-	}
-
-	return list, count, nil
+	return true
+}
+
+// removeDeletedComments removes all (ordinary comment and change comment) threads
+// (the top level comment and all replies to it), but only if all comments
+// in the thread are deleted. Just one non-deleted reply in a thread will cause
+// the entire thread to be included in the resulting list.
+func removeDeletedComments(list []*types.PullReqActivity) []*types.PullReqActivity {
+	var (
+		threadIdx int
+		threadLen int
+		listIdx   int
+	)
+
+	inspectThread := func() {
+		if threadLen > 0 && !allCommentsDeleted(list[threadIdx:threadIdx+threadLen]) {
+			copy(list[listIdx:listIdx+threadLen], list[threadIdx:threadIdx+threadLen])
+			listIdx += threadLen
+		}
+		threadLen = 0
+	}
+
+	for i, act := range list {
+		if act.Deleted != nil {
+			act.Text = "" // return deleted comments, but remove their content
+		}
+
+		if act.Kind == enum.PullReqActivityKindComment || act.Kind == enum.PullReqActivityKindChangeComment {
+			if threadLen == 0 || list[threadIdx].Order != act.Order {
+				inspectThread()
+				threadIdx = i
+				threadLen = 1
+			} else {
+				threadLen++
+			}
+			continue
+		}
+
+		inspectThread()
+
+		list[listIdx] = act
+		listIdx++
+	}
+
+	inspectThread()
+
+	return list[:listIdx]
 }

app/api/controller/pullreq/activity_list_test.go

@@ -0,0 +1,126 @@
+// Copyright 2023 Harness, Inc.
+//
+// Licensed 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 pullreq
+
+import (
+	"testing"
+
+	"github.com/harness/gitness/types"
+	"github.com/harness/gitness/types/enum"
+
+	"golang.org/x/exp/slices"
+)
+
+func TestRemoveDeletedComments(t *testing.T) {
+	type activity struct {
+		id       int64
+		kind     enum.PullReqActivityKind
+		order    int64
+		subOrder int64
+		deleted  *int64
+	}
+
+	var n int64
+	d := &n
+
+	tests := []struct {
+		name  string
+		input []activity
+		want  []int64
+	}{
+		{
+			name: "nothing-deleted",
+			input: []activity{
+				{id: 1, kind: enum.PullReqActivityKindComment, order: 0, subOrder: 0, deleted: nil},
+				{id: 2, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 0, deleted: nil},
+				{id: 3, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 1, deleted: nil},
+				{id: 4, kind: enum.PullReqActivityKindComment, order: 2, subOrder: 0, deleted: nil},
+				{id: 5, kind: enum.PullReqActivityKindComment, order: 2, subOrder: 1, deleted: nil},
+			},
+			want: []int64{1, 2, 3, 4, 5},
+		},
+		{
+			name: "deleted-thread",
+			input: []activity{
+				{id: 1, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 0, deleted: d},
+				{id: 2, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 1, deleted: d},
+				{id: 3, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 2, deleted: d},
+			},
+			want: []int64{},
+		},
+		{
+			name: "deleted-top-level-replies-not",
+			input: []activity{
+				{id: 1, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 0, deleted: d},
+				{id: 2, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 1, deleted: nil},
+				{id: 3, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 2, deleted: d},
+			},
+			want: []int64{1, 2, 3},
+		},
+		{
+			name: "deleted-all-replies",
+			input: []activity{
+				{id: 1, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 0, deleted: nil},
+				{id: 2, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 1, deleted: d},
+				{id: 3, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 2, deleted: d},
+			},
+			want: []int64{1, 2, 3},
+		},
+		{
+			name: "complex",
+			input: []activity{
+				// kind=system, deleted, must not be removed
+				{id: 1, kind: enum.PullReqActivityKindSystem, order: 0, subOrder: 0, deleted: d},
+				// thread size=1, not deleted
+				{id: 2, kind: enum.PullReqActivityKindComment, order: 1, subOrder: 0, deleted: nil},
+				// thread size=1, deleted
+				{id: 3, kind: enum.PullReqActivityKindComment, order: 2, subOrder: 0, deleted: d},
+				// kind=system, not deleted, must not be removed
+				{id: 4, kind: enum.PullReqActivityKindSystem, order: 3, subOrder: 0, deleted: nil},
+				// thread size=2, not deleted
+				{id: 5, kind: enum.PullReqActivityKindComment, order: 4, subOrder: 0, deleted: nil},
+				{id: 6, kind: enum.PullReqActivityKindComment, order: 4, subOrder: 1, deleted: d},
+				// thread size=2, change comment, deleted
+				{id: 7, kind: enum.PullReqActivityKindChangeComment, order: 5, subOrder: 0, deleted: d},
+				{id: 8, kind: enum.PullReqActivityKindChangeComment, order: 5, subOrder: 1, deleted: d},
+			},
+			want: []int64{1, 2, 4, 5, 6},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			list := make([]*types.PullReqActivity, len(test.input))
+			for i := range test.input {
+				list[i] = &types.PullReqActivity{
+					ID:       test.input[i].id,
+					Kind:     test.input[i].kind,
+					Order:    test.input[i].order,
+					SubOrder: test.input[i].subOrder,
+					Deleted:  test.input[i].deleted,
+				}
+			}
+
+			var got []int64
+			for _, act := range removeDeletedComments(list) {
+				got = append(got, act.ID)
+			}
+
+			if !slices.Equal(test.want, got) {
+				t.Errorf("want=%v got=%v", test.want, got)
+			}
+		})
+	}
+}

app/api/handler/pullreq/activity_list.go

@@ -46,13 +46,12 @@ func HandleListActivities(pullreqCtrl *pullreq.Controller) http.HandlerFunc {
 			return
 		}
 
-		list, total, err := pullreqCtrl.ActivityList(ctx, session, repoRef, pullreqNumber, filter)
+		list, err := pullreqCtrl.ActivityList(ctx, session, repoRef, pullreqNumber, filter)
 		if err != nil {
 			render.TranslatedUserError(w, err)
 			return
 		}
 
-		render.PaginationLimit(r, w, int(total))
 		render.JSON(w, http.StatusOK, list)
 	}
 }