Keep truncated lists on the stream reload path

A same-list inbox drag was always answered with a 204 (no frame
reload), even when the target list's visible window is truncated
(Backlogs::InboxComponent renders only a head/tail slice plus a
show-more marker row). Trusting the client's optimistic reorder there
left the truncated window stale: the moved row could stay visible when
it should now hide behind the marker, or a newly-eligible row would
never appear.

sortable-lists/list-dom.ts gains hasTruncationMarkerRow, checking for
the existing sortablePreviousItemIdAttribute marker on a rows
container. sortable-lists.controller.ts's handleDrop uses it to decide
whether a move is optimistic; buildMoveFormData now only appends the
optimistic param when true, so a truncated target list falls back to
the same full turbo-stream reload menu moves already use, while the
optimistic DOM reorder still happens client-side for responsiveness.
Adds a Selenium feature spec covering a same-list drag into a
stubbed-small truncated inbox, verifying the visible window and the
marker's collapsed-item id are both recomputed after the drag.

Also hardens resolveRowsContainer against an invalid
rowsContainerSelector value: querySelector can throw a SyntaxError on
a malformed selector, which previously propagated instead of falling
back to the list element like every other unmatched-selector case.
Wraps the lookup in try/catch, warning and returning the list element
on failure.

Author: myabc

frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts

