Refactor Issues Subscription (#8738)

* FIX: getIssueWatchers() get only aktive suscriber

* save query to work later with it or not ...

* fix test + add new case

* corect tests + GetIssueWatch

* API issue_subscripton: Put/Delete require tocken

* remove redundant code

* swagger specify return value

* remove unused binding

* remove note
because I'll implement this in a different way and in another PR

* ID should be unique!

* use xorm session

* Revert "use xorm session"

This reverts commit c1de540147199f2f1a8dd0d008f54af3603e2229.

* better test code
* more acurate comments
* use assert.False/True instead of Equal

* use more assert methodes

3 files changed, 33 insertions, 8 deletions

diff --git a/models/fixtures/issue_watch.yml b/models/fixtures/issue_watch.yml
index 75351eb17f..4bc3ff1b8b 100644
--- a/models/fixtures/issue_watch.yml
+++ b/models/fixtures/issue_watch.yml
@@ -13,3 +13,19 @@
   is_watching: false
   created_unix: 946684800
   updated_unix: 946684800
+
+-
+  id: 3
+  user_id: 2
+  issue_id: 7
+  is_watching: true
+  created_unix: 946684800
+  updated_unix: 946684800
+
+-
+  id: 4
+  user_id: 1
+  issue_id: 7
+  is_watching: false
+  created_unix: 946684800
+  updated_unix: 946684800
diff --git a/models/issue_watch.go b/models/issue_watch.go
index 3d7d48a125..e42e371a1f 100644
--- a/models/issue_watch.go
+++ b/models/issue_watch.go
@@ -56,6 +56,7 @@ func getIssueWatch(e Engine, userID, issueID int64) (iw *IssueWatch, exists bool
 	exists, err = e.
 		Where("user_id = ?", userID).
 		And("issue_id = ?", issueID).
+		And("is_watching = ?", true).
 		Get(iw)
 	return
 }
@@ -80,6 +81,7 @@ func GetIssueWatchers(issueID int64) (IssueWatchList, error) {
 func getIssueWatchers(e Engine, issueID int64) (watches IssueWatchList, err error) {
 	err = e.
 		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").
diff --git a/models/issue_watch_test.go b/models/issue_watch_test.go
index ce0d2045ca..1d0473426e 100644
--- a/models/issue_watch_test.go
+++ b/models/issue_watch_test.go
@@ -15,26 +15,26 @@ func TestCreateOrUpdateIssueWatch(t *testing.T) {
 
 	assert.NoError(t, CreateOrUpdateIssueWatch(3, 1, true))
 	iw := AssertExistsAndLoadBean(t, &IssueWatch{UserID: 3, IssueID: 1}).(*IssueWatch)
-	assert.Equal(t, true, iw.IsWatching)
+	assert.True(t, iw.IsWatching)
 
 	assert.NoError(t, CreateOrUpdateIssueWatch(1, 1, false))
 	iw = AssertExistsAndLoadBean(t, &IssueWatch{UserID: 1, IssueID: 1}).(*IssueWatch)
-	assert.Equal(t, false, iw.IsWatching)
+	assert.False(t, iw.IsWatching)
 }
 
 func TestGetIssueWatch(t *testing.T) {
 	assert.NoError(t, PrepareTestDatabase())
 
 	_, exists, err := GetIssueWatch(9, 1)
-	assert.Equal(t, true, exists)
+	assert.True(t, exists)
 	assert.NoError(t, err)
 
 	_, exists, err = GetIssueWatch(2, 2)
-	assert.Equal(t, true, exists)
+	assert.False(t, exists)
 	assert.NoError(t, err)
 
 	_, exists, err = GetIssueWatch(3, 1)
-	assert.Equal(t, false, exists)
+	assert.False(t, exists)
 	assert.NoError(t, err)
 }
 
@@ -44,13 +44,20 @@ func TestGetIssueWatchers(t *testing.T) {
 	iws, err := GetIssueWatchers(1)
 	assert.NoError(t, err)
 	// Watcher is inactive, thus 0
-	assert.Equal(t, 0, len(iws))
+	assert.Len(t, iws, 0)
 
 	iws, err = GetIssueWatchers(2)
 	assert.NoError(t, err)
-	assert.Equal(t, 1, len(iws))
+	// Watcher is explicit not watching
+	assert.Len(t, iws, 0)
 
 	iws, err = GetIssueWatchers(5)
 	assert.NoError(t, err)
-	assert.Equal(t, 0, len(iws))
+	// Issue has no Watchers
+	assert.Len(t, iws, 0)
+
+	iws, err = GetIssueWatchers(7)
+	assert.NoError(t, err)
+	// Issue has one watcher
+	assert.Len(t, iws, 1)
 }