mirror of
https://github.com/go-gitea/gitea.git
synced 2026-05-16 19:07:41 +02:00
Fix review feedback bugs
- EditProject: wrap field updates and ChangeProjectStatus in db.WithTx so a status-change failure doesn't leave a partially applied PATCH. - Validate EditProjectOption.State against open/closed; 422 on other values instead of silently treating them as open. - Align missing-issue status to 404 (the URL targets a missing resource); update existing test that was asserting the old 422. - RemoveIssueFromProjectColumn: verify the project_issue row matches the URL column before clearing the issue's project assignment, since IssueAssignOrRemoveProject(projectID=0) detaches the issue from any project regardless of column. Returns 404 if the issue isn't in this column. New test covers the cross-column case. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
This commit is contained in:
parent
438f367ab3
commit
efe43882d5
@ -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)
|
||||
|
||||
@ -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
|
||||
})
|
||||
}
|
||||
|
||||
@ -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)
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user