fix(projects): move only the dragged card on project boards (#40182)

The board reported every card of the target column on each drag, and the
server wrote all of them into that column. A stale board in another
browser therefore pulled cards moved by someone else back to their old
column.

The client now sends only the dragged issue and its predecessor, and the
server computes the position in one transaction:

- Only the dragged issue changes column, the others are just renumbered.
- A predecessor that is no longer in the column puts the card at the
end.

Added an e2e test.

Fixes https://github.com/go-gitea/gitea/issues/21243
Fixes https://github.com/go-gitea/gitea/issues/32070
Fixes https://github.com/go-gitea/gitea/issues/13498

---
Assisted-by: Codet:claude

---------

Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
Lunny Xiao
2026-11-09 21:39:22 +00:00
committed by GitHub
parent 2d6dca9abb
commit 3561478109
11 changed files with 176 additions and 65 deletions

View File

@@ -211,7 +211,7 @@ export default defineConfig([
'@typescript-eslint/no-magic-numbers': [0], '@typescript-eslint/no-magic-numbers': [0],
'@typescript-eslint/no-meaningless-void-operator': [0], '@typescript-eslint/no-meaningless-void-operator': [0],
'@typescript-eslint/no-misused-new': [2], '@typescript-eslint/no-misused-new': [2],
'@typescript-eslint/no-misused-promises': [2, {checksVoidReturn: {attributes: false, arguments: false}}], '@typescript-eslint/no-misused-promises': [2, {checksVoidReturn: {attributes: false, arguments: false, properties: false}}],
'@typescript-eslint/no-misused-spread': [2], '@typescript-eslint/no-misused-spread': [2],
'@typescript-eslint/no-mixed-enums': [0], '@typescript-eslint/no-mixed-enums': [0],
'@typescript-eslint/no-namespace': [2], '@typescript-eslint/no-namespace': [2],

View File

@@ -43,6 +43,41 @@ func columnIssueIDs(column *Column) []int64 {
return []int64{column.ID} return []int64{column.ID}
} }
// LockProject serializes concurrent card moves of a project, avoiding row-lock deadlocks between them.
func LockProject(ctx context.Context, projectID int64) error {
_, err := db.GetEngine(ctx).Exec("UPDATE `project` SET title=title WHERE id=?", projectID)
return err
}
// ColumnIssueSorting is an issue's position in a column.
type ColumnIssueSorting struct {
IssueID int64 `xorm:"issue_id"`
Sorting int64 `xorm:"sorting"`
}
// GetColumnIssueSortings returns the column's issues except excludeIssueID in board order.
func GetColumnIssueSortings(ctx context.Context, column *Column, excludeIssueID int64) (rows []ColumnIssueSorting, _ error) {
err := db.GetEngine(ctx).Table("project_issue").
Select("project_issue.issue_id, project_issue.sorting").
Join("INNER", "issue", "issue.id = project_issue.issue_id").
Where("project_issue.project_id=?", column.ProjectID).
In("project_issue.project_board_id", columnIssueIDs(column)).
And("project_issue.issue_id<>?", excludeIssueID).
OrderBy("project_issue.sorting ASC, issue.created_unix DESC, issue.id DESC").
Find(&rows)
return rows, err
}
// SetIssueSorting sets the issue's sorting if it is still in the column.
func SetIssueSorting(ctx context.Context, column *Column, issueID, sorting int64) error {
_, err := db.GetEngine(ctx).
Where("project_id=? AND issue_id=?", column.ProjectID, issueID).
In("project_board_id", columnIssueIDs(column)).
Cols("sorting").
Update(&ProjectIssue{Sorting: sorting})
return err
}
// IsIssueInColumn reports whether the issue is placed in the column. // IsIssueInColumn reports whether the issue is placed in the column.
func IsIssueInColumn(ctx context.Context, issueID int64, column *Column) (bool, error) { func IsIssueInColumn(ctx context.Context, issueID int64, column *Column) (bool, error) {
return db.GetEngine(ctx). return db.GetEngine(ctx).

View File

@@ -154,50 +154,30 @@ func MoveIssues(ctx *context.Context) {
return return
} }
type movedIssuesForm struct { form := &struct {
Issues []struct { IssueID int64 `json:"issueID"`
IssueID int64 `json:"issueID"` PrevIssueID int64 `json:"prevIssueID"`
Sorting int64 `json:"sorting"` }{}
} `json:"issues"` if err := json.NewDecoder(ctx.Req.Body).Decode(form); err != nil {
} ctx.ServerError("DecodeMovedIssueForm", err)
form := &movedIssuesForm{}
if err := json.NewDecoder(ctx.Req.Body).Decode(&form); err != nil {
ctx.ServerError("DecodeMovedIssuesForm", err)
return return
} }
issueIDs := make([]int64, 0, len(form.Issues)) issue, err := issues_model.GetIssueByID(ctx, form.IssueID)
sortedIssueIDs := make(map[int64]int64)
for _, issue := range form.Issues {
issueIDs = append(issueIDs, issue.IssueID)
sortedIssueIDs[issue.Sorting] = issue.IssueID
}
movedIssues, err := issues_model.GetIssuesByIDs(ctx, issueIDs)
if err != nil { if err != nil {
ctx.ServerError("GetIssueByID", err) ctx.ServerError("GetIssueByID", err)
return return
} }
if err = issue.LoadRepo(ctx); err != nil {
if len(movedIssues) != len(form.Issues) { ctx.ServerError("LoadRepo", err)
ctx.ServerError("some issues do not exist", errors.New("some issues do not exist"))
return return
} }
if !project.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo) {
if _, err = movedIssues.LoadRepositories(ctx); err != nil { ctx.ServerError("Some issue's repoID is not equal to project's repoID", errors.New("Some issue's repoID is not equal to project's repoID"))
ctx.ServerError("LoadRepositories", err)
return return
} }
if err = project_service.MoveIssueAfter(ctx, ctx.Doer, column, issue, form.PrevIssueID); err != nil {
for _, issue := range movedIssues { ctx.ServerError("MoveIssueAfter", err)
if !project.CanBeAccessedByOwnerRepo(issue.Repo.OwnerID, issue.Repo) {
ctx.ServerError("Some issue's repoID is not equal to project's repoID", errors.New("Some issue's repoID is not equal to project's repoID"))
return
}
}
if err = project_service.MoveIssuesOnProjectColumn(ctx, ctx.Doer, column, sortedIssueIDs); err != nil {
ctx.ServerError("MoveIssuesOnProjectColumn", err)
return return
} }

View File

@@ -84,7 +84,7 @@ func TestMoveIssuesNeedsOnlyProjectWrite(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
ctx.SetPathParam("id", strconv.FormatInt(project.ID, 10)) ctx.SetPathParam("id", strconv.FormatInt(project.ID, 10))
ctx.SetPathParam("columnID", strconv.FormatInt(column.ID, 10)) ctx.SetPathParam("columnID", strconv.FormatInt(column.ID, 10))
ctx.Req.Body = io.NopCloser(strings.NewReader(fmt.Sprintf(`{"issues":[{"issueID":%d,"sorting":0}]}`, issue.ID))) ctx.Req.Body = io.NopCloser(strings.NewReader(fmt.Sprintf(`{"issueID":%d,"prevIssueID":0}`, issue.ID)))
MoveIssues(ctx) MoveIssues(ctx)
assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus()) assert.Equal(t, http.StatusOK, ctx.Resp.WrittenStatus())

View File

@@ -99,10 +99,35 @@ func RemoveIssueFromColumn(ctx context.Context, doer *user_model.User, issue *is
return nil return nil
} }
// MoveIssuesOnProjectColumn moves or keeps issues in a column and sorts them inside that column // MoveIssueAfter places the issue after prevIssueID, first for 0, last when it left the column.
func MoveIssuesOnProjectColumn(ctx context.Context, doer *user_model.User, column *project_model.Column, sortedIssueIDs map[int64]int64) error { func MoveIssueAfter(ctx context.Context, doer *user_model.User, column *project_model.Column, issue *issues_model.Issue, prevIssueID int64) error {
moves, err := db.WithTx2(ctx, func(ctx context.Context) ([]columnMove, error) { moves, err := db.WithTx2(ctx, func(ctx context.Context) ([]columnMove, error) {
return moveIssuesOnProjectColumn(ctx, doer, column, sortedIssueIDs) if err := project_model.LockProject(ctx, column.ProjectID); err != nil {
return nil, err
}
others, err := project_model.GetColumnIssueSortings(ctx, column, issue.ID)
if err != nil {
return nil, err
}
pos := util.Iif(prevIssueID == 0, 0, len(others))
if i := slices.IndexFunc(others, func(row project_model.ColumnIssueSorting) bool { return row.IssueID == prevIssueID }); i >= 0 {
pos = i + 1
}
moves, err := moveIssuesOnProjectColumn(ctx, doer, column, map[int64]int64{int64(pos): issue.ID})
if err != nil {
return nil, err
}
for i, other := range others {
newSorting := int64(i + util.Iif(i >= pos, 1, 0))
if other.Sorting == newSorting {
continue
}
if err := project_model.SetIssueSorting(ctx, column, other.IssueID, newSorting); err != nil {
return nil, err
}
}
return moves, nil
}) })
if err != nil { if err != nil {
return err return err

View File

@@ -156,7 +156,7 @@ func Test_Projects(t *testing.T) {
// move the issue into a non-default column of the first project only // move the issue into a non-default column of the first project only
targetColumn := &project_model.Column{Title: "target", ProjectID: projects[0].ID} targetColumn := &project_model.Column{Title: "target", ProjectID: projects[0].ID}
assert.NoError(t, project_model.NewColumn(t.Context(), targetColumn)) assert.NoError(t, project_model.NewColumn(t.Context(), targetColumn))
assert.NoError(t, MoveIssuesOnProjectColumn(t.Context(), user2, targetColumn, map[int64]int64{0: issue11.ID})) assert.NoError(t, MoveIssueAfter(t.Context(), user2, targetColumn, issue11, 0))
unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{ unittest.AssertExistsAndLoadBean(t, &project_model.ProjectIssue{
IssueID: issue11.ID, ProjectID: projects[0].ID, ProjectColumnID: targetColumn.ID, IssueID: issue11.ID, ProjectID: projects[0].ID, ProjectColumnID: targetColumn.ID,
@@ -166,6 +166,44 @@ func Test_Projects(t *testing.T) {
}) })
}) })
t.Run("Moving an issue never moves other cards", func(t *testing.T) {
project := &project_model.Project{Title: "incremental move", RepoID: 1, Type: project_model.TypeRepository}
assert.NoError(t, project_model.NewProject(t.Context(), project))
defer func() {
assert.NoError(t, project_model.DeleteProjectByID(t.Context(), project.ID))
}()
colA := &project_model.Column{Title: "A", ProjectID: project.ID}
colB := &project_model.Column{Title: "B", ProjectID: project.ID}
assert.NoError(t, project_model.NewColumn(t.Context(), colA))
assert.NoError(t, project_model.NewColumn(t.Context(), colB))
for i, id := range []int64{1, 2, 3, 5} {
assert.NoError(t, db.Insert(t.Context(), &project_model.ProjectIssue{IssueID: id, ProjectID: project.ID, ProjectColumnID: colA.ID, Sorting: int64(i)}))
}
issue := func(id int64) *issues_model.Issue {
return unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: id})
}
order := func(col *project_model.Column) (ids []int64) {
rows, err := project_model.GetColumnIssueSortings(t.Context(), col, 0)
assert.NoError(t, err)
for i, row := range rows {
assert.EqualValues(t, i, row.Sorting)
ids = append(ids, row.IssueID)
}
return ids
}
assert.NoError(t, MoveIssueAfter(t.Context(), user2, colB, issue(1), 0))
assert.NoError(t, MoveIssueAfter(t.Context(), user2, colA, issue(2), 1))
assert.Equal(t, []int64{3, 5, 2}, order(colA))
assert.NoError(t, MoveIssueAfter(t.Context(), user2, colA, issue(5), 0))
assert.Equal(t, []int64{5, 3, 2}, order(colA))
assert.NoError(t, MoveIssueAfter(t.Context(), user2, colA, issue(5), 3))
assert.Equal(t, []int64{3, 5, 2}, order(colA))
assert.Equal(t, []int64{1}, order(colB))
assert.NoError(t, project_model.SetIssueSorting(t.Context(), colA, 1, 99))
assert.Equal(t, []int64{1}, order(colB))
})
t.Run("Repository projects", func(t *testing.T) { t.Run("Repository projects", func(t *testing.T) {
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})

View File

@@ -1,6 +1,25 @@
import {env} from 'node:process'; import {env} from 'node:process';
import {test, expect} from '@playwright/test'; import {test, expect} from '@playwright/test';
import {login, apiCreateRepo, apiCreateIssue, apiDeleteRepo, createProject, createProjectColumn, randomString} from './utils.ts'; import {
login, apiCreateRepo, apiCreateIssue, apiCreateProject, apiDeleteRepo, createProject, createProjectColumn, randomString,
} from './utils.ts';
import type {Page} from '@playwright/test';
const projectCards = (page: Page, column: string) => page.locator('.project-column', {hasText: column}).locator('.cards');
async function dragProjectCardToTop(page: Page, title: string, column: string) {
const card = page.locator('.issue-card', {hasText: title});
const cards = projectCards(page, column);
const moved = page.waitForResponse((resp) => resp.url().endsWith('/move'));
await card.hover();
await page.mouse.down();
await card.hover({position: {x: 10, y: 10}});
await expect(card).toHaveClass(/card-ghost/); // sortable ignores dragover until its deferred drag start ran
await cards.hover({position: {x: 10, y: 10}});
await expect(cards.locator('.issue-card').first()).toContainText(title);
await page.mouse.up();
await moved;
}
test('assign issue to project and change column', async ({page}) => { test('assign issue to project and change column', async ({page}) => {
const repoName = `e2e-issue-project-${randomString(8)}`; const repoName = `e2e-issue-project-${randomString(8)}`;
@@ -441,3 +460,25 @@ test('select projects on new issue page shows in sidebar', async ({page}) => {
await apiDeleteRepo(page.request, env.GITEA_TEST_E2E_USER, repoName); await apiDeleteRepo(page.request, env.GITEA_TEST_E2E_USER, repoName);
} }
}); });
test('project board drag keeps a card moved in another tab', async ({page}) => {
const owner = env.GITEA_TEST_E2E_USER;
const repo = `e2e-project-board-${randomString(8)}`;
const [otherPage] = await Promise.all([
page.context().newPage(), login(page), apiCreateRepo(page.request, {name: repo, autoInit: false}),
]);
try {
const projectID = await apiCreateProject(page.request, owner, repo, 'Board', {templateType: 'basic_kanban'});
for (const title of ['first', 'second']) await apiCreateIssue(page.request, {owner, repo, title, projects: [projectID]});
await Promise.all([page, otherPage].map((board) => board.goto(`/${owner}/${repo}/projects/${projectID}`)));
await dragProjectCardToTop(otherPage, 'first', 'To Do');
await dragProjectCardToTop(page, 'second', 'Backlog');
await page.reload();
await expect(projectCards(page, 'Backlog').locator('.issue-card')).toHaveText([/second/]);
await expect(projectCards(page, 'To Do').locator('.issue-card')).toHaveText([/first/]);
} finally {
await apiDeleteRepo(page.request, owner, repo);
}
});

View File

@@ -130,10 +130,10 @@ export async function createProjectColumn(requestContext: APIRequestContext, own
}), 'createProjectColumn'); }), 'createProjectColumn');
} }
export async function apiCreateProject(requestContext: APIRequestContext, owner: string, repo: string, title: string): Promise<number> { export async function apiCreateProject(requestContext: APIRequestContext, owner: string, repo: string, title: string, {templateType}: {templateType?: string} = {}): Promise<number> {
const response = await apiRetry(() => requestContext.post(`${baseUrl()}/api/v1/repos/${owner}/${repo}/projects`, { const response = await apiRetry(() => requestContext.post(`${baseUrl()}/api/v1/repos/${owner}/${repo}/projects`, {
headers: apiHeaders(), headers: apiHeaders(),
data: {title}, data: {title, template_type: templateType},
}), 'apiCreateProject'); }), 'apiCreateProject');
return (await response.json()).id; return (await response.json()).id;
} }

