From 49a0d19fa3273e78ee51b781d8eb8719220b2159 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 8 Jun 2026 23:12:09 -0700 Subject: [PATCH] feat(api): Add assignees APIs (#37330) Follow https://docs.github.com/en/enterprise-server@3.20/rest/issues/assignees?apiVersion=2022-11-28 Fix #33576 And it also fixed some possible dead-lock problem. --------- Signed-off-by: Lunny Xiao Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Nicolas Co-authored-by: Zettat123 --- models/issues/assignees.go | 10 +- models/issues/assignees_test.go | 6 +- models/perm/access/repo_permission.go | 4 +- modules/structs/issue.go | 5 + routers/api/v1/api.go | 5 + routers/api/v1/repo/collaborators.go | 37 ++++ routers/api/v1/repo/issue.go | 2 +- routers/api/v1/repo/issue_assignee.go | 238 ++++++++++++++++++++++ routers/api/v1/repo/pull.go | 2 +- routers/api/v1/swagger/options.go | 2 + routers/web/repo/issue.go | 2 +- services/context/repo.go | 2 +- services/issue/assignee.go | 222 +++++++++++++++++---- services/issue/assignee_test.go | 55 +++++- services/issue/issue.go | 18 +- services/pull/pull.go | 18 +- services/repository/collaboration.go | 2 +- templates/swagger/v1_json.tmpl | 237 ++++++++++++++++++++++ templates/swagger/v1_openapi3_json.tmpl | 251 ++++++++++++++++++++++++ tests/integration/api_issue_test.go | 87 ++++++++ tests/integration/api_repo_test.go | 18 +- 21 files changed, 1155 insertions(+), 68 deletions(-) create mode 100644 routers/api/v1/repo/issue_assignee.go diff --git a/models/issues/assignees.go b/models/issues/assignees.go index a45ab468eb..91823b4ec4 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -64,8 +64,8 @@ func GetAssigneeIDsByIssue(ctx context.Context, issueID int64) ([]int64, error) } // IsUserAssignedToIssue returns true when the user is assigned to the issue -func IsUserAssignedToIssue(ctx context.Context, issue *Issue, user *user_model.User) (isAssigned bool, err error) { - return db.Exist[IssueAssignees](ctx, builder.Eq{"assignee_id": user.ID, "issue_id": issue.ID}) +func IsUserAssignedToIssue(ctx context.Context, issue *Issue, userID int64) (isAssigned bool, err error) { + return db.Exist[IssueAssignees](ctx, builder.Eq{"assignee_id": userID, "issue_id": issue.ID}) } type AssignedIssuesOptions struct { @@ -170,7 +170,7 @@ func toggleUserAssignee(ctx context.Context, issue *Issue, assigneeID int64) (re } // MakeIDsFromAPIAssigneesToAdd returns an array with all assignee IDs -func MakeIDsFromAPIAssigneesToAdd(ctx context.Context, oneAssignee string, multipleAssignees []string) (assigneeIDs []int64, err error) { +func MakeIDsFromAPIAssigneesToAdd(ctx context.Context, oneAssignee string, multipleAssignees []string) ([]int64, error) { var requestAssignees []string // Keeping the old assigning method for compatibility reasons @@ -184,7 +184,5 @@ func MakeIDsFromAPIAssigneesToAdd(ctx context.Context, oneAssignee string, multi } // Get the IDs of all assignees - assigneeIDs, err = user_model.GetUserIDsByNames(ctx, requestAssignees, false) - - return assigneeIDs, err + return user_model.GetUserIDsByNames(ctx, requestAssignees, false) } diff --git a/models/issues/assignees_test.go b/models/issues/assignees_test.go index 58a1edc02b..57c5262607 100644 --- a/models/issues/assignees_test.go +++ b/models/issues/assignees_test.go @@ -40,7 +40,7 @@ func TestUpdateAssignee(t *testing.T) { assert.NoError(t, err) // Check if he got removed - isAssigned, err := issues_model.IsUserAssignedToIssue(t.Context(), issue, user1) + isAssigned, err := issues_model.IsUserAssignedToIssue(t.Context(), issue, user1.ID) assert.NoError(t, err) assert.False(t, isAssigned) @@ -56,12 +56,12 @@ func TestUpdateAssignee(t *testing.T) { } // Check if the user is assigned - isAssigned, err = issues_model.IsUserAssignedToIssue(t.Context(), issue, user2) + isAssigned, err = issues_model.IsUserAssignedToIssue(t.Context(), issue, user2.ID) assert.NoError(t, err) assert.True(t, isAssigned) // This user should not be assigned - isAssigned, err = issues_model.IsUserAssignedToIssue(t.Context(), issue, &user_model.User{ID: 4}) + isAssigned, err = issues_model.IsUserAssignedToIssue(t.Context(), issue, 4) assert.NoError(t, err) assert.False(t, isAssigned) } diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 6e52177f0a..2d53b0699f 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -567,9 +567,9 @@ func HasAccessUnit(ctx context.Context, user *user_model.User, repo *repo_model. // CanBeAssigned return true if user can be assigned to issue or pull requests in repo // Currently any write access (code, issues or pr's) is assignable, to match assignee list in user interface. -func CanBeAssigned(ctx context.Context, user *user_model.User, repo *repo_model.Repository, _ bool) (bool, error) { +func CanBeAssigned(ctx context.Context, user *user_model.User, repo *repo_model.Repository) (bool, error) { if user.IsOrganization() { - return false, fmt.Errorf("organization can't be added as assignee [user_id: %d, repo_id: %d]", user.ID, repo.ID) + return false, util.NewInvalidArgumentErrorf("organization can't be added as assignee [user_id: %d, repo_id: %d]", user.ID, repo.ID) } perm, err := GetIndividualUserRepoPermission(ctx, repo, user) if err != nil { diff --git a/modules/structs/issue.go b/modules/structs/issue.go index 68618a3191..54e62ad2bb 100644 --- a/modules/structs/issue.go +++ b/modules/structs/issue.go @@ -125,6 +125,11 @@ type EditIssueOption struct { ContentVersion *int `json:"content_version"` } +// IssueAssigneesOption options for adding/removing issue assignees +type IssueAssigneesOption struct { + Assignees []string `json:"assignees"` +} + // EditDeadlineOption options for creating a deadline type EditDeadlineOption struct { // required:true diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 618181eb1c..e44c0cb43e 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1226,6 +1226,7 @@ func Routes() *web.Router { }) }, reqToken()) m.Get("/assignees", reqToken(), reqAnyRepoReader(), repo.GetAssignees) + m.Get("/assignees/{assignee}", reqToken(), reqAnyRepoReader(), repo.CheckRepoIssueAssignee) m.Get("/reviewers", reqToken(), reqAnyRepoReader(), repo.GetReviewers) m.Group("/teams", func() { m.Get("", reqAnyRepoReader(), repo.ListTeams) @@ -1517,6 +1518,10 @@ func Routes() *web.Router { m.Combo("").Get(repo.GetIssue). Patch(reqToken(), bind(api.EditIssueOption{}), repo.EditIssue). Delete(reqToken(), reqAdmin(), context.ReferencesGitRepo(), repo.DeleteIssue) + m.Combo("/assignees"). + Post(reqToken(), mustNotBeArchived, bind(api.IssueAssigneesOption{}), repo.AddIssueAssignees). + Delete(reqToken(), mustNotBeArchived, bind(api.IssueAssigneesOption{}), repo.DeleteIssueAssignees) + m.Get("/assignees/{assignee}", repo.CheckIssueAssignee) m.Group("/comments", func() { m.Combo("").Get(repo.ListIssueComments). Post(reqToken(), mustNotBeArchived, bind(api.CreateIssueCommentOption{}), repo.CreateIssueComment) diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index e254d5e128..07c0b95e09 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -367,5 +367,42 @@ func GetAssignees(ctx *context.APIContext) { ctx.APIErrorInternal(err) return } + ctx.JSON(http.StatusOK, convert.ToUsers(ctx, ctx.Doer, assignees)) } + +// CheckRepoIssueAssignee check if a user can be assigned to issues in a repository +func CheckRepoIssueAssignee(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/assignees/{assignee} repository repoCheckAssignee + // --- + // summary: Check if a user can be assigned to issues in a repository + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: assignee + // in: path + // description: username of the user to check for being an assignee + // type: string + // required: true + // responses: + // "204": + // "$ref": "#/responses/empty" + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + if checkAssignableUser(ctx, ctx.PathParam("assignee"), ctx.Repo.Repository) { + ctx.Status(http.StatusNoContent) + } +} diff --git a/routers/api/v1/repo/issue.go b/routers/api/v1/repo/issue.go index 9946afc8b7..fe5772fa46 100644 --- a/routers/api/v1/repo/issue.go +++ b/routers/api/v1/repo/issue.go @@ -670,7 +670,7 @@ func CreateIssue(ctx *context.APIContext) { return } - valid, err := access_model.CanBeAssigned(ctx, assignee, ctx.Repo.Repository, false) + valid, err := access_model.CanBeAssigned(ctx, assignee, ctx.Repo.Repository) if err != nil { ctx.APIErrorInternal(err) return diff --git a/routers/api/v1/repo/issue_assignee.go b/routers/api/v1/repo/issue_assignee.go new file mode 100644 index 0000000000..d045102930 --- /dev/null +++ b/routers/api/v1/repo/issue_assignee.go @@ -0,0 +1,238 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repo + +import ( + "net/http" + + issues_model "gitea.dev/models/issues" + access_model "gitea.dev/models/perm/access" + repo_model "gitea.dev/models/repo" + user_model "gitea.dev/models/user" + api "gitea.dev/modules/structs" + "gitea.dev/modules/web" + "gitea.dev/services/context" + "gitea.dev/services/convert" + issue_service "gitea.dev/services/issue" +) + +// AddIssueAssignees add assignees to an issue +func AddIssueAssignees(ctx *context.APIContext) { + // swagger:operation POST /repos/{owner}/{repo}/issues/{index}/assignees issue issueAddAssignees + // --- + // summary: Add assignees to an issue + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true + // - name: body + // in: body + // required: true + // schema: + // "$ref": "#/definitions/IssueAssigneesOption" + // responses: + // "201": + // "$ref": "#/responses/Issue" + // "400": + // "$ref": "#/responses/error" + // "403": + // "$ref": "#/responses/forbidden" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" + + opts := web.GetForm(ctx).(*api.IssueAssigneesOption) + updateIssueAssignees(ctx, *opts, true) +} + +// DeleteIssueAssignees remove assignees from an issue +func DeleteIssueAssignees(ctx *context.APIContext) { + // swagger:operation DELETE /repos/{owner}/{repo}/issues/{index}/assignees issue issueRemoveAssignees + // --- + // summary: Remove assignees from an issue + // consumes: + // - application/json + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true + // - name: body + // in: body + // required: true + // schema: + // "$ref": "#/definitions/IssueAssigneesOption" + // responses: + // "200": + // "$ref": "#/responses/Issue" + // "403": + // "$ref": "#/responses/forbidden" + // "404": + // "$ref": "#/responses/notFound" + // "422": + // "$ref": "#/responses/validationError" + + opts := web.GetForm(ctx).(*api.IssueAssigneesOption) + updateIssueAssignees(ctx, *opts, false) +} + +// CheckIssueAssignee check if a user can be assigned to an issue +func CheckIssueAssignee(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/issues/{index}/assignees/{assignee} issue issueCheckAssignee + // --- + // summary: Check if a user can be assigned to an issue + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: owner of the repo + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repo + // type: string + // required: true + // - name: index + // in: path + // description: index of the issue + // type: integer + // format: int64 + // required: true + // - name: assignee + // in: path + // description: username of the user to check for being an assignee + // type: string + // required: true + // responses: + // "204": + // "$ref": "#/responses/empty" + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("index")) + if err != nil { + ctx.APIErrorAuto(err) + return + } + + if !ctx.Repo.Permission.CanReadIssuesOrPulls(issue.IsPull) { + ctx.APIErrorNotFound() + return + } + + if checkAssignableUser(ctx, ctx.PathParam("assignee"), ctx.Repo.Repository) { + ctx.Status(http.StatusNoContent) + } +} + +// checkAssignableUser resolves assigneeName and verifies the user can be assigned to issues in repo. +// Returns true only when the user resolves AND is assignable; the caller is responsible for writing the 204. +// On any failure it writes the appropriate API response and returns false. +func checkAssignableUser(ctx *context.APIContext, assigneeName string, repo *repo_model.Repository) bool { + assignee, err := user_model.GetUserByName(ctx, assigneeName) + if err != nil { + ctx.APIErrorAuto(err) + return false + } + + canAssign, err := access_model.CanBeAssigned(ctx, assignee, repo) + if err != nil { + ctx.APIErrorAuto(err) + return false + } + + if !canAssign { + ctx.APIErrorNotFound() + return false + } + + return true +} + +func updateIssueAssignees(ctx *context.APIContext, opts api.IssueAssigneesOption, isAdd bool) { + issue, err := issues_model.GetIssueByIndex(ctx, ctx.Repo.Repository.ID, ctx.PathParamInt64("index")) + if err != nil { + ctx.APIErrorAuto(err) + return + } + + if !ctx.Repo.Permission.CanWriteIssuesOrPulls(issue.IsPull) { + ctx.Status(http.StatusForbidden) + return + } + + if err := issue.LoadAttributes(ctx); err != nil { + ctx.APIErrorInternal(err) + return + } + + assigneeIDs, err := user_model.GetUserIDsByNames(ctx, opts.Assignees, false) + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.APIError(http.StatusUnprocessableEntity, err.Error()) + return + } + ctx.APIErrorAuto(err) + return + } + + if isAdd { + err = issue_service.AddAssignees(ctx, issue, ctx.Doer, assigneeIDs) + } else { + err = issue_service.RemoveAssignees(ctx, issue, ctx.Doer, assigneeIDs) + } + + if err != nil { + ctx.APIErrorAuto(err) + return + } + + issue, err = issues_model.GetIssueByID(ctx, issue.ID) + if err != nil { + ctx.APIErrorInternal(err) + return + } + + status := http.StatusOK + if isAdd { + status = http.StatusCreated + } + ctx.JSON(status, convert.ToAPIIssue(ctx, ctx.Doer, issue)) +} diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index dcfaa3d3ac..d5c912d243 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -545,7 +545,7 @@ func CreatePullRequest(ctx *context.APIContext) { return } - valid, err := access_model.CanBeAssigned(ctx, assignee, repo, true) + valid, err := access_model.CanBeAssigned(ctx, assignee, repo) if err != nil { ctx.APIErrorInternal(err) return diff --git a/routers/api/v1/swagger/options.go b/routers/api/v1/swagger/options.go index e59857682e..c234b8ec84 100644 --- a/routers/api/v1/swagger/options.go +++ b/routers/api/v1/swagger/options.go @@ -36,6 +36,8 @@ type swaggerParameterBodies struct { EditIssueOption api.EditIssueOption // in:body EditDeadlineOption api.EditDeadlineOption + // in:body + IssueAssigneesOption api.IssueAssigneesOption // in:body CreateIssueCommentOption api.CreateIssueCommentOption diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index e37484e2ad..10c9d7d726 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -458,7 +458,7 @@ func UpdateIssueAssignee(ctx *context.Context) { return } - valid, err := access_model.CanBeAssigned(ctx, assignee, issue.Repo, issue.IsPull) + valid, err := access_model.CanBeAssigned(ctx, assignee, issue.Repo) if err != nil { ctx.ServerError("canBeAssigned", err) return diff --git a/services/context/repo.go b/services/context/repo.go index 9c7f20aadd..fa816ba6ad 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -242,7 +242,7 @@ func (r *Repository) CanUseTimetracker(ctx context.Context, issue *issues_model. // Checking for following: // 1. Is timetracker enabled // 2. Is the user a contributor, admin, poster or assignee and do the repository policies require this? - isAssigned, _ := issues_model.IsUserAssignedToIssue(ctx, issue, user) + isAssigned, _ := issues_model.IsUserAssignedToIssue(ctx, issue, user.ID) return r.Repository.IsTimetrackerEnabled(ctx) && (!r.Repository.AllowOnlyContributorsToTrackTime(ctx) || r.Permission.CanWriteIssuesOrPulls(issue.IsPull) || issue.IsPoster(user.ID) || isAssigned) } diff --git a/services/issue/assignee.go b/services/issue/assignee.go index a3d1879d85..4b24115507 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -6,6 +6,7 @@ package issue import ( "context" + "gitea.dev/models/db" issues_model "gitea.dev/models/issues" access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" @@ -14,8 +15,7 @@ import ( notify_service "gitea.dev/services/notify" ) -// DeleteNotPassedAssignee deletes all assignees who aren't passed via the "assignees" array -func DeleteNotPassedAssignee(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assignees []*user_model.User) (err error) { +func toBeRemovedAssignees(issue *issues_model.Issue, assignees []*user_model.User) (toBeRemovedAssignees []*user_model.User) { var found bool oriAssignees := make([]*user_model.User, len(issue.Assignees)) _ = copy(oriAssignees, issue.Assignees) @@ -31,28 +31,54 @@ func DeleteNotPassedAssignee(ctx context.Context, issue *issues_model.Issue, doe if !found { // This function also does comments and hooks, which is why we call it separately instead of directly removing the assignees here - if _, _, err := ToggleAssigneeWithNotify(ctx, issue, doer, assignee.ID); err != nil { - return err - } + toBeRemovedAssignees = append(toBeRemovedAssignees, assignee) + } + } + return toBeRemovedAssignees +} + +// DeleteNotPassedAssignee deletes all assignees who aren't passed via the "assignees" array +func DeleteNotPassedAssignee(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assignees []*user_model.User) (err error) { + toBeRemoved := toBeRemovedAssignees(issue, assignees) + + for _, assignee := range toBeRemoved { + // This function also does comments and hooks, which is why we call it separately instead of directly removing the assignees here + removed, comment, err := ToggleAssignee(ctx, issue, doer, assignee) + if err != nil { + return err + } + if removed { + notify_service.IssueChangeAssignee(ctx, doer, issue, assignee, true, comment) } } return nil } -// ToggleAssigneeWithNoNotify changes a user between assigned and not assigned for this issue, and make issue comment for it. -func ToggleAssigneeWithNotify(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *issues_model.Comment, err error) { - removed, comment, err = issues_model.ToggleIssueAssignee(ctx, issue, doer, assigneeID) +// ToggleAssignee changes a user between assigned and not assigned for this issue, and make issue comment for it. +func ToggleAssignee(ctx context.Context, issue *issues_model.Issue, doer, assignee *user_model.User) (removed bool, comment *issues_model.Comment, err error) { + removed, comment, err = issues_model.ToggleIssueAssignee(ctx, issue, doer, assignee.ID) if err != nil { return false, nil, err } + issue.AssigneeID = assignee.ID + issue.Assignee = assignee + + return removed, comment, nil +} + +// ToggleAssignee changes a user between assigned and not assigned for this issue, and make issue comment for it. +func ToggleAssigneeWithNotify(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeID int64) (removed bool, comment *issues_model.Comment, err error) { assignee, err := user_model.GetUserByID(ctx, assigneeID) if err != nil { return false, nil, err } - issue.AssigneeID = assigneeID - issue.Assignee = assignee + + removed, comment, err = ToggleAssignee(ctx, issue, doer, assignee) + if err != nil { + return false, nil, err + } notify_service.IssueChangeAssignee(ctx, doer, issue, assignee, removed, comment) @@ -81,43 +107,85 @@ func UpdateAssignees(ctx context.Context, issue *issues_model.Issue, oneAssignee return err } - if user_model.IsUserBlockedBy(ctx, doer, assignee.ID) { - return user_model.ErrBlockedUser + if err := validateAssignee(ctx, issue, doer, assignee); err != nil { + return err } allNewAssignees = append(allNewAssignees, assignee) } - // Delete all old assignees not passed - if err = DeleteNotPassedAssignee(ctx, issue, doer, allNewAssignees); err != nil { + assigneeCommentMap := make(map[int64]*issues_model.Comment) + assigneeRemovedCommentMap := make(map[int64]*issues_model.Comment) + assigneeRemoved := make(map[int64]*user_model.User) + if err := db.WithTx(ctx, func(ctx context.Context) error { + // Delete all old assignees not passed. + toBeRemoved := toBeRemovedAssignees(issue, allNewAssignees) + + for _, assignee := range toBeRemoved { + // This function also does comments and hooks, which is why we call it separately instead of directly removing the assignees here + removed, comment, err := ToggleAssignee(ctx, issue, doer, assignee) + if err != nil { + return err + } + if removed { + assigneeRemoved[assignee.ID] = assignee + assigneeRemovedCommentMap[assignee.ID] = comment + } + } + + // Add all new assignees. + // Update the assignee. The function will check if the user exists, is already + // assigned (which he shouldn't as we deleted all assignees before) and + // has access to the repo. + for _, assignee := range allNewAssignees { + // Extra method to prevent double adding (which would result in removing). + comment, err := AddAssigneeIfNotAssigned(ctx, issue, doer, assignee) + if err != nil { + return err + } + assigneeCommentMap[assignee.ID] = comment + } + + return nil + }); err != nil { return err } - // Add all new assignees - // Update the assignee. The function will check if the user exists, is already - // assigned (which he shouldn't as we deleted all assignees before) and - // has access to the repo. + for _, assignee := range assigneeRemoved { + notify_service.IssueChangeAssignee(ctx, doer, issue, assignee, true, assigneeRemovedCommentMap[assignee.ID]) + } + for _, assignee := range allNewAssignees { - // Extra method to prevent double adding (which would result in removing) - _, err = AddAssigneeIfNotAssigned(ctx, issue, doer, assignee.ID, true) - if err != nil { - return err + comment := assigneeCommentMap[assignee.ID] + if comment != nil { + notify_service.IssueChangeAssignee(ctx, doer, issue, assignee, false, comment) } } - return err + return nil +} + +func validateAssignee(ctx context.Context, issue *issues_model.Issue, doer, assignee *user_model.User) error { + if user_model.IsUserBlockedBy(ctx, doer, assignee.ID) { + return user_model.ErrBlockedUser + } + + valid, err := access_model.CanBeAssigned(ctx, assignee, issue.Repo) + if err != nil { + return err + } + if !valid { + return repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: assignee.ID, RepoName: issue.Repo.Name} + } + + return nil } // AddAssigneeIfNotAssigned adds an assignee only if he isn't already assigned to the issue. // Also checks for access of assigned user -func AddAssigneeIfNotAssigned(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeID int64, notify bool) (comment *issues_model.Comment, err error) { - assignee, err := user_model.GetUserByID(ctx, assigneeID) - if err != nil { - return nil, err - } - +func AddAssigneeIfNotAssigned(ctx context.Context, issue *issues_model.Issue, doer, assignee *user_model.User) (comment *issues_model.Comment, err error) { // Check if the user is already assigned - isAssigned, err := issues_model.IsUserAssignedToIssue(ctx, issue, assignee) + isAssigned, err := issues_model.IsUserAssignedToIssue(ctx, issue, assignee.ID) if err != nil { return nil, err } @@ -126,18 +194,92 @@ func AddAssigneeIfNotAssigned(ctx context.Context, issue *issues_model.Issue, do return nil, nil //nolint:nilnil // return nil because the user is already assigned } - valid, err := access_model.CanBeAssigned(ctx, assignee, issue.Repo, issue.IsPull) - if err != nil { + if err := validateAssignee(ctx, issue, doer, assignee); err != nil { return nil, err } - if !valid { - return nil, repo_model.ErrUserDoesNotHaveAccessToRepo{UserID: assigneeID, RepoName: issue.Repo.Name} - } - if notify { - _, comment, err = ToggleAssigneeWithNotify(ctx, issue, doer, assigneeID) - return comment, err - } - _, comment, err = issues_model.ToggleIssueAssignee(ctx, issue, doer, assigneeID) + _, comment, err = issues_model.ToggleIssueAssignee(ctx, issue, doer, assignee.ID) return comment, err } + +// AddAssignees adds multiple assignees to an issue atomically. +func AddAssignees(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeIDs []int64) error { + assigneeCommentMap := make(map[int64]*issues_model.Comment) + assignees := make(map[int64]*user_model.User) + if err := db.WithTx(ctx, func(ctx context.Context) error { + for _, assigneeID := range assigneeIDs { + isAssigned, err := issues_model.IsUserAssignedToIssue(ctx, issue, assigneeID) + if err != nil { + return err + } + if isAssigned { + continue + } + + assignee, err := user_model.GetUserByID(ctx, assigneeID) + if err != nil { + return err + } + if err := validateAssignee(ctx, issue, doer, assignee); err != nil { + return err + } + + comment, err := AddAssigneeIfNotAssigned(ctx, issue, doer, assignee) + if err != nil { + return err + } + assignees[assigneeID] = assignee + assigneeCommentMap[assigneeID] = comment + } + + return nil + }); err != nil { + return err + } + + if len(assignees) > 0 { + for assigneeID, assignee := range assignees { + notify_service.IssueChangeAssignee(ctx, doer, issue, assignee, false, assigneeCommentMap[assigneeID]) + } + } + return nil +} + +// RemoveAssignees removes multiple assignees from an issue atomically. +func RemoveAssignees(ctx context.Context, issue *issues_model.Issue, doer *user_model.User, assigneeIDs []int64) error { + assigneeCommentMap := make(map[int64]*issues_model.Comment) + assignees := make(map[int64]*user_model.User) + if err := db.WithTx(ctx, func(ctx context.Context) error { + for _, assigneeID := range assigneeIDs { + isAssigned, err := issues_model.IsUserAssignedToIssue(ctx, issue, assigneeID) + if err != nil { + return err + } + if !isAssigned { + continue + } + removed, comment, err := issues_model.ToggleIssueAssignee(ctx, issue, doer, assigneeID) + if err != nil { + return err + } + if removed { + assignee, err := user_model.GetUserByID(ctx, assigneeID) + if err != nil { + return err + } + assignees[assigneeID] = assignee + assigneeCommentMap[assigneeID] = comment + } + } + return nil + }); err != nil { + return err + } + + if len(assignees) > 0 { + for assigneeID, assignee := range assignees { + notify_service.IssueChangeAssignee(ctx, doer, issue, assignee, true, assigneeCommentMap[assigneeID]) + } + } + return nil +} diff --git a/services/issue/assignee_test.go b/services/issue/assignee_test.go index d020df717d..4c427a2849 100644 --- a/services/issue/assignee_test.go +++ b/services/issue/assignee_test.go @@ -6,6 +6,7 @@ package issue import ( "testing" + "gitea.dev/models/db" issues_model "gitea.dev/models/issues" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" @@ -29,7 +30,7 @@ func TestDeleteNotPassedAssignee(t *testing.T) { assert.NoError(t, err) // Check if he got removed - isAssigned, err := issues_model.IsUserAssignedToIssue(t.Context(), issue, user1) + isAssigned, err := issues_model.IsUserAssignedToIssue(t.Context(), issue, user1.ID) assert.NoError(t, err) assert.True(t, isAssigned) @@ -44,3 +45,55 @@ func TestDeleteNotPassedAssignee(t *testing.T) { assert.Empty(t, issue.Assignees) assert.Empty(t, issue.Assignee) } + +func TestAddAssigneeIfNotAssignedBlocked(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue, err := issues_model.GetIssueByID(t.Context(), 1) + assert.NoError(t, err) + assert.NoError(t, issue.LoadRepo(t.Context())) + + doer, err := user_model.GetUserByID(t.Context(), 4) + assert.NoError(t, err) + + assignee, err := user_model.GetUserByID(t.Context(), 2) + assert.NoError(t, err) + + assert.NoError(t, db.Insert(t.Context(), &user_model.Blocking{ + BlockerID: assignee.ID, + BlockeeID: doer.ID, + })) + + _, err = AddAssigneeIfNotAssigned(t.Context(), issue, doer, assignee) + assert.ErrorIs(t, err, user_model.ErrBlockedUser) + + isAssigned, err := issues_model.IsUserAssignedToIssue(t.Context(), issue, assignee.ID) + assert.NoError(t, err) + assert.False(t, isAssigned) +} + +func TestAddAssigneesBlockedIsAtomic(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + issue, err := issues_model.GetIssueByID(t.Context(), 1) + assert.NoError(t, err) + assert.NoError(t, issue.LoadAttributes(t.Context())) + + doer, err := user_model.GetUserByID(t.Context(), 2) + assert.NoError(t, err) + + blockedAssignee, err := user_model.GetUserByID(t.Context(), 40) + assert.NoError(t, err) + + assert.NoError(t, db.Insert(t.Context(), &user_model.Blocking{ + BlockerID: blockedAssignee.ID, + BlockeeID: doer.ID, + })) + + err = AddAssignees(t.Context(), issue, doer, []int64{doer.ID, blockedAssignee.ID}) + assert.ErrorIs(t, err, user_model.ErrBlockedUser) + + assigneeIDs, err := issues_model.GetAssigneeIDsByIssue(t.Context(), issue.ID) + assert.NoError(t, err) + assert.ElementsMatch(t, []int64{1}, assigneeIDs) +} diff --git a/services/issue/issue.go b/services/issue/issue.go index 1c7fb7761e..511357372f 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -32,14 +32,24 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_mo return user_model.ErrBlockedUser } + assigneeCommentMap := make(map[int64]*issues_model.Comment) + assignees := make(map[int64]*user_model.User) if err := db.WithTx(ctx, func(ctx context.Context) error { if err := issues_model.NewIssue(ctx, repo, issue, labelIDs, uuids); err != nil { return err } for _, assigneeID := range assigneeIDs { - if _, err := AddAssigneeIfNotAssigned(ctx, issue, issue.Poster, assigneeID, true); err != nil { + assignee, err := user_model.GetUserByID(ctx, assigneeID) + if err != nil { + log.Error("GetUserByID: %v", err) + continue + } + assignees[assigneeID] = assignee + comment, err := AddAssigneeIfNotAssigned(ctx, issue, issue.Poster, assignee) + if err != nil { return err } + assigneeCommentMap[assigneeID] = comment } if len(projectIDs) > 0 { err := issues_model.IssueAssignOrRemoveProject(ctx, issue, issue.Poster, projectIDs) @@ -65,6 +75,12 @@ func NewIssue(ctx context.Context, repo *repo_model.Repository, issue *issues_mo notify_service.IssueChangeMilestone(ctx, issue.Poster, issue, 0) } + if len(assigneeIDs) > 0 { + for _, assignee := range assignees { + notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assignee.ID]) + } + } + return nil } diff --git a/services/pull/pull.go b/services/pull/pull.go index a601450ff2..a12adab99d 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -96,7 +96,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } assigneeCommentMap := make(map[int64]*issues_model.Comment) - + assignees := make(map[int64]*user_model.User) var reviewNotifiers []*issue_service.ReviewRequestNotifier if err := db.WithTx(ctx, func(ctx context.Context) error { if err := issues_model.NewPullRequest(ctx, repo, issue, labelIDs, uuids, pr); err != nil { @@ -104,10 +104,16 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { } for _, assigneeID := range assigneeIDs { - comment, err := issue_service.AddAssigneeIfNotAssigned(ctx, issue, issue.Poster, assigneeID, false) + assignee, err := user_model.GetUserByID(ctx, assigneeID) + if err != nil { + log.Error("GetUserByID: %v", err) + continue + } + comment, err := issue_service.AddAssigneeIfNotAssigned(ctx, issue, issue.Poster, assignee) if err != nil { return err } + assignees[assigneeID] = assignee assigneeCommentMap[assigneeID] = comment } @@ -186,12 +192,8 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { if issue.Milestone != nil { notify_service.IssueChangeMilestone(ctx, issue.Poster, issue, 0) } - for _, assigneeID := range assigneeIDs { - assignee, err := user_model.GetUserByID(ctx, assigneeID) - if err != nil { - return ErrDependenciesLeft - } - notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assigneeID]) + for _, assignee := range assignees { + notify_service.IssueChangeAssignee(ctx, issue.Poster, issue, assignee, false, assigneeCommentMap[assignee.ID]) } return nil diff --git a/services/repository/collaboration.go b/services/repository/collaboration.go index 9bbafb5c0c..68bb88cf87 100644 --- a/services/repository/collaboration.go +++ b/services/repository/collaboration.go @@ -100,7 +100,7 @@ func DeleteCollaboration(ctx context.Context, repo *repo_model.Repository, colla } func ReconsiderRepoIssuesAssignee(ctx context.Context, repo *repo_model.Repository, user *user_model.User) error { - if canAssigned, err := access_model.CanBeAssigned(ctx, user, repo, true); err != nil || canAssigned { + if canAssigned, err := access_model.CanBeAssigned(ctx, user, repo); err != nil || canAssigned { return err } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 0463c31d2a..13347e0fce 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -6766,6 +6766,52 @@ } } }, + "/repos/{owner}/{repo}/assignees/{assignee}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Check if a user can be assigned to issues in a repository", + "operationId": "repoCheckAssignee", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "username of the user to check for being an assignee", + "name": "assignee", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/avatar": { "post": { "produces": [ @@ -10962,6 +11008,183 @@ } } }, + "/repos/{owner}/{repo}/issues/{index}/assignees": { + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Add assignees to an issue", + "operationId": "issueAddAssignees", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/IssueAssigneesOption" + } + } + ], + "responses": { + "201": { + "$ref": "#/responses/Issue" + }, + "400": { + "$ref": "#/responses/error" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + }, + "delete": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Remove assignees from an issue", + "operationId": "issueRemoveAssignees", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, + { + "name": "body", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/IssueAssigneesOption" + } + } + ], + "responses": { + "200": { + "$ref": "#/responses/Issue" + }, + "403": { + "$ref": "#/responses/forbidden" + }, + "404": { + "$ref": "#/responses/notFound" + }, + "422": { + "$ref": "#/responses/validationError" + } + } + } + }, + "/repos/{owner}/{repo}/issues/{index}/assignees/{assignee}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "issue" + ], + "summary": "Check if a user can be assigned to an issue", + "operationId": "issueCheckAssignee", + "parameters": [ + { + "type": "string", + "description": "owner of the repo", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repo", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "format": "int64", + "description": "index of the issue", + "name": "index", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "username of the user to check for being an assignee", + "name": "assignee", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "$ref": "#/responses/empty" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/issues/{index}/blocks": { "get": { "produces": [ @@ -26838,6 +27061,20 @@ }, "x-go-package": "gitea.dev/modules/structs" }, + "IssueAssigneesOption": { + "description": "IssueAssigneesOption options for adding/removing issue assignees", + "type": "object", + "properties": { + "assignees": { + "type": "array", + "items": { + "type": "string" + }, + "x-go-name": "Assignees" + } + }, + "x-go-package": "gitea.dev/modules/structs" + }, "IssueConfig": { "type": "object", "properties": { diff --git a/templates/swagger/v1_openapi3_json.tmpl b/templates/swagger/v1_openapi3_json.tmpl index fcc1603ce1..b4ea4187fe 100644 --- a/templates/swagger/v1_openapi3_json.tmpl +++ b/templates/swagger/v1_openapi3_json.tmpl @@ -6992,6 +6992,20 @@ "type": "object", "x-go-package": "gitea.dev/modules/structs" }, + "IssueAssigneesOption": { + "description": "IssueAssigneesOption options for adding/removing issue assignees", + "properties": { + "assignees": { + "items": { + "type": "string" + }, + "type": "array", + "x-go-name": "Assignees" + } + }, + "type": "object", + "x-go-package": "gitea.dev/modules/structs" + }, "IssueConfig": { "properties": { "blank_issues_enabled": { @@ -17814,6 +17828,55 @@ ] } }, + "/repos/{owner}/{repo}/assignees/{assignee}": { + "get": { + "operationId": "repoCheckAssignee", + "parameters": [ + { + "description": "owner of the repo", + "in": "path", + "name": "owner", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "name of the repo", + "in": "path", + "name": "repo", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "username of the user to check for being an assignee", + "in": "path", + "name": "assignee", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "204": { + "$ref": "#/components/responses/empty" + }, + "400": { + "$ref": "#/components/responses/error" + }, + "404": { + "$ref": "#/components/responses/notFound" + } + }, + "summary": "Check if a user can be assigned to issues in a repository", + "tags": [ + "repository" + ] + } + }, "/repos/{owner}/{repo}/avatar": { "delete": { "operationId": "repoDeleteAvatar", @@ -22371,6 +22434,194 @@ ] } }, + "/repos/{owner}/{repo}/issues/{index}/assignees": { + "delete": { + "operationId": "issueRemoveAssignees", + "parameters": [ + { + "description": "owner of the repo", + "in": "path", + "name": "owner", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "name of the repo", + "in": "path", + "name": "repo", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "index of the issue", + "in": "path", + "name": "index", + "required": true, + "schema": { + "format": "int64", + "type": "integer" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IssueAssigneesOption" + } + } + }, + "required": true, + "x-originalParamName": "body" + }, + "responses": { + "200": { + "$ref": "#/components/responses/Issue" + }, + "403": { + "$ref": "#/components/responses/forbidden" + }, + "404": { + "$ref": "#/components/responses/notFound" + }, + "422": { + "$ref": "#/components/responses/validationError" + } + }, + "summary": "Remove assignees from an issue", + "tags": [ + "issue" + ] + }, + "post": { + "operationId": "issueAddAssignees", + "parameters": [ + { + "description": "owner of the repo", + "in": "path", + "name": "owner", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "name of the repo", + "in": "path", + "name": "repo", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "index of the issue", + "in": "path", + "name": "index", + "required": true, + "schema": { + "format": "int64", + "type": "integer" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IssueAssigneesOption" + } + } + }, + "required": true, + "x-originalParamName": "body" + }, + "responses": { + "201": { + "$ref": "#/components/responses/Issue" + }, + "400": { + "$ref": "#/components/responses/error" + }, + "403": { + "$ref": "#/components/responses/forbidden" + }, + "404": { + "$ref": "#/components/responses/notFound" + }, + "422": { + "$ref": "#/components/responses/validationError" + } + }, + "summary": "Add assignees to an issue", + "tags": [ + "issue" + ] + } + }, + "/repos/{owner}/{repo}/issues/{index}/assignees/{assignee}": { + "get": { + "operationId": "issueCheckAssignee", + "parameters": [ + { + "description": "owner of the repo", + "in": "path", + "name": "owner", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "name of the repo", + "in": "path", + "name": "repo", + "required": true, + "schema": { + "type": "string" + } + }, + { + "description": "index of the issue", + "in": "path", + "name": "index", + "required": true, + "schema": { + "format": "int64", + "type": "integer" + } + }, + { + "description": "username of the user to check for being an assignee", + "in": "path", + "name": "assignee", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "204": { + "$ref": "#/components/responses/empty" + }, + "400": { + "$ref": "#/components/responses/error" + }, + "404": { + "$ref": "#/components/responses/notFound" + } + }, + "summary": "Check if a user can be assigned to an issue", + "tags": [ + "issue" + ] + } + }, "/repos/{owner}/{repo}/issues/{index}/blocks": { "delete": { "operationId": "issueRemoveIssueBlocking", diff --git a/tests/integration/api_issue_test.go b/tests/integration/api_issue_test.go index 67a7e2b590..b77775d173 100644 --- a/tests/integration/api_issue_test.go +++ b/tests/integration/api_issue_test.go @@ -13,6 +13,7 @@ import ( "time" auth_model "gitea.dev/models/auth" + "gitea.dev/models/db" issues_model "gitea.dev/models/issues" repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" @@ -35,6 +36,7 @@ func TestAPIIssue(t *testing.T) { t.Run("IssueContentVersion", testAPIIssueContentVersion) t.Run("CreateIssue", testAPICreateIssue) t.Run("CreateIssueParallel", testAPICreateIssueParallel) + t.Run("IssueAssignees", testAPIIssueAssignees) t.Run("IssueProjects", testAPIIssueProjects) } @@ -495,6 +497,91 @@ func testAPIIssueContentVersion(t *testing.T) { }) } +func testAPIIssueAssignees(t *testing.T) { + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: issue.RepoID}) + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + + session := loginUser(t, owner.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteIssue) + + urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assignees", owner.Name, repo.Name, issue.Index) + getAssigneeIDs := func(issueID int64) []int64 { + assigneeIDs, err := issues_model.GetAssigneeIDsByIssue(t.Context(), issueID) + assert.NoError(t, err) + return assigneeIDs + } + + t.Run("NonWriter", func(t *testing.T) { + req := NewRequestWithJSON(t, "POST", urlStr, &api.IssueAssigneesOption{Assignees: []string{"user40"}}). + AddTokenAuth(getUserToken(t, "user5", auth_model.AccessTokenScopeWriteIssue)) + MakeRequest(t, req, http.StatusForbidden) + }) + + t.Run("MissingIssue", func(t *testing.T) { + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/issues/99999/assignees", owner.Name, repo.Name), &api.IssueAssigneesOption{Assignees: []string{"user40"}}). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) + }) + + t.Run("UnknownAssignee", func(t *testing.T) { + req := NewRequestWithJSON(t, "POST", urlStr, &api.IssueAssigneesOption{Assignees: []string{"does-not-exist"}}). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusUnprocessableEntity) + apiErr := DecodeJSON(t, resp, &api.APIError{}) + assert.Equal(t, "user does not exist [uid: 0, name: does-not-exist]", apiErr.Message) + }) + + t.Run("OrganizationAssignee", func(t *testing.T) { + req := NewRequestWithJSON(t, "POST", urlStr, &api.IssueAssigneesOption{Assignees: []string{"org3"}}). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusBadRequest) + + checkReq := NewRequest(t, "GET", fmt.Sprintf("%s/%s", urlStr, "org3")).AddTokenAuth(token) + MakeRequest(t, checkReq, http.StatusBadRequest) + }) + + t.Run("BlockedAssigneeIsAtomic", func(t *testing.T) { + blockedIssue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 5}) + blockedURL := fmt.Sprintf("/api/v1/repos/%s/%s/issues/%d/assignees", owner.Name, repo.Name, blockedIssue.Index) + blockedToken := getUserToken(t, "user40", auth_model.AccessTokenScopeWriteIssue) + + assert.Empty(t, getAssigneeIDs(blockedIssue.ID)) + assert.NoError(t, db.Insert(t.Context(), &user_model.Blocking{ + BlockerID: owner.ID, + BlockeeID: 40, + })) + + req := NewRequestWithJSON(t, "POST", blockedURL, &api.IssueAssigneesOption{Assignees: []string{"user1", "user2"}}). + AddTokenAuth(blockedToken) + MakeRequest(t, req, http.StatusForbidden) + assert.Empty(t, getAssigneeIDs(blockedIssue.ID)) + }) + + t.Run("HappyPath", func(t *testing.T) { + req := NewRequestWithJSON(t, "POST", urlStr, &api.IssueAssigneesOption{Assignees: []string{"user40"}}). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusCreated) + apiIssue := DecodeJSON(t, resp, &api.Issue{}) + assert.Len(t, apiIssue.Assignees, 2) + assert.ElementsMatch(t, []int64{1, 40}, []int64{apiIssue.Assignees[0].ID, apiIssue.Assignees[1].ID}) + + checkReq := NewRequest(t, "GET", fmt.Sprintf("%s/%s", urlStr, "user40")).AddTokenAuth(token) + MakeRequest(t, checkReq, http.StatusNoContent) + + // This endpoint checks assignability, not current assignment membership. + checkReq = NewRequest(t, "GET", fmt.Sprintf("%s/%s", urlStr, "user5")).AddTokenAuth(token) + MakeRequest(t, checkReq, http.StatusNoContent) + + req = NewRequestWithJSON(t, "DELETE", urlStr, &api.IssueAssigneesOption{Assignees: []string{"user1"}}). + AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + apiIssue = DecodeJSON(t, resp, &api.Issue{}) + assert.Len(t, apiIssue.Assignees, 1) + assert.Equal(t, int64(40), apiIssue.Assignees[0].ID) + }) +} + func testAPIIssueProjects(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index 45d904c261..47d8effaab 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -721,11 +721,25 @@ func TestAPIRepoGetAssignees(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) req := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/assignees", user.Name, repo.Name). AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) assignees := DecodeJSON(t, resp, []*api.User{}) - assert.Len(t, assignees, 2) + assert.Len(t, assignees, 1) + + assignee := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/assignees/%s", user.Name, repo.Name, assignee.Name). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + + nonAssignee := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/assignees/%s", user.Name, repo.Name, nonAssignee.Name). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) + + req = NewRequestf(t, "GET", "/api/v1/repos/%s/%s/assignees/%s", user.Name, repo.Name, "org3"). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusBadRequest) }