From 0c41e8f6d6f070a28e98917a9ae08e75be8b360b Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sun, 16 Aug 2026 04:05:31 +0100 Subject: [PATCH 1/8] [AGILE-363] Resolve batch positions Establishes one DOM resolution for availability and execution. Validates contiguous blocks and explicit unavailable reasons while preserving singular placement and sparse-list boundaries. https://community.openproject.org/wp/AGILE-363 --- .../dynamic/sortable-lists/list-dom.spec.ts | 140 ++++++++++++++++++ .../dynamic/sortable-lists/list-dom.ts | 117 +++++++++++++++ 2 files changed, 257 insertions(+) diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts index ec0171281798..3a80cedcd17f 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts @@ -36,6 +36,8 @@ import { itemMobility, permittedDestinations, reorderRows, + resolveBlockMove, + resolveBlockMoveAvailability, sortableItemMobilityAttribute, resolveDirectionalPreviousItemId, resolveMoveAvailability, @@ -664,6 +666,144 @@ describe('directional move helpers', () => { }); }); +describe('block move helpers', () => { + function fixture():{ + rowsContainer:HTMLUListElement; + items:HTMLLIElement[]; + marker:HTMLLIElement; + divider:HTMLLIElement; + } { + const rowsContainer = document.createElement('ul'); + const items = ['1', '2', '3', '4'].map((id) => { + const row = document.createElement('li'); + row.setAttribute('data-sortable-lists--item-id-value', id); + return row; + }); + const marker = document.createElement('li'); + marker.setAttribute('data-sortable-lists-prev-item-id', 'last-hidden'); + marker.setAttribute('data-sortable-lists-omitted-count', '3'); + const divider = document.createElement('li'); + divider.classList.add('divider'); + + rowsContainer.append(...items, marker, divider); + + return { rowsContainer, items, marker, divider }; + } + + it('resolves one card in all four directions', () => { + const { rowsContainer, items: [, second] } = fixture(); + + expect(resolveBlockMove({ itemElements: [second], direction: 'top', rowsContainer })) + .toEqual({ available: true, rows: [second], previousItemId: null }); + expect(resolveBlockMove({ itemElements: [second], direction: 'up', rowsContainer })) + .toEqual({ available: true, rows: [second], previousItemId: null }); + expect(resolveBlockMove({ itemElements: [second], direction: 'down', rowsContainer })) + .toEqual({ available: true, rows: [second], previousItemId: '3' }); + expect(resolveBlockMove({ itemElements: [second], direction: 'bottom', rowsContainer })) + .toEqual({ available: true, rows: [second], previousItemId: '4' }); + }); + + it('resolves an adjacent block in all four directions and excludes selected predecessors', () => { + const { rowsContainer, items: [, second, third] } = fixture(); + + expect(resolveBlockMove({ itemElements: [second, third], direction: 'top', rowsContainer })) + .toEqual({ available: true, rows: [second, third], previousItemId: null }); + expect(resolveBlockMove({ itemElements: [second, third], direction: 'up', rowsContainer })) + .toEqual({ available: true, rows: [second, third], previousItemId: null }); + expect(resolveBlockMove({ itemElements: [second, third], direction: 'down', rowsContainer })) + .toEqual({ available: true, rows: [second, third], previousItemId: '4' }); + expect(resolveBlockMove({ itemElements: [second, third], direction: 'bottom', rowsContainer })) + .toEqual({ available: true, rows: [second, third], previousItemId: '4' }); + }); + + it('rejects an empty or fixed selection as not orderable', () => { + const { rowsContainer, items: [first, second] } = fixture(); + second.setAttribute(sortableItemMobilityAttribute, 'fixed'); + + expect(resolveBlockMove({ itemElements: [], direction: 'top', rowsContainer })) + .toEqual({ available: false, reason: 'not-orderable' }); + expect(resolveBlockMove({ itemElements: [first, second], direction: 'top', rowsContainer })) + .toEqual({ available: false, reason: 'not-orderable' }); + }); + + it('rejects selected elements from different containers', () => { + const { rowsContainer, items: [first] } = fixture(); + const { items: [foreign] } = fixture(); + + expect(resolveBlockMove({ itemElements: [first, foreign], direction: 'top', rowsContainer })) + .toEqual({ available: false, reason: 'cross-list' }); + }); + + it('rejects sparse, reverse-order, and duplicate input', () => { + const { rowsContainer, items: [first, second, third] } = fixture(); + + expect(resolveBlockMove({ itemElements: [first, third], direction: 'top', rowsContainer })) + .toEqual({ available: false, reason: 'non-contiguous' }); + expect(resolveBlockMove({ itemElements: [third, second], direction: 'top', rowsContainer })) + .toEqual({ available: false, reason: 'non-contiguous' }); + expect(resolveBlockMove({ itemElements: [second, second], direction: 'top', rowsContainer })) + .toEqual({ available: false, reason: 'non-contiguous' }); + }); + + it('rejects up and down across an adjacent truncation marker', () => { + const { rowsContainer, items: [, second, third], marker } = fixture(); + + second.after(marker); + expect(resolveBlockMove({ itemElements: [second], direction: 'down', rowsContainer })) + .toEqual({ available: false, reason: 'truncation-boundary' }); + expect(resolveBlockMove({ itemElements: [third], direction: 'up', rowsContainer })) + .toEqual({ available: false, reason: 'truncation-boundary' }); + }); + + it('rejects up and down across an unaddressable gap', () => { + const { rowsContainer, items: [, second, third], divider } = fixture(); + + second.after(divider); + expect(resolveBlockMove({ itemElements: [second], direction: 'down', rowsContainer })) + .toEqual({ available: false, reason: 'unaddressable-gap' }); + expect(resolveBlockMove({ itemElements: [third], direction: 'up', rowsContainer })) + .toEqual({ available: false, reason: 'unaddressable-gap' }); + }); + + it('keeps top and bottom available across loaded sparse-list boundaries', () => { + const { rowsContainer, items: [, second, third, fourth], marker } = fixture(); + + second.after(marker); + expect(resolveBlockMove({ itemElements: [third], direction: 'top', rowsContainer })) + .toEqual({ available: true, rows: [third], previousItemId: null }); + expect(resolveBlockMove({ itemElements: [second], direction: 'bottom', rowsContainer })) + .toEqual({ available: true, rows: [second], previousItemId: '4' }); + expect(resolveBlockMove({ itemElements: [third], direction: 'bottom', rowsContainer })) + .toEqual({ available: true, rows: [third], previousItemId: '4' }); + expect(resolveBlockMove({ itemElements: [fourth], direction: 'top', rowsContainer })) + .toEqual({ available: true, rows: [fourth], previousItemId: null }); + }); + + it('rejects every move that returns the block to its effective placement', () => { + const { rowsContainer, items: [first, second, third, fourth] } = fixture(); + + expect(resolveBlockMove({ itemElements: [first], direction: 'top', rowsContainer })) + .toEqual({ available: false, reason: 'no-op' }); + expect(resolveBlockMove({ itemElements: [first, second], direction: 'up', rowsContainer })) + .toEqual({ available: false, reason: 'no-op' }); + expect(resolveBlockMove({ itemElements: [fourth], direction: 'down', rowsContainer })) + .toEqual({ available: false, reason: 'no-op' }); + expect(resolveBlockMove({ itemElements: [third, fourth], direction: 'bottom', rowsContainer })) + .toEqual({ available: false, reason: 'no-op' }); + }); + + it('derives availability by resolving all four directions', () => { + const { rowsContainer, items: [first, second, third, fourth] } = fixture(); + + expect(resolveBlockMoveAvailability({ itemElements: [first, second], rowsContainer })) + .toEqual({ top: false, up: false, down: true, bottom: true }); + expect(resolveBlockMoveAvailability({ itemElements: [second, third], rowsContainer })) + .toEqual({ top: true, up: true, down: true, bottom: true }); + expect(resolveBlockMoveAvailability({ itemElements: [third, fourth], rowsContainer })) + .toEqual({ top: true, up: true, down: false, bottom: false }); + }); +}); + describe('resolveItemPosition', () => { function item(id:string, label?:string):HTMLLIElement { const row = document.createElement('li'); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts index 1fc123b16aea..9f2ec9a363d8 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts @@ -477,6 +477,123 @@ export function resolveDirectionalPreviousItemId({ export type MoveAvailability = Record; +export interface BlockMoveResolution { + available:true; + rows:HTMLElement[]; + previousItemId:string|null; +} + +export type BlockMoveUnavailableReason = + | 'not-orderable' + | 'cross-list' + | 'non-contiguous' + | 'truncation-boundary' + | 'unaddressable-gap' + | 'no-op'; + +export interface BlockMoveUnavailable { + available:false; + reason:BlockMoveUnavailableReason; +} + +export type BlockMoveResult = BlockMoveResolution|BlockMoveUnavailable; + +export function resolveBlockMove({ + itemElements, + direction, + rowsContainer, +}:{ + itemElements:HTMLElement[]; + direction:MoveDirection; + rowsContainer:HTMLElement; +}):BlockMoveResult { + const unavailable = (reason:BlockMoveUnavailableReason):BlockMoveUnavailable => ({ available: false, reason }); + if (itemElements.length === 0 || itemElements.some((item) => !isOrderableItem(item))) { + return unavailable('not-orderable'); + } + + const rows = listRows(rowsContainer); + const selectedRows = itemElements.map((item) => rowOf(rowsContainer, item)); + if (selectedRows.some((row) => row === null)) { + return unavailable('cross-list'); + } + + const block = selectedRows as HTMLElement[]; + const indexes = block.map((row) => rows.indexOf(row)); + if ( + new Set(block).size !== block.length || + indexes.some((index, offset) => offset > 0 && index !== indexes[offset - 1] + 1) + ) { + return unavailable('non-contiguous'); + } + + const itemRows = rows.filter((row) => isItemRow(row, rowsContainer)); + const itemIndexes = block.map((row) => itemRows.indexOf(row)); + if (itemIndexes.some((index, offset) => index < 0 || (offset > 0 && index !== itemIndexes[offset - 1] + 1))) { + return unavailable('non-contiguous'); + } + + const firstRowIndex = indexes[0]; + const lastRowIndex = indexes[indexes.length - 1]; + const firstItemIndex = itemIndexes[0]; + const lastItemIndex = itemIndexes[itemIndexes.length - 1]; + + let previousItemId:string|null|undefined; + switch (direction) { + case 'top': + previousItemId = firstItemIndex === 0 ? undefined : null; + break; + case 'bottom': { + if (lastItemIndex === itemRows.length - 1) return unavailable('no-op'); + const selected = new Set(block); + const anchor = [...itemRows].reverse().find((row) => !selected.has(row as HTMLElement)); + const anchorId = anchor ? resolvePreviousItemId(anchor, rowsContainer) : null; + previousItemId = anchorId ?? undefined; + break; + } + case 'up': { + if (firstItemIndex === 0) return unavailable('no-op'); + const adjacent = rows[firstRowIndex - 1]; + if (!isItemRow(adjacent, rowsContainer)) { + return unavailable(adjacent && rowOmittedCount(adjacent) > 0 ? 'truncation-boundary' : 'unaddressable-gap'); + } + const anchor = rows[firstRowIndex - 2]; + if (anchor && !isAddressableRow(anchor, rowsContainer)) return unavailable('unaddressable-gap'); + previousItemId = anchor ? resolvePreviousItemId(anchor, rowsContainer) : null; + break; + } + case 'down': { + if (lastItemIndex === itemRows.length - 1) return unavailable('no-op'); + const adjacent = rows[lastRowIndex + 1]; + if (!isItemRow(adjacent, rowsContainer)) { + return unavailable(adjacent && rowOmittedCount(adjacent) > 0 ? 'truncation-boundary' : 'unaddressable-gap'); + } + const adjacentId = resolvePreviousItemId(adjacent, rowsContainer); + previousItemId = adjacentId ?? undefined; + break; + } + } + + return previousItemId === undefined + ? unavailable(firstItemIndex === 0 || lastItemIndex === itemRows.length - 1 ? 'no-op' : 'unaddressable-gap') + : { available: true, rows: block, previousItemId }; +} + +export function resolveBlockMoveAvailability({ + itemElements, + rowsContainer, +}:{ + itemElements:HTMLElement[]; + rowsContainer:HTMLElement; +}):MoveAvailability { + return { + top: resolveBlockMove({ itemElements, rowsContainer, direction: 'top' }).available, + up: resolveBlockMove({ itemElements, rowsContainer, direction: 'up' }).available, + down: resolveBlockMove({ itemElements, rowsContainer, direction: 'down' }).available, + bottom: resolveBlockMove({ itemElements, rowsContainer, direction: 'bottom' }).available, + }; +} + // Availability of all four directional moves for the item, or null when it is // not (yet) a row of the container. The row scan happens once; the four // per-direction resolutions only index into it. From 32d36e3bfb5ebb81bf97f784eaf3fe1321f97b1c Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sun, 16 Aug 2026 04:11:49 +0100 Subject: [PATCH 2/8] [AGILE-363] Validate block row ownership Verifies that every resolved row owns its supplied item and prevents nested lists from substituting an outer host row. https://community.openproject.org/wp/AGILE-363 --- .../dynamic/sortable-lists/list-dom.spec.ts | 31 +++++++++++++++++++ .../dynamic/sortable-lists/list-dom.ts | 4 ++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts index 3a80cedcd17f..76460e9e8b07 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts @@ -734,6 +734,26 @@ describe('block move helpers', () => { .toEqual({ available: false, reason: 'cross-list' }); }); + it('rejects an inner-list item that resolves to its outer host row', () => { + const rowsContainer = document.createElement('ul'); + const outerHost = document.createElement('li'); + outerHost.setAttribute('data-sortable-lists--item-id-value', 'outer-1'); + const nestedRows = document.createElement('ul'); + const inner = document.createElement('li'); + inner.setAttribute('data-sortable-lists--item-id-value', 'inner-1'); + nestedRows.append(inner); + outerHost.append(nestedRows); + + const outerSecond = document.createElement('li'); + outerSecond.setAttribute('data-sortable-lists--item-id-value', 'outer-2'); + const outerThird = document.createElement('li'); + outerThird.setAttribute('data-sortable-lists--item-id-value', 'outer-3'); + rowsContainer.append(outerHost, outerSecond, outerThird); + + expect(resolveBlockMove({ itemElements: [inner, outerSecond], direction: 'bottom', rowsContainer })) + .toEqual({ available: false, reason: 'cross-list' }); + }); + it('rejects sparse, reverse-order, and duplicate input', () => { const { rowsContainer, items: [first, second, third] } = fixture(); @@ -765,6 +785,17 @@ describe('block move helpers', () => { .toEqual({ available: false, reason: 'unaddressable-gap' }); }); + it('keeps fixed rows addressable as up and down neighbours', () => { + const { rowsContainer, items: [first, second, third] } = fixture(); + first.setAttribute(sortableItemMobilityAttribute, 'fixed'); + third.setAttribute(sortableItemMobilityAttribute, 'fixed'); + + expect(resolveBlockMove({ itemElements: [second], direction: 'up', rowsContainer })) + .toEqual({ available: true, rows: [second], previousItemId: null }); + expect(resolveBlockMove({ itemElements: [second], direction: 'down', rowsContainer })) + .toEqual({ available: true, rows: [second], previousItemId: '3' }); + }); + it('keeps top and bottom available across loaded sparse-list boundaries', () => { const { rowsContainer, items: [, second, third, fourth], marker } = fixture(); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts index 9f2ec9a363d8..fa67f081584c 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts @@ -514,7 +514,9 @@ export function resolveBlockMove({ const rows = listRows(rowsContainer); const selectedRows = itemElements.map((item) => rowOf(rowsContainer, item)); - if (selectedRows.some((row) => row === null)) { + if (selectedRows.some((row, index) => ( + row === null || resolveItemElement(row, rowsContainer) !== itemElements[index] + ))) { return unavailable('cross-list'); } From 07eeb5163b45ac22c8cc3fe669f2afd528bdaca9 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sun, 16 Aug 2026 04:16:52 +0100 Subject: [PATCH 3/8] [AGILE-363] Gate batch position actions Resolves availability over the invoker's prospective scope and exposes only moves the live block resolver can execute. Preserves destinations for contiguous same-list selections. https://community.openproject.org/wp/AGILE-363 --- .../dynamic/sortable-lists.controller.spec.ts | 82 ++++++++++++++++--- .../dynamic/sortable-lists.controller.ts | 15 ++-- .../dynamic/sortable-lists/drag-and-drop.ts | 3 +- .../sortable-lists/item.controller.spec.ts | 33 +++++++- .../sortable-lists/menu-availability.spec.ts | 8 +- .../sortable-lists/menu-availability.ts | 7 +- 6 files changed, 122 insertions(+), 26 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts index 37a9af707852..6594915fc454 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts @@ -1210,7 +1210,7 @@ describe('Sortable lists controller', () => { }); it('reports per-direction move availability for gating', async () => { - const { root, firstSourceItem } = renderFixture(); + const { root, firstSourceItem } = renderSelectableRoot(); await ctx.nextFrame(); const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; @@ -1220,16 +1220,78 @@ describe('Sortable lists controller', () => { }); }); - // A card that takes no part in ordering must not be offered a move the - // click path would then refuse. - it('reports no available direction for a non-movable item', async () => { - const { root, firstSourceItem } = renderFixture(); - firstSourceItem.setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); - await ctx.nextFrame(); - const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + describe('batch move availability', () => { + const unavailable = { top: false, up: false, down: false, bottom: false }; + const select = (element:HTMLElement, init:MouseEventInit = {}) => { + element.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, ...init })); + }; - expect(controller.moveAvailability(firstSourceItem)).toEqual({ - top: false, up: false, down: false, bottom: false, + it('reports the available directions for a selected contiguous block', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + select(items[1]); + select(items[2], { ctrlKey: true }); + + expect(controller.moveAvailability(items[1])).toEqual({ + top: true, up: true, down: false, bottom: false, + }); + }); + + it('reports every direction unavailable for a sparse selection', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + select(items[0]); + select(items[2], { ctrlKey: true }); + + expect(controller.moveAvailability(items[0])).toEqual(unavailable); + }); + + it('reports every direction unavailable for a cross-list selection', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + select(items[2]); + select(items[3], { ctrlKey: true }); + + expect(controller.moveAvailability(items[2])).toEqual(unavailable); + }); + + it('reports one-card availability for an unselected invoker without changing the selection', async () => { + const { root, items } = renderSelectableRoot(); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + select(items[0]); + select(items[1], { ctrlKey: true }); + + expect(controller.moveAvailability(items[3])).toEqual({ + top: false, up: false, down: true, bottom: true, + }); + expect(items.filter((item) => item.hasAttribute('data-batch-selected'))).toEqual([items[0], items[1]]); + }); + + it('reports normal within-list availability for a confined contiguous block', async () => { + const { root, items } = renderSelectableRoot(); + items[0].setAttribute('data-sortable-lists--item-mobility-value', 'confined'); + items[1].setAttribute('data-sortable-lists--item-mobility-value', 'confined'); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + select(items[0]); + select(items[1], { ctrlKey: true }); + + expect(controller.moveAvailability(items[0])).toEqual({ + top: false, up: false, down: true, bottom: true, + }); + }); + + it('reports null for a fixed invoker', async () => { + const { root, items } = renderSelectableRoot(); + items[0].setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); + await ctx.nextFrame(); + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + + expect(controller.moveAvailability(items[0])).toBeNull(); }); }); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts index f962c25b3808..1763b786f955 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts @@ -55,11 +55,11 @@ import { itemAcceptsDestination, reorderRows, resolveDirectionalPreviousItemId, + resolveBlockMoveAvailability, resolveItemId, resolveItemLabel, resolveItemPosition, resolveItemType, - resolveMoveAvailability, restoreRowPositions, rowOf, rowsRemainAt, @@ -498,14 +498,19 @@ export default class SortableListsController extends Controller imp // gating; the click path re-resolves the live DOM. moveAvailability(itemElement:HTMLElement):MoveAvailability|null { if (!isOrderableItem(itemElement)) { - return { - top: false, up: false, down: false, bottom: false, - }; + return null; + } + + const scope = this.actionScopeFor(itemElement); + if (scope.kind === 'refused') { + return null; } const list = this.ownerListOf(itemElement); - return list ? resolveMoveAvailability({ itemElement, rowsContainer: list.rowsContainer }) : null; + return list + ? resolveBlockMoveAvailability({ itemElements: scope.items, rowsContainer: list.rowsContainer }) + : null; } moveToDestination(itemElement:HTMLElement, target:DestinationIdentity):void { diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts index fd2dd41121d0..38591b802c66 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts @@ -108,7 +108,8 @@ export interface SortableListsRoot { availableDestinations(scope:ActionScope, candidates:DestinationIdentity[]):DestinationIdentity[]; moveToDestination(itemElement:HTMLElement, target:DestinationIdentity):void; moveInDirection(itemElement:HTMLElement, direction:MoveDirection):void; - // A snapshot for menu gating; the click path re-resolves against the live DOM. + // A snapshot for menu gating over the invoker's prospective action scope; + // the click path re-resolves against the live DOM. moveAvailability(itemElement:HTMLElement):MoveAvailability|null; // The element of the list an item currently belongs to; null outside any // registered list. Items carry no list reference, so the root resolves it. diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts index acfbcd137442..72da4217e743 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.spec.ts @@ -1612,7 +1612,7 @@ describe('Sortable lists item controller', () => { expect(menu.showItem).toHaveBeenCalledWith(moveToInbox); }); - it('hides destination actions and the position submenu for a true multi-card scope', async () => { + it('shows available batch position directions when every destination action is hidden', async () => { const { el, menu } = renderItemWithMenu(1, true); document.body.appendChild(el); const controller = await mountItemController(el); @@ -1622,6 +1622,7 @@ describe('Sortable lists item controller', () => { const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false }); actionScopeFor.mockReturnValue(scope); availableDestinations.mockReturnValue([]); + root.moveAvailability = () => ({ top: false, up: true, down: true, bottom: false }); controller.connectRoot(root); const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '1' }]); @@ -1632,11 +1633,39 @@ describe('Sortable lists item controller', () => { const divider = el.querySelector('li[data-sortable-lists--item-target="moveDivider"]')!; expect(menu.hideItem).toHaveBeenCalledWith(moveToSprint); expect(menu.hideItem).toHaveBeenCalledWith(moveToInbox); - expect(menu.hideItem).toHaveBeenCalledWith(moveMenu); + expect(menu.showItem).toHaveBeenCalledWith(moveMenu); expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'top')); + expect(menu.showItem).toHaveBeenCalledWith(liFor(el, 'up')); + expect(menu.showItem).toHaveBeenCalledWith(liFor(el, 'down')); + expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'bottom')); + expect(divider.hasAttribute('hidden')).toBe(false); + }); + + it('hides an all-unavailable batch position submenu and its directions', async () => { + const { el, menu } = renderItemWithMenu(1, true); + document.body.appendChild(el); + const controller = await mountItemController(el); + const scope:ActionScope = { kind: 'batch', items: [el] }; + const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false }); + actionScopeFor.mockReturnValue(scope); + availableDestinations.mockReturnValue([]); + root.moveAvailability = () => ({ top: false, up: false, down: false, bottom: false }); + controller.connectRoot(root); + + const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '1' }]); + await menuCtx!.nextFrame(); + + const moveMenu = el.querySelector('li[data-sortable-lists--item-target="moveMenu"]')!; + const divider = el.querySelector('li[data-sortable-lists--item-target="moveDivider"]')!; + expect(menu.hideItem).toHaveBeenCalledWith(moveToSprint); + expect(menu.hideItem).toHaveBeenCalledWith(moveMenu); + for (const direction of ['top', 'up', 'down', 'bottom']) { + expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, direction)); + } expect(divider.hasAttribute('hidden')).toBe(true); actionScopeFor.mockReturnValue({ kind: 'batch', items: [el] }); + root.moveAvailability = () => ({ top: true, up: true, down: false, bottom: false }); el.querySelector('action-menu > anchored-position')!.dispatchEvent(new ToggleEvent('toggle', { newState: 'open' })); expect(moveMenu.hasAttribute('hidden')).toBe(false); expect(liFor(el, 'top').hasAttribute('hidden')).toBe(false); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.spec.ts index e957ba1d3b86..6f712618c002 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.spec.ts @@ -51,7 +51,7 @@ function fixture(hideUnavailable = true) { hideUnavailable, identifier: 'sortable-lists-item', availableDestinations: vi.fn(() => []), - moveAvailability: vi.fn(() => ({ top: true, up: true, down: false, bottom: false })), + moveAvailability: vi.fn(() => ({ top: false, up: false, down: false, bottom: false })), }; return { input, divider, destination, moveMenu, moveItem }; } @@ -63,13 +63,13 @@ describe('menu availability projection', () => { for (const element of [divider, destination, moveMenu, moveItem]) { expect(element.hasAttribute('hidden')).toBe(true); } - expect(input.moveAvailability).not.toHaveBeenCalled(); + expect(input.moveAvailability).toHaveBeenCalled(); }); - it('restores the position group when the live scope becomes a singleton', () => { + it('restores the position group when the batch becomes positionable', () => { const { input, divider, moveMenu, moveItem } = fixture(); refreshMenuAvailability(input); - input.scope = { kind: 'batch', items: [document.createElement('div')] }; + input.moveAvailability = vi.fn(() => ({ top: true, up: true, down: false, bottom: false })); refreshMenuAvailability(input); for (const element of [divider, moveMenu, moveItem]) { expect(element.hasAttribute('hidden')).toBe(false); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.ts index 7015b9d54cb0..f4e72ef23e61 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.ts @@ -58,13 +58,12 @@ export function refreshMenuAvailability(input:MenuAvailabilityInput):void { setAvailability(item, candidates.length > 0 && input.availableDestinations(input.scope, candidates).length > 0); } - const multiItem = input.scope.kind === 'batch' && input.scope.items.length > 1; - const availability = multiItem ? null : input.moveAvailability(); - if (multiItem || availability) { + const availability = input.moveAvailability(); + if (availability) { let available = 0; for (const item of input.moveItems) { const direction = item.getAttribute(`data-${input.identifier}-direction-param`); - const enabled = !multiItem && isMoveDirection(direction) && !!availability?.[direction]; + const enabled = isMoveDirection(direction) && availability[direction]; setAvailability(item, enabled); if (enabled) available += 1; } From 02b253664466d61e8e40a0e5d8cfc562f2cb957e Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sun, 16 Aug 2026 04:24:00 +0100 Subject: [PATCH 4/8] [AGILE-363] Move selected blocks by menu Routes selection roots through the shared block resolver and moves the ordered block through the optimistic collection path. Keeps roots without selection on their singular member endpoint. https://community.openproject.org/wp/AGILE-363 --- .../dynamic/sortable-lists.controller.spec.ts | 195 ++++++++++++++++++ .../dynamic/sortable-lists.controller.ts | 55 ++++- 2 files changed, 249 insertions(+), 1 deletion(-) diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts index 6594915fc454..cfa455ab720f 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts @@ -1196,6 +1196,201 @@ describe('Sortable lists controller', () => { expect(body.body.get('prev_id')).toBe('2'); }); + describe('batch menu moves', () => { + function renderBatchMenuFixture() { + const elements = renderSelectableRoot({ + optimistic: true, + collectionMoveUrl: '/projects/demo/backlogs/work_packages/move', + }); + elements.sourceList.setAttribute('data-sortable-lists--list-type-value', 'sprint'); + elements.sourceList.setAttribute('data-sortable-lists--list-id-value', '8'); + elements.sourceList.append(elements.items[3]); + + return elements; + } + + function selectItems(...items:HTMLElement[]):void { + items.forEach((item, index) => { + item.dispatchEvent(new MouseEvent('click', { + bubbles: true, + cancelable: true, + ctrlKey: index > 0, + })); + }); + } + + function selectedIds(root:HTMLElement):string[] { + return Array.from(root.querySelectorAll('[data-batch-selected]')) + .map((item) => item.getAttribute('data-sortable-lists--item-id-value')!); + } + + function controllerFor(root:HTMLElement):SortableListsControllerType { + return ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + } + + it('moves the ordered selection immediately and excludes it from the predecessor', async () => { + let resolveMove!:(response:Response) => void; + fetchMock.mockImplementationOnce(() => new Promise((resolve) => { + resolveMove = resolve; + })); + const { root, sourceList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[2]); + + controllerFor(root).moveInDirection(items[1], 'down'); + + expect(itemIds(sourceList)).toEqual(['1', '4', '2', '3']); + const [url, options] = fetchMock.mock.lastCall as [string, { body:FormData }]; + expect(url).toBe('/projects/demo/backlogs/work_packages/move?optimistic=true'); + expect([...options.body.entries()]).toEqual([ + ['ids[]', '2'], + ['ids[]', '3'], + ['list_type', 'sprint'], + ['list_id', '8'], + ['prev_id', '4'], + ]); + + resolveMove(new Response('', { status: 200 })); + await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false)); + }); + + it('clears the selection after a successful collection move', async () => { + const { root, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[2]); + + controllerFor(root).moveInDirection(items[1], 'down'); + + await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false)); + expect(selectedIds(root)).toEqual([]); + }); + + it('rolls a rejected collection move back and preserves the selection', async () => { + fetchMock.mockResolvedValueOnce(new Response('', { status: 422 })); + const { root, sourceList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[2]); + + controllerFor(root).moveInDirection(items[1], 'down'); + + await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false)); + expect(itemIds(sourceList)).toEqual(['1', '2', '3', '4']); + expect(selectedIds(root)).toEqual(['2', '3']); + }); + + it('warns when a rejected collection move cannot be rolled back safely', async () => { + let resolveMove!:(response:Response) => void; + fetchMock.mockImplementationOnce(() => new Promise((resolve) => { + resolveMove = resolve; + })); + const { root, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[2]); + + controllerFor(root).moveInDirection(items[1], 'down'); + items[2].remove(); + resolveMove(new Response('', { status: 422 })); + + await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false)); + expect(announceSpy).toHaveBeenCalledWith( + expect.stringContaining('Check the items'), + expect.objectContaining({ politeness: 'assertive' }), + ); + }); + + it('does not request a move when the invoker has no owned list', async () => { + const { root } = renderBatchMenuFixture(); + await ctx.nextFrame(); + + controllerFor(root).moveInDirection(itemRow('99'), 'down'); + await flushPromises(); + + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('does not request a move when the live selection is unavailable', async () => { + const { root, sourceList, targetList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[4]); + + controllerFor(root).moveInDirection(items[1], 'down'); + await flushPromises(); + + expect(itemIds(sourceList)).toEqual(['1', '2', '3', '4']); + expect(itemIds(targetList)).toEqual(['5']); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + // A menu opened on an unselected card can go stale: by the time the item + // is activated its direction may be a no-op. The refusal must leave the + // batch the user built alone rather than replacing it with the invoker. + it('leaves the selection alone when an unselected invoker cannot move', async () => { + const { root, sourceList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[2]); + + controllerFor(root).moveInDirection(items[0], 'top'); + await flushPromises(); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(itemIds(sourceList)).toEqual(['1', '2', '3', '4']); + expect(selectedIds(root)).toEqual(['2', '3']); + }); + + it('does not request a no-op block move', async () => { + const { root, sourceList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[0], items[1]); + + controllerFor(root).moveInDirection(items[0], 'top'); + await flushPromises(); + + expect(itemIds(sourceList)).toEqual(['1', '2', '3', '4']); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('uses the collection contract for an unselected one-card invoker', async () => { + const { root, sourceList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + + controllerFor(root).moveInDirection(items[0], 'down'); + await flushPromises(); + + expect(itemIds(sourceList)).toEqual(['2', '1', '3', '4']); + const [url, options] = fetchMock.mock.lastCall as [string, { body:FormData }]; + expect(url).toBe('/projects/demo/backlogs/work_packages/move?optimistic=true'); + expect(options.body.getAll('ids[]')).toEqual(['1']); + }); + + it('preserves the selection and refuses the action while busy', async () => { + const { root, sourceList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[2]); + root.setAttribute('data-sortable-lists-busy', 'true'); + + controllerFor(root).moveInDirection(items[0], 'down'); + await flushPromises(); + + expect(itemIds(sourceList)).toEqual(['1', '2', '3', '4']); + expect(selectedIds(root)).toEqual(['2', '3']); + expect(fetchMock).not.toHaveBeenCalled(); + }); + }); + + it('retains the singular member contract without collection capability', async () => { + const { root, sourceList, firstSourceItem } = renderFixture({ optimistic: true }); + await ctx.nextFrame(); + + const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; + controller.moveInDirection(firstSourceItem, 'down'); + await flushPromises(); + + expect(itemIds(sourceList)).toEqual(['2', '1', '3']); + const [url, options] = fetchMock.mock.lastCall as [string, { body:FormData }]; + expect(url).toBe('/move/1?optimistic=true'); + expect(options.body.getAll('ids[]')).toEqual([]); + }); + it('refuses a directional move for a non-movable item', async () => { const { root, sourceList, firstSourceItem } = renderFixture(); firstSourceItem.setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts index 1763b786f955..1738591d0508 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts @@ -54,8 +54,9 @@ import { isOrderableItem, itemAcceptsDestination, reorderRows, - resolveDirectionalPreviousItemId, + resolveBlockMove, resolveBlockMoveAvailability, + resolveDirectionalPreviousItemId, resolveItemId, resolveItemLabel, resolveItemPosition, @@ -544,6 +545,58 @@ export default class SortableListsController extends Controller imp return; } + if (this.selection && this.hasCollectionMoveUrlValue) { + // Resolved without mutating: every check below can still refuse the + // move, and a stale menu must not replace the user's batch with the + // invoker for a move that then never runs. + const scope = this.actionScopeFor(itemElement); + if (scope.kind === 'refused') { + return; + } + + const list = this.ownerListOf(itemElement); + if (!list) { + return; + } + + const resolution = resolveBlockMove({ + itemElements: scope.items, + direction, + rowsContainer: list.rowsContainer, + }); + const moveUrl = this.resolveCollectionMoveUrl(); + if (!resolution.available || !moveUrl) { + return; + } + + // Scope members are resolved candidates, so both identity attributes + // exist; refusing on a mismatch keeps the moved rows and the submitted + // ids from ever diverging. + const items = scope.items.flatMap((element):SelectionItem[] => { + const type = resolveItemType(element); + const id = resolveItemId(element); + return type && id ? [{ type, id }] : []; + }); + if (items.length !== scope.items.length) { + return; + } + + // Committed only now the move is known executable: invoking a position + // action on an unselected card selects it, and a failed request keeps + // that selection for the retry. + this.selectForAction(itemElement); + + void this.performMove({ + rows: resolution.rows, + items, + rowsContainer: list.rowsContainer, + listData: list.listData, + previousItemId: resolution.previousItemId, + moveUrl, + }); + return; + } + const list = this.ownerListOf(itemElement); if (!list) { return; From 250791d0b1ed97d1596fc1107b91e230fa46102b Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sun, 16 Aug 2026 04:38:46 +0100 Subject: [PATCH 5/8] [AGILE-363] Cover batch position actions Exercises every direction and gating boundary through Selenium. Covers optimistic order, feedback, cleanup, and 422 rollback while protecting one-card, sparse, cross-list, and confined scopes. https://community.openproject.org/wp/AGILE-363 --- .../work_package_card_menu_component_spec.rb | 4 +- .../batch_destination_menu_spec.rb | 7 +- .../work_packages/batch_move_via_menu_spec.rb | 202 ++++++++++++++++++ .../backlogs/spec/support/pages/backlog.rb | 34 +++ 4 files changed, 243 insertions(+), 4 deletions(-) create mode 100644 modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb diff --git a/modules/backlogs/spec/components/backlogs/work_package_card_menu_component_spec.rb b/modules/backlogs/spec/components/backlogs/work_package_card_menu_component_spec.rb index c3c3766a8e8b..9836ab41cbc8 100644 --- a/modules/backlogs/spec/components/backlogs/work_package_card_menu_component_spec.rb +++ b/modules/backlogs/spec/components/backlogs/work_package_card_menu_component_spec.rb @@ -245,9 +245,11 @@ def render_component(work_package: self.work_package, "[data-action='click->sortable-lists--item#move']" ) end - expect(page).to have_css("li[data-sortable-lists--item-target='moveMenu']") + expect(page).to have_css("li[data-sortable-lists--item-target='moveMenu']", count: 1) expect(page).to have_no_field("direction", type: :hidden) expect(page).to have_no_field("prev_id", type: :hidden) + expect(page).to have_no_field("ids[]", type: :hidden) + expect(page).to have_no_css("form[action$='/backlogs/work_packages/move']") end context "when the work package is the only item in its list" do diff --git a/modules/backlogs/spec/features/work_packages/batch_destination_menu_spec.rb b/modules/backlogs/spec/features/work_packages/batch_destination_menu_spec.rb index 8c98b90e7f7e..3eb6ca1ba3dc 100644 --- a/modules/backlogs/spec/features/work_packages/batch_destination_menu_spec.rb +++ b/modules/backlogs/spec/features/work_packages/batch_destination_menu_spec.rb @@ -203,18 +203,19 @@ end end - context "with a multi-card action scope" do + context "with a contiguous multi-card action scope" do let!(:sprint) { create(:sprint, project:, name: "Sprint") } let!(:destination_bucket) { create(:backlog_bucket, project:, name: "Destination bucket") } let!(:first_story) { create(:work_package, project:, type:, sprint:, position: 1) } let!(:second_story) { create(:work_package, project:, type:, sprint:, position: 2) } + let!(:third_story) { create(:work_package, project:, type:, sprint:, position: 3) } - it "omits Move to position from a multi-card action scope" do + it "offers Move to position for a contiguous multi-card action scope" do backlogs_page.visit! backlogs_page.select_cards(first_story, second_story) - backlogs_page.expect_no_work_package_action(first_story, "Move to position") + backlogs_page.expect_move_to_position_available(first_story, action: "Move down") backlogs_page.expect_work_package_action(first_story, "Move to backlog bucket") end end diff --git a/modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb b/modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb new file mode 100644 index 000000000000..532828498fb5 --- /dev/null +++ b/modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb @@ -0,0 +1,202 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" +require_relative "../../support/pages/backlog" + +RSpec.describe "Move selected backlog cards to a position via their menu", + :js, + :selenium, + :settings_reset, + with_ee: %i[readonly_work_packages] do + let!(:project) do + create(:project, types: [type], enabled_module_names: %w(work_package_tracking backlogs)) + end + let(:type) { create(:type) } + let(:manage_sprint_items_role) do + create(:project_role, + permissions: %i[view_sprints manage_sprint_items view_work_packages edit_work_packages]) + end + let(:backlogs_page) { Pages::Backlog.new(project) } + + current_user do + create(:user, member_with_roles: { project => manage_sprint_items_role }) + end + + def create_sprint_stories(sprint, count: 6, status: nil) + Array.new(count) do |index| + create(:work_package, **{ project:, type:, sprint:, status:, position: index + 1 }.compact) + end + end + + it "moves a contiguous block to the top and then down while preserving its order" do + sprint = create(:sprint, project:, name: "Sprint") + stories = create_sprint_stories(sprint) + backlogs_page.visit! + + backlogs_page.select_contiguous_cards(stories[2], stories[3]) + backlogs_page.expect_move_to_position_available(stories[2], action: "Move to top") + backlogs_page.move_selected_cards(invoker: stories[2], action: "Move to top") + + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[2], stories[3], stories[0], stories[1], stories[4], stories[5]] + ) + backlogs_page.expect_polite_announcement("2 work packages moved to positions 1 through 2 of 6") + backlogs_page.expect_persisted_sprint_order( + sprint, + stories[2], stories[3], stories[0], stories[1], stories[4], stories[5] + ) + backlogs_page.expect_no_selected_cards + + backlogs_page.select_contiguous_cards(stories[2], stories[3]) + backlogs_page.move_selected_cards(invoker: stories[3], action: "Move down") + + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[0], stories[2], stories[3], stories[1], stories[4], stories[5]] + ) + backlogs_page.expect_persisted_sprint_order( + sprint, + stories[0], stories[2], stories[3], stories[1], stories[4], stories[5] + ) + end + + it "moves a contiguous block to the bottom and then up while preserving its order" do + sprint = create(:sprint, project:, name: "Sprint") + stories = create_sprint_stories(sprint) + backlogs_page.visit! + + backlogs_page.select_contiguous_cards(stories[2], stories[3]) + backlogs_page.move_selected_cards(invoker: stories[3], action: "Move to bottom") + + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[0], stories[1], stories[4], stories[5], stories[2], stories[3]] + ) + backlogs_page.expect_persisted_sprint_order( + sprint, + stories[0], stories[1], stories[4], stories[5], stories[2], stories[3] + ) + backlogs_page.expect_no_selected_cards + + backlogs_page.select_contiguous_cards(stories[2], stories[3]) + backlogs_page.move_selected_cards(invoker: stories[2], action: "Move up") + + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[0], stories[1], stories[4], stories[2], stories[3], stories[5]] + ) + backlogs_page.expect_persisted_sprint_order( + sprint, + stories[0], stories[1], stories[4], stories[2], stories[3], stories[5] + ) + end + + it "keeps the singular menu contract for an unselected card" do + sprint = create(:sprint, project:, name: "Sprint") + stories = create_sprint_stories(sprint, count: 3) + backlogs_page.visit! + + backlogs_page.expect_move_to_position_available(stories[1], action: "Move up") + backlogs_page.move_selected_cards(invoker: stories[1], action: "Move up") + + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[1], stories[0], stories[2]] + ) + backlogs_page.expect_polite_announcement("#{stories[1].to_fs(:caption)} moved to position 1 of 3") + backlogs_page.expect_persisted_sprint_order(sprint, stories[1], stories[0], stories[2]) + end + + it "omits positional actions for sparse and cross-list selections" do + sprint = create(:sprint, project:, name: "Sprint") + other_sprint = create(:sprint, project:, name: "Other sprint") + stories = create_sprint_stories(sprint, count: 4) + other_story = create(:work_package, project:, type:, sprint: other_sprint) + backlogs_page.visit! + + backlogs_page.select_cards(stories[0], stories[2]) + backlogs_page.expect_move_to_position_unavailable(stories[0]) + backlogs_page.clear_card_selection(stories[0]) + + backlogs_page.select_cards(stories[0], other_story) + backlogs_page.expect_move_to_position_unavailable(stories[0]) + end + + it "moves a contiguous confined block within its list" do + sprint = create(:sprint, project:, name: "Sprint") + readonly_status = create(:status, is_readonly: true) + stories = create_sprint_stories(sprint, count: 4, status: readonly_status) + backlogs_page.visit! + + backlogs_page.select_contiguous_cards(stories[1], stories[2]) + backlogs_page.expect_move_to_position_available(stories[1], action: "Move down") + backlogs_page.move_selected_cards(invoker: stories[1], action: "Move down") + + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[0], stories[3], stories[1], stories[2]] + ) + backlogs_page.expect_persisted_sprint_order(sprint, stories[0], stories[3], stories[1], stories[2]) + end + + it "omits one-step actions at truncation boundaries and block no-ops" do + stub_const("Backlogs::InboxComponent::TRUNCATE_MIDDLE", 2) + inbox_stories = Array.new(5) do |index| + create(:work_package, project:, type:, position: index + 1) + end + backlogs_page.visit! + + backlogs_page.select_contiguous_cards(inbox_stories[0], inbox_stories[1]) + backlogs_page.expect_move_to_position_unavailable(inbox_stories[0], action: "Move down") + backlogs_page.expect_move_to_position_unavailable(inbox_stories[0], action: "Move to top") + backlogs_page.clear_card_selection(inbox_stories[0]) + + backlogs_page.select_cards(inbox_stories.last) + backlogs_page.expect_move_to_position_unavailable(inbox_stories.last, action: "Move to bottom") + end + + it "rolls a rejected move back and preserves the selected block" do + sprint = create(:sprint, project:, name: "Sprint") + stories = create_sprint_stories(sprint, count: 4) + backlogs_page.visit! + + backlogs_page.select_contiguous_cards(stories[1], stories[2]) + stories[2].destroy! + backlogs_page.move_selected_cards(invoker: stories[1], action: "Move down") + + backlogs_page.expect_move_error( + I18n.t("backlogs.work_packages.move_collection.work_packages_not_found") + ) + backlogs_page.expect_sprint_items_in_order(sprint, items: stories) + backlogs_page.expect_selected_cards_in_order(stories[1], stories[2]) + end +end diff --git a/modules/backlogs/spec/support/pages/backlog.rb b/modules/backlogs/spec/support/pages/backlog.rb index 227a370c151c..7109c93bb1b5 100644 --- a/modules/backlogs/spec/support/pages/backlog.rb +++ b/modules/backlogs/spec/support/pages/backlog.rb @@ -745,16 +745,50 @@ def selected_card_ids def select_cards(*work_packages) work_packages.each { |work_package| toggle_card(work_package) } + expect(page).to have_css("[data-batch-selected]", count: work_packages.size) + end + + # Settles on the range's own endpoints rather than a card count, which + # only the caller knows for a span wider than a pair. + def select_contiguous_cards(first, last) + toggle_card(first) + extend_selection_to(last) + expect(page).to have_css("#{work_package_selector(first)}[data-batch-selected]") + expect(page).to have_css("#{work_package_selector(last)}[data-batch-selected]") end def expect_selected_cards_in_order(*work_packages) + expect(page).to have_css("[data-batch-selected]", count: work_packages.size) expect(selected_card_ids).to eq(work_packages.map { |work_package| work_package.id.to_s }) end def expect_no_selected_cards + expect(page).to have_no_css("[data-batch-selected]") expect(selected_card_ids).to be_empty end + def move_selected_cards(invoker:, action:, wait: false) + click_in_work_package_move_submenu(invoker, action, wait:) + end + + def expect_move_to_position_available(work_package, action: nil) + within_work_package_move_submenu(work_package) do |submenu| + next unless action + + expect(submenu).to have_selector(:menuitem, text: action, exact_text: true) + end + end + + def expect_move_to_position_unavailable(work_package, action: nil) + if action + within_work_package_move_submenu(work_package) do |submenu| + expect(submenu).to have_no_selector(:menuitem, text: action, exact_text: true) + end + else + expect_no_work_package_action(work_package, "Move to position") + end + end + def clear_card_selection(work_package) work_package_card(work_package).send_keys(:escape) expect_no_selected_cards From 7f7b18547fdaefe5bd48ed64752eea9c59b10f38 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sun, 16 Aug 2026 04:54:14 +0100 Subject: [PATCH 6/8] [AGILE-363] Guard batch position capability Resolves collection capability before availability or mutation. Makes incomplete selection roots fail closed while keeping roots without selection on their singular member behavior. https://community.openproject.org/wp/AGILE-363 --- .../dynamic/sortable-lists.controller.spec.ts | 62 +++++++++++++------ .../dynamic/sortable-lists.controller.ts | 16 +++-- .../selection-orchestrator.spec.ts | 59 ------------------ .../sortable-lists/selection-orchestrator.ts | 7 --- 4 files changed, 53 insertions(+), 91 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts index cfa455ab720f..ab05b65e3d3f 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts @@ -1197,11 +1197,16 @@ describe('Sortable lists controller', () => { }); describe('batch menu moves', () => { - function renderBatchMenuFixture() { + function renderBatchMenuFixture({ + collectionMoveUrl = '/projects/demo/backlogs/work_packages/move', + }:{ collectionMoveUrl?:string|null } = {}) { const elements = renderSelectableRoot({ optimistic: true, - collectionMoveUrl: '/projects/demo/backlogs/work_packages/move', + collectionMoveUrl, }); + if (collectionMoveUrl === '') { + elements.root.setAttribute('data-sortable-lists-collection-move-url-value', ''); + } elements.sourceList.setAttribute('data-sortable-lists--list-type-value', 'sprint'); elements.sourceList.setAttribute('data-sortable-lists--list-id-value', '8'); elements.sourceList.append(elements.items[3]); @@ -1375,6 +1380,35 @@ describe('Sortable lists controller', () => { expect(selectedIds(root)).toEqual(['2', '3']); expect(fetchMock).not.toHaveBeenCalled(); }); + + it.each([ + ['missing', null], + ['blank', ''], + ])('reports every direction unavailable when the collection URL is %s', async (_state, collectionMoveUrl) => { + const { root, items } = renderBatchMenuFixture({ collectionMoveUrl }); + await ctx.nextFrame(); + selectItems(items[0], items[1]); + + expect(controllerFor(root).moveAvailability(items[0])).toEqual({ + top: false, up: false, down: false, bottom: false, + }); + }); + + it.each([ + ['missing', null], + ['blank', ''], + ])('preserves a selected block without requesting when the collection URL is %s', async (_state, collectionMoveUrl) => { + const { root, sourceList, items } = renderBatchMenuFixture({ collectionMoveUrl }); + await ctx.nextFrame(); + selectItems(items[0], items[1]); + + controllerFor(root).moveInDirection(items[0], 'down'); + await flushPromises(); + + expect(itemIds(sourceList)).toEqual(['1', '2', '3', '4']); + expect(selectedIds(root)).toEqual(['1', '2']); + expect(fetchMock).not.toHaveBeenCalled(); + }); }); it('retains the singular member contract without collection capability', async () => { @@ -1405,7 +1439,7 @@ describe('Sortable lists controller', () => { }); it('reports per-direction move availability for gating', async () => { - const { root, firstSourceItem } = renderSelectableRoot(); + const { root, firstSourceItem } = renderSelectableRoot({ collectionMoveUrl: '/collection-move-url' }); await ctx.nextFrame(); const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; @@ -1422,7 +1456,7 @@ describe('Sortable lists controller', () => { }; it('reports the available directions for a selected contiguous block', async () => { - const { root, items } = renderSelectableRoot(); + const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/collection-move-url' }); await ctx.nextFrame(); const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; select(items[1]); @@ -1434,7 +1468,7 @@ describe('Sortable lists controller', () => { }); it('reports every direction unavailable for a sparse selection', async () => { - const { root, items } = renderSelectableRoot(); + const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/collection-move-url' }); await ctx.nextFrame(); const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; select(items[0]); @@ -1444,7 +1478,7 @@ describe('Sortable lists controller', () => { }); it('reports every direction unavailable for a cross-list selection', async () => { - const { root, items } = renderSelectableRoot(); + const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/collection-move-url' }); await ctx.nextFrame(); const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; select(items[2]); @@ -1454,7 +1488,7 @@ describe('Sortable lists controller', () => { }); it('reports one-card availability for an unselected invoker without changing the selection', async () => { - const { root, items } = renderSelectableRoot(); + const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/collection-move-url' }); await ctx.nextFrame(); const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; select(items[0]); @@ -1467,7 +1501,7 @@ describe('Sortable lists controller', () => { }); it('reports normal within-list availability for a confined contiguous block', async () => { - const { root, items } = renderSelectableRoot(); + const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/collection-move-url' }); items[0].setAttribute('data-sortable-lists--item-mobility-value', 'confined'); items[1].setAttribute('data-sortable-lists--item-mobility-value', 'confined'); await ctx.nextFrame(); @@ -1864,18 +1898,6 @@ describe('Sortable lists controller', () => { expect(document.querySelectorAll('[data-batch-selected]')).toHaveLength(0); }); - it('collapses a wider batch onto the card a menu move names', async () => { - const { root, items } = renderSelectableRoot(); - await ctx.nextFrame(); - const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; - items[1].dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true })); - items[2].dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true })); - - controller.moveInDirection(items[0], 'down'); - - expect(items.filter((item) => item.hasAttribute('data-batch-selected'))).toEqual([items[0]]); - }); - // A card that is not part of the batch collapses it onto itself, which is // a count change a screen-reader user has to hear. Dragging a member // instead carries the whole batch — see the "batch dragging" block below. diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts index 1738591d0508..a0582576ecba 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts @@ -507,6 +507,10 @@ export default class SortableListsController extends Controller imp return null; } + if (!this.resolveCollectionMoveUrl()) { + return { top: false, up: false, down: false, bottom: false }; + } + const list = this.ownerListOf(itemElement); return list @@ -545,7 +549,12 @@ export default class SortableListsController extends Controller imp return; } - if (this.selection && this.hasCollectionMoveUrlValue) { + if (this.selection) { + const moveUrl = this.resolveCollectionMoveUrl(); + if (!moveUrl) { + return; + } + // Resolved without mutating: every check below can still refuse the // move, and a stale menu must not replace the user's batch with the // invoker for a move that then never runs. @@ -564,8 +573,7 @@ export default class SortableListsController extends Controller imp direction, rowsContainer: list.rowsContainer, }); - const moveUrl = this.resolveCollectionMoveUrl(); - if (!resolution.available || !moveUrl) { + if (!resolution.available) { return; } @@ -618,8 +626,6 @@ export default class SortableListsController extends Controller imp return; } - this.selection?.collapseForAction(itemElement); - void this.performMove({ rows: [sourceRow], items: null, diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.spec.ts index 00b6fa97cace..53557e83f409 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.spec.ts @@ -861,65 +861,6 @@ describe('SelectionOrchestrator', () => { }); }); - // A menu action relocates one card, so it collapses the batch onto that - // card rather than leaving a wider selection to outlive the move. - describe('collapseForAction', () => { - it('collapses a wider batch onto a member', () => { - const orchestrator = new SelectionOrchestrator(hostFor(root)); - orchestrator.handleClick(clickOn(item('1'))); - orchestrator.handleClick(clickOn(item('3'), { ctrlKey: true })); - - expect(orchestrator.collapseForAction(item('1'))).toEqual({ kind: 'batch', items: [item('1')] }); - expect(selectedIds()).toEqual(['1']); - }); - - it('collapses a wider batch onto a card outside it', () => { - const orchestrator = new SelectionOrchestrator(hostFor(root)); - orchestrator.handleClick(clickOn(item('1'))); - orchestrator.handleClick(clickOn(item('2'), { ctrlKey: true })); - - expect(orchestrator.collapseForAction(item('3')).items).toEqual([item('3')]); - expect(selectedIds()).toEqual(['3']); - }); - - it('leaves an empty selection empty', () => { - const orchestrator = new SelectionOrchestrator(hostFor(root)); - - expect(orchestrator.collapseForAction(item('2')).items).toEqual([item('2')]); - expect(orchestrator.hasSelection).toBe(false); - }); - - it('refuses a fixed card without disturbing the batch', () => { - item('3').setAttribute('data-sortable-lists--item-mobility-value', 'fixed'); - const orchestrator = new SelectionOrchestrator(hostFor(root)); - orchestrator.handleClick(clickOn(item('1'))); - - expect(orchestrator.collapseForAction(item('3'))).toEqual({ kind: 'refused', items: [] }); - expect(selectedIds()).toEqual(['1']); - }); - - it('announces the collapse of a wider batch', () => { - const orchestrator = new SelectionOrchestrator(hostFor(root)); - orchestrator.handleClick(clickOn(item('1'))); - orchestrator.handleClick(clickOn(item('3'), { ctrlKey: true })); - announceSpy.mockClear(); - - orchestrator.collapseForAction(item('1')); - - expect(announceSpy.mock.calls.map((call) => call[0])).toEqual(['[selected:1]']); - }); - - it('stays silent collapsing a single selection onto itself', () => { - const orchestrator = new SelectionOrchestrator(hostFor(root)); - orchestrator.handleClick(clickOn(item('1'))); - announceSpy.mockClear(); - - orchestrator.collapseForAction(item('1')); - - expect(announceSpy).not.toHaveBeenCalled(); - }); - }); - describe('clearSilently', () => { it('clears model, anchor and presentation without an announcement', () => { const orchestrator = new SelectionOrchestrator(hostFor(root)); diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts index adf02d4a90ce..08bd369c0fe0 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts @@ -112,13 +112,6 @@ export class SelectionOrchestrator { return this.resolveActionScope(itemElement, 'replace-if-unselected'); } - // A menu action relocates exactly this card, so a wider batch collapses - // onto it rather than outliving the move. It must not manufacture a - // selection where the user made none: a failed move would otherwise leave - // that card selected. - collapseForAction(itemElement:HTMLElement):ActionScope { - return this.resolveActionScope(itemElement, this.hasSelection ? 'replace' : 'none'); - } private resolveActionScope(itemElement:HTMLElement, mutation:ScopeMutation):ActionScope { const candidate = resolveCandidate(this.host.rootElement, itemElement); From c87a88c3ffb1884c8b41073397ecdf72581a0c5c Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Sun, 16 Aug 2026 23:54:41 +0100 Subject: [PATCH 7/8] [AGILE-363] Restore singular position gating Restores per-direction availability for plain sortable roots while keeping selection roots on block and capability gating. Protects blank predecessors and removes the obsolete collapse shim. https://community.openproject.org/wp/AGILE-363 --- .../dynamic/sortable-lists.controller.spec.ts | 22 ++++++++++++++++-- .../dynamic/sortable-lists.controller.ts | 23 ++++++++++--------- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts index ab05b65e3d3f..4cbb4a39aff5 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts @@ -1259,6 +1259,24 @@ describe('Sortable lists controller', () => { await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false)); }); + it('serializes a top block move with an explicit blank predecessor', async () => { + const { root, sourceList, items } = renderBatchMenuFixture(); + await ctx.nextFrame(); + selectItems(items[1], items[2]); + + controllerFor(root).moveInDirection(items[1], 'top'); + + expect(itemIds(sourceList)).toEqual(['2', '3', '1', '4']); + const options = fetchMock.mock.lastCall?.[1] as { body:FormData }; + expect([...options.body.entries()]).toEqual([ + ['ids[]', '2'], + ['ids[]', '3'], + ['list_type', 'sprint'], + ['list_id', '8'], + ['prev_id', ''], + ]); + }); + it('clears the selection after a successful collection move', async () => { const { root, items } = renderBatchMenuFixture(); await ctx.nextFrame(); @@ -1438,8 +1456,8 @@ describe('Sortable lists controller', () => { expect(fetchMock).not.toHaveBeenCalled(); }); - it('reports per-direction move availability for gating', async () => { - const { root, firstSourceItem } = renderSelectableRoot({ collectionMoveUrl: '/collection-move-url' }); + it('reports per-direction availability for a plain sortable root', async () => { + const { root, firstSourceItem } = renderFixture(); await ctx.nextFrame(); const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType; diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts index a0582576ecba..b008e1a056cf 100644 --- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts +++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts @@ -61,6 +61,7 @@ import { resolveItemLabel, resolveItemPosition, resolveItemType, + resolveMoveAvailability, restoreRowPositions, rowOf, rowsRemainAt, @@ -493,29 +494,29 @@ export default class SortableListsController extends Controller imp return this.element.hasAttribute(sortableListsBusyAttribute); } - // A direction is offered exactly when the move resolver can produce a - // target for it, and never for an item moveInDirection would refuse below. - // Null means the item is not in an owned list yet. A snapshot for menu - // gating; the click path re-resolves the live DOM. + // A direction is offered exactly when the move resolver can produce a target + // for it, which keeps the menu honest about truncated lists. Null means the + // item is not in an owned list yet, or takes no part in ordering. A snapshot + // for menu gating; the click path re-resolves the live DOM. moveAvailability(itemElement:HTMLElement):MoveAvailability|null { - if (!isOrderableItem(itemElement)) { + const list = this.ownerListOf(itemElement); + if (!list || !isOrderableItem(itemElement)) { return null; } const scope = this.actionScopeFor(itemElement); if (scope.kind === 'refused') { - return null; + return resolveMoveAvailability({ + itemElement, + rowsContainer: list.rowsContainer, + }); } if (!this.resolveCollectionMoveUrl()) { return { top: false, up: false, down: false, bottom: false }; } - const list = this.ownerListOf(itemElement); - - return list - ? resolveBlockMoveAvailability({ itemElements: scope.items, rowsContainer: list.rowsContainer }) - : null; + return resolveBlockMoveAvailability({ itemElements: scope.items, rowsContainer: list.rowsContainer }); } moveToDestination(itemElement:HTMLElement, target:DestinationIdentity):void { From 771c8e616e5925b0b45dfdcb55d1f08f03f312c2 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Wed, 19 Aug 2026 19:57:21 +0100 Subject: [PATCH 8/8] Assert position outcomes through the page Replaces the persisted-order database checks with a reload and a rendered-order assertion, so the spec proves persistence by what the user sees surviving the reload. Payload and ordering semantics remain covered by the controller unit specs and the request specs. --- .../work_packages/batch_move_via_menu_spec.rb | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb b/modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb index 532828498fb5..1494cd381c23 100644 --- a/modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb +++ b/modules/backlogs/spec/features/work_packages/batch_move_via_menu_spec.rb @@ -70,10 +70,6 @@ def create_sprint_stories(sprint, count: 6, status: nil) items: [stories[2], stories[3], stories[0], stories[1], stories[4], stories[5]] ) backlogs_page.expect_polite_announcement("2 work packages moved to positions 1 through 2 of 6") - backlogs_page.expect_persisted_sprint_order( - sprint, - stories[2], stories[3], stories[0], stories[1], stories[4], stories[5] - ) backlogs_page.expect_no_selected_cards backlogs_page.select_contiguous_cards(stories[2], stories[3]) @@ -83,9 +79,11 @@ def create_sprint_stories(sprint, count: 6, status: nil) sprint, items: [stories[0], stories[2], stories[3], stories[1], stories[4], stories[5]] ) - backlogs_page.expect_persisted_sprint_order( + + backlogs_page.visit! + backlogs_page.expect_sprint_items_in_order( sprint, - stories[0], stories[2], stories[3], stories[1], stories[4], stories[5] + items: [stories[0], stories[2], stories[3], stories[1], stories[4], stories[5]] ) end @@ -101,10 +99,6 @@ def create_sprint_stories(sprint, count: 6, status: nil) sprint, items: [stories[0], stories[1], stories[4], stories[5], stories[2], stories[3]] ) - backlogs_page.expect_persisted_sprint_order( - sprint, - stories[0], stories[1], stories[4], stories[5], stories[2], stories[3] - ) backlogs_page.expect_no_selected_cards backlogs_page.select_contiguous_cards(stories[2], stories[3]) @@ -114,9 +108,11 @@ def create_sprint_stories(sprint, count: 6, status: nil) sprint, items: [stories[0], stories[1], stories[4], stories[2], stories[3], stories[5]] ) - backlogs_page.expect_persisted_sprint_order( + + backlogs_page.visit! + backlogs_page.expect_sprint_items_in_order( sprint, - stories[0], stories[1], stories[4], stories[2], stories[3], stories[5] + items: [stories[0], stories[1], stories[4], stories[2], stories[3], stories[5]] ) end @@ -133,7 +129,12 @@ def create_sprint_stories(sprint, count: 6, status: nil) items: [stories[1], stories[0], stories[2]] ) backlogs_page.expect_polite_announcement("#{stories[1].to_fs(:caption)} moved to position 1 of 3") - backlogs_page.expect_persisted_sprint_order(sprint, stories[1], stories[0], stories[2]) + + backlogs_page.visit! + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[1], stories[0], stories[2]] + ) end it "omits positional actions for sparse and cross-list selections" do @@ -165,7 +166,12 @@ def create_sprint_stories(sprint, count: 6, status: nil) sprint, items: [stories[0], stories[3], stories[1], stories[2]] ) - backlogs_page.expect_persisted_sprint_order(sprint, stories[0], stories[3], stories[1], stories[2]) + + backlogs_page.visit! + backlogs_page.expect_sprint_items_in_order( + sprint, + items: [stories[0], stories[3], stories[1], stories[2]] + ) end it "omits one-step actions at truncation boundaries and block no-ops" do