View File

@@ -84,7 +84,7 @@ func TestProjectWorkflowsWeb(t *testing.T) {
assert.Contains(t, NewHTMLParser(t, resp.Body).Find(".timeline-item.event .comment-text-line").Text(), "Project workflow Item added to project") assert.Contains(t, NewHTMLParser(t, resp.Body).Find(".timeline-item.event .comment-text-line").Text(), "Project workflow Item added to project")
req = NewRequestWithJSON(t, "POST", link+"/3/move", map[string]any{ req = NewRequestWithJSON(t, "POST", link+"/3/move", map[string]any{
"issues": []map[string]any{{"issueID": created.ID, "sorting": 0}}, "issueID": created.ID, "prevIssueID": 0,
}) })
session.MakeRequest(t, req, http.StatusOK) session.MakeRequest(t, req, http.StatusOK)

View File

@@ -16,27 +16,19 @@ function updateIssueCount(card: HTMLElement): void {
} }
async function moveIssue({item, from, to, oldIndex}: SortableEvent): Promise<void> { async function moveIssue({item, from, to, oldIndex}: SortableEvent): Promise<void> {
const columnCards = to.querySelectorAll('.issue-card');
updateIssueCount(from); updateIssueCount(from);
updateIssueCount(to); updateIssueCount(to);
const columnSorting = { const prevCard = item.previousElementSibling;
issues: Array.from(columnCards, (card, i) => ({ const resp = await performFetchActionRequest(item, {
issueID: parseInt(card.getAttribute('data-issue')!), method: 'POST',
sorting: i, url: `${to.getAttribute('data-url')}/move`,
})), data: {
}; issueID: parseInt(item.getAttribute('data-issue')!),
prevIssueID: prevCard ? parseInt(prevCard.getAttribute('data-issue')!) : 0,
try { },
await POST(`${to.getAttribute('data-url')}/move`, { });
data: columnSorting, if (!resp && oldIndex !== undefined) from.insertBefore(item, from.children[oldIndex]);
});
} catch (error) {
console.error(error);
if (oldIndex !== undefined) {
from.insertBefore(item, from.children[oldIndex]);
}
}
} }
function initRepoProjectSortable(): void { function initRepoProjectSortable(): void {
@@ -49,7 +41,7 @@ function initRepoProjectSortable(): void {
handle: '.project-column-header', handle: '.project-column-header',
delayOnTouchOnly: true, delayOnTouchOnly: true,
delay: 500, delay: 500,
onSort: async () => { // eslint-disable-line @typescript-eslint/no-misused-promises -- Sortable ignores the returned promise, the body catches its own errors onSort: async () => {
boardColumns = mainBoard.querySelectorAll<HTMLElement>('.project-column'); boardColumns = mainBoard.querySelectorAll<HTMLElement>('.project-column');
const columnSorting = { const columnSorting = {
@@ -73,8 +65,8 @@ function initRepoProjectSortable(): void {
const boardCardList = boardColumn.querySelector<HTMLElement>('.cards')!; const boardCardList = boardColumn.querySelector<HTMLElement>('.cards')!;
createSortable(boardCardList, { createSortable(boardCardList, {
group: 'shared', group: 'shared',
onAdd: moveIssue, // eslint-disable-line @typescript-eslint/no-misused-promises -- Sortable ignores the returned promise, moveIssue catches its own errors onAdd: moveIssue,
onUpdate: moveIssue, // eslint-disable-line @typescript-eslint/no-misused-promises -- Sortable ignores the returned promise, moveIssue catches its own errors onUpdate: moveIssue,
delayOnTouchOnly: true, delayOnTouchOnly: true,
delay: 500, delay: 500,
}); });

View File

@@ -31,7 +31,7 @@ export function attachTribute(element: HTMLElement) {
}; };
const mentionCollection: TributeCollection<Mention> = { const mentionCollection: TributeCollection<Mention> = {
values: async (_query: string, cb: (matches: Mention[]) => void) => { // eslint-disable-line @typescript-eslint/no-misused-promises -- tributejs ignores the returned promise, results arrive via the callback values: async (_query: string, cb: (matches: Mention[]) => void) => {
cb(mentionsUrl ? await fetchMentions(mentionsUrl) : []); cb(mentionsUrl ? await fetchMentions(mentionsUrl) : []);
}, },
requireLeadingSpace: true, requireLeadingSpace: true,