Merge pull request 'Teach activities.GetFeeds() how to avoid returning duplicates' (#3598) from algernon/forgejo:action-inaction-reaction into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3598
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

models/activities/action.go

@@ -431,14 +431,15 @@ func (a *Action) GetIssueContent(ctx context.Context) string {
 // GetFeedsOptions options for retrieving feeds
 type GetFeedsOptions struct {
 	db.ListOptions
-	RequestedUser   *user_model.User       // the user we want activity for
+	RequestedUser        *user_model.User       // the user we want activity for
-	RequestedTeam   *organization.Team     // the team we want activity for
+	RequestedTeam        *organization.Team     // the team we want activity for
-	RequestedRepo   *repo_model.Repository // the repo we want activity for
+	RequestedRepo        *repo_model.Repository // the repo we want activity for
-	Actor           *user_model.User       // the user viewing the activity
+	Actor                *user_model.User       // the user viewing the activity
-	IncludePrivate  bool                   // include private actions
+	IncludePrivate       bool                   // include private actions
-	OnlyPerformedBy bool                   // only actions performed by requested user
+	OnlyPerformedBy      bool                   // only actions performed by requested user
-	IncludeDeleted  bool                   // include deleted actions
+	OnlyPerformedByActor bool                   // only actions performed by the original actor
-	Date            string                 // the day we want activity for: YYYY-MM-DD
+	IncludeDeleted       bool                   // include deleted actions
+	Date                 string                 // the day we want activity for: YYYY-MM-DD
 }
 
 // GetFeeds returns actions according to the provided options
@@ -481,6 +482,10 @@ func ActivityReadable(user, doer *user_model.User) bool {
 func activityQueryCondition(ctx context.Context, opts GetFeedsOptions) (builder.Cond, error) {
 	cond := builder.NewCond()
 
+	if opts.OnlyPerformedByActor {
+		cond = cond.And(builder.Expr("`action`.user_id = `action`.act_user_id"))
+	}
+
 	if opts.RequestedTeam != nil && opts.RequestedUser == nil {
 		org, err := user_model.GetUserByID(ctx, opts.RequestedTeam.OrgID)
 		if err != nil {

release-notes/8.0.0/fix/3598.md

@@ -0,0 +1 @@
+Fixed an issue that resulted in repository activity feeds (including RSS and Atom feeds) containing repeated activities.

routers/api/v1/repo/repo.go

@@ -1307,11 +1307,12 @@ func ListRepoActivityFeeds(ctx *context.APIContext) {
 	listOptions := utils.GetListOptions(ctx)
 
 	opts := activities_model.GetFeedsOptions{
-		RequestedRepo:  ctx.Repo.Repository,
+		RequestedRepo:        ctx.Repo.Repository,
-		Actor:          ctx.Doer,
+		OnlyPerformedByActor: true,
-		IncludePrivate: true,
+		Actor:                ctx.Doer,
-		Date:           ctx.FormString("date"),
+		IncludePrivate:       true,
-		ListOptions:    listOptions,
+		Date:                 ctx.FormString("date"),
+		ListOptions:          listOptions,
 	}
 
 	feeds, count, err := activities_model.GetFeeds(ctx, opts)

routers/web/feed/repo.go

@@ -16,10 +16,11 @@ import (
 // ShowRepoFeed shows user activity on the repo as RSS / Atom feed
 func ShowRepoFeed(ctx *context.Context, repo *repo_model.Repository, formatType string) {
 	actions, _, err := activities_model.GetFeeds(ctx, activities_model.GetFeedsOptions{
-		RequestedRepo:  repo,
+		OnlyPerformedByActor: true,
-		Actor:          ctx.Doer,
+		RequestedRepo:        repo,
-		IncludePrivate: true,
+		Actor:                ctx.Doer,
-		Date:           ctx.FormString("date"),
+		IncludePrivate:       true,
+		Date:                 ctx.FormString("date"),
 	})
 	if err != nil {
 		ctx.ServerError("GetFeeds", err)

tests/integration/api_repo_activities_test.go

@@ -0,0 +1,88 @@
+// Copyright 2024 The Forgejo Authors c/o Codeberg e.V.. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	auth_model "code.gitea.io/gitea/models/auth"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/tests"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAPIRepoActivitiyFeeds(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
+	repo, _, f := CreateDeclarativeRepoWithOptions(t, owner, DeclarativeRepoOptions{})
+	defer f()
+
+	feedURL := fmt.Sprintf("/api/v1/repos/%s/activities/feeds", repo.FullName())
+	assertAndReturnActivities := func(t *testing.T, length int) []api.Activity {
+		t.Helper()
+
+		req := NewRequest(t, "GET", feedURL)
+		resp := MakeRequest(t, req, http.StatusOK)
+		var activities []api.Activity
+		DecodeJSON(t, resp, &activities)
+
+		assert.Len(t, activities, length)
+
+		return activities
+	}
+	createIssue := func(t *testing.T) {
+		session := loginUser(t, owner.Name)
+		token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue)
+		urlStr := fmt.Sprintf("/api/v1/repos/%s/issues?state=all", repo.FullName())
+		req := NewRequestWithJSON(t, "POST", urlStr, &api.CreateIssueOption{
+			Title: "ActivityFeed test",
+			Body:  "Nothing to see here!",
+		}).AddTokenAuth(token)
+		MakeRequest(t, req, http.StatusCreated)
+	}
+
+	t.Run("repo creation", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		// Upon repo creation, there's a single activity.
+		assertAndReturnActivities(t, 1)
+	})
+
+	t.Run("single watcher, single issue", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		// After creating an issue, we'll have two activities.
+		createIssue(t)
+		assertAndReturnActivities(t, 2)
+	})
+
+	t.Run("a new watcher, no new activities", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		watcher := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+		watcherSession := loginUser(t, watcher.Name)
+		watcherToken := getTokenForLoggedInUser(t, watcherSession, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeReadUser)
+
+		req := NewRequest(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/subscription", repo.FullName())).
+			AddTokenAuth(watcherToken)
+		MakeRequest(t, req, http.StatusOK)
+
+		assertAndReturnActivities(t, 2)
+	})
+
+	t.Run("multiple watchers, new issue", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		// After creating a second issue, we'll have three activities, even
+		// though we have multiple watchers.
+		createIssue(t)
+		assertAndReturnActivities(t, 3)
+	})
+}