fix(projects): scope project-issue move to its own project
MoveIssuesOnProjectColumn updated `project_issue` with a WHERE clause on issue_id only. An issue assigned to several projects has one project_issue row per project, so moving it within one project rewrote project_board_id for every project the issue belonged to, detaching it from all the others. Scope the UPDATE to (issue_id, project_id) so only the target project's row changes. Mirrors the fix already present in upstream/main. Adds an integration regression test asserting an issue in two user projects keeps its column in the other project after a move. Fixes #17.
This commit is contained in:
@@ -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 {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -37,6 +37,7 @@ func TestAPIUserProjects(t *testing.T) {
|
|||||||
t.Run("RemoveIssueFromProjectColumn", testAPIUserRemoveIssueFromProjectColumn)
|
t.Run("RemoveIssueFromProjectColumn", testAPIUserRemoveIssueFromProjectColumn)
|
||||||
t.Run("ListProjectColumnIssues", testAPIUserListProjectColumnIssues)
|
t.Run("ListProjectColumnIssues", testAPIUserListProjectColumnIssues)
|
||||||
t.Run("MoveProjectIssue", testAPIUserMoveProjectIssue)
|
t.Run("MoveProjectIssue", testAPIUserMoveProjectIssue)
|
||||||
|
t.Run("MoveProjectIssueMultiProjectIsolation", testAPIUserMoveProjectIssueMultiProjectIsolation)
|
||||||
t.Run("Permissions", testAPIUserProjectPermissions)
|
t.Run("Permissions", testAPIUserProjectPermissions)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -502,6 +503,44 @@ func testAPIUserMoveProjectIssue(t *testing.T) {
|
|||||||
MakeRequest(t, req, http.StatusNotFound)
|
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) {
|
func testAPIUserProjectPermissions(t *testing.T) {
|
||||||
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
owner := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
|
||||||
other := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user5"})
|
other := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user5"})
|
||||||
|
|||||||
Reference in New Issue
Block a user