Auto-subscribe user to repository when they commit/tag to it (#7657)

* Add support for AUTO_WATCH_ON_CHANGES and AUTO_WATCH_ON_CLONE

* Update models/repo_watch.go

Co-Authored-By: Lauris BH <lauris@nix.lv>

* Round up changes suggested by lafriks

* Added changes suggested from automated tests

* Updated deleteUser to take RepoWatchModeDont into account, corrected inverted DefaultWatchOnClone and DefaultWatchOnChanges behaviour, updated and added tests.

* Reinsert import "github.com/Unknwon/com" on http.go

* Add migration for new column `watch`.`mode`

* Remove serv code

* Remove WATCH_ON_CLONE; use hooks, add integrations

* Renamed watch_test.go to repo_watch_test.go

* Correct fmt

* Add missing EOL

* Correct name of test function

* Reword cheat and ini descriptions

* Add update to migration to ensure column value

* Clarify comment

Co-Authored-By: zeripath <art27@cantab.net>

* Simplify if condition

custom/conf/app.ini.sample

@@ -501,6 +501,9 @@ SHOW_REGISTRATION_BUTTON = true
 ; When adding a repo to a team or creating a new repo all team members will watch the
 ; repo automatically if enabled
 AUTO_WATCH_NEW_REPOS = true
+; Default value for AutoWatchOnChanges
+; Make the user watch a repository When they commit for the first time
+AUTO_WATCH_ON_CHANGES = false
 
 [webhook]
 ; Hook task queue length, increase if webhook shooting starts hanging

docs/content/doc/advanced/config-cheat-sheet.en-us.md

@@ -303,6 +303,7 @@ relation to port exhaustion.
   on this instance.
 - `SHOW_REGISTRATION_BUTTON`: **! DISABLE\_REGISTRATION**: Show Registration Button
 - `AUTO_WATCH_NEW_REPOS`: **true**: Enable this to let all organisation users watch new repos when they are created
+- `AUTO_WATCH_ON_CHANGES`: **false**: Enable this to make users watch a repository after their first commit to it
 - `DEFAULT_ORG_VISIBILITY`: **public**: Set default visibility mode for organisations, either "public", "limited" or "private".
 - `DEFAULT_ORG_MEMBER_VISIBLE`: **false** True will make the membership of the users visible when added to the organisation.
 

integrations/repo_watch_test.go

@@ -0,0 +1,24 @@
+// 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 integrations
+
+import (
+	"net/url"
+	"testing"
+
+	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/setting"
+)
+
+func TestRepoWatch(t *testing.T) {
+	onGiteaRun(t, func(t *testing.T, giteaURL *url.URL) {
+		// Test round-trip auto-watch
+		setting.Service.AutoWatchOnChanges = true
+		session := loginUser(t, "user2")
+		models.AssertNotExistsBean(t, &models.Watch{UserID: 2, RepoID: 3})
+		testEditFile(t, session, "user3", "repo3", "master", "README.md", "Hello, World (Edited for watch)\n")
+		models.AssertExistsAndLoadBean(t, &models.Watch{UserID: 2, RepoID: 3, Mode: models.RepoWatchModeAuto})
+	})
+}

models/consistency.go

@@ -84,14 +84,17 @@ func (user *User) checkForConsistency(t *testing.T) {
 func (repo *Repository) checkForConsistency(t *testing.T) {
 	assert.Equal(t, repo.LowerName, strings.ToLower(repo.Name), "repo: %+v", repo)
 	assertCount(t, &Star{RepoID: repo.ID}, repo.NumStars)
-	assertCount(t, &Watch{RepoID: repo.ID}, repo.NumWatches)
 	assertCount(t, &Milestone{RepoID: repo.ID}, repo.NumMilestones)
 	assertCount(t, &Repository{ForkID: repo.ID}, repo.NumForks)
 	if repo.IsFork {
 		AssertExistsAndLoadBean(t, &Repository{ID: repo.ForkID})
 	}
 
-	actual := getCount(t, x.Where("is_pull=?", false), &Issue{RepoID: repo.ID})
+	actual := getCount(t, x.Where("Mode<>?", RepoWatchModeDont), &Watch{RepoID: repo.ID})
+	assert.EqualValues(t, repo.NumWatches, actual,
+		"Unexpected number of watches for repo %+v", repo)
+
+	actual = getCount(t, x.Where("is_pull=?", false), &Issue{RepoID: repo.ID})
 	assert.EqualValues(t, repo.NumIssues, actual,
 		"Unexpected number of issues for repo %+v", repo)
 

models/fixtures/repository.yml

@@ -10,7 +10,7 @@
   num_closed_pulls: 0
   num_milestones: 3
   num_closed_milestones: 1
-  num_watches: 3
+  num_watches: 4
   status: 0
 
 -

models/fixtures/watch.yml

@@ -2,13 +2,28 @@
   id: 1
   user_id: 1
   repo_id: 1
+  mode: 1 # normal
 
 -
   id: 2
   user_id: 4
   repo_id: 1
+  mode: 1 # normal
 
 -
   id: 3
   user_id: 9
   repo_id: 1
+  mode: 1 # normal
+
+-
+  id: 4
+  user_id: 8
+  repo_id: 1
+  mode: 2 # don't watch
+
+-
+  id: 5
+  user_id: 11
+  repo_id: 1
+  mode: 3 # auto 

models/migrations/migrations.go

@@ -266,6 +266,8 @@ var migrations = []Migration{
 	NewMigration("remove unnecessary columns from label", removeLabelUneededCols),
 	// v105 -> v106
 	NewMigration("add includes_all_repositories to teams", addTeamIncludesAllRepositories),
+	// v106 -> v107
+	NewMigration("add column `mode` to table watch", addModeColumnToWatch),
 }
 
 // Migrate database to current version

models/migrations/v106.go

@@ -0,0 +1,26 @@
+// 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 (
+	"xorm.io/xorm"
+)
+
+// RepoWatchMode specifies what kind of watch the user has on a repository
+type RepoWatchMode int8
+
+// Watch is connection request for receiving repository notification.
+type Watch struct {
+	ID   int64         `xorm:"pk autoincr"`
+	Mode RepoWatchMode `xorm:"SMALLINT NOT NULL DEFAULT 1"`
+}
+
+func addModeColumnToWatch(x *xorm.Engine) (err error) {
+	if err = x.Sync2(new(Watch)); err != nil {
+		return
+	}
+	_, err = x.Exec("UPDATE `watch` SET `mode` = 1")
+	return err
+}

models/repo.go

@@ -2410,8 +2410,8 @@ func CheckRepoStats() {
 	checkers := []*repoChecker{
 		// Repository.NumWatches
 		{
-			"SELECT repo.id FROM `repository` repo WHERE repo.num_watches!=(SELECT COUNT(*) FROM `watch` WHERE repo_id=repo.id)",
-			"UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=?) WHERE id=?",
+			"SELECT repo.id FROM `repository` repo WHERE repo.num_watches!=(SELECT COUNT(*) FROM `watch` WHERE repo_id=repo.id AND mode<>2)",
+			"UPDATE `repository` SET num_watches=(SELECT COUNT(*) FROM `watch` WHERE repo_id=? AND mode<>2) WHERE id=?",
 			"repository count 'num_watches'",
 		},
 		// Repository.NumStars

models/repo_watch.go

@@ -4,42 +4,118 @@
 
 package models
 
-import "fmt"
+import (
+	"fmt"
+
+	"code.gitea.io/gitea/modules/setting"
+)
+
+// RepoWatchMode specifies what kind of watch the user has on a repository
+type RepoWatchMode int8
+
+const (
+	// RepoWatchModeNone don't watch
+	RepoWatchModeNone RepoWatchMode = iota // 0
+	// RepoWatchModeNormal watch repository (from other sources)
+	RepoWatchModeNormal // 1
+	// RepoWatchModeDont explicit don't auto-watch
+	RepoWatchModeDont // 2
+	// RepoWatchModeAuto watch repository (from AutoWatchOnChanges)
+	RepoWatchModeAuto // 3
+)
 
 // Watch is connection request for receiving repository notification.
 type Watch struct {
 	ID     int64         `xorm:"pk autoincr"`
 	UserID int64         `xorm:"UNIQUE(watch)"`
 	RepoID int64         `xorm:"UNIQUE(watch)"`
+	Mode   RepoWatchMode `xorm:"SMALLINT NOT NULL DEFAULT 1"`
 }
 
-func isWatching(e Engine, userID, repoID int64) bool {
-	has, _ := e.Get(&Watch{UserID: userID, RepoID: repoID})
-	return has
+// getWatch gets what kind of subscription a user has on a given repository; returns dummy record if none found
+func getWatch(e Engine, userID, repoID int64) (Watch, error) {
+	watch := Watch{UserID: userID, RepoID: repoID}
+	has, err := e.Get(&watch)
+	if err != nil {
+		return watch, err
+	}
+	if !has {
+		watch.Mode = RepoWatchModeNone
+	}
+	return watch, nil
+}
+
+// Decodes watchability of RepoWatchMode
+func isWatchMode(mode RepoWatchMode) bool {
+	return mode != RepoWatchModeNone && mode != RepoWatchModeDont
 }
 
 // IsWatching checks if user has watched given repository.
 func IsWatching(userID, repoID int64) bool {
-	return isWatching(x, userID, repoID)
+	watch, err := getWatch(x, userID, repoID)
+	return err == nil && isWatchMode(watch.Mode)
 }
 
-func watchRepo(e Engine, userID, repoID int64, watch bool) (err error) {
-	if watch {
-		if isWatching(e, userID, repoID) {
+func watchRepoMode(e Engine, watch Watch, mode RepoWatchMode) (err error) {
+	if watch.Mode == mode {
 		return nil
 	}
-		if _, err = e.Insert(&Watch{RepoID: repoID, UserID: userID}); err != nil {
+	if mode == RepoWatchModeAuto && (watch.Mode == RepoWatchModeDont || isWatchMode(watch.Mode)) {
+		// Don't auto watch if already watching or deliberately not watching
+		return nil
+	}
+
+	hadrec := watch.Mode != RepoWatchModeNone
+	needsrec := mode != RepoWatchModeNone
+	repodiff := 0
+
+	if isWatchMode(mode) && !isWatchMode(watch.Mode) {
+		repodiff = 1
+	} else if !isWatchMode(mode) && isWatchMode(watch.Mode) {
+		repodiff = -1
+	}
+
+	watch.Mode = mode
+
+	if !hadrec && needsrec {
+		watch.Mode = mode
+		if _, err = e.Insert(watch); err != nil {
 			return err
 		}
-		_, err = e.Exec("UPDATE `repository` SET num_watches = num_watches + 1 WHERE id = ?", repoID)
-	} else {
-		if !isWatching(e, userID, repoID) {
-			return nil
+	} else if needsrec {
+		watch.Mode = mode
+		if _, err := e.ID(watch.ID).AllCols().Update(watch); err != nil {
+			return err
 		}
-		if _, err = e.Delete(&Watch{0, userID, repoID}); err != nil {
+	} else if _, err = e.Delete(Watch{ID: watch.ID}); err != nil {
 		return err
 	}
-		_, err = e.Exec("UPDATE `repository` SET num_watches = num_watches - 1 WHERE id = ?", repoID)
+	if repodiff != 0 {
+		_, err = e.Exec("UPDATE `repository` SET num_watches = num_watches + ? WHERE id = ?", repodiff, watch.RepoID)
+	}
+	return err
+}
+
+// WatchRepoMode watch repository in specific mode.
+func WatchRepoMode(userID, repoID int64, mode RepoWatchMode) (err error) {
+	var watch Watch
+	if watch, err = getWatch(x, userID, repoID); err != nil {
+		return err
+	}
+	return watchRepoMode(x, watch, mode)
+}
+
+func watchRepo(e Engine, userID, repoID int64, doWatch bool) (err error) {
+	var watch Watch
+	if watch, err = getWatch(e, userID, repoID); err != nil {
+		return err
+	}
+	if !doWatch && watch.Mode == RepoWatchModeAuto {
+		err = watchRepoMode(e, watch, RepoWatchModeDont)
+	} else if !doWatch {
+		err = watchRepoMode(e, watch, RepoWatchModeNone)
+	} else {
+		err = watchRepoMode(e, watch, RepoWatchModeNormal)
 	}
 	return err
 }
@@ -52,6 +128,7 @@ func WatchRepo(userID, repoID int64, watch bool) (err error) {
 func getWatchers(e Engine, repoID int64) ([]*Watch, error) {
 	watches := make([]*Watch, 0, 10)
 	return watches, e.Where("`watch`.repo_id=?", repoID).
+		And("`watch`.mode<>?", RepoWatchModeDont).
 		And("`user`.is_active=?", true).
 		And("`user`.prohibit_login=?", false).
 		Join("INNER", "`user`", "`user`.id = `watch`.user_id").
@@ -67,7 +144,8 @@ func GetWatchers(repoID int64) ([]*Watch, error) {
 func (repo *Repository) GetWatchers(page int) ([]*User, error) {
 	users := make([]*User, 0, ItemsPerPage)
 	sess := x.Where("watch.repo_id=?", repo.ID).
-		Join("LEFT", "watch", "`user`.id=`watch`.user_id")
+		Join("LEFT", "watch", "`user`.id=`watch`.user_id").
+		And("`watch`.mode<>?", RepoWatchModeDont)
 	if page > 0 {
 		sess = sess.Limit(ItemsPerPage, (page-1)*ItemsPerPage)
 	}
@@ -137,3 +215,22 @@ func notifyWatchers(e Engine, act *Action) error {
 func NotifyWatchers(act *Action) error {
 	return notifyWatchers(x, act)
 }
+
+func watchIfAuto(e Engine, userID, repoID int64, isWrite bool) error {
+	if !isWrite || !setting.Service.AutoWatchOnChanges {
+		return nil
+	}
+	watch, err := getWatch(e, userID, repoID)
+	if err != nil {
+		return err
+	}
+	if watch.Mode != RepoWatchModeNone {
+		return nil
+	}
+	return watchRepoMode(e, watch, RepoWatchModeAuto)
+}
+
+// WatchIfAuto subscribes to repo if AutoWatchOnChanges is set
+func WatchIfAuto(userID int64, repoID int64, isWrite bool) error {
+	return watchIfAuto(x, userID, repoID, isWrite)
+}

models/repo_watch_test.go

@@ -7,6 +7,8 @@ package models
 import (
 	"testing"
 
+	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
@@ -15,8 +17,10 @@ func TestIsWatching(t *testing.T) {
 
 	assert.True(t, IsWatching(1, 1))
 	assert.True(t, IsWatching(4, 1))
+	assert.True(t, IsWatching(11, 1))
 
 	assert.False(t, IsWatching(1, 5))
+	assert.False(t, IsWatching(8, 1))
 	assert.False(t, IsWatching(NonexistentID, NonexistentID))
 }
 
@@ -78,7 +82,7 @@ func TestNotifyWatchers(t *testing.T) {
 	}
 	assert.NoError(t, NotifyWatchers(action))
 
-	// One watchers are inactive, thus action is only created for user 8, 1, 4
+	// One watchers are inactive, thus action is only created for user 8, 1, 4, 11
 	AssertExistsAndLoadBean(t, &Action{
 		ActUserID: action.ActUserID,
 		UserID:    8,
@@ -97,4 +101,88 @@ func TestNotifyWatchers(t *testing.T) {
 		RepoID:    action.RepoID,
 		OpType:    action.OpType,
 	})
+	AssertExistsAndLoadBean(t, &Action{
+		ActUserID: action.ActUserID,
+		UserID:    11,
+		RepoID:    action.RepoID,
+		OpType:    action.OpType,
+	})
+}
+
+func TestWatchIfAuto(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository)
+	watchers, err := repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, repo.NumWatches)
+
+	setting.Service.AutoWatchOnChanges = false
+
+	prevCount := repo.NumWatches
+
+	// Must not add watch
+	assert.NoError(t, WatchIfAuto(8, 1, true))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+
+	// Should not add watch
+	assert.NoError(t, WatchIfAuto(10, 1, true))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+
+	setting.Service.AutoWatchOnChanges = true
+
+	// Must not add watch
+	assert.NoError(t, WatchIfAuto(8, 1, true))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+
+	// Should not add watch
+	assert.NoError(t, WatchIfAuto(12, 1, false))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+
+	// Should add watch
+	assert.NoError(t, WatchIfAuto(12, 1, true))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount+1)
+
+	// Should remove watch, inhibit from adding auto
+	assert.NoError(t, WatchRepo(12, 1, false))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+
+	// Must not add watch
+	assert.NoError(t, WatchIfAuto(12, 1, true))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+}
+
+func TestWatchRepoMode(t *testing.T) {
+	assert.NoError(t, PrepareTestDatabase())
+
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1}, 0)
+
+	assert.NoError(t, WatchRepoMode(12, 1, RepoWatchModeAuto))
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1}, 1)
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1, Mode: RepoWatchModeAuto}, 1)
+
+	assert.NoError(t, WatchRepoMode(12, 1, RepoWatchModeNormal))
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1}, 1)
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1, Mode: RepoWatchModeNormal}, 1)
+
+	assert.NoError(t, WatchRepoMode(12, 1, RepoWatchModeDont))
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1}, 1)
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1, Mode: RepoWatchModeDont}, 1)
+
+	assert.NoError(t, WatchRepoMode(12, 1, RepoWatchModeNone))
+	AssertCount(t, &Watch{UserID: 12, RepoID: 1}, 0)
 }

