diff --git a/routers/api/v1/repo/project.go b/routers/api/v1/repo/project.go index cb9177dc1b..cc4110d139 100644 --- a/routers/api/v1/repo/project.go +++ b/routers/api/v1/repo/project.go @@ -277,7 +277,15 @@ func EditProject(ctx *context.APIContext) { opts.CardType = optional.Some(project_model.CardType(*form.CardType)) } if form.State != nil { - opts.IsClosed = optional.Some(*form.State == string(api.StateClosed)) + switch api.StateType(*form.State) { + case api.StateOpen: + opts.IsClosed = optional.Some(false) + case api.StateClosed: + opts.IsClosed = optional.Some(true) + default: + ctx.APIError(http.StatusUnprocessableEntity, "state must be 'open' or 'closed'") + return + } } if err := project_service.UpdateProject(ctx, project, opts); err != nil { ctx.APIErrorInternal(err) @@ -765,9 +773,25 @@ func assignIssueToProjectColumn(ctx *context.APIContext, add bool) { return } - projectID := int64(0) - if add { - projectID = column.ProjectID + projectID := column.ProjectID + if !add { + // Confirm the issue is currently in this specific column before removing, + // since IssueAssignOrRemoveProject(projectID=0) clears the issue's project + // assignment unconditionally. + exists, err := db.GetEngine(ctx).Exist(&project_model.ProjectIssue{ + IssueID: issue.ID, + ProjectID: column.ProjectID, + ProjectColumnID: column.ID, + }) + if err != nil { + ctx.APIErrorInternal(err) + return + } + if !exists { + ctx.APIErrorNotFound() + return + } + projectID = 0 } if err := issues_model.IssueAssignOrRemoveProject(ctx, issue, ctx.Doer, projectID, column.ID); err != nil { ctx.APIErrorInternal(err) diff --git a/services/projects/project.go b/services/projects/project.go index d64769cb87..8d4296cfdd 100644 --- a/services/projects/project.go +++ b/services/projects/project.go @@ -6,6 +6,7 @@ package project import ( "context" + "code.gitea.io/gitea/models/db" project_model "code.gitea.io/gitea/models/project" "code.gitea.io/gitea/modules/optional" ) @@ -18,24 +19,26 @@ type UpdateProjectOptions struct { IsClosed optional.Option[bool] } -// UpdateProject applies the provided options to the project. +// UpdateProject applies the provided options to the project atomically. func UpdateProject(ctx context.Context, project *project_model.Project, opts UpdateProjectOptions) error { - if opts.Title.Has() { - project.Title = opts.Title.Value() - } - if opts.Description.Has() { - project.Description = opts.Description.Value() - } - if opts.CardType.Has() { - project.CardType = opts.CardType.Value() - } - if err := project_model.UpdateProject(ctx, project); err != nil { - return err - } - if opts.IsClosed.Has() && opts.IsClosed.Value() != project.IsClosed { - if err := project_model.ChangeProjectStatus(ctx, project, opts.IsClosed.Value()); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + if opts.Title.Has() { + project.Title = opts.Title.Value() + } + if opts.Description.Has() { + project.Description = opts.Description.Value() + } + if opts.CardType.Has() { + project.CardType = opts.CardType.Value() + } + if err := project_model.UpdateProject(ctx, project); err != nil { return err } - } - return nil + if opts.IsClosed.Has() && opts.IsClosed.Value() != project.IsClosed { + if err := project_model.ChangeProjectStatus(ctx, project, opts.IsClosed.Value()); err != nil { + return err + } + } + return nil + }) } diff --git a/tests/integration/api_repo_project_test.go b/tests/integration/api_repo_project_test.go index 6785f8d276..f2740556e9 100644 --- a/tests/integration/api_repo_project_test.go +++ b/tests/integration/api_repo_project_test.go @@ -230,6 +230,13 @@ func testAPIChangeProjectStatus(t *testing.T) { DecodeJSON(t, resp, &updatedProject) assert.False(t, updatedProject.IsClosed) + + // Invalid state value must be rejected + bogus := "reopen" + req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/repos/%s/%s/projects/%d", owner.Name, repo.Name, project.ID), &api.EditProjectOption{ + State: &bogus, + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusUnprocessableEntity) } func testAPIDeleteProject(t *testing.T) { @@ -528,7 +535,7 @@ func testAPIAddIssueToProjectColumn(t *testing.T) { // Test adding non-existent issue req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/projects/%d/columns/%d/issues/%d", owner.Name, repo.Name, project.ID, column1.ID, 99999), nil).AddTokenAuth(token) - MakeRequest(t, req, http.StatusUnprocessableEntity) + MakeRequest(t, req, http.StatusNotFound) // Test adding to non-existent column req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/projects/%d/columns/99999/issues/%d", owner.Name, repo.Name, project.ID, issue.ID), nil).AddTokenAuth(token) @@ -605,12 +612,29 @@ func testAPIRemoveIssueFromProjectColumn(t *testing.T) { err = project_model.NewColumn(t.Context(), column) assert.NoError(t, err) + otherColumn := &project_model.Column{ + Title: "Other Column", + ProjectID: project.ID, + CreatorID: owner.ID, + } + err = project_model.NewColumn(t.Context(), otherColumn) + assert.NoError(t, err) + err = issues_model.IssueAssignOrRemoveProject(t.Context(), issue, owner, project.ID, column.ID) assert.NoError(t, err) token := getUserToken(t, owner.Name, auth_model.AccessTokenScopeWriteIssue) - req := NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/projects/%d/columns/%d/issues/%d", owner.Name, repo.Name, project.ID, column.ID, issue.ID), nil). + // Removing via a column the issue does not live in must 404 and not detach the issue + req := NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/projects/%d/columns/%d/issues/%d", owner.Name, repo.Name, project.ID, otherColumn.ID, issue.ID), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) + unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{ + ProjectID: project.ID, + IssueID: issue.ID, + }) + + req = NewRequestWithJSON(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/%s/projects/%d/columns/%d/issues/%d", owner.Name, repo.Name, project.ID, column.ID, issue.ID), nil). AddTokenAuth(token) MakeRequest(t, req, http.StatusNoContent)