diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index bd64680c8c787..5cb02fb9f47e5 100644 --- a/models/fixtures/comment.yml +++ b/models/fixtures/comment.yml @@ -9,7 +9,7 @@ - id: 2 type: 0 # comment - poster_id: 3 # user not watching (see watch.yml) + poster_id: 3 # the user not explicitly not watching (see issue_watch.yml) issue_id: 1 # in repo_id 1 content: "good work!" created_unix: 946684811 @@ -17,7 +17,7 @@ - id: 3 type: 0 # comment - poster_id: 5 # user not watching (see watch.yml) + poster_id: 5 # the user not explicitly not watching (see issue_watch.yml) issue_id: 1 # in repo_id 1 content: "meh..." created_unix: 946684812 diff --git a/models/issues/issue_subscriber.go b/models/issues/issue_subscriber.go new file mode 100644 index 0000000000000..7c0f21622ed6c --- /dev/null +++ b/models/issues/issue_subscriber.go @@ -0,0 +1,103 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues + +import ( + "context" + + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + user_model "code.gitea.io/gitea/models/user" + + "xorm.io/builder" +) + +// CheckIssueSubscriber check if a user is subscribing an issue +// it takes participants and repo watch into account +func CheckIssueSubscriber(user *user_model.User, issue *Issue) (bool, error) { + iw, exist, err := GetIssueWatch(db.DefaultContext, user.ID, issue.ID) + if err != nil { + return false, err + } + if exist { + return iw.IsWatching, nil + } + w, err := repo_model.GetWatch(db.DefaultContext, user.ID, issue.RepoID) + if err != nil { + return false, err + } + return repo_model.IsWatchMode(w.Mode) || IsUserParticipantsOfIssue(user, issue), nil +} + +// GetIssueSubscribers returns subscribers of a given issue +func GetIssueSubscribers(ctx context.Context, issue *Issue, listOptions db.ListOptions) (user_model.UserList, error) { + subscribeUserIds := builder.Select("`issue_watch`.user_id"). + From("issue_watch"). + Where(builder.Eq{"`issue_watch`.issue_id": issue.ID, "`issue_watch`.is_watching": true}) + + unsubscribeUserIds := builder.Select("`issue_watch`.user_id"). + From("issue_watch"). + Where(builder.Eq{"`issue_watch`.issue_id": issue.ID, "`issue_watch`.is_watching": false}) + + participantsUserIds := builder.Select("`comment`.poster_id"). + From("comment"). + Where(builder.Eq{"`comment`.issue_id": issue.ID}). + And(builder.In("`comment`.type", CommentTypeComment, CommentTypeCode, CommentTypeReview)) + + repoSubscribeUserIds := builder.Select("`watch`.user_id"). + From("watch"). + Where(builder.Eq{"`watch`.repo_id": issue.RepoID}). + And(builder.NotIn("`watch`.mode", repo_model.WatchModeDont, repo_model.WatchModeNone)) + + sess := db.GetEngine(ctx).Table("user"). + Where(builder.Or( + builder.In("id", subscribeUserIds), + builder.In("id", repoSubscribeUserIds), + builder.In("id", participantsUserIds), + builder.In("id", issue.PosterID), + ). + And(builder.NotIn("id", unsubscribeUserIds)). + And(builder.Eq{"`user`.is_active": true, "`user`.prohibit_login": false})) + + if listOptions.Page != 0 { + sess = db.SetSessionPagination(sess, &listOptions) + users := make(user_model.UserList, 0, listOptions.PageSize) + return users, sess.Find(&users) + } + users := make(user_model.UserList, 0, 8) + return users, sess.Find(&users) +} + +// CountIssueSubscribers count subscribers of a given issue +func CountIssueSubscribers(ctx context.Context, issue *Issue) (int64, error) { + subscribeUserIds := builder.Select("`issue_watch`.user_id"). + From("issue_watch"). + Where(builder.Eq{"`issue_watch`.issue_id": issue.ID, "`issue_watch`.is_watching": true}) + + unsubscribeUserIds := builder.Select("`issue_watch`.user_id"). + From("issue_watch"). + Where(builder.Eq{"`issue_watch`.issue_id": issue.ID, "`issue_watch`.is_watching": false}) + + participantsUserIds := builder.Select("`comment`.poster_id"). + From("comment"). + Where(builder.Eq{"`comment`.issue_id": issue.ID}). + And(builder.In("`comment`.type", CommentTypeComment, CommentTypeCode, CommentTypeReview)) + + repoSubscribeUserIds := builder.Select("`watch`.user_id"). + From("watch"). + Where(builder.Eq{"`watch`.repo_id": issue.RepoID}). + And(builder.NotIn("`watch`.mode", repo_model.WatchModeDont, repo_model.WatchModeNone)) + + sess := db.GetEngine(ctx).Table("user"). + Where(builder.Or( + builder.In("id", subscribeUserIds), + builder.In("id", repoSubscribeUserIds), + builder.In("id", participantsUserIds), + builder.In("id", issue.PosterID), + ). + And(builder.NotIn("id", unsubscribeUserIds)). + And(builder.Eq{"`user`.is_active": true, "`user`.prohibit_login": false})) + + return sess.Count(new(user_model.User)) +} diff --git a/models/issues/issue_subscriber_test.go b/models/issues/issue_subscriber_test.go new file mode 100644 index 0000000000000..712bf25dcf578 --- /dev/null +++ b/models/issues/issue_subscriber_test.go @@ -0,0 +1,69 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package issues_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + issues_model "code.gitea.io/gitea/models/issues" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestGetIssueWatchers(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issueList := issues_model.IssueList{ + unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}), // repo 1 + unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 2}), // repo 1 + unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 5}), // repo 1 + unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 7}), // repo 2 + } + + iws, err := issues_model.GetIssueSubscribers(db.DefaultContext, issueList[0], db.ListOptions{}) + assert.NoError(t, err) + // +isIssueWatching[] + // -noIssueWatching[] + // +participates[2,3,5] + // +poster[1] + // +repoWatch[1,4,9,11] + // -inactive[3,9] + // => [1,4,5,11] + assert.Len(t, iws, 4) + + iws, err = issues_model.GetIssueSubscribers(db.DefaultContext, issueList[1], db.ListOptions{}) + assert.NoError(t, err) + // +isIssueWatching[] + // -noIssueWatching[2] + // +participates[1] + // +poster[1] + // +repoWatch[1,4,9,11] + // -inactive[3,9] + // => [1,4,11] + assert.Len(t, iws, 3) + + iws, err = issues_model.GetIssueSubscribers(db.DefaultContext, issueList[2], db.ListOptions{}) + assert.NoError(t, err) + // +isIssueWatching[] + // -noIssueWatching[] + // +participates[] + // +poster[2] + // +repoWatch[1,4,9,11] + // -inactive[3,9] + // => [1,2,4,11] + assert.Len(t, iws, 4) + + iws, err = issues_model.GetIssueSubscribers(db.DefaultContext, issueList[3], db.ListOptions{}) + assert.NoError(t, err) + // +isIssueWatching[] + // -noIssueWatching[] + // +participates[] + // +poster[2] + // +repoWatch[] + // -inactive[3,9] + // => [2] + assert.Len(t, iws, 1) +} diff --git a/models/issues/issue_watch.go b/models/issues/issue_watch.go index 1efc0ea6871c4..6a94078f34da3 100644 --- a/models/issues/issue_watch.go +++ b/models/issues/issue_watch.go @@ -7,16 +7,14 @@ import ( "context" "code.gitea.io/gitea/models/db" - repo_model "code.gitea.io/gitea/models/repo" - user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/timeutil" ) // IssueWatch is connection request for receiving issue notification. type IssueWatch struct { ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"UNIQUE(watch) NOT NULL"` IssueID int64 `xorm:"UNIQUE(watch) NOT NULL"` + UserID int64 `xorm:"UNIQUE(watch) NOT NULL"` IsWatching bool `xorm:"NOT NULL"` CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` @@ -67,23 +65,6 @@ func GetIssueWatch(ctx context.Context, userID, issueID int64) (iw *IssueWatch, return iw, exists, err } -// CheckIssueWatch check if an user is watching an issue -// it takes participants and repo watch into account -func CheckIssueWatch(user *user_model.User, issue *Issue) (bool, error) { - iw, exist, err := GetIssueWatch(db.DefaultContext, user.ID, issue.ID) - if err != nil { - return false, err - } - if exist { - return iw.IsWatching, nil - } - w, err := repo_model.GetWatch(db.DefaultContext, user.ID, issue.RepoID) - if err != nil { - return false, err - } - return repo_model.IsWatchMode(w.Mode) || IsUserParticipantsOfIssue(user, issue), nil -} - // GetIssueWatchersIDs returns IDs of subscribers or explicit unsubscribers to a given issue id // but avoids joining with `user` for performance reasons // User permissions must be verified elsewhere if required @@ -96,34 +77,6 @@ func GetIssueWatchersIDs(ctx context.Context, issueID int64, watching bool) ([]i Find(&ids) } -// GetIssueWatchers returns watchers/unwatchers of a given issue -func GetIssueWatchers(ctx context.Context, issueID int64, listOptions db.ListOptions) (IssueWatchList, error) { - sess := db.GetEngine(ctx). - Where("`issue_watch`.issue_id = ?", issueID). - And("`issue_watch`.is_watching = ?", true). - And("`user`.is_active = ?", true). - And("`user`.prohibit_login = ?", false). - Join("INNER", "`user`", "`user`.id = `issue_watch`.user_id") - - if listOptions.Page != 0 { - sess = db.SetSessionPagination(sess, &listOptions) - watches := make([]*IssueWatch, 0, listOptions.PageSize) - return watches, sess.Find(&watches) - } - watches := make([]*IssueWatch, 0, 8) - return watches, sess.Find(&watches) -} - -// CountIssueWatchers count watchers/unwatchers of a given issue -func CountIssueWatchers(ctx context.Context, issueID int64) (int64, error) { - return db.GetEngine(ctx). - Where("`issue_watch`.issue_id = ?", issueID). - And("`issue_watch`.is_watching = ?", true). - And("`user`.is_active = ?", true). - And("`user`.prohibit_login = ?", false). - Join("INNER", "`user`", "`user`.id = `issue_watch`.user_id").Count(new(IssueWatch)) -} - // RemoveIssueWatchersByRepoID remove issue watchers by repoID func RemoveIssueWatchersByRepoID(ctx context.Context, userID, repoID int64) error { _, err := db.GetEngine(ctx). diff --git a/models/issues/issue_watch_test.go b/models/issues/issue_watch_test.go index 4f44487f567ad..ab8783675fdd6 100644 --- a/models/issues/issue_watch_test.go +++ b/models/issues/issue_watch_test.go @@ -41,27 +41,3 @@ func TestGetIssueWatch(t *testing.T) { assert.False(t, exists) assert.NoError(t, err) } - -func TestGetIssueWatchers(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - iws, err := issues_model.GetIssueWatchers(db.DefaultContext, 1, db.ListOptions{}) - assert.NoError(t, err) - // Watcher is inactive, thus 0 - assert.Len(t, iws, 0) - - iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 2, db.ListOptions{}) - assert.NoError(t, err) - // Watcher is explicit not watching - assert.Len(t, iws, 0) - - iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 5, db.ListOptions{}) - assert.NoError(t, err) - // Issue has no Watchers - assert.Len(t, iws, 0) - - iws, err = issues_model.GetIssueWatchers(db.DefaultContext, 7, db.ListOptions{}) - assert.NoError(t, err) - // Issue has one watcher - assert.Len(t, iws, 1) -} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 9f4acda23699a..263546ae78ee8 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -528,6 +528,8 @@ var migrations = []Migration{ NewMigration("Add Version to ActionRun table", v1_21.AddVersionToActionRunTable), // v273 -> v274 NewMigration("Add Action Schedule Table", v1_21.AddActionScheduleTable), + // v274 -> v275 + NewMigration("Adjust index order of issue_watch table ", v1_21.AdjustIssueWatchIndexOrder), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_21/v274.go b/models/migrations/v1_21/v274.go new file mode 100644 index 0000000000000..e73e828611a89 --- /dev/null +++ b/models/migrations/v1_21/v274.go @@ -0,0 +1,23 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_21 //nolint +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func AdjustIssueWatchIndexOrder(x *xorm.Engine) error { + type IssueWatch struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"UNIQUE(watch) NOT NULL"` + IssueID int64 `xorm:"UNIQUE(watch) NOT NULL"` + IsWatching bool `xorm:"NOT NULL"` + CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` + } + // Drop the old index :(user_id,issue_id) + // Then automatically created new index => (issue_id,user_id) + return x.DropIndexes(new(IssueWatch)) +} diff --git a/routers/api/v1/repo/issue_subscription.go b/routers/api/v1/repo/issue_subscription.go index 5a05471264653..04d4a4aa4bc39 100644 --- a/routers/api/v1/repo/issue_subscription.go +++ b/routers/api/v1/repo/issue_subscription.go @@ -132,7 +132,7 @@ func setIssueSubscription(ctx *context.APIContext, watch bool) { return } - current, err := issues_model.CheckIssueWatch(user, issue) + current, err := issues_model.CheckIssueSubscriber(user, issue) if err != nil { ctx.Error(http.StatusInternalServerError, "CheckIssueWatch", err) return @@ -196,14 +196,14 @@ func CheckIssueSubscription(ctx *context.APIContext) { return } - watching, err := issues_model.CheckIssueWatch(ctx.Doer, issue) + subscribed, err := issues_model.CheckIssueSubscriber(ctx.Doer, issue) if err != nil { ctx.InternalServerError(err) return } ctx.JSON(http.StatusOK, api.WatchInfo{ - Subscribed: watching, - Ignored: !watching, + Subscribed: subscribed, + Ignored: !subscribed, Reason: nil, CreatedAt: issue.CreatedUnix.AsTime(), URL: issue.APIURL() + "/subscriptions", @@ -262,30 +262,20 @@ func GetIssueSubscribers(ctx *context.APIContext) { return } - iwl, err := issues_model.GetIssueWatchers(ctx, issue.ID, utils.GetListOptions(ctx)) + users, err := issues_model.GetIssueSubscribers(ctx, issue, utils.GetListOptions(ctx)) if err != nil { - ctx.Error(http.StatusInternalServerError, "GetIssueWatchers", err) + ctx.Error(http.StatusInternalServerError, "GetIssueSubscribers", err) return } - userIDs := make([]int64, 0, len(iwl)) - for _, iw := range iwl { - userIDs = append(userIDs, iw.UserID) - } - - users, err := user_model.GetUsersByIDs(userIDs) - if err != nil { - ctx.Error(http.StatusInternalServerError, "GetUsersByIDs", err) - return - } apiUsers := make([]*api.User, 0, len(users)) for _, v := range users { apiUsers = append(apiUsers, convert.ToUser(ctx, v, ctx.Doer)) } - count, err := issues_model.CountIssueWatchers(ctx, issue.ID) + count, err := issues_model.CountIssueSubscribers(ctx, issue) if err != nil { - ctx.Error(http.StatusInternalServerError, "CountIssueWatchers", err) + ctx.Error(http.StatusInternalServerError, "CountIssueSubscribers", err) return } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index f5e8f80ddfe2e..73c3e64735c35 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -1408,7 +1408,7 @@ func ViewIssue(ctx *context.Context) { if ctx.Doer != nil { iw.UserID = ctx.Doer.ID iw.IssueID = issue.ID - iw.IsWatching, err = issues_model.CheckIssueWatch(ctx.Doer, issue) + iw.IsWatching, err = issues_model.CheckIssueSubscriber(ctx.Doer, issue) if err != nil { ctx.ServerError("CheckIssueWatch", err) return diff --git a/tests/integration/incoming_email_test.go b/tests/integration/incoming_email_test.go index b4478f57809fa..053d5b0885db9 100644 --- a/tests/integration/incoming_email_test.go +++ b/tests/integration/incoming_email_test.go @@ -154,7 +154,7 @@ func TestIncomingEmail(t *testing.T) { t.Run("Unsubscribe", func(t *testing.T) { defer tests.PrintCurrentTest(t)() - watching, err := issues_model.CheckIssueWatch(user, issue) + watching, err := issues_model.CheckIssueSubscriber(user, issue) assert.NoError(t, err) assert.True(t, watching) @@ -169,7 +169,7 @@ func TestIncomingEmail(t *testing.T) { assert.NoError(t, handler.Handle(db.DefaultContext, content, user, payload)) - watching, err = issues_model.CheckIssueWatch(user, issue) + watching, err = issues_model.CheckIssueSubscriber(user, issue) assert.NoError(t, err) assert.False(t, watching) })