models/user.go

@@ -1082,7 +1082,7 @@ func deleteUser(e *xorm.Session, u *User) error {
 	// ***** START: Watch *****
 	watchedRepoIDs := make([]int64, 0, 10)
 	if err = e.Table("watch").Cols("watch.repo_id").
-		Where("watch.user_id = ?", u.ID).Find(&watchedRepoIDs); err != nil {
+		Where("watch.user_id = ?", u.ID).And("watch.mode <>?", RepoWatchModeDont).Find(&watchedRepoIDs); err != nil {
 		return fmt.Errorf("get all watches: %v", err)
 	}
 	if _, err = e.Decr("num_watches").In("id", watchedRepoIDs).NoAutoTime().Update(new(Repository)); err != nil {
@@ -1543,6 +1543,7 @@ func GetStarredRepos(userID int64, private bool) ([]*Repository, error) {
 // GetWatchedRepos returns the repos watched by a particular user
 func GetWatchedRepos(userID int64, private bool) ([]*Repository, error) {
 	sess := x.Where("watch.user_id=?", userID).
+		And("`watch`.mode<>?", RepoWatchModeDont).
 		Join("LEFT", "watch", "`repository`.id=`watch`.repo_id")
 	if !private {
 		sess = sess.And("is_private=?", false)

modules/repofiles/update.go

@@ -504,5 +504,10 @@ func PushUpdate(repo *models.Repository, branch string, opts models.PushUpdateOp
 	if opts.RefFullName == git.BranchPrefix+repo.DefaultBranch {
 		models.UpdateRepoIndexer(repo)
 	}
+
+	if err = models.WatchIfAuto(opts.PusherID, repo.ID, true); err != nil {
+		log.Warn("Fail to perform auto watch on user %v for repo %v: %v", opts.PusherID, repo.ID, err)
+	}
+
 	return nil
 }

modules/setting/service.go

@@ -44,6 +44,7 @@ var Service struct {
 	NoReplyAddress                          string
 	EnableUserHeatmap                       bool
 	AutoWatchNewRepos                       bool
+	AutoWatchOnChanges                      bool
 	DefaultOrgMemberVisible                 bool
 
 	// OpenID settings
@@ -85,6 +86,7 @@ func newService() {
 	Service.NoReplyAddress = sec.Key("NO_REPLY_ADDRESS").MustString("noreply.example.org")
 	Service.EnableUserHeatmap = sec.Key("ENABLE_USER_HEATMAP").MustBool(true)
 	Service.AutoWatchNewRepos = sec.Key("AUTO_WATCH_NEW_REPOS").MustBool(true)
+	Service.AutoWatchOnChanges = sec.Key("AUTO_WATCH_ON_CHANGES").MustBool(false)
 	Service.DefaultOrgVisibility = sec.Key("DEFAULT_ORG_VISIBILITY").In("public", structs.ExtractKeysFromMapString(structs.VisibilityModes))
 	Service.DefaultOrgVisibilityMode = structs.VisibilityModes[Service.DefaultOrgVisibility]
 	Service.DefaultOrgMemberVisible = sec.Key("DEFAULT_ORG_MEMBER_VISIBLE").MustBool()