diff --git a/services/projects/issue.go b/services/projects/issue.go index 3655a000ce..6478ffd2a7 100644 --- a/services/projects/issue.go +++ b/services/projects/issue.go @@ -85,7 +85,16 @@ func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, colum } } - _, err = db.Exec(ctx, "UPDATE `project_issue` SET project_board_id=?, sorting=? WHERE issue_id=?", column.ID, sorting, issueID) + // Scope the update to this issue *in this project*. Without the + // project_id predicate, an issue that belongs to several projects + // would have every project_issue row rewritten to the target + // column, detaching it from all other projects. + _, err = db.GetEngine(ctx).Table("project_issue"). + Where("issue_id = ? AND project_id = ?", issueID, column.ProjectID). + Update(map[string]any{ + "project_board_id": column.ID, + "sorting": sorting, + }) if err != nil { return err } diff --git a/tests/integration/api_user_project_test.go b/tests/integration/api_user_project_test.go index be7ecd99a1..403f922493 100644 --- a/tests/integration/api_user_project_test.go +++ b/tests/integration/api_user_project_test.go @@ -37,6 +37,7 @@ func TestAPIUserProjects(t *testing.T) { t.Run("RemoveIssueFromProjectColumn", testAPIUserRemoveIssueFromProjectColumn) t.Run("ListProjectColumnIssues", testAPIUserListProjectColumnIssues) t.Run("MoveProjectIssue", testAPIUserMoveProjectIssue) + t.Run("MoveProjectIssueMultiProjectIsolation", testAPIUserMoveProjectIssueMultiProjectIsolation) t.Run("Permissions", testAPIUserProjectPermissions) } @@ -502,6 +503,44 @@ func testAPIUserMoveProjectIssue(t *testing.T) { MakeRequest(t, req, http.StatusNotFound) } +// Regression for #17: moving an issue's column in one user project must not +// rewrite its column in other projects the issue also belongs to. +func testAPIUserMoveProjectIssueMultiProjectIsolation(t *testing.T) { + owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1}) + + p1 := makeUserProject(t, owner) + p2 := makeUserProject(t, owner) + + p1ColA := &project_model.Column{Title: "p1-A", ProjectID: p1.ID, CreatorID: owner.ID} + assert.NoError(t, project_model.NewColumn(t.Context(), p1ColA)) + p1ColB := &project_model.Column{Title: "p1-B", ProjectID: p1.ID, CreatorID: owner.ID} + assert.NoError(t, project_model.NewColumn(t.Context(), p1ColB)) + p2Col := &project_model.Column{Title: "p2", ProjectID: p2.ID, CreatorID: owner.ID} + assert.NoError(t, project_model.NewColumn(t.Context(), p2Col)) + + assert.NoError(t, issues_model.IssueAssignOrRemoveProject(t.Context(), issue, owner, []int64{p1.ID, p2.ID})) + assert.NoError(t, projects_service.MoveIssuesOnProjectColumn(t.Context(), owner, p1ColA, map[int64]int64{0: issue.ID})) + assert.NoError(t, projects_service.MoveIssuesOnProjectColumn(t.Context(), owner, p2Col, map[int64]int64{0: issue.ID})) + + token := getUserToken(t, owner.Name, auth_model.AccessTokenScopeWriteIssue) + + // move the issue inside p1 only + req := NewRequestWithJSON(t, "POST", + fmt.Sprintf("/api/v1/users/%s/projects/%d/issues/%d/move", owner.Name, p1.ID, issue.ID), + &api.MoveProjectIssueOption{ColumnID: p1ColB.ID}, + ).AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + + // p1 updated as requested + pi1 := unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{ProjectID: p1.ID, IssueID: issue.ID}) + assert.Equal(t, p1ColB.ID, pi1.ProjectColumnID) + + // p2 must be untouched + pi2 := unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{ProjectID: p2.ID, IssueID: issue.ID}) + assert.Equal(t, p2Col.ID, pi2.ProjectColumnID, "issue must remain in its original column in other projects") +} + func testAPIUserProjectPermissions(t *testing.T) { owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) other := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user5"})