Fix API Bug (fail on empty assignees) (#8873)

* keep sure if assigneeIDs == nil -> do nothing

* fix #8872

* Revert "keep sure if assigneeIDs == nil -> do nothing"

-> go handle it itself preaty well

This reverts commit e72d94129c4666d5151f6131cb2f8c1de127d9d0.

* clarity comparson

Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>
* simplify

* Update models/issue_assignees.go

Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>
* Update issue_assignees.go

* simplify more

* add --if oneAssignee != ""-- again

* Update models/issue_assignees.go

Co-Authored-By: guillep2k <18600385+guillep2k@users.noreply.github.com>
* CI.restart()

* Update issue_assignees.go

* add Test for GetUserIDsByNames

* add Test for MakeIDsFromAPIAssigneesToAdd

* fix test

diff --git a/models/issue_assignees.go b/models/issue_assignees.go
index e15b718eb2afc58c0c7ffae23129ad48864337ef..70bed039c2e7d52632dff2ff162e7cd7eff9ce9e 100644 (file)
--- a/models/issue_assignees.go
+++ b/models/issue_assignees.go
@@ -7,6 +7,8 @@ package models
 import (
        "fmt"
 
+       "code.gitea.io/gitea/modules/util"
+
        "xorm.io/xorm"
 )
 
@@ -171,25 +173,20 @@ func toggleUserAssignee(e *xorm.Session, issue *Issue, assigneeID int64) (remove
 // MakeIDsFromAPIAssigneesToAdd returns an array with all assignee IDs
 func MakeIDsFromAPIAssigneesToAdd(oneAssignee string, multipleAssignees []string) (assigneeIDs []int64, err error) {
 
+       var requestAssignees []string
+
        // Keeping the old assigning method for compatibility reasons
-       if oneAssignee != "" {
-
-               // Prevent double adding assignees
-               var isDouble bool
-               for _, assignee := range multipleAssignees {
-                       if assignee == oneAssignee {
-                               isDouble = true
-                               break
-                       }
-               }
+       if oneAssignee != "" && !util.IsStringInSlice(oneAssignee, multipleAssignees) {
+               requestAssignees = append(requestAssignees, oneAssignee)
+       }
 
-               if !isDouble {
-                       multipleAssignees = append(multipleAssignees, oneAssignee)
-               }
+       //Prevent empty assignees
+       if len(multipleAssignees) > 0 && multipleAssignees[0] != "" {
+               requestAssignees = append(requestAssignees, multipleAssignees...)
        }
 
        // Get the IDs of all assignees
-       assigneeIDs, err = GetUserIDsByNames(multipleAssignees, false)
+       assigneeIDs, err = GetUserIDsByNames(requestAssignees, false)
 
        return
 }

diff --git a/models/issue_assignees_test.go b/models/issue_assignees_test.go
index 163234b16779350c9d842b16aefaa57955be0086..79257013f8431f883017288e37f8062d33e18c90 100644 (file)
--- a/models/issue_assignees_test.go
+++ b/models/issue_assignees_test.go
@@ -59,3 +59,24 @@ func TestUpdateAssignee(t *testing.T) {
        assert.NoError(t, err)
        assert.False(t, isAssigned)
 }
+
+func TestMakeIDsFromAPIAssigneesToAdd(t *testing.T) {
+       IDs, err := MakeIDsFromAPIAssigneesToAdd("", []string{""})
+       assert.NoError(t, err)
+       assert.Equal(t, []int64{}, IDs)
+
+       IDs, err = MakeIDsFromAPIAssigneesToAdd("", []string{"none_existing_user"})
+       assert.Error(t, err)
+
+       IDs, err = MakeIDsFromAPIAssigneesToAdd("user1", []string{"user1"})
+       assert.NoError(t, err)
+       assert.Equal(t, []int64{1}, IDs)
+
+       IDs, err = MakeIDsFromAPIAssigneesToAdd("user2", []string{""})
+       assert.NoError(t, err)
+       assert.Equal(t, []int64{2}, IDs)
+
+       IDs, err = MakeIDsFromAPIAssigneesToAdd("", []string{"user1", "user2"})
+       assert.NoError(t, err)
+       assert.Equal(t, []int64{1, 2}, IDs)
+}

diff --git a/models/user_test.go b/models/user_test.go
index bcb955817c3307bd120071dad67dd42beb13f76c..f3952422afd3367a87019dd394de7c02501970ff 100644 (file)
--- a/models/user_test.go
+++ b/models/user_test.go
@@ -373,3 +373,16 @@ func TestCreateUser_Issue5882(t *testing.T) {
                assert.NoError(t, DeleteUser(v.user))
        }
 }
+
+func TestGetUserIDsByNames(t *testing.T) {
+
+       //ignore non existing
+       IDs, err := GetUserIDsByNames([]string{"user1", "user2", "none_existing_user"}, true)
+       assert.NoError(t, err)
+       assert.Equal(t, []int64{1, 2}, IDs)
+
+       //ignore non existing
+       IDs, err = GetUserIDsByNames([]string{"user1", "do_not_exist"}, false)
+       assert.Error(t, err)
+       assert.Equal(t, []int64(nil), IDs)
+}