diff --git a/integrations/issue_test.go b/integrations/issue_test.go index 1454d75885019..6ff5d7ef775ba 100644 --- a/integrations/issue_test.go +++ b/integrations/issue_test.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/references" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" + repo_service "code.gitea.io/gitea/services/repository" "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" @@ -57,6 +58,38 @@ func TestNoLoginViewIssues(t *testing.T) { MakeRequest(t, req, http.StatusOK) } +func TestIssueAutoSubscription(t *testing.T) { + defer prepareTestEnv(t)() + + user := models.AssertExistsAndLoadBean(t, &models.User{ID: 1}).(*models.User) + otherUser := models.AssertExistsAndLoadBean(t, &models.User{ID: 2}).(*models.User) + session := loginUser(t, user.Name) + + // create repo + repo, err := repo_service.CreateRepository(otherUser, otherUser, models.CreateRepoOptions{ + Name: "TESTIssueAutoSubscriptionRepo", + License: "MIT", + Readme: "Default", + IsPrivate: false, + AutoInit: true, + }) + assert.NoError(t, err) + + //make sure repo is not subscribed + models.AssertNotExistsBean(t, &models.Watch{UserID: user.ID, RepoID: repo.ID}) + + //create new issue + testNewIssue(t, session, repo.OwnerName, repo.Name, "some title text", "some body text") + newIssue, err := models.GetIssueByIndex(repo.ID, 1) + assert.NoError(t, err) + + time.Sleep(1 * time.Second) + + models.AssertNotExistsBean(t, &models.Watch{UserID: user.ID, RepoID: repo.ID}) + issueWatch := models.AssertExistsAndLoadBean(t, &models.IssueWatch{UserID: user.ID, IssueID: newIssue.ID}).(*models.IssueWatch) + assert.Equal(t, models.IssueWatchModeAuto, issueWatch.Mode) +} + func TestViewIssuesSortByType(t *testing.T) { defer prepareTestEnv(t)() diff --git a/models/fixtures/issue_watch.yml b/models/fixtures/issue_watch.yml index 4bc3ff1b8b987..ef00975d6fdb9 100644 --- a/models/fixtures/issue_watch.yml +++ b/models/fixtures/issue_watch.yml @@ -2,7 +2,7 @@ id: 1 user_id: 9 issue_id: 1 - is_watching: true + mode: 1 #IssueWatchModeNormal created_unix: 946684800 updated_unix: 946684800 @@ -10,7 +10,7 @@ id: 2 user_id: 2 issue_id: 2 - is_watching: false + mode: 2 #IssueWatchModeDont created_unix: 946684800 updated_unix: 946684800 @@ -18,7 +18,7 @@ id: 3 user_id: 2 issue_id: 7 - is_watching: true + mode: 1 #IssueWatchModeNormal created_unix: 946684800 updated_unix: 946684800 @@ -26,6 +26,14 @@ id: 4 user_id: 1 issue_id: 7 - is_watching: false + mode: 2 #IssueWatchModeDont + created_unix: 946684800 + updated_unix: 946684800 + +- + id: 5 + user_id: 1 + issue_id: 8 + mode: 3 #IssueWatchModeAuto created_unix: 946684800 updated_unix: 946684800 diff --git a/models/issue_watch.go b/models/issue_watch.go index dea6aa5a52330..f0a9f9d24fb0a 100644 --- a/models/issue_watch.go +++ b/models/issue_watch.go @@ -5,15 +5,31 @@ package models import ( + "fmt" + "time" + + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" ) +// IssueWatchMode specifies what kind of watch the user has on a issue +type IssueWatchMode int8 + +const ( + // IssueWatchModeNormal watch issue + IssueWatchModeNormal IssueWatchMode = iota + 1 // 1 + // IssueWatchModeDont explicit don't watch + IssueWatchModeDont // 2 + // IssueWatchModeAuto watch issue (from AutoWatchOnIssueChanges) + IssueWatchModeAuto // 3 +) + // 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"` - IsWatching bool `xorm:"NOT NULL"` + Mode IssueWatchMode `xorm:"NOT NULL DEFAULT 1"` CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` } @@ -21,40 +37,40 @@ type IssueWatch struct { // IssueWatchList contains IssueWatch type IssueWatchList []*IssueWatch -// CreateOrUpdateIssueWatch set watching for a user and issue -func CreateOrUpdateIssueWatch(userID, issueID int64, isWatching bool) error { - iw, exists, err := getIssueWatch(x, userID, issueID) - if err != nil { +// CreateOrUpdateIssueWatchMode set IssueWatchMode for a user and issue +func CreateOrUpdateIssueWatchMode(userID, issueID int64, mode IssueWatchMode) error { + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + if err := createOrUpdateIssueWatchMode(sess, userID, issueID, mode); err != nil { return err } + return sess.Commit() +} - if !exists { - iw = &IssueWatch{ - UserID: userID, - IssueID: issueID, - IsWatching: isWatching, - } - - if _, err := x.Insert(iw); err != nil { - return err - } - } else { - iw.IsWatching = isWatching - - if _, err := x.ID(iw.ID).Cols("is_watching", "updated_unix").Update(iw); err != nil { - return err - } +func createOrUpdateIssueWatchMode(e Engine, userID, issueID int64, mode IssueWatchMode) error { + if _, err := e.Exec(fmt.Sprintf("INSERT INTO issue_watch(user_id,issue_id,mode,created_unix,updated_unix) SELECT %d,%d,%d,%d,%d WHERE NOT EXISTS(SELECT 1 FROM issue_watch WHERE user_id = %d AND issue_id = %d);", userID, issueID, mode, time.Now().Unix(), time.Now().Unix(), userID, issueID)); err != nil { + return err + } + iw, exist, err := getIssueWatch(e, userID, issueID) + if err != nil && !exist { + return err + } + iw.Mode = mode + iw.UpdatedUnix = timeutil.TimeStampNow() + if _, err := e.ID(iw.ID).Cols("updated_unix", "mode").Update(iw); err != nil { + return err } return nil } // GetIssueWatch returns all IssueWatch objects from db by user and issue -// the current Web-UI need iw object for watchers AND explicit non-watchers func GetIssueWatch(userID, issueID int64) (iw *IssueWatch, exists bool, err error) { return getIssueWatch(x, userID, issueID) } -// Return watcher AND explicit non-watcher if entry in db exist func getIssueWatch(e Engine, userID, issueID int64) (iw *IssueWatch, exists bool, err error) { iw = new(IssueWatch) exists, err = e. @@ -67,28 +83,40 @@ func getIssueWatch(e Engine, userID, issueID int64) (iw *IssueWatch, exists bool // 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 -func GetIssueWatchersIDs(issueID int64, watching bool) ([]int64, error) { - return getIssueWatchersIDs(x, issueID, watching) +func GetIssueWatchersIDs(issueID int64, modes ...IssueWatchMode) ([]int64, error) { + if len(modes) == 0 { + modes = []IssueWatchMode{IssueWatchModeNormal, IssueWatchModeAuto} + } + return getIssueWatchersIDs(x, issueID, modes...) } -func getIssueWatchersIDs(e Engine, issueID int64, watching bool) ([]int64, error) { +func getIssueWatchersIDs(e Engine, issueID int64, modes ...IssueWatchMode) ([]int64, error) { ids := make([]int64, 0, 64) + if len(modes) == 0 { + return nil, fmt.Errorf("no IssueWatchMode set") + } return ids, e.Table("issue_watch"). Where("issue_id=?", issueID). - And("is_watching = ?", watching). + In("mode", modes). Select("user_id"). Find(&ids) } -// GetIssueWatchers returns watchers/unwatchers of a given issue -func GetIssueWatchers(issueID int64, listOptions ListOptions) (IssueWatchList, error) { - return getIssueWatchers(x, issueID, listOptions) +// GetIssueWatchers returns IssueWatch entry's based on modes of a given issue +func GetIssueWatchers(issueID int64, listOptions ListOptions, modes ...IssueWatchMode) (IssueWatchList, error) { + if len(modes) == 0 { + modes = []IssueWatchMode{IssueWatchModeNormal, IssueWatchModeAuto} + } + return getIssueWatchers(x, issueID, listOptions, modes...) } -func getIssueWatchers(e Engine, issueID int64, listOptions ListOptions) (IssueWatchList, error) { +func getIssueWatchers(e Engine, issueID int64, listOptions ListOptions, modes ...IssueWatchMode) (IssueWatchList, error) { + if len(modes) == 0 { + return nil, fmt.Errorf("no IssueWatchMode set") + } sess := e. Where("`issue_watch`.issue_id = ?", issueID). - And("`issue_watch`.is_watching = ?", true). + In("`issue_watch`.mode", modes). And("`user`.is_active = ?", true). And("`user`.prohibit_login = ?", false). Join("INNER", "`user`", "`user`.id = `issue_watch`.user_id") @@ -109,3 +137,19 @@ func removeIssueWatchersByRepoID(e Engine, userID int64, repoID int64) error { Delete(new(IssueWatch)) return err } + +// IsWatching is true if user iw watching either repo or issue (backwards compatibility) +func (iw IssueWatch) IsWatching() bool { + issue, err := GetIssueByID(iw.IssueID) + if err != nil { + // fail silent since template expect only bool + log.Warn("IssueWatch.IsWatching: GetIssueByID: ", err) + return false + } + // if RepoWatch is true ... + if IsWatching(iw.UserID, issue.RepoID) && iw.Mode != IssueWatchModeDont { + return true + } + + return iw.Mode == IssueWatchModeNormal || iw.Mode == IssueWatchModeAuto +} diff --git a/models/issue_watch_test.go b/models/issue_watch_test.go index 762b1486c6461..a02dff988986f 100644 --- a/models/issue_watch_test.go +++ b/models/issue_watch_test.go @@ -6,6 +6,7 @@ package models import ( "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -13,13 +14,25 @@ import ( func TestCreateOrUpdateIssueWatch(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - assert.NoError(t, CreateOrUpdateIssueWatch(3, 1, true)) + assert.NoError(t, CreateOrUpdateIssueWatchMode(3, 1, IssueWatchModeNormal)) iw := AssertExistsAndLoadBean(t, &IssueWatch{UserID: 3, IssueID: 1}).(*IssueWatch) - assert.True(t, iw.IsWatching) + assert.EqualValues(t, IssueWatchModeNormal, iw.Mode) - assert.NoError(t, CreateOrUpdateIssueWatch(1, 1, false)) + assert.NoError(t, DeleteIssueWatch(3, 1)) + AssertNotExistsBean(t, &IssueWatch{UserID: 3, IssueID: 1}) + + assert.NoError(t, CreateOrUpdateIssueWatchMode(3, 1, IssueWatchModeAuto)) + iw = AssertExistsAndLoadBean(t, &IssueWatch{UserID: 3, IssueID: 1}).(*IssueWatch) + assert.EqualValues(t, IssueWatchModeAuto, iw.Mode) + + assert.NoError(t, CreateOrUpdateIssueWatchMode(1, 1, IssueWatchModeAuto)) + iw = AssertExistsAndLoadBean(t, &IssueWatch{UserID: 1, IssueID: 1}).(*IssueWatch) + assert.EqualValues(t, IssueWatchModeAuto, iw.Mode) + + time.Sleep(1 * time.Second) + assert.NoError(t, CreateOrUpdateIssueWatchMode(1, 1, IssueWatchModeDont)) iw = AssertExistsAndLoadBean(t, &IssueWatch{UserID: 1, IssueID: 1}).(*IssueWatch) - assert.False(t, iw.IsWatching) + assert.EqualValues(t, IssueWatchModeDont, iw.Mode) } func TestGetIssueWatch(t *testing.T) { @@ -32,11 +45,17 @@ func TestGetIssueWatch(t *testing.T) { iw, exists, err := GetIssueWatch(2, 2) assert.True(t, exists) assert.NoError(t, err) - assert.EqualValues(t, false, iw.IsWatching) + assert.False(t, iw.IsWatching()) _, exists, err = GetIssueWatch(3, 1) assert.False(t, exists) assert.NoError(t, err) + + assert.False(t, IsWatching(1, 10)) + iw, exists, err = GetIssueWatch(1, 8) + assert.True(t, exists) + assert.NoError(t, err) + assert.True(t, iw.IsWatching()) } func TestGetIssueWatchers(t *testing.T) { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index ce3f77ba4e26f..573332f76616e 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -188,6 +188,8 @@ var migrations = []Migration{ NewMigration("Fix topic repository count", fixTopicRepositoryCount), // v127 -> v128 NewMigration("add repository code language statistics", addLanguageStats), + // v128 -> v129 + NewMigration("Change from IsWatching to Modes at IssueWatch", addIssueWatchModes), } // Migrate database to current version diff --git a/models/migrations/v128.go b/models/migrations/v128.go new file mode 100644 index 0000000000000..3c91347e9b7da --- /dev/null +++ b/models/migrations/v128.go @@ -0,0 +1,48 @@ +// Copyright 2019 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "fmt" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func addIssueWatchModes(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"` + Mode models.IssueWatchMode `xorm:"NOT NULL DEFAULT 1"` + CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` + } + + sess := x.NewSession() + defer sess.Close() + if err := sess.Begin(); err != nil { + return err + } + + if err := sess.Sync2(new(IssueWatch)); err != nil { + return fmt.Errorf("Sync2: %v", err) + } + + if _, err := sess.Where("is_watching = ?", false).Cols("mode").Update(&models.IssueWatch{Mode: models.IssueWatchModeDont}); err != nil { + return err + } + if _, err := sess.Where("is_watching = ?", true).Cols("mode").Update(&models.IssueWatch{Mode: models.IssueWatchModeNormal}); err != nil { + return err + } + + if err := dropTableColumns(sess, "issue_watch", "is_watching"); err != nil { + return err + } + + return sess.Commit() +} diff --git a/models/notification.go b/models/notification.go index c52d6c557a5d6..ceae881c1a07f 100644 --- a/models/notification.go +++ b/models/notification.go @@ -144,7 +144,7 @@ func createOrUpdateIssueNotifications(e Engine, issueID, commentID int64, notifi return err } - issueWatches, err := getIssueWatchersIDs(e, issueID, true) + issueWatches, err := getIssueWatchersIDs(e, issueID, IssueWatchModeNormal, IssueWatchModeAuto) if err != nil { return err } @@ -163,7 +163,7 @@ func createOrUpdateIssueNotifications(e Engine, issueID, commentID int64, notifi // dont notify user who cause notification delete(toNotify, notificationAuthorID) // explicit unwatch on issue - issueUnWatches, err := getIssueWatchersIDs(e, issueID, false) + issueUnWatches, err := getIssueWatchersIDs(e, issueID, IssueWatchModeDont) if err != nil { return err } @@ -171,6 +171,14 @@ func createOrUpdateIssueNotifications(e Engine, issueID, commentID int64, notifi delete(toNotify, id) } + // if Issue/Comment creator/updater is not subscribed ... auto subscribe to issue + if _, ok := toNotify[notificationAuthorID]; !ok { + + if err := createOrUpdateIssueWatchMode(e, notificationAuthorID, issueID, IssueWatchModeAuto); err != nil { + return err + } + } + err = issue.loadRepo(e) if err != nil { return err diff --git a/routers/api/v1/repo/issue_subscription.go b/routers/api/v1/repo/issue_subscription.go index 0406edd207844..ce3c2ed692c77 100644 --- a/routers/api/v1/repo/issue_subscription.go +++ b/routers/api/v1/repo/issue_subscription.go @@ -125,7 +125,12 @@ func setIssueSubscription(ctx *context.APIContext, watch bool) { return } - if err := models.CreateOrUpdateIssueWatch(user.ID, issue.ID, watch); err != nil { + mode := models.IssueWatchModeNormal + if !watch { + mode = models.IssueWatchModeDont + } + + if err := models.CreateOrUpdateIssueWatchMode(user.ID, issue.ID, mode); err != nil { ctx.Error(http.StatusInternalServerError, "CreateOrUpdateIssueWatch", err) return } @@ -184,7 +189,7 @@ func GetIssueSubscribers(ctx *context.APIContext) { return } - iwl, err := models.GetIssueWatchers(issue.ID, utils.GetListOptions(ctx)) + iwl, err := models.GetIssueWatchers(issue.ID, utils.GetListOptions(ctx), models.IssueWatchModeNormal, models.IssueWatchModeAuto) if err != nil { ctx.Error(http.StatusInternalServerError, "GetIssueWatchers", err) return diff --git a/routers/repo/issue.go b/routers/repo/issue.go index fdade2795d164..857642ad18b7d 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -702,9 +702,8 @@ func ViewIssue(ctx *context.Context) { } if !exists { iw = &models.IssueWatch{ - UserID: ctx.User.ID, - IssueID: issue.ID, - IsWatching: models.IsWatching(ctx.User.ID, ctx.Repo.Repository.ID), + UserID: ctx.User.ID, + IssueID: issue.ID, } } } diff --git a/routers/repo/issue_watch.go b/routers/repo/issue_watch.go index 07671af13a145..c977a3aae7e59 100644 --- a/routers/repo/issue_watch.go +++ b/routers/repo/issue_watch.go @@ -48,8 +48,13 @@ func IssueWatch(ctx *context.Context) { return } - if err := models.CreateOrUpdateIssueWatch(ctx.User.ID, issue.ID, watch); err != nil { - ctx.ServerError("CreateOrUpdateIssueWatch", err) + mode := models.IssueWatchModeNormal + if !watch { + mode = models.IssueWatchModeDont + } + + if err := models.CreateOrUpdateIssueWatchMode(ctx.User.ID, issue.ID, mode); err != nil { + ctx.ServerError("CreateOrUpdateIssueWatchMode", err) return }