Multiple assignees (#3705)

4 files changed, 118 insertions, 100 deletions

diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go
index cc033554f3..211d8045a4 100644
--- a/routers/api/v1/repo/issue.go
+++ b/routers/api/v1/repo/issue.go
@@ -178,25 +178,22 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) {
 		DeadlineUnix: deadlineUnix,
 	}
 
-	if ctx.Repo.IsWriter() {
-		if len(form.Assignee) > 0 {
-			assignee, err := models.GetUserByName(form.Assignee)
-			if err != nil {
-				if models.IsErrUserNotExist(err) {
-					ctx.Error(422, "", fmt.Sprintf("Assignee does not exist: [name: %s]", form.Assignee))
-				} else {
-					ctx.Error(500, "GetUserByName", err)
-				}
-				return
-			}
-			issue.AssigneeID = assignee.ID
+	// Get all assignee IDs
+	assigneeIDs, err := models.MakeIDsFromAPIAssigneesToAdd(form.Assignee, form.Assignees)
+	if err != nil {
+		if models.IsErrUserNotExist(err) {
+			ctx.Error(422, "", fmt.Sprintf("Assignee does not exist: [name: %s]", err))
+		} else {
+			ctx.Error(500, "AddAssigneeByName", err)
 		}
-		issue.MilestoneID = form.Milestone
-	} else {
-		form.Labels = nil
+		return
 	}
 
-	if err := models.NewIssue(ctx.Repo.Repository, issue, form.Labels, nil); err != nil {
+	if err := models.NewIssue(ctx.Repo.Repository, issue, form.Labels, assigneeIDs, nil); err != nil {
+		if models.IsErrUserDoesNotHaveAccessToRepo(err) {
+			ctx.Error(400, "UserDoesNotHaveAccessToRepo", err)
+			return
+		}
 		ctx.Error(500, "NewIssue", err)
 		return
 	}
@@ -209,7 +206,6 @@ func CreateIssue(ctx *context.APIContext, form api.CreateIssueOption) {
 	}
 
 	// Refetch from database to assign some automatic values
-	var err error
 	issue, err = models.GetIssueByID(issue.ID)
 	if err != nil {
 		ctx.Error(500, "GetIssueByID", err)
@@ -272,6 +268,7 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 		issue.Content = *form.Body
 	}
 
+	// Update the deadline
 	var deadlineUnix util.TimeStamp
 	if form.Deadline != nil && !form.Deadline.IsZero() {
 		deadlineUnix = util.TimeStamp(form.Deadline.Unix())
@@ -282,28 +279,28 @@ func EditIssue(ctx *context.APIContext, form api.EditIssueOption) {
 		return
 	}
 
-	if ctx.Repo.IsWriter() && form.Assignee != nil &&
-		(issue.Assignee == nil || issue.Assignee.LowerName != strings.ToLower(*form.Assignee)) {
-		if len(*form.Assignee) == 0 {
-			issue.AssigneeID = 0
-		} else {
-			assignee, err := models.GetUserByName(*form.Assignee)
-			if err != nil {
-				if models.IsErrUserNotExist(err) {
-					ctx.Error(422, "", fmt.Sprintf("assignee does not exist: [name: %s]", *form.Assignee))
-				} else {
-					ctx.Error(500, "GetUserByName", err)
-				}
-				return
-			}
-			issue.AssigneeID = assignee.ID
+	// Add/delete assignees
+
+	// Deleting is done the Github way (quote from their api documentation):
+	// https://developer.github.com/v3/issues/#edit-an-issue
+	// "assignees" (array): Logins for Users to assign to this issue.
+	// Pass one or more user logins to replace the set of assignees on this Issue.
+	// Send an empty array ([]) to clear all assignees from the Issue.
+
+	if ctx.Repo.IsWriter() && (form.Assignees != nil || form.Assignee != nil) {
+
+		oneAssignee := ""
+		if form.Assignee != nil {
+			oneAssignee = *form.Assignee
 		}
 
-		if err = models.UpdateIssueUserByAssignee(issue); err != nil {
-			ctx.Error(500, "UpdateIssueUserByAssignee", err)
+		err = models.UpdateAPIAssignee(issue, oneAssignee, form.Assignees, ctx.User)
+		if err != nil {
+			ctx.Error(500, "UpdateAPIAssignee", err)
 			return
 		}
 	}
+
 	if ctx.Repo.IsWriter() && form.Milestone != nil &&
 		issue.MilestoneID != *form.Milestone {
 		oldMilestoneID := issue.MilestoneID
diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 3ea4b8d327..78d96c647f 100644
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -211,26 +211,6 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
 		milestoneID = milestone.ID
 	}
 
-	if len(form.Assignee) > 0 {
-		assigneeUser, err := models.GetUserByName(form.Assignee)
-		if err != nil {
-			if models.IsErrUserNotExist(err) {
-				ctx.Error(422, "", fmt.Sprintf("assignee does not exist: [name: %s]", form.Assignee))
-			} else {
-				ctx.Error(500, "GetUserByName", err)
-			}
-			return
-		}
-
-		assignee, err := repo.GetAssigneeByID(assigneeUser.ID)
-		if err != nil {
-			ctx.Error(500, "GetAssigneeByID", err)
-			return
-		}
-
-		assigneeID = assignee.ID
-	}
-
 	patch, err := headGitRepo.GetPatch(prInfo.MergeBase, headBranch)
 	if err != nil {
 		ctx.Error(500, "GetPatch", err)
@@ -266,7 +246,22 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
 		Type:         models.PullRequestGitea,
 	}
 
-	if err := models.NewPullRequest(repo, prIssue, labelIDs, []string{}, pr, patch); err != nil {
+	// Get all assignee IDs
+	assigneeIDs, err := models.MakeIDsFromAPIAssigneesToAdd(form.Assignee, form.Assignees)
+	if err != nil {
+		if models.IsErrUserNotExist(err) {
+			ctx.Error(422, "", fmt.Sprintf("Assignee does not exist: [name: %s]", err))
+		} else {
+			ctx.Error(500, "AddAssigneeByName", err)
+		}
+		return
+	}
+
+	if err := models.NewPullRequest(repo, prIssue, labelIDs, []string{}, pr, patch, assigneeIDs); err != nil {
+		if models.IsErrUserDoesNotHaveAccessToRepo(err) {
+			ctx.Error(400, "UserDoesNotHaveAccessToRepo", err)
+			return
+		}
 		ctx.Error(500, "NewPullRequest", err)
 		return
 	} else if err := pr.PushToBaseRepo(); err != nil {
@@ -335,6 +330,7 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
 		issue.Content = form.Body
 	}
 
+	// Update Deadline
 	var deadlineUnix util.TimeStamp
 	if form.Deadline != nil && !form.Deadline.IsZero() {
 		deadlineUnix = util.TimeStamp(form.Deadline.Unix())
@@ -345,28 +341,27 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
 		return
 	}
 
-	if ctx.Repo.IsWriter() && len(form.Assignee) > 0 &&
-		(issue.Assignee == nil || issue.Assignee.LowerName != strings.ToLower(form.Assignee)) {
-		if len(form.Assignee) == 0 {
-			issue.AssigneeID = 0
-		} else {
-			assignee, err := models.GetUserByName(form.Assignee)
-			if err != nil {
-				if models.IsErrUserNotExist(err) {
-					ctx.Error(422, "", fmt.Sprintf("assignee does not exist: [name: %s]", form.Assignee))
-				} else {
-					ctx.Error(500, "GetUserByName", err)
-				}
-				return
-			}
-			issue.AssigneeID = assignee.ID
-		}
+	// Add/delete assignees
 
-		if err = models.UpdateIssueUserByAssignee(issue); err != nil {
-			ctx.Error(500, "UpdateIssueUserByAssignee", err)
+	// Deleting is done the Github way (quote from their api documentation):
+	// https://developer.github.com/v3/issues/#edit-an-issue
+	// "assignees" (array): Logins for Users to assign to this issue.
+	// Pass one or more user logins to replace the set of assignees on this Issue.
+	// Send an empty array ([]) to clear all assignees from the Issue.
+
+	if ctx.Repo.IsWriter() && (form.Assignees != nil || len(form.Assignee) > 0) {
+
+		err = models.UpdateAPIAssignee(issue, form.Assignee, form.Assignees, ctx.User)
+		if err != nil {
+			if models.IsErrUserNotExist(err) {
+				ctx.Error(422, "", fmt.Sprintf("Assignee does not exist: [name: %s]", err))
+			} else {
+				ctx.Error(500, "UpdateAPIAssignee", err)
+			}
 			return
 		}
 	}
+
 	if ctx.Repo.IsWriter() && form.Milestone != 0 &&
 		issue.MilestoneID != form.Milestone {
 		oldMilestoneID := issue.MilestoneID
diff --git a/routers/repo/issue.go b/routers/repo/issue.go
index c7aef25594..33ba3e0d64 100644
--- a/routers/repo/issue.go
+++ b/routers/repo/issue.go
@@ -364,7 +364,7 @@ func NewIssue(ctx *context.Context) {
 }
 
 // ValidateRepoMetas check and returns repository's meta informations
-func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64, int64, int64) {
+func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64, []int64, int64) {
 	var (
 		repo = ctx.Repo.Repository
 		err  error
@@ -372,11 +372,11 @@ func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64
 
 	labels := RetrieveRepoMetas(ctx, ctx.Repo.Repository)
 	if ctx.Written() {
-		return nil, 0, 0
+		return nil, nil, 0
 	}
 
 	if !ctx.Repo.IsWriter() {
-		return nil, 0, 0
+		return nil, nil, 0
 	}
 
 	var labelIDs []int64
@@ -385,7 +385,7 @@ func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64
 	if len(form.LabelIDs) > 0 {
 		labelIDs, err = base.StringsToInt64s(strings.Split(form.LabelIDs, ","))
 		if err != nil {
-			return nil, 0, 0
+			return nil, nil, 0
 		}
 		labelIDMark := base.Int64sToMap(labelIDs)
 
@@ -407,23 +407,35 @@ func ValidateRepoMetas(ctx *context.Context, form auth.CreateIssueForm) ([]int64
 		ctx.Data["Milestone"], err = repo.GetMilestoneByID(milestoneID)
 		if err != nil {
 			ctx.ServerError("GetMilestoneByID", err)
-			return nil, 0, 0
+			return nil, nil, 0
 		}
 		ctx.Data["milestone_id"] = milestoneID
 	}
 
-	// Check assignee.
-	assigneeID := form.AssigneeID
-	if assigneeID > 0 {
-		ctx.Data["Assignee"], err = repo.GetAssigneeByID(assigneeID)
+	// Check assignees
+	var assigneeIDs []int64
+	if len(form.AssigneeIDs) > 0 {
+		assigneeIDs, err = base.StringsToInt64s(strings.Split(form.AssigneeIDs, ","))
 		if err != nil {
-			ctx.ServerError("GetAssigneeByID", err)
-			return nil, 0, 0
+			return nil, nil, 0
+		}
+
+		// Check if the passed assignees actually exists and has write access to the repo
+		for _, aID := range assigneeIDs {
+			_, err = repo.GetUserIfHasWriteAccess(aID)
+			if err != nil {
+				ctx.ServerError("GetUserIfHasWriteAccess", err)
+				return nil, nil, 0
+			}
 		}
-		ctx.Data["assignee_id"] = assigneeID
 	}
 
-	return labelIDs, milestoneID, assigneeID
+	// Keep the old assignee id thingy for compatibility reasons
+	if form.AssigneeID > 0 {
+		assigneeIDs = append(assigneeIDs, form.AssigneeID)
+	}
+
+	return labelIDs, assigneeIDs, milestoneID
 }
 
 // NewIssuePost response for creating new issue
@@ -440,7 +452,7 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
 		attachments []string
 	)
 
-	labelIDs, milestoneID, assigneeID := ValidateRepoMetas(ctx, form)
+	labelIDs, assigneeIDs, milestoneID := ValidateRepoMetas(ctx, form)
 	if ctx.Written() {
 		return
 	}
@@ -460,11 +472,14 @@ func NewIssuePost(ctx *context.Context, form auth.CreateIssueForm) {
 		PosterID:    ctx.User.ID,
 		Poster:      ctx.User,
 		MilestoneID: milestoneID,
-		AssigneeID:  assigneeID,
 		Content:     form.Content,
 		Ref:         form.Ref,
 	}
-	if err := models.NewIssue(repo, issue, labelIDs, attachments); err != nil {
+	if err := models.NewIssue(repo, issue, labelIDs, assigneeIDs, attachments); err != nil {
+		if models.IsErrUserDoesNotHaveAccessToRepo(err) {
+			ctx.Error(400, "UserDoesNotHaveAccessToRepo", err.Error())
+			return
+		}
 		ctx.ServerError("NewIssue", err)
 		return
 	}
@@ -702,8 +717,8 @@ func ViewIssue(ctx *context.Context) {
 				comment.Milestone = ghostMilestone
 			}
 		} else if comment.Type == models.CommentTypeAssignees {
-			if err = comment.LoadAssignees(); err != nil {
-				ctx.ServerError("LoadAssignees", err)
+			if err = comment.LoadAssigneeUser(); err != nil {
+				ctx.ServerError("LoadAssigneeUser", err)
 				return
 			}
 		}
@@ -912,13 +927,20 @@ func UpdateIssueAssignee(ctx *context.Context) {
 	}
 
 	assigneeID := ctx.QueryInt64("id")
+	action := ctx.Query("action")
+
 	for _, issue := range issues {
-		if issue.AssigneeID == assigneeID {
-			continue
-		}
-		if err := issue.ChangeAssignee(ctx.User, assigneeID); err != nil {
-			ctx.ServerError("ChangeAssignee", err)
-			return
+		switch action {
+		case "clear":
+			if err := models.DeleteNotPassedAssignee(issue, ctx.User, []*models.User{}); err != nil {
+				ctx.ServerError("ClearAssignees", err)
+				return
+			}
+		default:
+			if err := issue.ChangeAssignee(ctx.User, assigneeID); err != nil {
+				ctx.ServerError("ChangeAssignee", err)
+				return
+			}
 		}
 	}
 	ctx.JSON(200, map[string]interface{}{
diff --git a/routers/repo/pull.go b/routers/repo/pull.go
index a852cee1f3..dfe07ca44a 100644
--- a/routers/repo/pull.go
+++ b/routers/repo/pull.go
@@ -775,7 +775,7 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 		return
 	}
 
-	labelIDs, milestoneID, assigneeID := ValidateRepoMetas(ctx, form)
+	labelIDs, assigneeIDs, milestoneID := ValidateRepoMetas(ctx, form)
 	if ctx.Written() {
 		return
 	}
@@ -811,7 +811,6 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 		PosterID:    ctx.User.ID,
 		Poster:      ctx.User,
 		MilestoneID: milestoneID,
-		AssigneeID:  assigneeID,
 		IsPull:      true,
 		Content:     form.Content,
 	}
@@ -828,7 +827,12 @@ func CompareAndPullRequestPost(ctx *context.Context, form auth.CreateIssueForm)
 	}
 	// FIXME: check error in the case two people send pull request at almost same time, give nice error prompt
 	// instead of 500.
-	if err := models.NewPullRequest(repo, pullIssue, labelIDs, attachments, pullRequest, patch); err != nil {
+
+	if err := models.NewPullRequest(repo, pullIssue, labelIDs, attachments, pullRequest, patch, assigneeIDs); err != nil {
+		if models.IsErrUserDoesNotHaveAccessToRepo(err) {
+			ctx.Error(400, "UserDoesNotHaveAccessToRepo", err.Error())
+			return
+		}
 		ctx.ServerError("NewPullRequest", err)
 		return
 	} else if err := pullRequest.PushToBaseRepo(); err != nil {