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

Author: Guillermo Prandi

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
 
 -
   id: 2

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/repo.go

@@ -2261,8 +2261,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

@@ -218,11 +218,11 @@ func NotifyWatchers(act *Action) error {
 
 func watchIfAuto(e Engine, userID, repoID int64, isWrite bool) error {
 	if isWrite {
-		if !setting.Service.AutoWatchOnClone {
+		if !setting.Service.AutoWatchOnChanges {
 			return nil
 		}
 	} else {
-		if !setting.Service.AutoWatchOnChanges {
+		if !setting.Service.AutoWatchOnClone {
 			return nil
 		}
 	}

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,106 @@ 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.AutoWatchOnClone = false
+	setting.Service.AutoWatchOnChanges = false
+
+	prevCount := repo.NumWatches
+
+	// Must not add watch
+	assert.NoError(t, WatchIfAuto(8, 1, false))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+
+	// Should not add watch
+	assert.NoError(t, WatchIfAuto(10, 1, false))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount)
+
+	setting.Service.AutoWatchOnClone = true
+
+	// 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)
+
+	// Should add watch
+	assert.NoError(t, WatchIfAuto(10, 1, false))
+	watchers, err = repo.GetWatchers(1)
+	assert.NoError(t, err)
+	assert.Len(t, watchers, prevCount+1)
+
+	prevCount++
+
+	setting.Service.AutoWatchOnClone = false
+	setting.Service.AutoWatchOnChanges = true
+
+	// Must not add watch
+	assert.NoError(t, WatchIfAuto(8, 1, false))
+	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, false))
+	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

@@ -1047,7 +1047,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 {