@@ -118,6 +118,12 @@ describe('Sortable lists controller', () => {
     return { root, sourceList, targetList, firstSourceItem: sourceList.querySelector<HTMLElement>('[data-sortable-lists--item-id-value="1"]')! };
   }
 
+  function truncationMarkerRow(previousItemId = 'hidden-item'):HTMLLIElement {
+    const row = document.createElement('li');
+    row.setAttribute('data-sortable-lists-prev-item-id', previousItemId);
+    return row;
+  }
+
   function renderScrollableFixture(values = '') {
     fixture.innerHTML = `
       <div data-controller="sortable-lists" ${values}>
@@ -291,6 +297,29 @@ describe('Sortable lists controller', () => {
     expect(options.body.get('prev_id')).toEqual('5');
   });
 
+  it('marks the move optimistic when the target list has no truncation marker row', async () => {
+    const { targetList, firstSourceItem } = renderFixture();
+
+    await ctx.nextFrame();
+    await dropCurrentItemOnList(firstSourceItem, targetList);
+
+    const options = fetchMock.mock.lastCall?.[1] as { body:FormData };
+
+    expect(options.body.get('optimistic')).toEqual('true');
+  });
+
+  it('does not mark the move optimistic when the target list has a truncation marker row', async () => {
+    const { targetList, firstSourceItem } = renderFixture();
+    targetList.append(truncationMarkerRow());
+
+    await ctx.nextFrame();
+    await dropCurrentItemOnList(firstSourceItem, targetList);
+
+    const options = fetchMock.mock.lastCall?.[1] as { body:FormData };
+
+    expect(options.body.get('optimistic')).toBeNull();
+  });
+
   it('resolves the move url from the template matching the item type', async () => {
     const { targetList, firstSourceItem } = renderFixture({
       moveUrlTemplates: { work_package: '/projects/demo/backlogs/work_packages/{id}/move' },

frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts

@@ -46,6 +46,7 @@ import {
 } from './sortable-lists/drag-and-drop';
 import {
   captureRowPositions,
+  hasTruncationMarkerRow,
   reorderRows,
   resolveSourceRow,
   restoreRowPositions,
@@ -150,15 +151,18 @@ export default class SortableListsController extends Controller<HTMLElement> {
       return;
     }
 
-    // Move the row optimistically, then persist. A same-list move is answered
-    // with 204 and this DOM order is final; a cross-list move gets a
-    // turbo-stream frame reload that reconciles the list. A failure rolls the
-    // row back to where it started.
+    // Move the row optimistically, then persist. A same-list move into a
+    // non-truncated list is answered with 204 and this DOM order is final; a
+    // cross-list move, or a same-list move into a truncated list (whose
+    // visible window is server-computed), gets a turbo-stream frame reload
+    // that reconciles the list. A failure rolls the row back to where it
+    // started.
     const rows = [sourceRow];
     const rollback = captureRowPositions(rows);
     reorderRows({ rows, container: intent.rowsContainer, previousItemId: intent.previousItemId });
 
-    const result = await this.moveItem({ intent, moveUrl });
+    const optimistic = !hasTruncationMarkerRow(intent.rowsContainer);
+    const result = await this.moveItem({ intent, moveUrl, optimistic });
 
     if (result.ok) {
       // After moveItem's finally: moving is false again, so listeners resumed
@@ -213,9 +217,17 @@ export default class SortableListsController extends Controller<HTMLElement> {
     return /^[a-z][a-z\d+\-.]*:/i.test(moveUrl) ? url.toString() : `${url.pathname}${url.search}${url.hash}`;
   }
 
-  private async moveItem({ intent, moveUrl }:{ intent:DropIntent; moveUrl:string }):Promise<MoveResult> {
+  private async moveItem({
+    intent,
+    moveUrl,
+    optimistic,
+  }:{
+    intent:DropIntent;
+    moveUrl:string;
+    optimistic:boolean;
+  }):Promise<MoveResult> {
     const request = new FetchRequest('put', moveUrl, {
-      body: buildMoveFormData({ intent, positionMode: this.positionMode }),
+      body: buildMoveFormData({ intent, positionMode: this.positionMode, optimistic }),
       responseKind: 'turbo-stream',
     });
 

frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.spec.ts

@@ -204,7 +204,7 @@ describe('sortable lists drag and drop helpers', () => {
 
   describe('buildMoveFormData', () => {
     it('builds a relative payload from the intent', () => {
-      const data = buildMoveFormData({ intent: intentFixture(), positionMode: 'relative' });
+      const data = buildMoveFormData({ intent: intentFixture(), positionMode: 'relative', optimistic: true });
       expect(data.get('list_type')).toBe('sprint');
       expect(data.get('list_id')).toBe('42');
       expect(data.get('prev_id')).toBe('a');
@@ -216,18 +216,28 @@ describe('sortable lists drag and drop helpers', () => {
       const data = buildMoveFormData({
         intent: intentFixture({ listId: null, previousItemId: null }),
         positionMode: 'relative',
+        optimistic: true,
       });
       expect(data.get('list_id')).toBe('');
       expect(data.get('prev_id')).toBe('');
     });
 
     it('builds an absolute payload with a computed position', () => {
-      const data = buildMoveFormData({ intent: intentFixture({ previousItemId: 'a' }), positionMode: 'absolute' });
+      const data = buildMoveFormData({
+        intent: intentFixture({ previousItemId: 'a' }),
+        positionMode: 'absolute',
+        optimistic: true,
+      });
       expect(data.get('target_id')).toBe('42');
       expect(data.get('position')).toBe('2');
       expect(data.get('optimistic')).toBe('true');
       expect(data.get('prev_id')).toBeNull();
     });
+
+    it('omits the optimistic param when the move is not optimistic', () => {
+      const data = buildMoveFormData({ intent: intentFixture(), positionMode: 'relative', optimistic: false });
+      expect(data.get('optimistic')).toBeNull();
+    });
   });
 
   // The drop target Pragmatic DnD reports is always the item controller's own

frontend/src/stimulus/controllers/dynamic/sortable-lists/drag-and-drop.ts

@@ -116,17 +116,24 @@ export function sortableListData({
 export type SortablePositionMode = 'relative'|'absolute';
 
 // One builder for both payload shapes so call sites hand over the whole drop
-// intent; every sortable-lists move is optimistic (the row has already been
-// reordered in the DOM), which the `optimistic` param signals to the server.
+// intent; the row has always already been reordered in the DOM, but the
+// server should only treat that reorder as final when `optimistic` is true —
+// truncated target lists opt out because the server must re-render the
+// visible window (which rows show, the truncation marker's metadata) rather
+// than trust the client's optimistic DOM order.
 export function buildMoveFormData({
   intent,
   positionMode,
+  optimistic,
 }:{
   intent:DropIntent;
   positionMode:SortablePositionMode;
+  optimistic:boolean;
 }):FormData {
   const data = new FormData();
-  data.append('optimistic', 'true');
+  if (optimistic) {
+    data.append('optimistic', 'true');
+  }
 
   if (positionMode === 'absolute') {
     data.append('target_id', intent.listData.listId ?? '');

frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.spec.ts

@@ -26,8 +26,11 @@
 // See COPYRIGHT and LICENSE files for more details.
 //++
 
+import { vi } from 'vitest';
+
 import {
   captureRowPositions,
+  hasTruncationMarkerRow,
   reorderRows,
   resolveItemElement,
   resolveItemPosition,
@@ -202,6 +205,36 @@ describe('sortable lists DOM helpers', () => {
       list.setAttribute('data-sortable-lists--list-rows-container-selector-value', ':scope > ul');
       expect(resolveRowsContainer(list)).toBe(list);
     });
+
+    it('falls back to the list element and warns when the selector value is invalid', () => {
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
+      const list = document.createElement('div');
+      list.setAttribute('data-sortable-lists--list-rows-container-selector-value', '[[');
+
+      expect(resolveRowsContainer(list)).toBe(list);
+      expect(warnSpy).toHaveBeenCalledTimes(1);
+      const message = warnSpy.mock.calls[0]?.[0] as string;
+      expect(message).toContain('data-sortable-lists--list-rows-container-selector-value');
+      expect(message).toContain('[[');
+
+      warnSpy.mockRestore();
+    });
+  });
+
+  describe('hasTruncationMarkerRow', () => {
+    it('returns false for a container with only item rows', () => {
+      const container = listElement();
+      container.append(itemRow('1'), itemRow('2'));
+
+      expect(hasTruncationMarkerRow(container)).toBe(false);
+    });
+
+    it('returns true when the container holds a truncation marker row', () => {
+      const container = listElement();
+      container.append(itemRow('1'), showMoreRow(), itemRow('2'));
+
+      expect(hasTruncationMarkerRow(container)).toBe(true);
+    });
   });
 
   describe('resolveRow', () => {

frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts

@@ -51,7 +51,21 @@ export function resolveRowsContainer(list:HTMLElement):HTMLElement {
     return list;
   }
 
-  return list.querySelector<HTMLElement>(selector) ?? list;
+  try {
+    return list.querySelector<HTMLElement>(selector) ?? list;
+  } catch (error) {
+    console.warn(`Invalid ${rowsContainerSelectorAttribute} selector "${selector}" on`, list, error);
+    return list;
+  }
+}
+
+// Whether a rows container holds a truncated list's "show more" marker row
+// (data-sortable-lists-prev-item-id on a non-item row). A truncated list's
+// visible window (which rows show, the marker's collapsed-item metadata) is
+// server-computed from the full ordering, so an optimistic client-side
+// reorder cannot be the final word there — the server must re-render it.
+export function hasTruncationMarkerRow(container:Element):boolean {
+  return container.querySelector(`[${sortablePreviousItemIdAttribute}]`) !== null;
 }
 
 // A row is the direct child of the rows container that contains the element.

modules/backlogs/spec/features/work_packages/drag_in_inbox_spec.rb

@@ -141,6 +141,46 @@
     end
   end
 
+  context "when the inbox is truncated" do
+    # tail_size = [TRUNCATE_MIDDLE / 5, 1].max = 1, so with TRUNCATE_MIDDLE
+    # stubbed to 2, the visible window is first(2) + last(1); the truncate
+    # threshold (TRUNCATE_MIDDLE + tail_size * 2 = 4) stays below the 5 work
+    # packages the outer example group already sets up, so the inbox truncates.
+    before do
+      stub_const("Backlogs::InboxComponent::TRUNCATE_MIDDLE", 2)
+    end
+
+    it "keeps the server-rendered truncation window consistent after a same-list drag" do
+      backlogs_page.visit!
+
+      backlogs_page.expect_work_packages_in_inbox_in_order(work_packages: [inbox_wp1, inbox_wp2])
+      backlogs_page.expect_inbox_item(inbox_wp5)
+      backlogs_page.expect_no_inbox_item(inbox_wp3)
+      backlogs_page.expect_no_inbox_item(inbox_wp4)
+      backlogs_page.expect_inbox_show_more
+      expect(backlogs_page.inbox_truncation_marker_previous_item_id).to eq(inbox_wp4.id.to_s)
+
+      # Same list, but the drop target (the inbox) is truncated: the server
+      # must recompute the visible window rather than trust the optimistic
+      # client-side reorder, so this move settles via a turbo-stream frame
+      # reload rather than the 204/sortable-lists:moved path a plain same-list
+      # drag takes. `cross_list: true` steers drag_work_package to wait on
+      # that frame reload (see sortable-lists.controller.ts's `optimistic`
+      # gate and Backlogs::InboxComponent's TRUNCATE_MIDDLE window).
+      backlogs_page
+        .drag_work_package(inbox_wp1, before: inbox_wp5, cross_list: true)
+
+      # inbox_wp1 moved behind inbox_wp4 (the item the marker names), pushing
+      # it out of the visible head and pulling inbox_wp3 into view instead.
+      backlogs_page.expect_work_packages_in_inbox_in_order(work_packages: [inbox_wp2, inbox_wp3])
+      backlogs_page.expect_inbox_item(inbox_wp5)
+      backlogs_page.expect_no_inbox_item(inbox_wp1)
+      backlogs_page.expect_no_inbox_item(inbox_wp4)
+      backlogs_page.expect_inbox_show_more
+      expect(backlogs_page.inbox_truncation_marker_previous_item_id).to eq(inbox_wp1.id.to_s)
+    end
+  end
+
   context "when lacking the permission to manage sprint items" do
     current_user do
       create(:user,

modules/backlogs/spec/support/pages/backlog.rb

@@ -167,6 +167,17 @@ def click_inbox_show_more
       wait_for_backlogs_network_idle
     end
 
+    # The truncation marker row's data-sortable-lists-prev-item-id names the
+    # last work package collapsed behind the "show more" row (the DOM contract
+    # sortable-lists relies on to anchor drops next to a hidden block). Reading
+    # it back lets specs assert the server recomputed the truncated window
+    # rather than trusting a stale client-side reorder.
+    def inbox_truncation_marker_previous_item_id
+      within_backlog_inbox do
+        find(".op-work-package-card-list--show-more-row", visible: :all)["data-sortable-lists-prev-item-id"]
+      end
+    end
+
     def expect_work_packages_in_inbox_in_order(work_packages: [])
       within_backlog_inbox do
         expect_work_packages_in_order work_packages: