diff --git a/frontend/src/stimulus/controllers/async-dialog.controller.spec.ts b/frontend/src/stimulus/controllers/async-dialog.controller.spec.ts
new file mode 100644
index 000000000000..6581308bc323
--- /dev/null
+++ b/frontend/src/stimulus/controllers/async-dialog.controller.spec.ts
@@ -0,0 +1,266 @@
+//-- 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.
+//++
+
+import type { FetchResponse } from '@rails/request.js';
+import { waitFor } from '@testing-library/dom';
+import { vi, type Mock } from 'vitest';
+
+import { setupStimulusTest, type StimulusTestContext } from 'core-stimulus/test-helpers';
+import { TurboHelpers } from 'core-turbo/helpers';
+import type AsyncDialogControllerType from './async-dialog.controller';
+
+const perform = vi.fn();
+const FetchRequest = vi.fn(function FetchRequestMock(
+ _method:string,
+ _url:string|URL,
+ _options:{ body?:FormData; responseKind:'turbo-stream' },
+) {
+ return { perform };
+});
+
+vi.doMock('@rails/request.js', () => ({ FetchRequest }));
+
+describe('Async dialog controller', () => {
+ let ctx:StimulusTestContext;
+ let AsyncDialogController:typeof AsyncDialogControllerType;
+ let showProgressBar:Mock;
+ let hideProgressBar:Mock;
+
+ beforeAll(async () => {
+ ({ default: AsyncDialogController } = await import('./async-dialog.controller'));
+ });
+
+ beforeEach(async () => {
+ FetchRequest.mockClear();
+ perform.mockReset();
+ showProgressBar = vi.spyOn(TurboHelpers, 'showProgressBar').mockImplementation(() => undefined);
+ hideProgressBar = vi.spyOn(TurboHelpers, 'hideProgressBar').mockImplementation(() => undefined);
+
+ ctx = await setupStimulusTest({
+ controllers: { 'async-dialog': AsyncDialogController },
+ });
+ });
+
+ afterEach(() => {
+ ctx.dispose();
+ vi.restoreAllMocks();
+ });
+
+ function turboStreamResponse({
+ ok = true,
+ unprocessableEntity = false,
+ }:{ ok?:boolean; unprocessableEntity?:boolean } = {}) {
+ const renderTurboStream = vi.fn().mockResolvedValue(undefined);
+ const response = {
+ ok,
+ unprocessableEntity,
+ isTurboStream: true,
+ renderTurboStream,
+ } as unknown as FetchResponse;
+ return { response, renderTurboStream };
+ }
+
+ function nonTurboResponse() {
+ const renderTurboStream = vi.fn().mockResolvedValue(undefined);
+ const response = {
+ ok: true,
+ unprocessableEntity: false,
+ isTurboStream: false,
+ renderTurboStream,
+ } as unknown as FetchResponse;
+ return { response, renderTurboStream };
+ }
+
+ function resolveLikeFetchRequest(response:FetchResponse) {
+ perform.mockImplementation(async () => {
+ if ((response.ok || response.unprocessableEntity) && response.isTurboStream) {
+ await response.renderTurboStream();
+ }
+
+ return response;
+ });
+ }
+
+ async function mountPostButton() {
+ await ctx.mount(`
+
+ `);
+
+ const form = ctx.container.querySelector('#dialog-form')!;
+ const button = ctx.container.querySelector('button')!;
+ Object.defineProperty(form, 'action', { configurable: true, value: '/dialog' });
+ return { button, form };
+ }
+
+ function trigger(controller:AsyncDialogControllerType, url?:string) {
+ return (controller as unknown as { triggerTurboStream(url?:string):Promise }).triggerTurboStream(url);
+ }
+
+ it('dispatches beforeLoad and submits form data captured after synchronous listeners run', async () => {
+ const { response } = turboStreamResponse();
+ resolveLikeFetchRequest(response);
+ const { button, form } = await mountPostButton();
+ let beforeLoad!:CustomEvent<{ form:HTMLFormElement|null }>;
+ button.addEventListener('async-dialog:beforeLoad', (event) => {
+ beforeLoad = event as CustomEvent<{ form:HTMLFormElement|null }>;
+ const input = document.createElement('input');
+ input.name = 'ids[]';
+ input.value = '7';
+ form.appendChild(input);
+ });
+
+ button.click();
+
+ await waitFor(() => expect(FetchRequest).toHaveBeenCalledOnce());
+ expect(beforeLoad.defaultPrevented).toBe(false);
+ expect(beforeLoad.detail.form).toBe(form);
+ const [, , options] = FetchRequest.mock.calls[0];
+ const submitted = options.body;
+ expect(submitted).toBeInstanceOf(FormData);
+ expect(FetchRequest).toHaveBeenCalledWith('post', '/dialog', {
+ body: submitted,
+ responseKind: 'turbo-stream',
+ });
+ if (!(submitted instanceof FormData)) throw new Error('Expected form data');
+ expect([...submitted.entries()]).toEqual([['authenticity_token', 'token'], ['ids[]', '7']]);
+ });
+
+ it('makes no request when beforeLoad is canceled', async () => {
+ const { button } = await mountPostButton();
+ button.addEventListener('async-dialog:beforeLoad', (event) => event.preventDefault());
+
+ button.click();
+ await ctx.nextFrame();
+
+ expect(FetchRequest).not.toHaveBeenCalled();
+ expect(showProgressBar).not.toHaveBeenCalled();
+ });
+
+ it('keeps existing anchors on GET with their href', async () => {
+ const { response } = turboStreamResponse();
+ resolveLikeFetchRequest(response);
+ await ctx.mount('Open');
+ const anchor = ctx.container.querySelector('a')!;
+
+ anchor.click();
+
+ await waitFor(() => expect(FetchRequest).toHaveBeenCalledOnce());
+ expect(FetchRequest).toHaveBeenCalledWith('GET', anchor.href, {
+ body: undefined,
+ responseKind: 'turbo-stream',
+ });
+ });
+
+ it('ignores repeated activation while a request is loading', async () => {
+ let resolveRequest!:(response:FetchResponse) => void;
+ perform.mockReturnValue(new Promise((resolve) => { resolveRequest = resolve; }));
+ const { button } = await mountPostButton();
+
+ button.click();
+ button.click();
+
+ expect(FetchRequest).toHaveBeenCalledOnce();
+ resolveRequest(turboStreamResponse().response);
+ await waitFor(() => expect(button).not.toHaveAttribute('aria-disabled'));
+ });
+
+ it.each([
+ ['a successful response', turboStreamResponse().response],
+ ['an unprocessable response', turboStreamResponse({ ok: false, unprocessableEntity: true }).response],
+ ])('clears loading state after %s', async (_label, response) => {
+ resolveLikeFetchRequest(response);
+ const { button } = await mountPostButton();
+
+ button.click();
+ expect(button).toHaveAttribute('aria-disabled', 'true');
+
+ await waitFor(() => expect(button).not.toHaveAttribute('aria-disabled'));
+ expect(showProgressBar).toHaveBeenCalledOnce();
+ expect(hideProgressBar).toHaveBeenCalledOnce();
+ });
+
+ it('renders a 500 Turbo Stream exactly once and clears loading state', async () => {
+ const { response, renderTurboStream } = turboStreamResponse({ ok: false });
+ resolveLikeFetchRequest(response);
+ const { button } = await mountPostButton();
+
+ button.click();
+
+ await waitFor(() => expect(renderTurboStream).toHaveBeenCalledOnce());
+ expect(button).not.toHaveAttribute('aria-disabled');
+ expect(hideProgressBar).toHaveBeenCalledOnce();
+ });
+
+ it('rejects a 200 non-Turbo response without rendering and clears loading state', async () => {
+ const { response, renderTurboStream } = nonTurboResponse();
+ resolveLikeFetchRequest(response);
+ const { button } = await mountPostButton();
+ const controller = ctx.getController('async-dialog', button);
+
+ await expect(trigger(controller)).rejects.toThrow('Response is not a Turbo Stream');
+
+ expect(renderTurboStream).not.toHaveBeenCalled();
+ expect(button).not.toHaveAttribute('aria-disabled');
+ expect(hideProgressBar).toHaveBeenCalledOnce();
+ });
+
+ it('clears loading state when the request rejects', async () => {
+ const error = new Error('network failed');
+ perform.mockRejectedValue(error);
+ const { button } = await mountPostButton();
+ const controller = ctx.getController('async-dialog', button);
+
+ await expect(trigger(controller)).rejects.toBe(error);
+
+ expect(button).not.toHaveAttribute('aria-disabled');
+ expect(hideProgressBar).toHaveBeenCalledOnce();
+ });
+
+ it('uses a custom event URL without including the associated form', async () => {
+ const { response } = turboStreamResponse();
+ resolveLikeFetchRequest(response);
+ const { button } = await mountPostButton();
+ const controller = ctx.getController('async-dialog', button);
+ let associatedForm:HTMLFormElement|null|undefined;
+ button.addEventListener('async-dialog:beforeLoad', (event) => {
+ associatedForm = (event as CustomEvent<{ form:HTMLFormElement|null }>).detail.form;
+ });
+
+ controller.handleOpenDialog(new CustomEvent('open', { detail: { url: '/override' } }));
+
+ await waitFor(() => expect(FetchRequest).toHaveBeenCalledOnce());
+ expect(associatedForm).toBeNull();
+ expect(FetchRequest).toHaveBeenCalledWith('GET', '/override', {
+ body: undefined,
+ responseKind: 'turbo-stream',
+ });
+ });
+});
diff --git a/frontend/src/stimulus/controllers/async-dialog.controller.ts b/frontend/src/stimulus/controllers/async-dialog.controller.ts
index 1896be76f699..32e70be9f66a 100644
--- a/frontend/src/stimulus/controllers/async-dialog.controller.ts
+++ b/frontend/src/stimulus/controllers/async-dialog.controller.ts
@@ -26,82 +26,89 @@
// See COPYRIGHT and LICENSE files for more details.
//++
-import { ApplicationController } from 'stimulus-use';
-import { renderStreamMessage } from '@hotwired/turbo';
-import { TurboHelpers } from 'core-turbo/helpers';
+import { Controller } from '@hotwired/stimulus';
+import { FetchRequest } from '@rails/request.js';
+import { performTurboStreamRequest, withProgressBar } from 'core-stimulus/helpers/request-helpers';
-export default class AsyncDialogController extends ApplicationController {
+export default class AsyncDialogController extends Controller {
static values = { disableDuringLoad: { type: Boolean, default: true } };
declare disableDuringLoadValue:boolean;
private loading = false;
- connect() {
- // Only bind events if we have an href to work with
- if (this.href) {
- this.bindEventListeners();
- }
- }
+ private readonly handleClick = (event:Event):void => {
+ event.preventDefault();
+ void this.triggerTurboStream();
+ };
- private bindEventListeners() {
- this.element.addEventListener('click', (event:MouseEvent) => {
+ private readonly handleKeydown = (event:Event):void => {
+ const keyboardEvent = event as KeyboardEvent;
+ if (keyboardEvent.key === 'Enter' || keyboardEvent.key === ' ') {
event.preventDefault();
- this.triggerTurboStream(this.href);
- });
+ void this.triggerTurboStream();
+ }
+ };
- this.element.addEventListener('keydown', (event:KeyboardEvent) => {
- if (event.key === 'Enter' || event.key === ' ') {
- event.preventDefault();
- this.triggerTurboStream(this.href);
- }
- });
+ connect():void {
+ if (this.href || (this.element instanceof HTMLButtonElement && this.element.form)) {
+ this.element.addEventListener('click', this.handleClick);
+ this.element.addEventListener('keydown', this.handleKeydown);
+ }
}
- private triggerTurboStream(url:string):void {
+ disconnect():void {
+ this.element.removeEventListener('click', this.handleClick);
+ this.element.removeEventListener('keydown', this.handleKeydown);
+ }
+
+ private async triggerTurboStream(urlOverride?:string):Promise {
if (this.disableDuringLoadValue && this.loading) return;
- if (this.disableDuringLoadValue) {
- this.loading = true;
- (this.element as HTMLElement).setAttribute('aria-disabled', 'true');
- }
- TurboHelpers.showProgressBar();
-
- void fetch(url, {
- method: this.method,
- headers: {
- Accept: 'text/vnd.turbo-stream.html',
- },
- }).then((response) => {
- const contentType = response.headers.get('Content-Type') ?? '';
- const isTurboStream = contentType.includes('text/vnd.turbo-stream.html');
-
- if (!isTurboStream) {
- return Promise.reject(new Error('Response is not a Turbo Stream'));
- }
+ const form = urlOverride
+ ? null
+ : this.element instanceof HTMLButtonElement ? this.element.form : null;
+ const event = this.dispatch('beforeLoad', { cancelable: true, detail: { form } });
+ if (event.defaultPrevented) return;
- return response.text();
- }).then((html) => {
- renderStreamMessage(html);
- }).finally(() => {
- if (this.disableDuringLoadValue) {
- this.loading = false;
- (this.element as HTMLElement).removeAttribute('aria-disabled');
- }
- TurboHelpers.hideProgressBar();
- });
+ const method = form?.method ?? this.method;
+ const url = urlOverride ?? form?.action ?? this.href;
+ const body = form ? new FormData(form) : undefined;
+
+ this.setLoading(true);
+
+ try {
+ await withProgressBar(performTurboStreamRequest(new FetchRequest(method, url, {
+ body,
+ responseKind: 'turbo-stream',
+ })));
+ } finally {
+ this.setLoading(false);
+ }
}
handleOpenDialog(event:CustomEvent<{ url:string }>):void {
// Trigger the dialog with custom URL
- this.triggerTurboStream(event.detail.url);
+ void this.triggerTurboStream(event.detail.url);
}
- get href() {
+ get href():string {
return (this.element as HTMLLinkElement).href;
}
- get method() {
+ get method():string {
return (this.element as HTMLLinkElement).dataset.turboMethod ?? 'GET';
}
+
+ private setLoading(loading:boolean):void {
+ this.loading = loading;
+
+ if (this.disableDuringLoadValue) {
+ if (loading) {
+ this.element.setAttribute('aria-disabled', 'true');
+ } else {
+ this.element.removeAttribute('aria-disabled');
+ }
+ }
+ }
}
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 f384215c5bed..37a9af707852 100644
--- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.spec.ts
@@ -58,6 +58,7 @@ import { attachClosestEdge } from '@atlaskit/pragmatic-drag-and-drop-hitbox/clos
import type { monitorForElements as monitorForElementsFn } from '@atlaskit/pragmatic-drag-and-drop/element/adapter';
import { waitFor } from '@testing-library/dom';
import { type Mock, type MockInstance } from 'vitest';
+import { TurboHelpers } from 'core-turbo/helpers';
import { LiveRegionElement } from '@primer/live-region-element';
import { setupStimulusTest, type StimulusTestContext } from 'core-stimulus/test-helpers';
import type SortableListsControllerType from './sortable-lists.controller';
@@ -1240,6 +1241,248 @@ describe('Sortable lists controller', () => {
expect(controller.moveAvailability(document.createElement('li'))).toBeNull();
});
+ it('resolves the owning list element of an item for the drag payload', async () => {
+ const { root, sourceList, firstSourceItem } = renderFixture();
+ await ctx.nextFrame();
+ const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType;
+
+ expect(controller.ownerListElementOf(firstSourceItem)).toBe(sourceList);
+ expect(controller.ownerListElementOf(document.createElement('li'))).toBeNull();
+ });
+
+ it('exposes batch action scopes and removes destinations occupied by every member', async () => {
+ const { root, items } = renderSelectableRoot();
+ await ctx.nextFrame();
+ const controller = ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType;
+
+ items[0].dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true }));
+ items[1].dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true }));
+ const scope = controller.actionScopeFor(items[1]);
+
+ expect(scope).toMatchObject({ kind: 'batch', items: [items[0], items[1]]});
+ expect(controller.availableDestinations(scope, [
+ { type: 'backlog_bucket', id: '1' },
+ { type: 'sprint', id: '1' },
+ ])).toEqual([{ type: 'sprint', id: '1' }]);
+ });
+
+ describe('direct destination moves', () => {
+ const destination = { type: 'inbox', id: null };
+
+ function destinationController(root:HTMLElement) {
+ return ctx.application.getControllerForElementAndIdentifier(root, 'sortable-lists') as SortableListsControllerType;
+ }
+
+ function selectItem(element:HTMLElement, init:MouseEventInit = {}):void {
+ element.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, ...init }));
+ }
+
+ function destinationSelected(element:HTMLElement):boolean {
+ return element.hasAttribute('data-batch-selected');
+ }
+
+ it.each([200, 422, 500])('cleans up destination progress after HTTP %s', async (status) => {
+ const show = vi.spyOn(TurboHelpers, 'showProgressBar').mockImplementation(() => undefined);
+ const hide = vi.spyOn(TurboHelpers, 'hideProgressBar').mockImplementation(() => undefined);
+ const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+ let finish!:(response:Response) => void;
+ fetchMock.mockReturnValueOnce(new Promise((resolve) => { finish = resolve; }));
+ selectItem(items[0]);
+ destinationController(root).moveToDestination(items[0], destination);
+ expect(show).toHaveBeenCalledOnce();
+ expect(hide).not.toHaveBeenCalled();
+ await vi.waitFor(() => expect(fetchMock).toHaveBeenCalled());
+ finish(new Response('', { status, headers: { 'Content-Type': 'text/vnd.turbo-stream.html' } }));
+ await vi.waitFor(() => expect(hide).toHaveBeenCalledOnce());
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false);
+ });
+
+ it('submits the selected scope in document order without optimistic or positional fields', async () => {
+ fetchMock.mockResolvedValueOnce(new Response('', {
+ headers: { 'Content-Type': 'text/vnd.turbo-stream.html' },
+ status: 422,
+ }));
+ const { root, sourceList, items } = renderSelectableRoot({
+ optimistic: true,
+ collectionMoveUrl: '/projects/demo/backlogs/work_packages/move',
+ });
+ sourceList.insertBefore(items[1], items[0]);
+ await ctx.nextFrame();
+ selectItem(items[0]);
+ selectItem(items[1], { ctrlKey: true });
+
+ destinationController(root).moveToDestination(items[0], destination);
+ await flushPromises();
+
+ const [requestUrl, requestOptions] = fetchMock.mock.lastCall as [string, { body:FormData }];
+ expect(requestUrl).toBe('/projects/demo/backlogs/work_packages/move');
+ expect([...requestOptions.body.entries()]).toEqual([
+ ['ids[]', '2'],
+ ['ids[]', '1'],
+ ['list_type', 'inbox'],
+ ['list_id', ''],
+ ]);
+ expect(requestOptions.body.has('prev_id')).toBe(false);
+ expect(requestOptions.body.has('optimistic')).toBe(false);
+ expect(items.filter(destinationSelected)).toEqual([items[0], items[1]]);
+ await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false));
+ expect(renderStreamMessageMock).toHaveBeenCalledOnce();
+ });
+
+ it('replaces an unrelated selection when an unselected card invokes the action', async () => {
+ fetchMock.mockResolvedValueOnce(new Response('', { status: 422 }));
+ const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+ selectItem(items[1]);
+ selectItem(items[2], { ctrlKey: true });
+
+ destinationController(root).moveToDestination(items[0], destination);
+ await flushPromises();
+
+ const requestOptions = fetchMock.mock.lastCall?.[1] as { body:FormData };
+ expect(requestOptions.body.getAll('ids[]')).toEqual(['1']);
+ expect(items.filter(destinationSelected)).toEqual([items[0]]);
+ });
+
+ it('does not reorder or clear selection before a successful frame stream reconciles the root', async () => {
+ fetchMock.mockResolvedValueOnce(new Response('', {
+ headers: { 'Content-Type': 'text/vnd.turbo-stream.html' },
+ status: 200,
+ }));
+ const { root, sourceList, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+ selectItem(items[0]);
+ selectItem(items[1], { ctrlKey: true });
+
+ destinationController(root).moveToDestination(items[0], destination);
+ await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false));
+
+ expect(itemIds(sourceList)).toEqual(['1', '2', '3']);
+ expect(items.filter(destinationSelected)).toEqual([items[0], items[1]]);
+ expect(renderStreamMessageMock).toHaveBeenCalledOnce();
+ });
+
+ it('does not mutate selection or submit while another move is busy', async () => {
+ const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+ selectItem(items[1]);
+ root.setAttribute('data-sortable-lists-busy', 'true');
+ const controller = destinationController(root);
+
+ const prospectiveScope = controller.selectForAction(items[0]);
+ controller.moveToDestination(items[0], destination);
+ await flushPromises();
+
+ expect(prospectiveScope).toMatchObject({ kind: 'batch'});
+ expect(items.filter(destinationSelected)).toEqual([items[1]]);
+ expect(fetchMock).not.toHaveBeenCalled();
+ });
+
+ it('clears a detached request busy marker when the cached root reconnects', async () => {
+ let resolveRequest!:(response:Response) => void;
+ fetchMock.mockImplementationOnce(() => new Promise((resolve) => {
+ resolveRequest = resolve;
+ }));
+ const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+
+ destinationController(root).moveToDestination(items[0], destination);
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(true);
+
+ root.remove();
+ await ctx.nextFrame();
+ resolveRequest(new Response('', { status: 200 }));
+ await flushPromises();
+
+ // The settled request deliberately leaves a detached element alone.
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(true);
+
+ fixture.append(root);
+ await ctx.nextFrame();
+ await ctx.nextFrame();
+
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false);
+
+ fetchMock.mockResolvedValueOnce(new Response('', {
+ headers: { 'Content-Type': 'text/vnd.turbo-stream.html' },
+ status: 422,
+ }));
+ destinationController(root).moveToDestination(items[0], destination);
+
+ await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2));
+ await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false));
+ });
+
+ // A destination move whose request never produces a stream (a network
+ // failure, or an error page) has no server flash to speak for it, so the
+ // client has to say something rather than just clearing the busy state.
+ it('dispatches an error toast when the destination request never streams', async () => {
+ const toastEvents:CustomEvent[] = [];
+ const onToast = (event:Event) => toastEvents.push(event as CustomEvent);
+ window.addEventListener('op:toasters:add', onToast);
+ fetchMock.mockRejectedValueOnce(new Error('network down'));
+ const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+
+ destinationController(root).moveToDestination(items[0], destination);
+ await flushPromises();
+
+ expect(toastEvents).toHaveLength(1);
+ expect(toastEvents[0].detail.type).toBe('error');
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false);
+
+ window.removeEventListener('op:toasters:add', onToast);
+ });
+
+ it('keeps a reconnected root busy until its detached request settles', async () => {
+ let resolveRequest!:(response:Response) => void;
+ fetchMock.mockImplementationOnce(() => new Promise((resolve) => {
+ resolveRequest = resolve;
+ }));
+ const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+
+ destinationController(root).moveToDestination(items[0], destination);
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(true);
+
+ root.remove();
+ await ctx.nextFrame();
+ fixture.append(root);
+ await ctx.nextFrame();
+ await ctx.nextFrame();
+
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(true);
+
+ destinationController(root).moveToDestination(items[1], destination);
+ await flushPromises();
+ expect(fetchMock).toHaveBeenCalledOnce();
+
+ resolveRequest(new Response('', { status: 200 }));
+ await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false));
+ });
+
+ it.each([
+ ['a 500 Turbo Stream response', () => Promise.resolve(new Response('', {
+ headers: { 'Content-Type': 'text/vnd.turbo-stream.html' },
+ status: 500,
+ })), 1],
+ ['a successful non-stream response', () => Promise.resolve(new Response('', { status: 200 })), 0],
+ ['a rejected request', () => Promise.reject(new Error('Network failure')), 0],
+ ])('clears busy state after %s', async (_description, request, streamRenderCount) => {
+ fetchMock.mockImplementationOnce(request);
+ const { root, items } = renderSelectableRoot({ collectionMoveUrl: '/batch/move' });
+ await ctx.nextFrame();
+
+ destinationController(root).moveToDestination(items[0], destination);
+ expect(root.hasAttribute('data-sortable-lists-busy')).toBe(true);
+ await flushPromises();
+
+ await waitFor(() => expect(root.hasAttribute('data-sortable-lists-busy')).toBe(false));
+ expect(renderStreamMessageMock).toHaveBeenCalledTimes(streamRenderCount);
+ });
+ });
+
describe('nested list topology', () => {
it('resolves the source row of a nested item against its innermost list', async () => {
const { fieldList, firstFieldItem } = renderNestedFixture();
@@ -1437,6 +1680,21 @@ describe('Sortable lists controller', () => {
expect(row.hasAttribute('data-batch-selected')).toBe(false);
});
+ it('clears the live selection when a consumer reports a completed move', async () => {
+ const { root, items } = renderSelectableRoot();
+ root.setAttribute(
+ 'data-action',
+ 'sortable-lists:test-move-completed@document->sortable-lists#clearSelectionAfterMove',
+ );
+ await ctx.nextFrame();
+ click(items[0]);
+ click(items[1], { ctrlKey: true });
+
+ document.dispatchEvent(new CustomEvent('sortable-lists:test-move-completed'));
+
+ expect(items.some(isSelected)).toBe(false);
+ });
+
it('selects only the clicked card on a plain click', async () => {
const { items } = renderSelectableRoot();
await ctx.nextFrame();
@@ -1987,17 +2245,17 @@ describe('Sortable lists controller', () => {
expect(items.some(isSelected)).toBe(false);
});
- // Escape clears local state only, so the busy gate must not swallow it.
- it('clears the batch on Escape even during a busy move', async () => {
+ it('preserves the batch and consumes Escape during a busy move', async () => {
const { root, items } = renderSelectableRoot();
await ctx.nextFrame();
click(items[0]);
items[0].focus();
root.setAttribute('data-sortable-lists-busy', 'true');
- keydown(items[0], 'Escape');
+ const event = keydown(items[0], 'Escape');
- expect(items.some(isSelected)).toBe(false);
+ expect(items.filter(isSelected)).toEqual([items[0]]);
+ expect(event.defaultPrevented).toBe(true);
});
// An unconsumed Escape still reaches dialogs and menus.
diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts
index bc24e635be14..f962c25b3808 100644
--- a/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists.controller.ts
@@ -36,6 +36,7 @@ import { announce } from '@primer/live-region-element';
import { debugLog } from 'core-app/shared/helpers/debug_output';
import { OPToastEvent } from 'core-app/shared/components/toaster/toast-event';
import { flipMove } from 'core-stimulus/helpers/flip-helper';
+import { performTurboStreamRequest, withProgressBar } from 'core-stimulus/helpers/request-helpers';
import { parseTemplate } from 'url-template';
import {
buildMoveFormData,
@@ -62,12 +63,15 @@ import {
restoreRowPositions,
rowOf,
rowsRemainAt,
+ permittedDestinations,
+ sameDestination,
sortableListsBusyAttribute,
type DestinationIdentity,
type MoveAvailability,
type MoveDirection,
} from './sortable-lists/list-dom';
import { SelectionOrchestrator, type SelectionHost } from './sortable-lists/selection-orchestrator';
+import { scopeIds, type ActionScope } from './sortable-lists/action-scope';
import { itemIdentity, orderedItemElements } from './sortable-lists/selection';
type CleanupFn = () => void;
@@ -120,8 +124,13 @@ export default class SortableListsController extends Controller imp
private monitorCleanupFn?:CleanupFn;
private healScheduled = false;
private reconcileScheduled = false;
+ private inFlightMoveRequests = 0;
connect():void {
+ // Busy belongs to in-flight controller work, not to cached DOM markup.
+ // Reconnecting before settlement keeps the root blocked; reconnecting a
+ // stale cached root after settlement clears the marker.
+ this.syncBusyState();
this.monitorCleanupFn = monitorForElements({
canMonitor: ({ source }) => !this.busy
&& isItemFromRoot(this.element, source.data),
@@ -235,6 +244,40 @@ export default class SortableListsController extends Controller imp
}
}
+ actionScopeFor(itemElement:HTMLElement):ActionScope {
+ return this.selection?.actionScopeFor(itemElement) ?? { kind: 'refused', items: [] };
+ }
+
+ selectForAction(itemElement:HTMLElement):ActionScope {
+ if (this.busy) {
+ return this.actionScopeFor(itemElement);
+ }
+
+ return this.selection?.selectForAction(itemElement) ?? { kind: 'refused', items: [] };
+ }
+
+ // Consumer-owned non-optimistic forms do not call performMove, so their
+ // successful move event is the shared boundary at which the live batch is
+ // cleared. Failed requests emit no completion event and keep the selection.
+ clearSelectionAfterMove():void {
+ this.selection?.clearSilently();
+ }
+
+ // Where the batch may move: the candidates every member accepts, minus the
+ // one they all already occupy, which would be a move to nowhere.
+ availableDestinations(scope:ActionScope, candidates:DestinationIdentity[]):DestinationIdentity[] {
+ if (scope.kind === 'refused') {
+ return [];
+ }
+
+ const ownerDestinationOf = (item:HTMLElement) => this.ownerDestinationOf(item);
+ const permitted = permittedDestinations({ items: scope.items, candidates, ownerDestinationOf });
+
+ return permitted.filter((target) => (
+ !scope.items.every((item) => sameDestination(ownerDestinationOf(item), target))
+ ));
+ }
+
// Frozen at drag start and consumed exactly once per drop, cancelled ones
// included: neither Escape nor a mid-drag morph can change what is
// submitted, and no stale batch leaks into the next drag.
@@ -465,6 +508,29 @@ export default class SortableListsController extends Controller imp
return list ? resolveMoveAvailability({ itemElement, rowsContainer: list.rowsContainer }) : null;
}
+ moveToDestination(itemElement:HTMLElement, target:DestinationIdentity):void {
+ if (this.busy) {
+ return;
+ }
+
+ const moveUrl = this.resolveCollectionMoveUrl(false);
+ if (!moveUrl) {
+ return;
+ }
+
+ const scope = this.selectForAction(itemElement);
+ if (scope.kind === 'refused') {
+ return;
+ }
+
+ const body = new FormData();
+ scopeIds(scope).forEach((id) => body.append('ids[]', id));
+ body.append('list_type', target.type);
+ body.append('list_id', target.id ?? '');
+
+ void this.submitDestinationMove(moveUrl, body);
+ }
+
moveInDirection(itemElement:HTMLElement, direction:MoveDirection):void {
// The menu is rendered server-side from a permission check that does not
// know about per-work-package movability, so a stale or over-permissive
@@ -516,6 +582,10 @@ export default class SortableListsController extends Controller imp
return containing.find((list) => !containing.some((other) => other !== list && list.element.contains(other.element))) ?? null;
}
+ ownerListElementOf(itemElement:HTMLElement):HTMLElement|null {
+ return this.ownerListOf(itemElement)?.element ?? null;
+ }
+
ownerRowsContainer(itemElement:HTMLElement):HTMLElement|null {
return this.ownerListOf(itemElement)?.rowsContainer ?? null;
}
@@ -611,20 +681,46 @@ export default class SortableListsController extends Controller imp
return this.hasCollectionMoveUrlValue && this.collectionMoveUrlValue !== '' ? this.collectionMoveUrlValue : null;
}
- private resolveCollectionMoveUrl():string|null {
+ private resolveCollectionMoveUrl(optimistic = this.optimisticValue):string|null {
const collectionMoveUrl = this.collectionMoveUrl;
if (!collectionMoveUrl) {
return null;
}
const url = new URL(collectionMoveUrl, window.location.href);
- if (this.optimisticValue) {
+ if (optimistic) {
url.searchParams.set('optimistic', 'true');
+ } else {
+ url.searchParams.delete('optimistic');
}
return relativeUrl(url);
}
+ private async submitDestinationMove(moveUrl:string, body:FormData):Promise {
+ const request = new FetchRequest(
+ 'put',
+ moveUrl,
+ {
+ body,
+ responseKind: 'turbo-stream',
+ },
+ );
+
+ this.startMoveRequest();
+ try {
+ await withProgressBar(performTurboStreamRequest(request));
+ } catch (error) {
+ // Only a request that never produced a stream lands here — a rejection
+ // streams its own flash. Without the toast the busy state would simply
+ // clear and the batch would look moved.
+ debugLog('Failed to move sortable list items to destination', error);
+ this.dispatchErrorToast();
+ } finally {
+ this.finishMoveRequest();
+ }
+ }
+
// Refused whole when a row is missing: a member that vanished mid-drag
// means a partial block would diverge from the ids the request claims.
private rowsForItems(items:SelectionItem[]):HTMLElement[]|null {
@@ -779,7 +875,7 @@ export default class SortableListsController extends Controller imp
},
);
- this.setBusy(true);
+ this.startMoveRequest();
try {
const response = await request.perform();
@@ -794,7 +890,26 @@ export default class SortableListsController extends Controller imp
debugLog('Failed to move sortable list item due to request error', error);
return { ok: false, showToast: true };
} finally {
- this.setBusy(false);
+ this.finishMoveRequest();
+ }
+ }
+
+ private startMoveRequest():void {
+ this.inFlightMoveRequests += 1;
+ this.syncBusyState();
+ }
+
+ private finishMoveRequest():void {
+ this.inFlightMoveRequests = Math.max(0, this.inFlightMoveRequests - 1);
+ this.syncBusyState();
+ }
+
+ private syncBusyState():void {
+ // A successful frame stream may already have replaced this root. Avoid
+ // mutating detached cached DOM; connect() will project the current count
+ // if this element is restored later.
+ if (this.element.isConnected) {
+ this.setBusy(this.inFlightMoveRequests > 0);
}
}
diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/action-scope.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/action-scope.ts
new file mode 100644
index 000000000000..3aed62c63a6b
--- /dev/null
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/action-scope.ts
@@ -0,0 +1,42 @@
+//-- 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.
+//++
+
+import { resolveItemId } from './list-dom';
+
+export type ActionScope =
+ | { kind:'batch'; items:HTMLElement[] }
+ | { kind:'refused'; items:[] };
+
+// The ids a scope's members carry, in the scope's own order. Derived on
+// demand rather than frozen into the scope: the elements are its identity,
+// and a stale id list would outlive a morph that replaced a row.
+export function scopeIds(scope:ActionScope):string[] {
+ return scope.items
+ .map((item) => resolveItemId(item))
+ .filter((id):id is string => id !== null);
+}
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 600f4d84b726..fd2dd41121d0 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
@@ -55,6 +55,7 @@ import {
type MoveAvailability,
type MoveDirection,
} from './list-dom';
+import type { ActionScope } from './action-scope';
// The Pragmatic DnD payloads exchanged between the sortable-lists root and
// item controllers, built on top of the DOM contract in list-dom.ts.
@@ -102,9 +103,16 @@ export interface SortableListData extends Record {
export interface SortableListsRoot {
readonly element:HTMLElement;
readonly busy:boolean;
+ actionScopeFor(itemElement:HTMLElement):ActionScope;
+ selectForAction(itemElement:HTMLElement):ActionScope;
+ 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.
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.
+ ownerListElementOf(itemElement:HTMLElement):HTMLElement|null;
// The rows container of the item's innermost owning list, or null when the
// item is not (yet) inside a list the root knows about.
ownerRowsContainer(itemElement:HTMLElement):HTMLElement|null;
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 1fa208a97d2d..acfbcd137442 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
@@ -58,6 +58,7 @@ import type { ActionEvent } from '@hotwired/stimulus';
import type ItemControllerType from './item.controller';
import type { SortableListsRoot } from './drag-and-drop';
import type { DestinationIdentity } from './list-dom';
+import type { ActionScope } from './action-scope';
describe('Sortable lists item controller', () => {
let draggable:typeof draggableFn;
@@ -104,8 +105,13 @@ describe('Sortable lists item controller', () => {
return {
element,
busy,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
moveAvailability: vi.fn(() => null),
+ ownerListElementOf: vi.fn(() => null),
ownerRowsContainer: vi.fn(ownerRowsContainer),
freezeDragBatch: vi.fn(() => 1),
markDragBatch: vi.fn(),
@@ -1135,8 +1141,13 @@ describe('Sortable lists item controller', () => {
controller.connectRoot({
element: row,
busy: false,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
moveAvailability: vi.fn(() => null),
+ ownerListElementOf: vi.fn(() => null),
ownerRowsContainer: vi.fn(() => null),
freezeDragBatch: vi.fn(() => 3),
markDragBatch: vi.fn(),
@@ -1228,8 +1239,13 @@ describe('Sortable lists item controller', () => {
controller.connectRoot({
element: row,
busy: false,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
moveAvailability: vi.fn(() => null),
+ ownerListElementOf: vi.fn(() => null),
ownerRowsContainer: vi.fn(() => null),
freezeDragBatch: vi.fn(() => 3),
markDragBatch: vi.fn(),
@@ -1363,13 +1379,13 @@ describe('Sortable lists item controller', () => {
const menuElement = document.createElement('action-menu');
// The divider opens the move group, so everything below it is what
// decides whether it still separates anything.
- menuElement.innerHTML = (withDivider ? '' : '')
- + ['top', 'up', 'down', 'bottom'].map((direction) => (
- `'
- )).join('');
+ menuElement.innerHTML = withDivider ? '' : '';
const parent = document.createElement('li');
parent.setAttribute('data-sortable-lists--item-target', 'moveMenu');
+ parent.innerHTML = ['top', 'up', 'down', 'bottom'].map((direction) => (
+ `'
+ )).join('');
menuElement.appendChild(parent);
el.appendChild(menuElement);
@@ -1384,11 +1400,24 @@ describe('Sortable lists item controller', () => {
isItemHidden: vi.fn((li:Element|null) => !!li?.hasAttribute('hidden')),
};
Object.assign(menuElement, menu);
+ const popover = document.createElement('anchored-position');
+ menuElement.append(popover);
+ Object.defineProperty(menuElement, 'popoverElement', { value: popover });
return { el, menu };
}
const liFor = (el:HTMLElement, direction:string) => el.querySelector(`li[data-sortable-lists--item-direction-param="${direction}"]`)!;
+ const destinationWithMetadata = (el:HTMLElement, metadata:string) => {
+ const item = document.createElement('li');
+ item.setAttribute('data-sortable-lists--item-target', 'destinationItem');
+ item.dataset.sortableListsDestinations = metadata;
+ el.querySelector('action-menu')!.append(item);
+ return item;
+ };
+ const destinationFor = (el:HTMLElement, candidates:{ type:string; id:string|null }[]) => (
+ destinationWithMetadata(el, JSON.stringify(candidates))
+ );
// Availability defaults to the first/last extremes so the position-driven
// specs read naturally; individual tests can override the map to exercise
// the marker-aware (truncated list) wiring.
@@ -1404,15 +1433,73 @@ describe('Sortable lists item controller', () => {
moveInDirection = vi.fn(),
availability = availabilityFromPosition(position),
) => ({
- element: el, busy: false, moveAvailability: () => availability, moveInDirection,
+ element: el,
+ busy: false,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
+ moveAvailability: () => availability,
+ moveInDirection,
} as unknown as SortableListsRoot);
+ function stubMenuRoot(el:HTMLElement, position:{ isFirst:boolean; isLast:boolean }) {
+ const actionScopeFor = vi.fn(():ActionScope => ({ kind: 'refused', items: [] }));
+ const availableDestinations = vi.fn((_scope:ActionScope, _candidates:DestinationIdentity[]):DestinationIdentity[] => []);
+ const root = { ...stubRoot(el, position), actionScopeFor, availableDestinations };
+
+ return { root, actionScopeFor, availableDestinations };
+ }
+
+ it('ignores opening tooltips and submenu overlays', async () => {
+ const { el } = renderItemWithMenu(1);
+ const controller = await mountItemController(el);
+ const { root, actionScopeFor } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ controller.connectRoot(root);
+ const tooltip = document.createElement('tool-tip');
+ const submenu = document.createElement('anchored-position');
+ el.append(tooltip, submenu);
+ tooltip.dispatchEvent(new ToggleEvent('toggle', { newState: 'open' }));
+ submenu.dispatchEvent(new ToggleEvent('toggle', { newState: 'open' }));
+ expect(actionScopeFor).not.toHaveBeenCalled();
+ });
+
+ it('projects a deferred fragment once after its menu has already opened', async () => {
+ const { el } = renderItemWithMenu(1);
+ const parent = el.querySelector('[data-sortable-lists--item-target="moveMenu"]')!;
+ parent.remove();
+ const controller = await mountItemController(el);
+ const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ controller.connectRoot(root);
+ el.querySelector('anchored-position')!.dispatchEvent(new ToggleEvent('toggle', { newState: 'open' }));
+ expect(actionScopeFor).not.toHaveBeenCalled();
+
+ el.querySelector('action-menu')!.append(parent);
+ destinationFor(el, [{ type: 'sprint', id: '1' }]);
+ destinationFor(el, [{ type: 'backlog_bucket', id: '2' }]);
+ destinationFor(el, [{ type: 'inbox', id: null }]);
+ await vi.waitFor(() => expect(availableDestinations).toHaveBeenCalledTimes(3));
+ expect(actionScopeFor).toHaveBeenCalledTimes(1);
+ });
+
+ it('cancels a queued refresh when disconnected', async () => {
+ const { el } = renderItemWithMenu(1);
+ const controller = await mountItemController(el);
+ const { root, actionScopeFor } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ controller.connectRoot(root);
+ controller.moveItemTargetConnected();
+ controller.disconnect();
+ await Promise.resolve();
+ expect(actionScopeFor).not.toHaveBeenCalled();
+ });
+
it('hides up/top for a first item and shows the rest', async () => {
const { el, menu } = renderItemWithMenu(1);
document.body.appendChild(el);
const controller = await mountItemController(el);
controller.connectRoot(stubRoot(el, { isFirst: true, isLast: false }));
controller.moveItemTargetConnected();
+ await Promise.resolve();
expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'top'));
expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'up'));
@@ -1429,6 +1516,7 @@ describe('Sortable lists item controller', () => {
const gappedAvailability = { top: true, up: true, down: false, bottom: true };
controller.connectRoot(stubRoot(el, { isFirst: false, isLast: false }, vi.fn(), gappedAvailability));
controller.moveItemTargetConnected();
+ await Promise.resolve();
expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'down'));
expect(menu.showItem).toHaveBeenCalledWith(liFor(el, 'up'));
@@ -1441,6 +1529,7 @@ describe('Sortable lists item controller', () => {
const controller = await mountItemController(el);
controller.connectRoot(stubRoot(el, { isFirst: true, isLast: false }));
controller.moveItemTargetConnected();
+ await Promise.resolve();
expect(menu.disableItem).toHaveBeenCalledWith(liFor(el, 'top'));
expect(menu.disableItem).toHaveBeenCalledWith(liFor(el, 'up'));
@@ -1456,20 +1545,172 @@ describe('Sortable lists item controller', () => {
const controller = await mountItemController(el);
controller.connectRoot(stubRoot(el, { isFirst: true, isLast: true }));
controller.moveItemTargetConnected();
+ await Promise.resolve();
const parent = el.querySelector('li[data-sortable-lists--item-target="moveMenu"]')!;
expect(menu.hideItem).toHaveBeenCalledWith(parent);
});
+ it('projects deferred destination items for the selected invoker', 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.mockImplementation((_scope, candidates) => (
+ candidates.filter((candidate) => candidate.type === 'inbox')
+ ));
+ controller.connectRoot(root);
+
+ const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '1' }]);
+ const moveToInbox = destinationFor(el, [{ type: 'inbox', id: null }]);
+ await menuCtx!.nextFrame();
+
+ expect(actionScopeFor).toHaveBeenCalledWith(el);
+ expect(menu.hideItem).toHaveBeenCalledWith(moveToSprint);
+ expect(menu.showItem).toHaveBeenCalledWith(moveToInbox);
+ });
+
+ it('recomputes selected multi-card and prospective one-card scopes whenever the menu opens', async () => {
+ const { el, menu } = renderItemWithMenu(1);
+ document.body.appendChild(el);
+ const controller = await mountItemController(el);
+ const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ const selectedPeer = document.createElement('div');
+ selectedPeer.setAttribute('data-sortable-lists--item-id-value', '2');
+ selectedPeer.setAttribute('data-sortable-lists--item-mobility-value', 'free');
+ const selectedScope:ActionScope = { kind: 'batch', items: [el, selectedPeer]};
+ const prospectiveScope:ActionScope = { kind: 'batch', items: [el]};
+ let activeScope = selectedScope;
+ actionScopeFor.mockImplementation(() => activeScope);
+ availableDestinations.mockImplementation((scope, candidates) => (
+ candidates.filter((candidate) => candidate.type === (scope === selectedScope ? 'sprint' : 'inbox'))
+ ));
+ controller.connectRoot(root);
+
+ const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '1' }]);
+ const moveToInbox = destinationFor(el, [{ type: 'inbox', id: null }]);
+ await menuCtx!.nextFrame();
+
+ expect(availableDestinations).toHaveBeenCalledWith(selectedScope, [{ type: 'sprint', id: '1' }]);
+ expect(availableDestinations).toHaveBeenCalledWith(selectedScope, [{ type: 'inbox', id: null }]);
+ expect(menu.showItem).toHaveBeenCalledWith(moveToSprint);
+ expect(menu.hideItem).toHaveBeenCalledWith(moveToInbox);
+ menu.hideItem.mockClear();
+ menu.showItem.mockClear();
+ activeScope = prospectiveScope;
+
+ const menuElement = el.querySelector('action-menu')!;
+ const toggle = new ToggleEvent('toggle', { newState: 'open', oldState: 'closed' });
+ menuElement.querySelector('anchored-position')!.dispatchEvent(toggle);
+
+ expect(actionScopeFor).toHaveBeenLastCalledWith(el);
+ expect(availableDestinations).toHaveBeenCalledWith(prospectiveScope, [{ type: 'sprint', id: '1' }]);
+ expect(availableDestinations).toHaveBeenCalledWith(prospectiveScope, [{ type: 'inbox', id: null }]);
+ expect(menu.hideItem).toHaveBeenCalledWith(moveToSprint);
+ expect(menu.showItem).toHaveBeenCalledWith(moveToInbox);
+ });
+
+ it('hides destination actions and the position submenu for a true multi-card scope', async () => {
+ const { el, menu } = renderItemWithMenu(1, true);
+ document.body.appendChild(el);
+ const controller = await mountItemController(el);
+ // Batch size is the member count: two elements, not one element with
+ // two ids.
+ const scope:ActionScope = { kind: 'batch', items: [el, document.createElement('li')] };
+ const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ actionScopeFor.mockReturnValue(scope);
+ availableDestinations.mockReturnValue([]);
+ controller.connectRoot(root);
+
+ const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '1' }]);
+ const moveToInbox = destinationFor(el, [{ type: 'inbox', id: null }]);
+ 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(moveToInbox);
+ expect(menu.hideItem).toHaveBeenCalledWith(moveMenu);
+ expect(menu.hideItem).toHaveBeenCalledWith(liFor(el, 'top'));
+ expect(divider.hasAttribute('hidden')).toBe(true);
+
+ actionScopeFor.mockReturnValue({ kind: 'batch', items: [el] });
+ 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);
+ expect(divider.hasAttribute('hidden')).toBe(false);
+ });
+
+ it('keeps only the current owner destination for a confined batch scope', async () => {
+ const { el, menu } = renderItemWithMenu(1);
+ el.setAttribute('data-sortable-lists--item-mobility-value', 'confined');
+ document.body.appendChild(el);
+ const controller = await mountItemController(el);
+ const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ const scope:ActionScope = { kind: 'batch', items: [el]};
+ actionScopeFor.mockReturnValue(scope);
+ availableDestinations.mockImplementation((_scope, candidates) => candidates.filter((candidate) => candidate.id === '12'));
+ controller.connectRoot(root);
+
+ const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '12' }, { type: 'sprint', id: '13' }]);
+ await menuCtx!.nextFrame();
+
+ expect(availableDestinations).toHaveBeenCalledWith(scope, [{ type: 'sprint', id: '12' }, { type: 'sprint', id: '13' }]);
+ expect(menu.showItem).toHaveBeenCalledWith(moveToSprint);
+ });
+
+ it('hides batch destinations for a fixed synthetic singular scope', async () => {
+ const { el, menu } = renderItemWithMenu(1);
+ el.setAttribute('data-sortable-lists--item-mobility-value', 'fixed');
+ document.body.appendChild(el);
+ const controller = await mountItemController(el);
+ const { root, actionScopeFor, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ const scope:ActionScope = { kind: 'refused', items: [] };
+ actionScopeFor.mockReturnValue(scope);
+ availableDestinations.mockReturnValue([]);
+ controller.connectRoot(root);
+
+ const moveToSprint = destinationFor(el, [{ type: 'sprint', id: '12' }]);
+ await menuCtx!.nextFrame();
+
+ expect(availableDestinations).toHaveBeenCalledWith(scope, [{ type: 'sprint', id: '12' }]);
+ expect(menu.hideItem).toHaveBeenCalledWith(moveToSprint);
+ });
+
+ it.each([
+ ['invalid JSON', '{invalid'],
+ ['an empty destination type', '[{"type":"","id":null}]'],
+ ['non-array JSON', '{"type":"sprint","id":"12"}'],
+ ['a malformed candidate member', '[{"type":"sprint","id":"12"},{"type":"sprint"}]'],
+ ])('fails closed for %s destination metadata', async (_description, metadata) => {
+ const { el, menu } = renderItemWithMenu(1);
+ document.body.appendChild(el);
+ const controller = await mountItemController(el);
+ const { root, availableDestinations } = stubMenuRoot(el, { isFirst: false, isLast: false });
+ controller.connectRoot(root);
+
+ const destination = destinationWithMetadata(el, metadata);
+ await menuCtx!.nextFrame();
+
+ expect(menu.hideItem).toHaveBeenCalledWith(destination);
+ expect(availableDestinations).not.toHaveBeenCalled();
+ });
+
// Regression: the divider is rendered server-side from a permission check
// alone, so an item with nowhere to move used to be left with a separator
// and nothing below it.
it('hides the divider when nothing below it is left visible', async () => {
const { el } = renderItemWithMenu(1, true);
+ const overlay = document.createElement('anchored-position');
+ overlay.setAttribute('popover', 'auto');
+ el.querySelector('action-menu')!.append(overlay);
document.body.appendChild(el);
const controller = await mountItemController(el);
controller.connectRoot(stubRoot(el, { isFirst: true, isLast: true }));
controller.moveItemTargetConnected();
+ await Promise.resolve();
const divider = el.querySelector('li[data-sortable-lists--item-target="moveDivider"]')!;
expect(divider.hasAttribute('hidden')).toBe(true);
@@ -1481,6 +1722,7 @@ describe('Sortable lists item controller', () => {
const controller = await mountItemController(el);
controller.connectRoot(stubRoot(el, { isFirst: true, isLast: false }));
controller.moveItemTargetConnected();
+ await Promise.resolve();
const divider = el.querySelector('li[data-sortable-lists--item-target="moveDivider"]')!;
expect(divider.hasAttribute('hidden')).toBe(false);
@@ -1496,6 +1738,7 @@ describe('Sortable lists item controller', () => {
const controller = await mountItemController(el);
controller.connectRoot(stubRoot(el, { isFirst: true, isLast: true }));
controller.moveItemTargetConnected();
+ await Promise.resolve();
const divider = el.querySelector('li[data-sortable-lists--item-target="moveDivider"]')!;
expect(divider.hasAttribute('hidden')).toBe(false);
@@ -1508,6 +1751,7 @@ describe('Sortable lists item controller', () => {
const controller = await mountItemController(el);
controller.connectRoot(stubRoot(el, { isFirst: false, isLast: false }, moveInDirection));
controller.moveItemTargetConnected();
+ await Promise.resolve();
liFor(el, 'down').click();
expect(moveInDirection).toHaveBeenCalledWith(el, 'down');
@@ -1532,7 +1776,7 @@ describe('Sortable lists item controller', () => {
expect(() => {
controller.connectRoot(stubRoot(el, { isFirst: true, isLast: true }, moveInDirection));
// @ts-expect-error exercising the guard directly
- controller.refreshMoveMenuAvailability?.();
+ controller.refreshActionAvailability?.();
}).not.toThrow();
// move() must also no-op rather than throw: there is no menu to read
@@ -1545,6 +1789,113 @@ describe('Sortable lists item controller', () => {
});
});
+ describe('destination activation', () => {
+ it('exposes direct destination movement as a required root capability', () => {
+ const moveToDestination = vi.fn();
+ const root:SortableListsRoot = { ...fakeRoot(), moveToDestination };
+ const item = document.createElement('div');
+
+ root.moveToDestination(item, { type: 'inbox', id: null });
+
+ expect(moveToDestination).toHaveBeenCalledWith(item, { type: 'inbox', id: null });
+ });
+
+ it('replaces stale generated inputs with the current action scope in document order', () => {
+ const element = document.createElement('div');
+ const [first, second] = ['2', '1'].map((id) => {
+ const member = document.createElement('div');
+ member.setAttribute('data-sortable-lists--item-id-value', id);
+ return member;
+ });
+ const selectForAction = vi.fn(():ActionScope => ({ kind: 'batch', items: [first, second] }));
+ const controller = connectedControllerFor(element, {
+ root: { ...fakeRoot(), selectForAction },
+ });
+ const form = document.createElement('form');
+ form.innerHTML = '';
+ const event = new CustomEvent('async-dialog:beforeLoad', {
+ cancelable: true,
+ detail: { form },
+ });
+
+ controller.prepareDialog(event);
+
+ expect(selectForAction).toHaveBeenCalledWith(element);
+ expect(event.defaultPrevented).toBe(false);
+ expect(Array.from(form.elements).map((input) => [(input as HTMLInputElement).name, (input as HTMLInputElement).value])).toEqual([
+ ['kept', 'yes'],
+ ['ids[]', '2'],
+ ['ids[]', '1'],
+ ]);
+ expect(form.querySelectorAll('[data-sortable-lists-generated-id]')).toHaveLength(2);
+ });
+
+ it('cancels dialog loading without resolving or mutating the action scope while the root is busy', () => {
+ const element = document.createElement('div');
+ const selectForAction = vi.fn(():ActionScope => ({ kind: 'batch', items: [element] }));
+ const controller = connectedControllerFor(element, {
+ root: { ...fakeRoot(), busy: true, selectForAction },
+ });
+ const form = document.createElement('form');
+ form.innerHTML = '';
+ const staleInput = form.querySelector('[data-sortable-lists-generated-id]');
+ const event = new CustomEvent<{ form:HTMLFormElement|null }>('async-dialog:beforeLoad', {
+ cancelable: true,
+ detail: { form },
+ });
+
+ controller.prepareDialog(event);
+
+ expect(event.defaultPrevented).toBe(true);
+ expect(selectForAction).not.toHaveBeenCalled();
+ expect(form.querySelector('[data-sortable-lists-generated-id]')).toBe(staleInput);
+ expect(Array.from(form.elements).map((input) => [(input as HTMLInputElement).name, (input as HTMLInputElement).value])).toEqual([
+ ['ids[]', 'stale'],
+ ['kept', 'yes'],
+ ]);
+ });
+
+ it('cancels dialog loading for a fixed invoker with no batch action scope', () => {
+ const element = document.createElement('div');
+ element.setAttribute('data-sortable-lists--item-mobility-value', 'fixed');
+ const selectForAction = vi.fn(():ActionScope => ({ kind: 'refused', items: [] }));
+ const controller = connectedControllerFor(element, {
+ root: { ...fakeRoot(), selectForAction },
+ });
+ const form = document.createElement('form');
+ const event = new CustomEvent('async-dialog:beforeLoad', {
+ cancelable: true,
+ detail: { form },
+ });
+
+ controller.prepareDialog(event);
+
+ expect(selectForAction).toHaveBeenCalledWith(element);
+ expect(event.defaultPrevented).toBe(true);
+ expect(form.querySelector('[name="ids[]"]')).toBeNull();
+ });
+
+ it('delegates direct destination metadata to the root', () => {
+ const element = document.createElement('div');
+ const moveToDestination = vi.fn();
+ const controller = connectedControllerFor(element, {
+ root: { ...fakeRoot(), moveToDestination },
+ });
+ Object.defineProperty(controller, 'hasMenuElement', { value: true });
+ Object.defineProperty(controller, 'menuElement', {
+ value: { isItemDisabled: vi.fn(() => false), isItemHidden: vi.fn(() => false) },
+ });
+ const item = document.createElement('li');
+ item.dataset.sortableListsDestinations = '[{"type":"inbox","id":null}]';
+ const event = new Event('click') as ActionEvent;
+ Object.defineProperty(event, 'currentTarget', { value: item });
+
+ controller.moveToDestination(event);
+
+ expect(moveToDestination).toHaveBeenCalledWith(element, { type: 'inbox', id: null });
+ });
+ });
+
describe('movability and focus', () => {
let ctx:StimulusTestContext;
@@ -1638,8 +1989,13 @@ describe('Sortable lists item controller', () => {
const root:SortableListsRoot = {
element: item,
busy: false,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
moveAvailability: vi.fn(() => null),
+ ownerListElementOf: vi.fn(() => null),
ownerRowsContainer: vi.fn(() => null),
freezeDragBatch: vi.fn(() => 1),
markDragBatch,
@@ -1666,8 +2022,13 @@ describe('Sortable lists item controller', () => {
const root:SortableListsRoot = {
element: item,
busy: false,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
moveAvailability: vi.fn(() => null),
+ ownerListElementOf: vi.fn(() => null),
ownerRowsContainer: vi.fn(() => null),
freezeDragBatch,
markDragBatch: vi.fn(),
diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts
index 80e7f2e281d5..e31e165b7cf3 100644
--- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/item.controller.ts
@@ -52,19 +52,23 @@ import {
} from './drag-and-drop';
import {
isMoveDirection,
+ parseDestinationCandidates,
isOrderableItem,
itemMobility,
resolveItemExternalUrl,
resolveItemLabel,
sortableItemSelector,
+ type DestinationIdentity,
} from './list-dom';
import { webLinkHref } from './external-data';
import { renderDragPreview } from './preview';
+import { scopeIds } from './action-scope';
+import { refreshMenuAvailability } from './menu-availability';
type CleanupFn = () => void;
export default class ItemController extends Controller implements RootAwareChild {
- static targets = ['handle', 'preview', 'moveItem', 'moveMenu', 'moveDivider', 'focus'];
+ static targets = ['handle', 'preview', 'destinationItem', 'moveItem', 'moveMenu', 'moveDivider', 'focus'];
static elements = { menu: 'action-menu' };
static values = {
@@ -89,6 +93,7 @@ export default class ItemController extends Controller implements R
declare readonly hasHandleTarget:boolean;
declare readonly previewTarget:HTMLElement;
declare readonly hasPreviewTarget:boolean;
+ declare readonly destinationItemTargets:HTMLElement[];
declare readonly moveItemTargets:HTMLElement[];
declare readonly moveMenuTarget:HTMLElement;
declare readonly hasMoveMenuTarget:boolean;
@@ -106,6 +111,7 @@ export default class ItemController extends Controller implements R
private cleanupFn?:CleanupFn;
private dropIndicatorElement?:HTMLElement;
private root?:SortableListsRoot;
+ private refreshToken?:object;
private readonly onMenuToggle = (event:Event):void => {
// The toggle event does not bubble, so listen in capture phase; recompute
@@ -113,8 +119,10 @@ export default class ItemController extends Controller implements R
// shifted siblings meanwhile. Read newState by duck typing rather than
// `instanceof ToggleEvent` so a browser without the ToggleEvent global
// cannot throw.
- if ((event as ToggleEvent).newState === 'open') {
- this.refreshMoveMenuAvailability();
+ if (this.hasMenuElement
+ && event.target === this.menuElement.popoverElement
+ && (event as ToggleEvent).newState === 'open') {
+ this.refreshActionAvailability();
}
};
@@ -125,6 +133,7 @@ export default class ItemController extends Controller implements R
}
disconnect():void {
+ this.refreshToken = undefined;
// A morph can remove a hovering row mid-drag; without this the drop indicator
// it owns on a sibling row is never cleared (no onDrop fires, and no other
// controller may clear a foreign owner), leaving a phantom drop line.
@@ -135,24 +144,30 @@ export default class ItemController extends Controller implements R
this.disconnectRoot();
}
- // A move item entering the DOM (inline or via a deferred fragment) triggers
- // an availability refresh here, but `this.root` is usually still unset at
- // this point (the outlet's connectRoot callback runs later), so this call
- // typically no-ops. The menu-open toggle handler is what actually
- // establishes correct availability, refreshing on every open once the root
- // is connected and after any reorder has shifted siblings. No
- // include-fragment knowledge, so both hooks work for any menu.
moveItemTargetConnected():void {
- this.refreshMoveMenuAvailability();
+ this.scheduleAvailabilityRefresh();
+ }
+
+ destinationItemTargetConnected():void {
+ this.scheduleAvailabilityRefresh();
+ }
+
+ private scheduleAvailabilityRefresh():void {
+ if (this.refreshToken) return;
+
+ const token = {};
+ this.refreshToken = token;
+ queueMicrotask(() => {
+ if (this.refreshToken !== token) return;
+
+ this.refreshToken = undefined;
+ if (this.element.isConnected) this.refreshActionAvailability();
+ });
}
move(event:ActionEvent):void {
const item = event.currentTarget;
- if (!isOrderableItem(this.element) || !this.hasMenuElement || !(item instanceof HTMLElement)) {
- return;
- }
-
- if (this.menuElement.isItemDisabled(item) || this.menuElement.isItemHidden(item)) {
+ if (!isOrderableItem(this.element) || !this.menuItemActionable(item)) {
return;
}
@@ -162,6 +177,43 @@ export default class ItemController extends Controller implements R
}
}
+ prepareDialog(event:CustomEvent<{ form:HTMLFormElement|null }>):void {
+ const root = this.root;
+ if (!root || root.busy) {
+ event.preventDefault();
+ return;
+ }
+
+ const scope = root.selectForAction(this.element);
+ const form = event.detail.form;
+ if (!form || !scope || scope.kind === 'refused') {
+ event.preventDefault();
+ return;
+ }
+
+ form.querySelectorAll('[data-sortable-lists-generated-id]').forEach((input) => input.remove());
+ scopeIds(scope).forEach((id) => {
+ const input = form.ownerDocument.createElement('input');
+ input.type = 'hidden';
+ input.name = 'ids[]';
+ input.value = id;
+ input.dataset.sortableListsGeneratedId = '';
+ form.append(input);
+ });
+ }
+
+ moveToDestination(event:ActionEvent):void {
+ const item = event.currentTarget;
+ if (!this.menuItemActionable(item)) {
+ return;
+ }
+
+ const candidates = this.destinationCandidates(item);
+ if (candidates.length === 1) {
+ this.root?.moveToDestination(this.element, candidates[0]);
+ }
+ }
+
// The focus host is the consumer's business: Backlogs puts the tab stop on
// the card inside the row, another consumer may focus the row itself.
focusItem():void {
@@ -463,81 +515,33 @@ export default class ItemController extends Controller implements R
this.dropIndicatorElement = undefined;
}
- private refreshMoveMenuAvailability():void {
+ private refreshActionAvailability():void {
+ this.refreshToken = undefined;
const root = this.root;
- if (!root || !this.hasMenuElement) {
- return;
- }
-
- // Null availability means the item is not in a list yet; leave the menu
- // alone until the outlet wiring settles.
- const availability = root.moveAvailability(this.element);
- if (!availability) {
+ if (!root || !this.hasMenuElement || (this.destinationItemTargets.length === 0 && this.moveItemTargets.length === 0)) {
return;
}
- let available = 0;
- for (const item of this.moveItemTargets) {
- // Outside a Stimulus action there is no event.params, so read the
- // param's backing attribute directly.
- const direction = item.getAttribute(`data-${this.identifier}-direction-param`);
- const enabled = isMoveDirection(direction) && availability[direction];
- this.setAvailability(item, enabled);
- if (enabled) {
- available += 1;
- }
- }
-
- if (this.hasMoveMenuTarget) {
- this.setAvailability(this.moveMenuTarget, available > 0);
- }
-
- this.refreshMoveDivider();
+ refreshMenuAvailability({
+ menu: this.menuElement,
+ scope: root.actionScopeFor(this.element),
+ destinationItems: this.destinationItemTargets,
+ moveItems: this.moveItemTargets,
+ moveMenu: this.hasMoveMenuTarget ? this.moveMenuTarget : null,
+ divider: this.hasMoveDividerTarget ? this.moveDividerTarget : null,
+ hideUnavailable: this.hideUnavailableValue,
+ identifier: this.identifier,
+ availableDestinations: (scope, candidates) => root.availableDestinations(scope, candidates),
+ moveAvailability: () => root.moveAvailability(this.element),
+ });
}
- // The divider that opens the move group is rendered server-side from a
- // permission check alone, so hiding the last entry below it would otherwise
- // leave a separator with nothing to separate. It never goes through
- // setAvailability: `disableItem` writes to the item's `.ActionListContent`,
- // which a divider does not have — and in that mode the group stays visible
- // anyway, only disabled.
- private refreshMoveDivider():void {
- if (!this.hasMoveDividerTarget || !this.hideUnavailableValue) {
- return;
- }
-
- const divider = this.moveDividerTarget;
- let sibling = divider.nextElementSibling;
-
- while (sibling) {
- if (!sibling.hasAttribute('hidden')) {
- divider.removeAttribute('hidden');
- return;
- }
-
- sibling = sibling.nextElementSibling;
- }
-
- divider.setAttribute('hidden', 'hidden');
+ private menuItemActionable(item:EventTarget|null):item is HTMLElement {
+ return this.hasMenuElement && item instanceof HTMLElement
+ && !this.menuElement.isItemDisabled(item) && !this.menuElement.isItemHidden(item);
}
- // Availability goes through the action-menu element's API: disableItem sets the
- // ActionListItem--disabled class plus aria-disabled on the item's content, and
- // hideItem toggles hidden. It operates on any descendant li, including the ones
- // in the nested move submenu. Default is hide; hideUnavailable=false switches to disable.
- private setAvailability(item:HTMLElement, available:boolean):void {
- const menu = this.menuElement;
-
- if (this.hideUnavailableValue) {
- if (available) {
- menu.showItem(item);
- } else {
- menu.hideItem(item);
- }
- } else if (available) {
- menu.enableItem(item);
- } else {
- menu.disableItem(item);
- }
+ private destinationCandidates(item:HTMLElement):DestinationIdentity[] {
+ return parseDestinationCandidates(item.dataset.sortableListsDestinations);
}
}
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 7fe83a4f7a2d..ec0171281798 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
@@ -26,11 +26,15 @@
// See COPYRIGHT and LICENSE files for more details.
//++
+import { environment } from '../../../../environments/environment';
+
import {
+ parseDestinationCandidates,
captureRowPositions,
isOrderableItem,
itemAcceptsDestination,
itemMobility,
+ permittedDestinations,
reorderRows,
sortableItemMobilityAttribute,
resolveDirectionalPreviousItemId,
@@ -43,6 +47,7 @@ import {
restoreRowPositions,
rowOf,
rowsRemainAt,
+ sameDestination,
} from './list-dom';
describe('sortable lists DOM helpers', () => {
@@ -455,6 +460,72 @@ describe('itemAcceptsDestination', () => {
});
});
+describe('destination intersection', () => {
+ const inbox = { type: 'inbox', id: null };
+ const sprint1 = { type: 'sprint', id: '1' };
+ const sprint2 = { type: 'sprint', id: '2' };
+ const bucket1 = { type: 'bucket', id: '1' };
+
+ function item(mobility:'fixed'|'confined'|'free' = 'free'):HTMLElement {
+ const element = document.createElement('li');
+ element.setAttribute(sortableItemMobilityAttribute, mobility);
+ return element;
+ }
+
+ it('permits every candidate for free-only members', () => {
+ const freeOne = item();
+ const freeTwo = item();
+
+ expect(permittedDestinations({
+ items: [freeOne, freeTwo],
+ candidates: [inbox, sprint1, sprint2, bucket1],
+ ownerDestinationOf: () => null,
+ })).toEqual([inbox, sprint1, sprint2, bucket1]);
+ });
+
+ it('intersects a confined member with free members', () => {
+ const confinedInSprint = item('confined');
+ const freeInBucket = item();
+ const ownerDestinationOf = (member:HTMLElement) => member === confinedInSprint ? sprint1 : bucket1;
+
+ expect(permittedDestinations({
+ items: [confinedInSprint, freeInBucket],
+ candidates: [inbox, sprint1, sprint2, bucket1],
+ ownerDestinationOf,
+ })).toEqual([sprint1]);
+ });
+
+ it('rejects confined members belonging to different destinations', () => {
+ const confinedInSprint = item('confined');
+ const confinedInBucket = item('confined');
+ const ownerDestinationOf = (member:HTMLElement) => member === confinedInSprint ? sprint1 : bucket1;
+
+ expect(permittedDestinations({
+ items: [confinedInSprint, confinedInBucket],
+ candidates: [inbox, sprint1, bucket1],
+ ownerDestinationOf,
+ })).toEqual([]);
+ });
+
+ it('rejects every destination when a member is fixed', () => {
+ expect(permittedDestinations({
+ items: [item('fixed'), item()],
+ candidates: [inbox, sprint1],
+ ownerDestinationOf: () => null,
+ })).toEqual([]);
+ });
+
+ it('identifies targets occupied by every member independently of permission', () => {
+ const freeInSprint = item();
+ const anotherFreeInSprint = item();
+ const items = [freeInSprint, anotherFreeInSprint];
+ const ownerDestinationOf = (_member:HTMLElement) => sprint1;
+ const permitted = permittedDestinations({ items, candidates: [sprint1, sprint2], ownerDestinationOf });
+
+ expect(permitted.filter((target) => !items.every((member) => sameDestination(ownerDestinationOf(member), target)))).toEqual([sprint2]);
+ });
+});
+
describe('directional move helpers', () => {
function container(ids:string[]):HTMLElement {
const ul = document.createElement('ul');
@@ -683,3 +754,36 @@ describe('resolveItemType', () => {
expect(resolveItemType(el)).toBeNull();
});
});
+
+
+describe('destination metadata', () => {
+ let wasProduction:boolean;
+ let log:ReturnType;
+ beforeEach(() => {
+ wasProduction = environment.production;
+ environment.production = false;
+ log = vi.spyOn(console, 'log').mockImplementation(() => undefined);
+ });
+ afterEach(() => {
+ environment.production = wasProduction;
+ vi.restoreAllMocks();
+ });
+
+ it.each([undefined, ''])('ignores missing metadata without logging', (metadata) => {
+ expect(parseDestinationCandidates(metadata)).toEqual([]);
+ expect(log).not.toHaveBeenCalled();
+ });
+
+ it.each(['{invalid', '{}', '[{"type":"","id":null}]', '[{"type":"sprint","id":1}]',
+ '[{"type":"sprint","id":"1"},{"type":"sprint"}]'])('rejects malformed metadata as a whole: %s', (metadata) => {
+ expect(parseDestinationCandidates(metadata)).toEqual([]);
+ expect(log).toHaveBeenCalledWith('[DEBUG] sortable-lists: malformed destination metadata');
+ });
+
+ it('preserves ordered destinations and the null inbox identity', () => {
+ expect(parseDestinationCandidates('[{"type":"sprint","id":"2"},{"type":"inbox","id":null}]')).toEqual([
+ { type: 'sprint', id: '2' }, { type: 'inbox', id: null },
+ ]);
+ expect(log).not.toHaveBeenCalled();
+ });
+});
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 d85962b6045c..1fc123b16aea 100644
--- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list-dom.ts
@@ -121,6 +121,33 @@ export interface DestinationIdentity {
id:string|null;
}
+export function isDestinationIdentity(candidate:unknown):candidate is DestinationIdentity {
+ if (typeof candidate !== 'object' || candidate === null) {
+ return false;
+ }
+
+ return 'type' in candidate
+ && 'id' in candidate
+ && typeof candidate.type === 'string'
+ && candidate.type.length > 0
+ && (typeof candidate.id === 'string' || candidate.id === null);
+}
+
+export function parseDestinationCandidates(raw:string|undefined):DestinationIdentity[] {
+ if (!raw) return [];
+
+ try {
+ const candidates:unknown = JSON.parse(raw);
+ if (Array.isArray(candidates) && candidates.every(isDestinationIdentity)) {
+ return candidates;
+ }
+ } catch {
+ // Invalid JSON and invalid identities share the same fail-closed result.
+ }
+ debugLog('sortable-lists: malformed destination metadata');
+ return [];
+}
+
export function sameDestination(left:DestinationIdentity|null, right:DestinationIdentity):boolean {
return left !== null && left.type === right.type && left.id === right.id;
}
@@ -142,6 +169,24 @@ export function itemAcceptsDestination(
}
}
+export function permittedDestinations({
+ items,
+ candidates,
+ ownerDestinationOf,
+}:{
+ items:HTMLElement[];
+ candidates:DestinationIdentity[];
+ ownerDestinationOf:(item:HTMLElement) => DestinationIdentity|null;
+}):DestinationIdentity[] {
+ if (items.length === 0) {
+ return [];
+ }
+
+ return candidates.filter((target) => (
+ items.every((item) => itemAcceptsDestination(item, target, ownerDestinationOf))
+ ));
+}
+
export function resolveItemType(element:Element):string|null {
const type = element.getAttribute(sortableItemTypeAttribute);
diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts
index 027d6882f171..718a61a7d531 100644
--- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/list.controller.spec.ts
@@ -37,6 +37,7 @@ import { setupStimulusTest, type StimulusTestContext } from 'core-stimulus/test-
import type ListControllerType from './list.controller';
import type { sortableItemData as sortableItemDataFn, SortableListsRoot } from './drag-and-drop';
import type { DestinationIdentity } from './list-dom';
+import type { ActionScope } from './action-scope';
// The list controller is tested in ISOLATION: the root drives the outlet
// hand-over in production (sortable-lists.controller.ts), so here we render only
@@ -75,8 +76,13 @@ describe('Sortable lists list controller', () => {
return {
element,
busy,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
moveAvailability: vi.fn(() => null),
+ ownerListElementOf: vi.fn(() => null),
ownerRowsContainer: vi.fn(() => null),
freezeDragBatch: vi.fn(() => 1),
markDragBatch: vi.fn(),
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
new file mode 100644
index 000000000000..e957ba1d3b86
--- /dev/null
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.spec.ts
@@ -0,0 +1,88 @@
+//-- 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.
+//++
+
+import { refreshMenuAvailability } from './menu-availability';
+
+function fixture(hideUnavailable = true) {
+ const list = document.createElement('ul');
+ list.innerHTML = '';
+ const [divider, destination, moveMenu] = Array.from(list.querySelectorAll('li'));
+ const moveItem = document.createElement('li');
+ moveItem.setAttribute('data-sortable-lists-item-direction-param', 'top');
+ moveMenu.append(moveItem);
+ const menu = {
+ showItem: (item:HTMLElement) => item.removeAttribute('hidden'),
+ hideItem: (item:HTMLElement) => item.setAttribute('hidden', ''),
+ enableItem: (item:HTMLElement) => item.removeAttribute('aria-disabled'),
+ disableItem: (item:HTMLElement) => item.setAttribute('aria-disabled', 'true'),
+ };
+ const input:Parameters[0] = {
+ menu,
+ scope: { kind: 'batch', items: [document.createElement('div'), document.createElement('div')] },
+ destinationItems: [destination],
+ moveItems: [moveItem],
+ moveMenu,
+ divider,
+ hideUnavailable,
+ identifier: 'sortable-lists-item',
+ availableDestinations: vi.fn(() => []),
+ moveAvailability: vi.fn(() => ({ top: true, up: true, down: false, bottom: false })),
+ };
+ return { input, divider, destination, moveMenu, moveItem };
+}
+
+describe('menu availability projection', () => {
+ it('ignores the trailing overlay when every real action is hidden', () => {
+ const { input, divider, destination, moveMenu, moveItem } = fixture();
+ refreshMenuAvailability(input);
+ for (const element of [divider, destination, moveMenu, moveItem]) {
+ expect(element.hasAttribute('hidden')).toBe(true);
+ }
+ expect(input.moveAvailability).not.toHaveBeenCalled();
+ });
+
+ it('restores the position group when the live scope becomes a singleton', () => {
+ const { input, divider, moveMenu, moveItem } = fixture();
+ refreshMenuAvailability(input);
+ input.scope = { kind: 'batch', items: [document.createElement('div')] };
+ refreshMenuAvailability(input);
+ for (const element of [divider, moveMenu, moveItem]) {
+ expect(element.hasAttribute('hidden')).toBe(false);
+ }
+ });
+
+ it('disables unavailable actions without hiding the group separator', () => {
+ const { input, divider, destination, moveMenu, moveItem } = fixture(false);
+ refreshMenuAvailability(input);
+ expect(divider.hasAttribute('hidden')).toBe(false);
+ for (const element of [destination, moveMenu, moveItem]) {
+ expect(element.getAttribute('aria-disabled')).toBe('true');
+ 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
new file mode 100644
index 000000000000..7015b9d54cb0
--- /dev/null
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/menu-availability.ts
@@ -0,0 +1,85 @@
+//-- 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.
+//++
+
+import type { ActionMenuElement } from '@openproject/primer-view-components/app/components/primer/alpha/action_menu/action_menu_element';
+import type { ActionScope } from './action-scope';
+import type { SortableListsRoot } from './drag-and-drop';
+import { isMoveDirection, parseDestinationCandidates, type DestinationIdentity } from './list-dom';
+
+interface MenuAvailabilityInput {
+ menu:Pick;
+ scope:ActionScope;
+ destinationItems:HTMLElement[];
+ moveItems:HTMLElement[];
+ moveMenu:HTMLElement|null;
+ divider:HTMLElement|null;
+ hideUnavailable:boolean;
+ identifier:string;
+ availableDestinations:(scope:ActionScope, candidates:DestinationIdentity[]) => DestinationIdentity[];
+ moveAvailability:() => ReturnType;
+}
+
+export function refreshMenuAvailability(input:MenuAvailabilityInput):void {
+ const setAvailability = (item:HTMLElement, available:boolean):void => {
+ if (input.hideUnavailable) {
+ input.menu[available ? 'showItem' : 'hideItem'](item);
+ } else {
+ input.menu[available ? 'enableItem' : 'disableItem'](item);
+ }
+ };
+
+ for (const item of input.destinationItems) {
+ const candidates = parseDestinationCandidates(item.dataset.sortableListsDestinations);
+ 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) {
+ 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];
+ setAvailability(item, enabled);
+ if (enabled) available += 1;
+ }
+ if (input.moveMenu) setAvailability(input.moveMenu, available > 0);
+ }
+
+ if (input.divider && input.hideUnavailable) {
+ let sibling = input.divider.nextElementSibling;
+ while (sibling) {
+ if (sibling instanceof HTMLLIElement && !sibling.hasAttribute('hidden')) {
+ input.divider.removeAttribute('hidden');
+ return;
+ }
+ sibling = sibling.nextElementSibling;
+ }
+ input.divider.setAttribute('hidden', 'hidden');
+ }
+}
diff --git a/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts b/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts
index 7436950072f0..0f571e1e2a15 100644
--- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/scrollable.controller.spec.ts
@@ -30,6 +30,7 @@ import { type autoScrollForElements as autoScrollForElementsFn } from '@atlaskit
import { setupStimulusTest, type StimulusTestContext } from 'core-stimulus/test-helpers';
import type ScrollableControllerType from './scrollable.controller';
import type { sortableItemData as sortableItemDataFn, SortableListsRoot } from './drag-and-drop';
+import type { ActionScope } from './action-scope';
vi.mock('@atlaskit/pragmatic-drag-and-drop-auto-scroll/element', () => ({
autoScrollForElements: vi.fn(() => vi.fn()),
@@ -70,8 +71,13 @@ describe('Sortable lists scrollable controller', () => {
return {
element,
busy: false,
+ actionScopeFor: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ selectForAction: vi.fn(():ActionScope => ({ kind: 'refused', items: [] })),
+ availableDestinations: vi.fn(() => []),
+ moveToDestination: vi.fn(),
moveInDirection: vi.fn(),
moveAvailability: vi.fn(() => null),
+ ownerListElementOf: vi.fn(() => null),
ownerRowsContainer: vi.fn(() => null),
freezeDragBatch: vi.fn(() => 1),
markDragBatch: vi.fn(),
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 8f189bc9d63a..adf02d4a90ce 100644
--- a/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts
+++ b/frontend/src/stimulus/controllers/dynamic/sortable-lists/selection-orchestrator.ts
@@ -26,6 +26,7 @@
// See COPYRIGHT and LICENSE files for more details.
//++
+import type { ActionScope } from './action-scope';
import { BatchSelection, type SelectionAnchor, type SelectionKey } from 'core-common/batch-selection';
import { announce } from '@primer/live-region-element';
import { resolveItemId, resolveItemType } from './list-dom';
@@ -59,10 +60,6 @@ export interface SelectionHost {
ownerRowsContainer(itemElement:HTMLElement):HTMLElement|null;
}
-export type ActionScope =
- | { kind:'batch'; items:HTMLElement[] }
- | { kind:'refused'; items:[] };
-
// What resolving an action scope does to the selection: nothing, replace it
// with the card alone only when the card is outside it, or replace it
// unconditionally.
@@ -400,6 +397,10 @@ export class SelectionOrchestrator {
}
event.preventDefault();
+ if (this.host.busy) {
+ return;
+ }
+
escapesClearedBySelection.add(event);
this.selection.clear();
this.renderSelection('selection');
diff --git a/frontend/src/stimulus/helpers/request-helpers.spec.ts b/frontend/src/stimulus/helpers/request-helpers.spec.ts
index 0fd3d0427112..a77be9bea8f3 100644
--- a/frontend/src/stimulus/helpers/request-helpers.spec.ts
+++ b/frontend/src/stimulus/helpers/request-helpers.spec.ts
@@ -26,10 +26,61 @@
// See COPYRIGHT and LICENSE files for more details.
//++
-import type { FetchResponse } from '@rails/request.js';
+import type { FetchRequest, FetchResponse } from '@rails/request.js';
import { TurboHelpers } from 'core-turbo/helpers';
-import { withLoadingIndicator, withProgressBar } from './request-helpers';
+import { performTurboStreamRequest, withLoadingIndicator, withProgressBar } from './request-helpers';
+
+describe('performTurboStreamRequest', () => {
+ function requestFor(response:Partial):FetchRequest {
+ return {
+ perform: vi.fn().mockResolvedValue(response),
+ } as unknown as FetchRequest;
+ }
+
+ it.each([
+ ['successful', { ok: true, unprocessableEntity: false }],
+ ['unprocessable', { ok: false, unprocessableEntity: true }],
+ ])('leaves request.js to render a %s stream', async (_name, status) => {
+ const renderTurboStream = vi.fn();
+ const response = {
+ ...status,
+ isTurboStream: true,
+ renderTurboStream,
+ } as unknown as FetchResponse;
+
+ await expect(performTurboStreamRequest(requestFor(response))).resolves.toBe(response);
+ expect(renderTurboStream).not.toHaveBeenCalled();
+ });
+
+ it('renders another unsuccessful Turbo Stream exactly once', async () => {
+ const renderTurboStream = vi.fn().mockResolvedValue(undefined);
+ const response = {
+ ok: false,
+ unprocessableEntity: false,
+ isTurboStream: true,
+ renderTurboStream,
+ } as unknown as FetchResponse;
+
+ await performTurboStreamRequest(requestFor(response));
+ expect(renderTurboStream).toHaveBeenCalledOnce();
+ });
+
+ it('rejects a non-Turbo response without rendering it', async () => {
+ const renderTurboStream = vi.fn();
+ const response = {
+ ok: true,
+ unprocessableEntity: false,
+ isTurboStream: false,
+ statusCode: 500,
+ renderTurboStream,
+ } as unknown as FetchResponse;
+
+ await expect(performTurboStreamRequest(requestFor(response)))
+ .rejects.toMatchObject({ name: 'FetchRequestError', _errorCode: 500, message: 'Response is not a Turbo Stream' });
+ expect(renderTurboStream).not.toHaveBeenCalled();
+ });
+});
describe('withLoadingIndicator', () => {
let indicator:HTMLElement|null;
diff --git a/frontend/src/stimulus/helpers/request-helpers.ts b/frontend/src/stimulus/helpers/request-helpers.ts
index a63f64b1aae8..98663cda838e 100644
--- a/frontend/src/stimulus/helpers/request-helpers.ts
+++ b/frontend/src/stimulus/helpers/request-helpers.ts
@@ -36,6 +36,20 @@ export function post(url:string|URL, options?:Options) {
return withLoadingIndicator(request.perform());
}
+export async function performTurboStreamRequest(request:FetchRequest):Promise {
+ const response = await request.perform();
+
+ if (!response.isTurboStream) {
+ throw new FetchRequestError(response.statusCode, 'Response is not a Turbo Stream');
+ }
+
+ if (!response.ok && !response.unprocessableEntity) {
+ await response.renderTurboStream();
+ }
+
+ return response;
+}
+
export function withLoadingIndicator(request:Promise) {
const loadingIndicator = document.querySelector('#global-loading-indicator');
invariant(loadingIndicator, 'Expected an Element with id global-loading-indicator to be present');
diff --git a/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.html.erb b/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.html.erb
index 366c690a9e72..2afb6182e796 100644
--- a/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.html.erb
+++ b/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.html.erb
@@ -27,26 +27,31 @@ See COPYRIGHT and LICENSE files for more details.
++#%>
-<%=
- render(Primer::Alpha::Dialog.new(title: t(".title"), id: DIALOG_ID, size: :medium)) do |d|
- d.with_header(variant: :large)
-
- d.with_body do
- available_buckets = buckets
- bucket_list_type = list_type
+<%= render(Primer::Alpha::Dialog.new(title: t(".title"), id: DIALOG_ID, size: :medium_portrait)) do |d| %>
+ <% d.with_header(variant: :large) %>
+ <% d.with_body do %>
+ <%=
primer_form_with(
url: move_action,
method: :put,
id: FORM_ID,
data: { turbo_stream: true }
) do |f|
+ selected_work_packages = work_packages
+ available_buckets = buckets
+ bucket_list_type = destination_list_type
+
render_inline_form(f) do |form|
+ selected_work_packages.each do |work_package|
+ form.hidden(name: "ids[]", value: work_package.id)
+ end
form.hidden(name: "list_type", value: bucket_list_type)
form.select_list(
name: "list_id",
label: BacklogBucket.human_attribute_name(:name),
- visually_hide_label: true
+ visually_hide_label: true,
+ aria: { describedby: SELECTION_LABEL_ID }
) do |select|
available_buckets.each do |bucket|
select.option(label: bucket.name, value: bucket.id)
@@ -54,9 +59,21 @@ See COPYRIGHT and LICENSE files for more details.
end
end
end
- end
+ %>
+
+ <%=
+ render(
+ Backlogs::SelectedWorkPackagesComponent.new(
+ work_packages:,
+ description_id: SELECTION_LABEL_ID,
+ mt: 3
+ )
+ )
+ %>
+ <% end %>
- d.with_footer(mt: 2) do
+ <% d.with_footer(show_divider: true) do %>
+ <%=
component_collection do |buttons|
buttons.with_component(Primer::Beta::Button.new(data: { close_dialog_id: DIALOG_ID })) do
t(:button_cancel)
@@ -68,6 +85,6 @@ See COPYRIGHT and LICENSE files for more details.
t(:button_move)
end
end
- end
- end
-%>
+ %>
+ <% end %>
+<% end %>
diff --git a/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.rb b/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.rb
index 14046c5bccbf..3f88deb5a753 100644
--- a/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.rb
+++ b/modules/backlogs/app/components/backlogs/move_to_bucket_dialog_component.rb
@@ -35,23 +35,22 @@ class MoveToBucketDialogComponent < ApplicationComponent
DIALOG_ID = "move-to-backlog-bucket-dialog"
FORM_ID = "move-to-backlog-bucket-dialog-form"
+ SELECTION_LABEL_ID = "move-to-backlog-bucket-dialog-selection"
- attr_reader :work_package, :project, :buckets, :move_action
+ attr_reader :work_packages, :buckets, :move_action
- def initialize(work_package:, project:, move_action:)
+ def initialize(work_packages:, buckets:, move_action:)
super()
- @work_package = work_package
- @project = project
+ @work_packages = work_packages
+ @buckets = buckets
@move_action = move_action
- @buckets = BacklogBucket.where(project:).order_alphabetically
- @buckets = @buckets.where.not(id: work_package.backlog_bucket_id) if work_package.backlog_bucket_id
end
private
- def list_type
- Backlogs::Target::BucketId.new(nil).list_type
+ def destination_list_type
+ Backlogs::Target::BucketId.list_type
end
end
end
diff --git a/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.html.erb b/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.html.erb
index 29698a64638d..b2e61f548360 100644
--- a/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.html.erb
+++ b/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.html.erb
@@ -27,32 +27,53 @@ See COPYRIGHT and LICENSE files for more details.
++#%>
-<%=
- render(Primer::Alpha::Dialog.new(title: t(".title"), id: DIALOG_ID, size: :medium)) do |d|
- d.with_header(variant: :large)
-
- d.with_body do
- available_sprints = sprints
- sprint_list_type = list_type
+<%= render(Primer::Alpha::Dialog.new(title: t(".title"), id: DIALOG_ID, size: :medium_portrait)) do |d| %>
+ <% d.with_header(variant: :large) %>
+ <% d.with_body do %>
+ <%=
primer_form_with(
url: move_action,
method: :put,
id: FORM_ID,
data: { turbo_stream: true }
) do |f|
+ selected_work_packages = work_packages
+ available_sprints = sprints
+ sprint_list_type = destination_list_type
+
render_inline_form(f) do |form|
+ selected_work_packages.each do |work_package|
+ form.hidden(name: "ids[]", value: work_package.id)
+ end
form.hidden(name: "list_type", value: sprint_list_type)
- form.select_list(name: "list_id", label: Sprint.human_model_name, visually_hide_label: true) do |select|
+ form.select_list(
+ name: "list_id",
+ label: Sprint.human_model_name,
+ visually_hide_label: true,
+ aria: { describedby: SELECTION_LABEL_ID }
+ ) do |select|
available_sprints.each do |sprint|
select.option(label: sprint.name, value: sprint.id)
end
end
end
end
- end
+ %>
+
+ <%=
+ render(
+ Backlogs::SelectedWorkPackagesComponent.new(
+ work_packages:,
+ description_id: SELECTION_LABEL_ID,
+ mt: 3
+ )
+ )
+ %>
+ <% end %>
- d.with_footer(mt: 2) do
+ <% d.with_footer(show_divider: true) do %>
+ <%=
component_collection do |buttons|
buttons.with_component(Primer::Beta::Button.new(data: { close_dialog_id: DIALOG_ID })) do
t(:button_cancel)
@@ -64,6 +85,6 @@ See COPYRIGHT and LICENSE files for more details.
t(:button_move)
end
end
- end
- end
-%>
+ %>
+ <% end %>
+<% end %>
diff --git a/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.rb b/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.rb
index bf0efcf387f2..39f38bceaafa 100644
--- a/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.rb
+++ b/modules/backlogs/app/components/backlogs/move_to_sprint_dialog_component.rb
@@ -35,23 +35,22 @@ class MoveToSprintDialogComponent < ApplicationComponent
DIALOG_ID = "move-to-sprint-dialog"
FORM_ID = "move-to-sprint-dialog-form"
+ SELECTION_LABEL_ID = "move-to-sprint-dialog-selection"
- attr_reader :work_package, :project, :sprints, :move_action
+ attr_reader :work_packages, :sprints, :move_action
- def initialize(work_package:, project:, move_action:)
+ def initialize(work_packages:, sprints:, move_action:)
super()
- @work_package = work_package
- @project = project
+ @work_packages = work_packages
+ @sprints = sprints
@move_action = move_action
- @sprints = Sprint.for_project(@project).visible.not_completed.order_by_date
- @sprints = @sprints.where.not(id: work_package.sprint_id) if work_package.sprint_id
end
private
- def list_type
- Backlogs::Target::SprintId.new(nil).list_type
+ def destination_list_type
+ Backlogs::Target::SprintId.list_type
end
end
end
diff --git a/modules/backlogs/app/components/backlogs/selected_work_packages_component.html.erb b/modules/backlogs/app/components/backlogs/selected_work_packages_component.html.erb
new file mode 100644
index 000000000000..d6a91cb0bbbb
--- /dev/null
+++ b/modules/backlogs/app/components/backlogs/selected_work_packages_component.html.erb
@@ -0,0 +1,59 @@
+<%#-- 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.
+
+++#%>
+
+<%= render(OpPrimer::InsetBoxComponent.new(**@system_arguments)) do %>
+ <%=
+ render(
+ Primer::Beta::Heading.new(
+ tag: :h2,
+ id: description_id,
+ font_size: :small,
+ font_weight: :bold,
+ mb: 2
+ )
+ ) { t(".label", count: work_packages.size) }
+ %>
+
+ <%= render(OpPrimer::ListComponent.new(ml: 0, tag: :ol)) do |list| %>
+ <% work_packages.each do |work_package| %>
+ <% list.with_item do %>
+ <%=
+ render(
+ WorkPackages::InfoLineComponent.new(
+ work_package:,
+ show_subject: true,
+ show_status: false,
+ my: 1
+ )
+ )
+ %>
+ <% end %>
+ <% end %>
+ <% end %>
+<% end %>
diff --git a/modules/backlogs/app/components/backlogs/selected_work_packages_component.rb b/modules/backlogs/app/components/backlogs/selected_work_packages_component.rb
new file mode 100644
index 000000000000..4552d03a4067
--- /dev/null
+++ b/modules/backlogs/app/components/backlogs/selected_work_packages_component.rb
@@ -0,0 +1,49 @@
+# 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.
+#++
+
+module Backlogs
+ # Lists the work packages a batch action is about to affect.
+ #
+ # The heading carries `description_id`; point the acting control's
+ # `aria-describedby` at that id so the count reaches assistive technology.
+ class SelectedWorkPackagesComponent < ApplicationComponent
+ include OpPrimer::ComponentHelpers
+
+ attr_reader :work_packages, :description_id
+
+ def initialize(work_packages:, description_id:, **system_arguments)
+ super()
+
+ @work_packages = work_packages
+ @description_id = description_id
+ @system_arguments = system_arguments
+ end
+ end
+end
diff --git a/modules/backlogs/app/components/backlogs/work_package_card_menu_component.html.erb b/modules/backlogs/app/components/backlogs/work_package_card_menu_component.html.erb
index 8136bb615e92..b339a5285b40 100644
--- a/modules/backlogs/app/components/backlogs/work_package_card_menu_component.html.erb
+++ b/modules/backlogs/app/components/backlogs/work_package_card_menu_component.html.erb
@@ -75,8 +75,9 @@ See COPYRIGHT and LICENSE files for more details.
id: dom_target(work_package, :menu, :move_to_inbox),
label: t(".action_menu.move_to_inbox"),
tag: :button,
- href: move_project_backlogs_work_package_path(project, work_package),
- form_arguments: { method: :put, inputs: [{ name: "list_type", value: inbox_list_type }] }
+ data: destination_data(inbox_list_type, []).merge(
+ action: "click->sortable-lists--item#moveToDestination:prevent"
+ )
) do |item|
item.with_leading_visual_icon(icon: :inbox)
end
@@ -86,8 +87,16 @@ See COPYRIGHT and LICENSE files for more details.
list.with_item(
id: dom_target(work_package, :menu, :move_to_backlog_bucket),
label: t(".action_menu.move_to_backlog_bucket"),
- href: move_to_bucket_dialog_project_backlogs_work_package_path(project, work_package),
- content_arguments: { data: { controller: "async-dialog" } }
+ tag: :button,
+ href: move_to_bucket_dialog_project_backlogs_work_packages_path(project),
+ form_arguments: { method: :post },
+ content_arguments: {
+ data: {
+ controller: "async-dialog",
+ action: "async-dialog:beforeLoad->sortable-lists--item#prepareDialog"
+ }
+ },
+ data: destination_data(bucket_list_type, bucket_ids)
) do |item|
item.with_leading_visual_icon(icon: :package)
end
@@ -97,8 +106,16 @@ See COPYRIGHT and LICENSE files for more details.
list.with_item(
id: dom_target(work_package, :menu, :move_to_sprint),
label: t(".action_menu.move_to_sprint"),
- href: move_to_sprint_dialog_project_backlogs_work_package_path(project, work_package),
- content_arguments: { data: { controller: "async-dialog" } }
+ tag: :button,
+ href: move_to_sprint_dialog_project_backlogs_work_packages_path(project),
+ form_arguments: { method: :post },
+ content_arguments: {
+ data: {
+ controller: "async-dialog",
+ action: "async-dialog:beforeLoad->sortable-lists--item#prepareDialog"
+ }
+ },
+ data: destination_data(sprint_list_type, sprint_ids)
) do |item|
item.with_leading_visual_icon(icon: :zap)
end
diff --git a/modules/backlogs/app/components/backlogs/work_package_card_menu_component.rb b/modules/backlogs/app/components/backlogs/work_package_card_menu_component.rb
index c93b030d452c..56e2bf1b916d 100644
--- a/modules/backlogs/app/components/backlogs/work_package_card_menu_component.rb
+++ b/modules/backlogs/app/components/backlogs/work_package_card_menu_component.rb
@@ -36,15 +36,15 @@ class WorkPackageCardMenuComponent < ApplicationComponent
include CommonHelper
include Concerns::WorkPackageMovability
- attr_reader :work_package, :project, :open_sprints_exist, :other_buckets_exist, :current_user
+ attr_reader :work_package, :project, :sprint_ids, :bucket_ids, :current_user
- def initialize(work_package:, project:, open_sprints_exist:, other_buckets_exist:, current_user: User.current)
+ def initialize(work_package:, project:, sprint_ids:, bucket_ids:, current_user: User.current)
super()
@work_package = work_package
@project = project
- @open_sprints_exist = open_sprints_exist
- @other_buckets_exist = other_buckets_exist
+ @sprint_ids = sprint_ids
+ @bucket_ids = bucket_ids
@current_user = current_user
end
@@ -62,15 +62,15 @@ def show_move_items?
end
def show_move_to_inbox?
- movable? && (work_package.sprint_id? || work_package.backlog_bucket_id?)
+ sortable?
end
def show_move_to_backlog_bucket?
- movable? && other_buckets_exist
+ sortable? && bucket_ids.any?
end
def show_move_to_sprint?
- movable? && open_sprints_exist
+ sortable? && sprint_ids.any?
end
def show_move_submenu?
@@ -106,5 +106,26 @@ def build_move_item(menu, label:, icon:, direction:)
def inbox_list_type
Backlogs::Target::InboxId.list_type
end
+
+ def sprint_list_type
+ Backlogs::Target::SprintId.list_type
+ end
+
+ def bucket_list_type
+ Backlogs::Target::BucketId.list_type
+ end
+
+ def destination_data(type, ids)
+ candidates = if type == Backlogs::Target::InboxId.list_type
+ [{ type:, id: nil }]
+ else
+ ids.map { |id| { type:, id: id.to_s } }
+ end
+
+ {
+ sortable_lists__item_target: "destinationItem",
+ sortable_lists_destinations: candidates.to_json
+ }
+ end
end
end
diff --git a/modules/backlogs/app/contracts/backlogs/work_packages/batch_move_params_contract.rb b/modules/backlogs/app/contracts/backlogs/work_packages/batch_move_params_contract.rb
index 5a714e049f6d..5ea7b6b3c22a 100644
--- a/modules/backlogs/app/contracts/backlogs/work_packages/batch_move_params_contract.rb
+++ b/modules/backlogs/app/contracts/backlogs/work_packages/batch_move_params_contract.rb
@@ -30,38 +30,12 @@
module Backlogs
module WorkPackages
- class BatchMoveParamsContract < ::ParamsContract
- validate :ids_distinct_and_present
- validate :batch_within_cap
+ class BatchMoveParamsContract < CollectionIdsContract
validate :target_resolvable
validate :predecessor_well_formed
- # BaseContract#errors returns model.errors whenever the model responds
- # to it, and project does: without this override, validating the
- # contract would clear and repopulate the live project's own error bag.
- def errors
- @errors ||= ActiveModel::Errors.new(self)
- end
-
private
- def ids
- Array(params[:ids]).map(&:to_s)
- end
-
- def ids_distinct_and_present
- return unless ids.empty? || ids.any?(&:blank?) || ids.uniq.length != ids.length
-
- errors.add(:base, I18n.t("backlogs.work_packages.move_collection.invalid_ids"))
- end
-
- def batch_within_cap
- return if ids.length <= BatchUpdateService::MAX_BATCH_SIZE
-
- errors.add(:base, I18n.t("backlogs.work_packages.move_collection.too_many_work_packages",
- max: BatchUpdateService::MAX_BATCH_SIZE))
- end
-
def target_resolvable
return if Backlogs::Target.from_list(params[:list_type], params[:list_id])
diff --git a/modules/backlogs/app/contracts/backlogs/work_packages/collection_ids_contract.rb b/modules/backlogs/app/contracts/backlogs/work_packages/collection_ids_contract.rb
new file mode 100644
index 000000000000..d9e9b3068ac6
--- /dev/null
+++ b/modules/backlogs/app/contracts/backlogs/work_packages/collection_ids_contract.rb
@@ -0,0 +1,64 @@
+# 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.
+#++
+
+module Backlogs
+ module WorkPackages
+ class CollectionIdsContract < ::ParamsContract
+ validate :ids_distinct_and_present
+ validate :batch_within_cap
+
+ # BaseContract#errors returns model.errors whenever the model responds
+ # to it, and project does: without this override, validating the
+ # contract would clear and repopulate the live project's own error bag.
+ def errors
+ @errors ||= ActiveModel::Errors.new(self)
+ end
+
+ private
+
+ def ids
+ Array(params[:ids]).map(&:to_s)
+ end
+
+ def ids_distinct_and_present
+ return unless ids.empty? || ids.any?(&:blank?) || ids.uniq.length != ids.length
+
+ errors.add(:base, I18n.t("backlogs.work_packages.move_collection.invalid_ids"))
+ end
+
+ def batch_within_cap
+ return if ids.length <= BatchUpdateService::MAX_BATCH_SIZE
+
+ errors.add(:base, I18n.t("backlogs.work_packages.move_collection.too_many_work_packages",
+ max: BatchUpdateService::MAX_BATCH_SIZE))
+ end
+ end
+ end
+end
diff --git a/modules/backlogs/app/controllers/backlogs/work_packages_controller.rb b/modules/backlogs/app/controllers/backlogs/work_packages_controller.rb
index 385097cff9ac..bd64f4bb23bb 100644
--- a/modules/backlogs/app/controllers/backlogs/work_packages_controller.rb
+++ b/modules/backlogs/app/controllers/backlogs/work_packages_controller.rb
@@ -36,15 +36,16 @@ class WorkPackagesController < BaseController
# split view open on the moved work package (see split-view-sync.controller.ts).
WORK_PACKAGE_MOVED_EVENT = "#{OpTurbo::ComponentStream::DISPATCHED_EVENT_PREFIX}backlogs:work-package-moved".freeze
- before_action :load_work_package, only: %i[menu move_to_sprint_dialog move_to_bucket_dialog move]
+ before_action :load_work_package, only: %i[menu move]
# Deferred ActionMenu items (Primer include-fragment).
def menu
+ availability = destination_availability([@work_package])
render(Backlogs::WorkPackageCardMenuComponent.new(
project: @project,
work_package: @work_package,
- open_sprints_exist: target_open_sprints.exists?,
- other_buckets_exist: target_buckets.exists?,
+ sprint_ids: availability.candidate_sprints.map(&:id),
+ bucket_ids: availability.candidate_buckets.map(&:id),
current_user:
),
layout: false)
@@ -60,27 +61,45 @@ def add_existing_dialog
container:
)
elsif target
- render_error_flash_message_via_turbo_stream(message: t(".target_not_found"))
+ render_error_flash_message_via_turbo_stream(
+ message: I18n.t("backlogs.work_packages.add_existing_dialog.target_not_found")
+ )
respond_with_turbo_streams(status: :not_found)
else
- render_error_flash_message_via_turbo_stream(message: t(".invalid_target"))
+ render_error_flash_message_via_turbo_stream(message: I18n.t("backlogs.work_packages.add_existing_dialog.invalid_target"))
respond_with_turbo_streams(status: :unprocessable_entity)
end
end
def move_to_sprint_dialog
+ work_packages = load_collection_work_packages
+ return if performed?
+
+ sprints = destination_availability(work_packages).sprints
+ if sprints.empty?
+ return render_move_collection_error(I18n.t("backlogs.work_packages.move_to_sprint_dialog.no_available_destinations"))
+ end
+
respond_with_dialog Backlogs::MoveToSprintDialogComponent.new(
- work_package: @work_package,
- project: @project,
- move_action: move_path
+ work_packages:,
+ sprints:,
+ move_action: move_collection_path
)
end
def move_to_bucket_dialog
+ work_packages = load_collection_work_packages
+ return if performed?
+
+ buckets = destination_availability(work_packages).buckets
+ if buckets.empty?
+ return render_move_collection_error(I18n.t("backlogs.work_packages.move_to_bucket_dialog.no_available_destinations"))
+ end
+
respond_with_dialog Backlogs::MoveToBucketDialogComponent.new(
- work_package: @work_package,
- project: @project,
- move_action: move_path
+ work_packages:,
+ buckets:,
+ move_action: move_collection_path
)
end
@@ -126,7 +145,9 @@ def move_collection # rubocop:disable Metrics/AbcSize
return render_move_collection_error(contract.errors.full_messages.join(" ")) unless contract.valid?
work_packages = find_collection_work_packages(move_collection_params[:ids])
- return render_move_collection_error(t(".work_packages_not_found")) unless work_packages
+ unless work_packages
+ return render_move_collection_error(I18n.t("backlogs.work_packages.move_collection.work_packages_not_found"))
+ end
# Snapshot before the call: move_after reloads mid-method and destroys
# dirty tracking, exactly as the member action's comment explains.
@@ -156,7 +177,7 @@ def render_update_collection_turbo_streams(call)
WORK_PACKAGE_MOVED_EVENT,
detail: { work_package_ids: call.result.map(&:id) }
)
- render_invisible_after_move_batch_flash(call.result)
+ render_collection_move_feedback(call.result)
else
render_error_flash_message_via_turbo_stream(
message: I18n.t(:notice_unsuccessful_update_with_reason, reason: batch_failure_reason(call))
@@ -166,6 +187,15 @@ def render_update_collection_turbo_streams(call)
respond_with_turbo_streams(status: call)
end
+ def render_collection_move_feedback(results)
+ invisible = invisible_after_move(results)
+ if invisible.any?
+ render_invisible_after_move_batch_flash(invisible)
+ elsif !optimistic_move?
+ render_collection_move_announcement(results)
+ end
+ end
+
# A member failure is reported with the member: the batch's own message
# is empty then, and the flash would not say which work package refused.
def batch_failure_reason(call)
@@ -175,8 +205,8 @@ def batch_failure_reason(call)
work_package = failed.result
return failed.message unless work_package.is_a?(WorkPackage)
- t("backlogs.work_packages.move_collection.member_failed",
- work_package: work_package.to_fs(:caption), reason: failed.message)
+ I18n.t("backlogs.work_packages.move_collection.member_failed",
+ work_package: work_package.to_fs(:caption), reason: failed.message)
end
def optimistic_same_list_batch_move?(call, source_targets)
@@ -184,21 +214,20 @@ def optimistic_same_list_batch_move?(call, source_targets)
destination = Backlogs::Target.for_work_package(call.result.first)
call.result.all? { |wp| source_targets[wp.id] == destination } &&
- requested_block_honored?(call.result)
+ requested_block_honored?(call.result, move_arguments: move_collection_params)
end
- # The batch form of requested_anchor_honored?: the first member sits where
- # the request anchored it and every further member directly below its
- # predecessor, which is the client's optimistic block.
- def requested_block_honored?(results) # rubocop:disable Metrics/AbcSize
- return false unless move_collection_params.key?(:prev_id)
+ # The first member occupies the requested anchor and any further members
+ # follow contiguously. This check serves both member and batch moves.
+ def requested_block_honored?(results, move_arguments:)
+ return false unless move_arguments.key?(:prev_id)
# One anchor query; the rest of the block is checked in memory.
# BatchUpdateService reloads every moved member before returning and
# the members share one target scope, so adjacent positions prove
# adjacency. A gap from elsewhere fails this check falsely, degrading
# to the full frame reload rather than skipping a needed one.
- prev_id = move_collection_params[:prev_id].presence
+ prev_id = move_arguments[:prev_id].presence
first = results.first
anchor_honored = prev_id ? first.higher_item&.id == prev_id.to_i : first.higher_item.nil?
@@ -232,19 +261,15 @@ def render_update_turbo_streams(call)
end
def render_invisible_after_move_flash(work_package)
- return unless work_package_invisible_after_move?(work_package)
-
- render_flash_message_via_turbo_stream(
- message: I18n.t(:notice_work_package_invisible_after_move, count: 1, backlog: target_list_name(work_package))
- )
+ invisible = invisible_after_move([work_package])
+ render_invisible_after_move_batch_flash(invisible) if invisible.any?
end
- # A member's own type or status can hide it independently of its
- # list-mates, so the first member cannot answer this for the batch.
- def render_invisible_after_move_batch_flash(results)
- invisible = results.select { |wp| work_package_invisible_after_move?(wp) }
- return if invisible.empty?
+ def invisible_after_move(results)
+ results.select { |wp| work_package_invisible_after_move?(wp) }
+ end
+ def render_invisible_after_move_batch_flash(invisible)
render_flash_message_via_turbo_stream(message: invisible_after_move_batch_message(invisible))
end
@@ -272,7 +297,7 @@ def render_move_announcement(work_package)
render_live_region_update_message(
message: t(
- ".moved_announcement",
+ "backlogs.work_packages.move.moved_announcement",
label: work_package.to_fs(:caption),
list: target_list_name(work_package),
position: index + 1,
@@ -281,6 +306,29 @@ def render_move_announcement(work_package)
)
end
+ def render_collection_move_announcement(work_packages)
+ return render_move_announcement(work_packages.first) if work_packages.one?
+
+ scope_ids = announcement_list_scope(work_packages.first).pluck(:id)
+ first = scope_ids.index(work_packages.first.id)
+ return unless first
+
+ render_live_region_update_message(
+ message: collection_move_announcement_message(work_packages, first:, total: scope_ids.size)
+ )
+ end
+
+ def collection_move_announcement_message(work_packages, first:, total:)
+ I18n.t(
+ "backlogs.work_packages.move_collection.moved_announcement",
+ count: work_packages.size,
+ list: target_list_name(work_packages.first),
+ first: first + 1,
+ last: first + work_packages.size,
+ total:
+ )
+ end
+
# The scopes the page renders, so the announced position matches what a
# sighted user sees: sprints are scoped to the project (shared sprints
# render only the project's items), buckets and the inbox go through the
@@ -301,15 +349,28 @@ def target_list_name(work_package)
end
def load_work_package
- @work_packages = WorkPackage.visible.where(project: @project).order_by_position
- @work_package = @work_packages.find(params.expect(:id))
+ @work_package = WorkPackage.visible.where(project: @project).find(params.expect(:id))
+ end
+
+ # The dialogs validate the id list alone: they open before a destination
+ # is chosen, so BatchMoveParamsContract, which also requires a resolvable
+ # target, cannot speak for them. Renders its own error and returns nil,
+ # so callers test `performed?`.
+ def load_collection_work_packages
+ ids = move_collection_params[:ids]
+
+ contract = Backlogs::WorkPackages::CollectionIdsContract.new(@project, current_user, params: { ids: })
+ return render_move_collection_error(contract.errors.full_messages.join(" ")) unless contract.valid?
+
+ find_collection_work_packages(ids) ||
+ render_move_collection_error(I18n.t("backlogs.work_packages.move_collection.work_packages_not_found"))
end
# Every submitted id must resolve to a distinct, visible work package of
# this project, in the submitted order: silently dropping a member would
# break the client's optimistic block. Nil when any id does not resolve.
def find_collection_work_packages(ids)
- found = WorkPackage.visible.where(project: @project, id: ids).index_by { |wp| wp.id.to_s }
+ found = WorkPackage.visible.where(project: @project, id: ids).includes(:type, :status).index_by { |wp| wp.id.to_s }
ordered = ids.map { |id| found[id.to_s] }
ordered.any?(&:nil?) ? nil : ordered
@@ -322,6 +383,14 @@ def render_move_collection_error(reason)
respond_with_turbo_streams(status: :unprocessable_entity)
end
+ def destination_availability(work_packages)
+ Backlogs::WorkPackages::DestinationAvailability.new(
+ project: @project,
+ user: current_user,
+ work_packages:
+ )
+ end
+
# params.expect guarantees a present, non-empty array of scalar ids; the
# optional placement and target fields go through permit instead.
def move_collection_params
@@ -331,19 +400,8 @@ def move_collection_params
end
end
- def move_path
- move_project_backlogs_work_package_path(@project, @work_package, backlog_filter_params)
- end
-
- def target_open_sprints
- Sprint.for_project(@project)
- .visible.not_completed
- .where.not(id: @work_package.sprint_id)
- end
-
- def target_buckets
- BacklogBucket.where(project: @project)
- .where.not(id: @work_package.backlog_bucket_id)
+ def move_collection_path
+ move_project_backlogs_work_packages_path(@project, backlog_filter_params)
end
# After a move the work package might no longer be visible: the page's active
@@ -386,14 +444,8 @@ def optimistic_same_list_move?(call, source_target)
# so they reconcile via reload.
def requested_anchor_honored?(work_package)
return false if move_params[:position].present?
- return false unless move_params.key?(:prev_id)
- prev_id = move_params[:prev_id].presence
- if prev_id
- work_package.higher_item&.id == prev_id.to_i
- else
- work_package.higher_item.nil?
- end
+ requested_block_honored?([work_package], move_arguments: move_params)
end
# Kept out of move_params: the service's keyword args reject it.
diff --git a/modules/backlogs/app/models/backlog_bucket.rb b/modules/backlogs/app/models/backlog_bucket.rb
index 99fe65b818b5..ac40649489bd 100644
--- a/modules/backlogs/app/models/backlog_bucket.rb
+++ b/modules/backlogs/app/models/backlog_bucket.rb
@@ -47,7 +47,7 @@ class BacklogBucket < ApplicationRecord
scope :order_alphabetically, -> { order(:name) }
- scopes :visible
+ scopes :visible, :assignable
validates :name, :project, presence: true
diff --git a/modules/backlogs/app/models/backlog_buckets/scopes/assignable.rb b/modules/backlogs/app/models/backlog_buckets/scopes/assignable.rb
new file mode 100644
index 000000000000..a03d36a37f7b
--- /dev/null
+++ b/modules/backlogs/app/models/backlog_buckets/scopes/assignable.rb
@@ -0,0 +1,39 @@
+# 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.
+#++
+
+module BacklogBuckets::Scopes::Assignable
+ extend ActiveSupport::Concern
+
+ class_methods do
+ def assignable(project:, user: User.current)
+ for_project(project).visible(user)
+ end
+ end
+end
diff --git a/modules/backlogs/app/models/backlogs/target.rb b/modules/backlogs/app/models/backlogs/target.rb
index f0983db39c7a..c2e3f19ec446 100644
--- a/modules/backlogs/app/models/backlogs/target.rb
+++ b/modules/backlogs/app/models/backlogs/target.rb
@@ -41,7 +41,10 @@ def name = I18n.t(:label_inbox)
end.new
SprintId = Data.define(:id) do
- def list_type = "sprint"
+ def self.list_type = "sprint"
+ delegate :list_type, to: :class
+
+ def lock_record = Sprint.find_by(id:)
def list_id = id
@@ -57,7 +60,10 @@ def container_for(project) = Sprint.for_project(project).visible.find_by(id:)
end
BucketId = Data.define(:id) do
- def list_type = "backlog_bucket"
+ def self.list_type = "backlog_bucket"
+ delegate :list_type, to: :class
+
+ def lock_record = BacklogBucket.find_by(id:)
def list_id = id
@@ -75,6 +81,8 @@ def container_for(project) = BacklogBucket.for_project(project).visible.find_by(
InboxId = Data.define do
def list_type = "inbox"
+ def lock_record = nil
+
def list_id = nil
def to_list_params = { list_type: }
diff --git a/modules/backlogs/app/services/backlogs/work_packages/batch_move_locks.rb b/modules/backlogs/app/services/backlogs/work_packages/batch_move_locks.rb
new file mode 100644
index 000000000000..61c5eafab5e2
--- /dev/null
+++ b/modules/backlogs/app/services/backlogs/work_packages/batch_move_locks.rb
@@ -0,0 +1,80 @@
+# 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.
+#++
+
+class Backlogs::WorkPackages::BatchMoveLocks
+ def initialize(project:, source_targets:, target:)
+ @project = project
+ @source_targets = source_targets
+ @destination = target.lock_record
+ end
+
+ # Source locks prevent moving members out after FinishService enumerates them.
+ # Ordering every source and target by [class, id] prevents deadlocks between
+ # batches with inverse source/target pairs.
+ def acquire_lifecycle
+ records = @source_targets.uniq.map(&:lock_record) + [@destination]
+ ordered = records.compact.uniq { |record| lifecycle_identity(record) }.sort_by { |record| lifecycle_identity(record) }
+ acquire_ordered(ordered)
+ end
+
+ def acquire_placement(target:, prev_id:)
+ return if prev_id.present? && target != Backlogs::Target::InboxId
+
+ suffix = ["backlogs_batch_update_destination", target.list_type, target.list_id].compact.join("_")
+ # rubocop:disable-next Lint/EmptyBlock -- transaction-scoped lock outlives this block
+ OpenProject::Mutex.with_advisory_lock_transaction(@project, suffix) {}
+ end
+
+ def acquire_members(work_packages:, anchor:)
+ acquire_ordered((work_packages + [anchor]).compact.uniq.sort_by(&:id))
+ end
+
+ def lock_destination_row
+ @destination&.lock!
+ rescue ActiveRecord::RecordNotFound
+ nil
+ end
+
+ private
+
+ def lifecycle_identity(record)
+ [record.class.name, record.id]
+ end
+
+ # Transaction-scoped locks survive these empty blocks. The gem forgets its
+ # per-thread lock entry, so inner services may request it again; PostgreSQL
+ # grants the transaction its already-held lock without waiting.
+ def acquire_ordered(records)
+ records.each do |record|
+ # rubocop:disable-next Lint/EmptyBlock -- transaction-scoped lock outlives this block
+ OpenProject::Mutex.with_advisory_lock_transaction(record) {}
+ end
+ end
+end
diff --git a/modules/backlogs/app/services/backlogs/work_packages/batch_update_service.rb b/modules/backlogs/app/services/backlogs/work_packages/batch_update_service.rb
index 29d9d4869496..3324bfc6977a 100644
--- a/modules/backlogs/app/services/backlogs/work_packages/batch_update_service.rb
+++ b/modules/backlogs/app/services/backlogs/work_packages/batch_update_service.rb
@@ -58,7 +58,7 @@ def initialize(user:, work_packages:)
def initial_prev_id = anchor ? anchor.id.to_s : ""
end
- def call(list_type: nil, list_id: nil, prev_id: nil) # rubocop:disable Metrics/AbcSize
+ def call(list_type: nil, list_id: nil, prev_id: nil) # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity
return empty_batch_failure if work_packages.empty?
contract = Backlogs::WorkPackages::BatchMoveParamsContract.new(
@@ -80,6 +80,12 @@ def call(list_type: nil, list_id: nil, prev_id: nil) # rubocop:disable Metrics/A
[work_package.id, Backlogs::Target.for_work_package(work_package)]
end
+ preflight = Backlogs::WorkPackages::DestinationAvailability.new(project: batch_project, user:, work_packages: [])
+ return unavailable_target_failure unless preflight.manage_permission? && preflight.candidate?(target)
+
+ # Each call needs fresh under-lock state, including when this instance is reused.
+ @destination_availability = nil
+
move_batch_in_transaction(target, prev_id, list_type:, list_id:)
end
@@ -103,17 +109,20 @@ def move_batch_in_transaction(target, prev_id, list_type:, list_id:)
end
def move_batch(target, prev_id, list_type:, list_id:)
- destination = raw_destination(target)
- acquire_ordered_locks(ordered_lifecycle_records(destination))
- acquire_placement_serialization_lock(target, prev_id)
+ locks = Backlogs::WorkPackages::BatchMoveLocks.new(
+ project: batch_project, source_targets: @batch_source_targets.values, target:
+ )
+ locks.acquire_lifecycle
+ locks.acquire_placement(target:, prev_id:)
placement = resolve_placement(target, prev_id)
return placement if placement.is_a?(ServiceResult)
- acquire_ordered_locks(lock_entries(placement.anchor))
+ locks.acquire_members(work_packages:, anchor: placement.anchor)
return stale_batch_failure unless cohort_intact?
- lock_destination_row!(destination)
- return unavailable_target_failure unless target_available?(target)
+ locks.lock_destination_row
+ destination_failure = revalidate_destination(target)
+ return destination_failure if destination_failure
anchor_failure = revalidate_anchor(placement, target)
return anchor_failure if anchor_failure
@@ -149,76 +158,6 @@ def move_members(placement, list_type:, list_id:) # rubocop:disable Metrics/AbcS
call
end
- # Sprint lifecycle services serialize on the Sprint model mutex before
- # enumerating and moving their work packages, so a batch has to join every
- # source lifecycle as well as the target's: locking only the target lets a
- # batch move a member out after FinishService has enumerated it. Sorted by
- # class and id, so two batches with inverse source/target pairs cannot
- # deadlock.
- def ordered_lifecycle_records(destination)
- source_records = @batch_source_targets.values.uniq.filter_map { |target| raw_destination(target) }
-
- (source_records + [destination])
- .compact
- .uniq { |record| lifecycle_lock_identity(record) }
- .sort_by { |record| lifecycle_lock_identity(record) }
- end
-
- def lifecycle_lock_identity(record)
- [record.class.name, record.id]
- end
-
- # Inbox has no lifecycle mutex. Even distinct explicit anchors share its
- # position scope, so their positions must not shift between read and insert.
- # Unanchored placement also needs serialization when the target is empty.
- def acquire_placement_serialization_lock(target, prev_id)
- return if prev_id.present? && target != Backlogs::Target::InboxId
-
- suffix = ["backlogs_batch_update_destination", target.list_type, target.list_id].compact.join("_")
- # rubocop:disable-next Lint/EmptyBlock -- the lock outlives the block; see acquire_ordered_locks
- OpenProject::Mutex.with_advisory_lock_transaction(batch_project, suffix) {}
- end
-
- # Ascending id order, so two overlapping batches request the same lock
- # sequence and neither waits on the other while holding one.
- def lock_entries(anchor)
- (work_packages + [anchor]).compact.uniq.sort_by(&:id)
- end
-
- # Transaction-scoped locks outlive their block until the enclosing
- # transaction ends, so each one is taken with an empty block in one flat
- # sequence. The gem's per-thread lock stack forgets the lock at block exit,
- # so the inner services re-request theirs; Postgres grants a lock the
- # session already holds without waiting.
- def acquire_ordered_locks(entries)
- entries.each do |entry|
- # rubocop:disable-next Lint/EmptyBlock -- the lock outlives the block; see the comment above
- OpenProject::Mutex.with_advisory_lock_transaction(entry) {}
- end
- end
-
- # Unscoped by policy so completion, deletion and reassignment all resolve
- # to the same advisory identity.
- def raw_destination(target)
- case target
- in Backlogs::Target::SprintId
- Sprint.find_by(id: target.list_id)
- in Backlogs::Target::BucketId
- BacklogBucket.find_by(id: target.list_id)
- in Backlogs::Target::InboxId
- nil
- end
- end
-
- # lock! reloads under FOR UPDATE, so a concurrent completion, deletion or
- # reassignment commits before the availability query runs. Inbox has no
- # destination row; its placement is serialized by the advisory lock alone.
- def lock_destination_row!(destination)
- destination&.lock!
- rescue ActiveRecord::RecordNotFound
- nil
- end
-
# The anchor is scoped to the batch project because the acts_as_list scope
# includes project_id: in a shared sprint another project's work package
# would pass a container-only comparison, yet be unresolvable for
@@ -251,34 +190,50 @@ def cohort_intact?
# project, same list, and for append still the last non-batch member. A
# concurrently moved anchor would otherwise fall through to move_after's
# silent insert-at-top.
- def revalidate_anchor(placement, target) # rubocop:disable Metrics/AbcSize
+ def revalidate_anchor(placement, target)
anchor = placement.anchor
+ return stale_predecessor_failure if append_anchor_changed?(placement, target)
return if anchor.nil?
anchor.reload
unless anchor.project_id == batch_project_id && Backlogs::Target.for_work_package(anchor) == target
- return stale_predecessor_failure
+ stale_predecessor_failure
end
-
- stale_predecessor_failure if placement.mode == :append && last_non_batch_member(target)&.id != anchor.id
rescue ActiveRecord::RecordNotFound
stale_predecessor_failure
end
+ def append_anchor_changed?(placement, target)
+ placement.mode == :append && last_non_batch_member(target)&.id != placement.anchor&.id
+ end
+
# The contract only revalidates a sprint or bucket target when the
# corresponding column changes, so a same-list reorder never triggers it
# and a sprint completed after the page loaded stays an accepted
- # destination. Mirrors the contract's own assignable_sprints and
- # backlog_bucket_belongs_to_project checks for every placement mode alike.
+ # destination. Judged on freshly loaded rows rather than the batch's
+ # loaded instances: a member's status, and so its mobility, may have
+ # changed since the controller loaded it, which is exactly what this check
+ # under the lock exists to catch.
+ def revalidate_destination(target)
+ return unavailable_target_failure unless destination_availability.manage_permission? && target_available?(target)
+
+ refused = destination_availability.refusing(target)
+ refused_members_failure(refused) if refused.any?
+ end
+
def target_available?(target)
- case target
- in Backlogs::Target::SprintId
- Sprint.assignable(project: batch_project, user:).exists?(id: target.list_id)
- in Backlogs::Target::BucketId
- BacklogBucket.for_project(batch_project).exists?(id: target.list_id)
- in Backlogs::Target::InboxId
- true
- end
+ destination_availability.candidate?(target)
+ end
+
+ # Freshly loaded rows, not the batch's loaded instances: a member's status,
+ # and so its mobility, may have changed since the controller loaded it,
+ # which is exactly what this check under the lock exists to catch.
+ def destination_availability
+ @destination_availability ||= Backlogs::WorkPackages::DestinationAvailability.new(
+ project: batch_project,
+ user:,
+ work_packages: WorkPackage.where(id: work_packages.map(&:id)).includes(:status).to_a
+ )
end
def last_non_batch_member(target)
@@ -319,6 +274,21 @@ def unavailable_target_failure
ServiceResult.failure(message: I18n.t("backlogs.work_packages.batch_update_service.unavailable_target"))
end
+ # Refused whole, before any member moves, but naming the members that
+ # refused rather than leaving the caller to guess.
+ def refused_members_failure(members)
+ failure = unavailable_target_failure
+ members.each do |member|
+ failure.add_dependent!(
+ ServiceResult.failure(
+ result: member,
+ message: I18n.t("backlogs.work_packages.batch_update_service.unavailable_target")
+ )
+ )
+ end
+ failure
+ end
+
def mixed_projects_failure
ServiceResult.failure(message: I18n.t("backlogs.work_packages.batch_update_service.mixed_projects"))
end
diff --git a/modules/backlogs/app/services/backlogs/work_packages/destination_availability.rb b/modules/backlogs/app/services/backlogs/work_packages/destination_availability.rb
new file mode 100644
index 000000000000..ea55d77efa8d
--- /dev/null
+++ b/modules/backlogs/app/services/backlogs/work_packages/destination_availability.rb
@@ -0,0 +1,87 @@
+# 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.
+#++
+
+class Backlogs::WorkPackages::DestinationAvailability
+ attr_reader :project, :user, :work_packages
+
+ def initialize(project:, user:, work_packages:)
+ @project = project
+ @user = user
+ @work_packages = work_packages
+ end
+
+ def manage_permission?
+ return @manage_permission if defined?(@manage_permission)
+
+ @manage_permission = user.allowed_in_project?(:manage_sprint_items, project)
+ end
+
+ def candidate_sprints
+ @candidate_sprints ||= Sprint.assignable(project:, user:).order_by_date.to_a
+ end
+
+ def candidate_buckets
+ @candidate_buckets ||= BacklogBucket.assignable(project:, user:).to_a
+ end
+
+ def candidate?(target)
+ candidate_targets.include?(target)
+ end
+
+ def refusing(target)
+ work_packages.reject { |work_package| accepts?(work_package, target) }
+ end
+
+ def sprints = offered(candidate_sprints)
+ def buckets = offered(candidate_buckets)
+
+ private
+
+ def candidate_targets
+ @candidate_targets ||= Set.new(
+ [Backlogs::Target::InboxId] +
+ (candidate_sprints + candidate_buckets).map { |container| Backlogs::Target.for(container) }
+ )
+ end
+
+ def accepts?(work_package, target)
+ !work_package.readonly_status? || Backlogs::Target.for_work_package(work_package) == target
+ end
+
+ def offered(containers)
+ return [] unless manage_permission?
+
+ containers.select do |container|
+ target = Backlogs::Target.for(container)
+ refusing(target).empty? &&
+ work_packages.any? { |wp| Backlogs::Target.for_work_package(wp) != target }
+ end
+ end
+end
diff --git a/modules/backlogs/app/views/backlogs/backlog/show.html.erb b/modules/backlogs/app/views/backlogs/backlog/show.html.erb
index a39f9f54998c..53ec6aa67c02 100644
--- a/modules/backlogs/app/views/backlogs/backlog/show.html.erb
+++ b/modules/backlogs/app/views/backlogs/backlog/show.html.erb
@@ -51,7 +51,10 @@ See COPYRIGHT and LICENSE files for more details.
data: {
controller: "backlogs--list-refresh backlogs--split-view-sync sortable-lists",
sortable_lists_optimistic_value: true,
- action: "#{Backlogs::WorkPackagesController::WORK_PACKAGE_MOVED_EVENT}@document->backlogs--split-view-sync#onWorkPackageMoved",
+ action: [
+ "#{Backlogs::WorkPackagesController::WORK_PACKAGE_MOVED_EVENT}@document->backlogs--split-view-sync#onWorkPackageMoved",
+ "#{Backlogs::WorkPackagesController::WORK_PACKAGE_MOVED_EVENT}@document->sortable-lists#clearSelectionAfterMove"
+ ].join(" "),
sortable_lists_move_url_template_value: backlogs_move_url_template(@project),
sortable_lists_collection_move_url_value: move_project_backlogs_work_packages_path(@project),
sortable_lists_max_batch_size_value: Backlogs::WorkPackages::BatchUpdateService::MAX_BATCH_SIZE,
diff --git a/modules/backlogs/config/locales/en.yml b/modules/backlogs/config/locales/en.yml
index 3dce5e68e083..a23915ee5d6a 100644
--- a/modules/backlogs/config/locales/en.yml
+++ b/modules/backlogs/config/locales/en.yml
@@ -208,6 +208,12 @@ en:
rebuild: "Rebuild"
rebuild_positions: "Rebuild positions"
remaining_hours: "Remaining work"
+
+ selected_work_packages_component:
+ label:
+ one: "1 selected work package"
+ other: "%{count} selected work packages"
+
sharing: "Sprint sharing"
show_burndown_chart: "Burndown chart"
@@ -295,9 +301,16 @@ en:
unexpected_failure: "The work packages could not be moved. Please try again."
move:
moved_announcement: "%{label} moved to %{list}, position %{position} of %{total}"
+ move_to_bucket_dialog:
+ no_available_destinations: "There are no available backlog buckets for these work packages."
+ move_to_sprint_dialog:
+ no_available_destinations: "There are no available sprints for these work packages."
move_collection:
invalid_ids: "The list of work packages to move is invalid."
member_failed: "%{work_package}: %{reason}"
+ moved_announcement:
+ one: "%{count} work package moved to %{list}, position %{first} of %{total}"
+ other: "%{count} work packages moved to %{list}, positions %{first} through %{last} of %{total}"
too_many_work_packages: "No more than %{max} work packages can be moved at once."
work_packages_not_found: "At least one work package could not be found in this project."
update_service:
diff --git a/modules/backlogs/config/routes.rb b/modules/backlogs/config/routes.rb
index 47c094093502..9c221379021a 100644
--- a/modules/backlogs/config/routes.rb
+++ b/modules/backlogs/config/routes.rb
@@ -99,14 +99,14 @@
collection do
get :add_existing_dialog
post :add_existing
+ post :move_to_sprint_dialog
+ post :move_to_bucket_dialog
put :move, action: :move_collection
end
member do
get :menu
put :move
- get :move_to_sprint_dialog
- get :move_to_bucket_dialog
end
end
diff --git a/modules/backlogs/spec/components/backlogs/move_to_bucket_dialog_component_spec.rb b/modules/backlogs/spec/components/backlogs/move_to_bucket_dialog_component_spec.rb
index 6af17413abea..6d85db18749c 100644
--- a/modules/backlogs/spec/components/backlogs/move_to_bucket_dialog_component_spec.rb
+++ b/modules/backlogs/spec/components/backlogs/move_to_bucket_dialog_component_spec.rb
@@ -35,11 +35,14 @@
current_user { admin }
let(:project) { create(:project) }
- let(:work_package) { create(:work_package, project:) }
- let(:move_path) { Rails.application.routes.url_helpers.move_project_backlogs_work_package_path(project, work_package) }
+ let(:first) { create(:work_package, project:) }
+ let(:second) { create(:work_package, project:) }
+ let(:work_packages) { [second, first] }
+ let(:move_path) { Rails.application.routes.url_helpers.move_project_backlogs_work_packages_path(project) }
+ let(:buckets) { [create(:backlog_bucket, project:, name: "Passed Bucket")] }
def render_component
- render_inline(described_class.new(work_package:, project:, move_action: move_path))
+ render_inline(described_class.new(work_packages:, buckets:, move_action: move_path))
end
it "renders the dialog with the correct title" do
@@ -48,23 +51,30 @@ def render_component
expect(page).to have_text(I18n.t(:"backlogs.move_to_bucket_dialog_component.title"))
end
- it "renders a form targeting the move path via PUT" do
+ it "renders an ordered collection form targeting the move path via PUT", :aggregate_failures do
render_component
+ expect(page).to have_text(second.subject)
+ expect(page).to have_text(first.subject)
+ expect(page.all("input[name='ids[]']", visible: :all).map(&:value))
+ .to eq([second.id.to_s, first.id.to_s])
expect(page).to have_element(:form, action: move_path, method: "post")
- expect(page).to have_element(:input, name: "_method", value: "put", visible: :all)
+ expect(page).to have_css("form[action='#{move_path}'] input[name='_method'][value='put']", visible: :all)
+ expect(page).to have_css(
+ "input[name='list_type'][value='#{Backlogs::Target::BucketId.new(nil).list_type}']",
+ visible: :all
+ )
end
- context "when params[:all] is true" do
- let(:move_path) do
- Rails.application.routes.url_helpers.move_project_backlogs_work_package_path(project, work_package, all: "true")
- end
-
- it "submits the move form with the all query preserved" do
- render_component
+ it "describes the bucket select by the selection heading" do
+ render_component
- expect(page).to have_element(:form, action: /all=true/)
- end
+ label = page.find_css("##{described_class::SELECTION_LABEL_ID}").first
+ expect(label.text).to include(I18n.t("backlogs.selected_work_packages_component.label", count: 2))
+ expect(page).to have_css(
+ "select[name='list_id'][aria-describedby~='#{described_class::SELECTION_LABEL_ID}']",
+ visible: :all
+ )
end
it "renders Cancel and Move buttons" do
@@ -74,42 +84,14 @@ def render_component
expect(page).to have_button(I18n.t(:button_move))
end
- context "when buckets exist" do
- let!(:bucket_a) { create(:backlog_bucket, project:, name: "Alpha") }
- let!(:bucket_b) { create(:backlog_bucket, project:, name: "Beta") }
-
- it "submits backlog_bucket list data" do
- render_component
-
- expect(page).to have_css(
- "input[name='list_type'][value='#{Backlogs::Target::BucketId.new(nil).list_type}']",
- visible: :all
- )
- expect(page).to have_element(:option, value: bucket_a.id, text: "Alpha")
- expect(page).to have_element(:option, value: bucket_b.id, text: "Beta")
- end
- end
-
- context "when a bucket belongs to a different project" do
- let!(:other_bucket) { create(:backlog_bucket, project: create(:project), name: "Other") }
-
- it "does not list buckets from other projects" do
- render_component
-
- expect(page).to have_no_css(:option, text: "Other")
- end
- end
-
- context "when the work package is already in a bucket" do
- let!(:current_bucket) { create(:backlog_bucket, project:, name: "Current") }
- let!(:target_bucket) { create(:backlog_bucket, project:, name: "Target") }
- let(:work_package) { create(:work_package, project:, backlog_bucket: current_bucket) }
+ context "when other bucket records exist" do
+ let!(:omitted_bucket) { create(:backlog_bucket, project:, name: "Omitted Bucket") }
- it "excludes the current bucket from the options" do
+ it "renders only the destinations supplied by the controller" do
render_component
- expect(page).to have_no_css(:option, text: "Current")
- expect(page).to have_element(:option, value: target_bucket.id, text: "Target")
+ expect(page).to have_css("option[value='#{buckets.first.id}']", text: "Passed Bucket")
+ expect(page).to have_no_css("option[value='#{omitted_bucket.id}']")
end
end
end
diff --git a/modules/backlogs/spec/components/backlogs/move_to_sprint_dialog_component_spec.rb b/modules/backlogs/spec/components/backlogs/move_to_sprint_dialog_component_spec.rb
index 7a4703ba1d53..b66f02787b82 100644
--- a/modules/backlogs/spec/components/backlogs/move_to_sprint_dialog_component_spec.rb
+++ b/modules/backlogs/spec/components/backlogs/move_to_sprint_dialog_component_spec.rb
@@ -35,11 +35,14 @@
current_user { admin }
let(:project) { create(:project) }
- let(:work_package) { create(:work_package, project:) }
- let(:move_path) { Rails.application.routes.url_helpers.move_project_backlogs_work_package_path(project, work_package) }
+ let(:first) { create(:work_package, project:) }
+ let(:second) { create(:work_package, project:) }
+ let(:work_packages) { [second, first] }
+ let(:move_path) { Rails.application.routes.url_helpers.move_project_backlogs_work_packages_path(project) }
+ let(:sprints) { [create(:sprint, project:, name: "Passed Sprint")] }
def render_component
- render_inline(described_class.new(work_package:, project:, move_action: move_path))
+ render_inline(described_class.new(work_packages:, sprints:, move_action: move_path))
end
it "renders the dialog with the correct title" do
@@ -48,23 +51,30 @@ def render_component
expect(page).to have_text(I18n.t(:"backlogs.move_to_sprint_dialog_component.title"))
end
- it "renders a form targeting the move path via PUT" do
+ it "renders an ordered collection form targeting the move path via PUT", :aggregate_failures do
render_component
+ expect(page).to have_text(second.subject)
+ expect(page).to have_text(first.subject)
+ expect(page.all("input[name='ids[]']", visible: :all).map(&:value))
+ .to eq([second.id.to_s, first.id.to_s])
expect(page).to have_element(:form, action: move_path, method: "post")
expect(page).to have_css("form[action='#{move_path}'] input[name='_method'][value='put']", visible: :all)
+ expect(page).to have_css(
+ "input[name='list_type'][value='#{Backlogs::Target::SprintId.new(nil).list_type}']",
+ visible: :all
+ )
end
- context "when params[:all] is true" do
- let(:move_path) do
- Rails.application.routes.url_helpers.move_project_backlogs_work_package_path(project, work_package, all: "true")
- end
-
- it "submits the move form with the all query preserved" do
- render_component
+ it "describes the sprint select by the selection heading" do
+ render_component
- expect(page).to have_css("form[action*='all=true']", visible: :all)
- end
+ label = page.find_css("##{described_class::SELECTION_LABEL_ID}").first
+ expect(label.text).to include(I18n.t("backlogs.selected_work_packages_component.label", count: 2))
+ expect(page).to have_css(
+ "select[name='list_id'][aria-describedby~='#{described_class::SELECTION_LABEL_ID}']",
+ visible: :all
+ )
end
it "renders Cancel and Save buttons" do
@@ -74,65 +84,14 @@ def render_component
expect(page).to have_button(I18n.t(:button_move))
end
- context "when in_planning and active sprints exist" do
- let!(:planning_sprint) { create(:sprint, project:, name: "Planning Sprint", status: "in_planning") }
- let!(:active_sprint) { create(:sprint, project:, name: "Active Sprint", status: "active") }
-
- it "submits sprint list data" do
- render_component
-
- expect(page).to have_css(
- "input[name='list_type'][value='#{Backlogs::Target::SprintId.new(nil).list_type}']",
- visible: :all
- )
- expect(page).to have_css("option[value='#{planning_sprint.id}']", text: "Planning Sprint")
- expect(page).to have_css("option[value='#{active_sprint.id}']", text: "Active Sprint")
- end
- end
-
- context "when a completed sprint exists" do
- let!(:completed_sprint) { create(:sprint, project:, name: "Old Sprint", status: "completed") }
-
- it "does not list the completed sprint" do
- render_component
-
- expect(page).to have_no_css("option", text: "Old Sprint")
- end
- end
-
- context "when a sprint belongs to a different project" do
- let!(:other_sprint) { create(:sprint, project: create(:project), name: "Other Sprint") }
-
- it "does not list sprints from other projects" do
- render_component
-
- expect(page).to have_no_css("option", text: "Other Sprint")
- end
- end
-
- context "when the work package is already in a sprint" do
- let!(:current_sprint) { create(:sprint, project:, name: "Current Sprint") }
- let!(:target_sprint) { create(:sprint, project:, name: "Target Sprint") }
- let(:work_package) { create(:work_package, project:, sprint: current_sprint) }
-
- it "excludes that sprint from the options" do
- render_component
-
- expect(page).to have_no_css("option", text: "Current Sprint")
- expect(page).to have_css("option[value='#{target_sprint.id}']", text: "Target Sprint")
- end
- end
-
- context "when the current user cannot view sprints in the project" do
- let(:other_user) { create(:user) }
- let!(:hidden_sprint) { create(:sprint, project:, name: "Hidden Sprint") }
-
- current_user { other_user }
+ context "when other sprint records exist" do
+ let!(:omitted_sprint) { create(:sprint, project:, name: "Omitted Sprint") }
- it "does not list sprints the user is not permitted to see" do
+ it "renders only the destinations supplied by the controller" do
render_component
- expect(page).to have_no_css("option", text: "Hidden Sprint")
+ expect(page).to have_css("option[value='#{sprints.first.id}']", text: "Passed Sprint")
+ expect(page).to have_no_css("option[value='#{omitted_sprint.id}']")
end
end
end
diff --git a/modules/backlogs/spec/components/backlogs/selected_work_packages_component_spec.rb b/modules/backlogs/spec/components/backlogs/selected_work_packages_component_spec.rb
new file mode 100644
index 000000000000..621351ff5bef
--- /dev/null
+++ b/modules/backlogs/spec/components/backlogs/selected_work_packages_component_spec.rb
@@ -0,0 +1,87 @@
+# 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 "rails_helper"
+
+RSpec.describe Backlogs::SelectedWorkPackagesComponent, type: :component do
+ shared_let(:admin) { create(:admin) }
+ current_user { admin }
+
+ shared_let(:project) { create(:project) }
+ shared_let(:epic_type) { create(:type, name: "Epic") }
+ shared_let(:feature_type) { create(:type, name: "Feature") }
+ shared_let(:epic) { create(:work_package, project:, type: epic_type, subject: "Contamination model") }
+ shared_let(:feature) { create(:work_package, project:, type: feature_type, subject: "Warning system") }
+
+ let(:description_id) { "move-dialog-selection" }
+
+ def render_component(work_packages: [feature, epic])
+ render_inline(described_class.new(work_packages:, description_id:))
+ end
+
+ it "labels the box and names every work package in the given order", :aggregate_failures do
+ render_component
+
+ expect(page).to have_text(I18n.t("backlogs.selected_work_packages_component.label", count: 2))
+ expect(page.text).to match(
+ /Feature.*#{feature.formatted_id}.*Warning system.*Epic.*#{epic.formatted_id}.*Contamination model/mi
+ )
+ end
+
+ it "links each work package to its full view" do
+ render_component
+
+ expect(page).to have_link(feature.formatted_id, href: "/work_packages/#{feature.id}")
+ expect(page).to have_link(epic.formatted_id, href: "/work_packages/#{epic.id}")
+ end
+
+ it "renders the selection as a semantic list", :aggregate_failures do
+ render_component
+
+ expect(page).to have_list_item count: 2
+ expect(page).to have_list_item position: 1, text: feature.subject
+ expect(page).to have_list_item position: 2, text: epic.subject
+ end
+
+ it "heads the box with the selected count on the description element" do
+ render_component
+
+ expect(page).to have_css(
+ "h2##{description_id}",
+ text: I18n.t("backlogs.selected_work_packages_component.label", count: 2)
+ )
+ end
+
+ it "heads a single selection in the singular" do
+ render_component(work_packages: [feature])
+
+ expect(page).to have_css("h2##{description_id}", text: "1 selected work package")
+ end
+end
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 68995222ff20..c3c3766a8e8b 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
@@ -72,12 +72,14 @@ def create_story(position:, subject: "Test Story", story_points: 5)
backlog_bucket: bucket)
end
- def render_component(work_package: self.work_package, open_sprints_exist: true, other_buckets_exist: true)
+ def render_component(work_package: self.work_package,
+ sprint_ids: [sprint].compact.map(&:id),
+ bucket_ids: [bucket].compact.map(&:id))
render_inline(described_class.new(
work_package:,
project:,
- open_sprints_exist:,
- other_buckets_exist:,
+ sprint_ids:,
+ bucket_ids:,
current_user: user
))
end
@@ -188,6 +190,22 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
end
describe "move menu items" do
+ it "renders project-level destination metadata including the invoking card's current target" do
+ other_sprint = create(:sprint, project:, name: "Sprint 2", start_date: Date.yesterday, finish_date: Date.tomorrow)
+ bucket = create(:backlog_bucket, project:)
+
+ render_component(sprint_ids: [sprint.id, other_sprint.id], bucket_ids: [bucket.id])
+
+ expect(page).to have_css(
+ "li[data-sortable-lists--item-target~='destinationItem']" \
+ "[data-sortable-lists-destinations*='\"type\":\"sprint\"']"
+ )
+ expect(page).to have_css("li[data-sortable-lists-destinations*='\"id\":\"#{sprint.id}\"']")
+ expect(page).to have_css(
+ "li[data-sortable-lists-destinations*='\"type\":\"#{Backlogs::Target::InboxId.list_type}\"']"
+ )
+ end
+
it "shows Move to top item with move-to-top icon" do
render_component
@@ -217,7 +235,7 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
end
it "always renders the four move items as client targets", :aggregate_failures do
- render_inline(described_class.new(work_package:, project:, open_sprints_exist: false, other_buckets_exist: false))
+ render_inline(described_class.new(work_package:, project:, sprint_ids: [], bucket_ids: []))
# The target, direction, and action live on the item , not the content button.
%w[top up down bottom].each do |direction|
@@ -264,7 +282,7 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
end
it "still shows the Move to position submenu (permission-gated only)" do
- render_component(open_sprints_exist: false, other_buckets_exist: false)
+ render_component(sprint_ids: [], bucket_ids: [])
expect(page).to have_selector(:menuitem, text: "Move to position")
end
@@ -273,11 +291,25 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
end
describe "Move to sprint item" do
+ it "loads the collection dialog with a POST form after freezing the action scope" do
+ render_component(sprint_ids: [sprint.id])
+
+ button = page.find("button#work_package_#{work_package.id}_menu_move_to_sprint")
+ form = button.find(:xpath, "ancestor::form")
+
+ expect(button["data-controller"]).to eq("async-dialog")
+ expect(button["data-action"]).to eq("async-dialog:beforeLoad->sortable-lists--item#prepareDialog")
+ expect(form[:method]).to eq("post")
+ expect(form[:action]).to end_with(
+ "/projects/#{project.identifier}/backlogs/work_packages/move_to_sprint_dialog"
+ )
+ end
+
context "when work package is in a sprint" do
it "is shown with zap icon" do
- render_component(open_sprints_exist: true)
+ render_component(sprint_ids: [sprint.id])
- expect(page).to have_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
expect(page).to have_octicon(:zap)
expect(page).to have_text(I18n.t(:"backlogs.work_package_card_menu_component.action_menu.move_to_sprint"))
end
@@ -288,9 +320,9 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
let(:bucket) { create(:backlog_bucket, project:) }
it "is shown" do
- render_component(open_sprints_exist: true)
+ render_component(sprint_ids: [create(:sprint, project:).id])
- expect(page).to have_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
end
end
@@ -298,20 +330,34 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
let(:sprint) { nil }
it "is shown" do
- render_component(open_sprints_exist: true)
+ render_component(sprint_ids: [create(:sprint, project:).id])
- expect(page).to have_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
end
end
- it "is hidden when open_sprints_exist is false" do
- render_component(open_sprints_exist: false)
+ it "is hidden when no sprint candidates exist" do
+ render_component(sprint_ids: [])
- expect(page).to have_no_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
+ expect(page).to have_no_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
end
end
describe "Move to backlog inbox item" do
+ it "binds a direct collection move without rendering a singular member form" do
+ render_component
+
+ item = page.find(
+ "li[data-sortable-lists--item-target~='destinationItem']" \
+ "[data-sortable-lists-destinations='[{\"type\":\"#{Backlogs::Target::InboxId.list_type}\",\"id\":null}]']"
+ )
+ button = item.find("button#work_package_#{work_package.id}_menu_move_to_inbox")
+
+ expect(item["data-action"]).to eq("click->sortable-lists--item#moveToDestination:prevent")
+ expect(button[:form]).to be_nil
+ expect(page).to have_no_field("list_type", type: :hidden)
+ end
+
context "when work package is in a sprint" do
it "is shown with inbox icon" do
render_component
@@ -319,7 +365,7 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_inbox\z/)
expect(page).to have_octicon(:inbox)
expect(page).to have_text(I18n.t(:"backlogs.work_package_card_menu_component.action_menu.move_to_inbox"))
- expect(page).to have_field("list_type", type: :hidden, with: Backlogs::Target::InboxId.list_type)
+ expect(page).to have_no_field("list_type", type: :hidden)
end
end
@@ -337,20 +383,35 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
context "when work package is already in the inbox (no sprint, no bucket)" do
let(:sprint) { nil }
- it "is hidden" do
+ it "renders the inbox candidate for client-side projection" do
render_component
- expect(page).to have_no_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_inbox\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_inbox\z/)
end
end
end
describe "Move to backlog bucket item" do
+ it "loads the collection dialog with a POST form after freezing the action scope" do
+ bucket = create(:backlog_bucket, project:)
+ render_component(bucket_ids: [bucket.id])
+
+ button = page.find("button#work_package_#{work_package.id}_menu_move_to_backlog_bucket")
+ form = button.find(:xpath, "ancestor::form")
+
+ expect(button["data-controller"]).to eq("async-dialog")
+ expect(button["data-action"]).to eq("async-dialog:beforeLoad->sortable-lists--item#prepareDialog")
+ expect(form[:method]).to eq("post")
+ expect(form[:action]).to end_with(
+ "/projects/#{project.identifier}/backlogs/work_packages/move_to_bucket_dialog"
+ )
+ end
+
context "when work package is in a sprint" do
it "is shown with package icon" do
- render_component(other_buckets_exist: true)
+ render_component(bucket_ids: [create(:backlog_bucket, project:).id])
- expect(page).to have_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
expect(page).to have_octicon(:package)
expect(page).to have_text(I18n.t(:"backlogs.work_package_card_menu_component.action_menu.move_to_backlog_bucket"))
end
@@ -361,9 +422,9 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
let(:bucket) { create(:backlog_bucket, project:) }
it "is shown" do
- render_component(other_buckets_exist: true)
+ render_component(bucket_ids: [bucket.id])
- expect(page).to have_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
end
end
@@ -371,16 +432,16 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
let(:sprint) { nil }
it "is shown" do
- render_component(other_buckets_exist: true)
+ render_component(bucket_ids: [create(:backlog_bucket, project:).id])
- expect(page).to have_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
end
end
- it "is hidden when other_buckets_exist is false" do
- render_component(other_buckets_exist: false)
+ it "is hidden when no bucket candidates exist" do
+ render_component(bucket_ids: [])
- expect(page).to have_no_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
+ expect(page).to have_no_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
end
end
@@ -392,12 +453,11 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
let(:work_package) { readonly_story }
context "with an Enterprise token", with_ee: %i[readonly_work_packages] do
- it "offers no move to another container", :aggregate_failures do
+ it "renders destination candidates for client-side confined-scope projection", :aggregate_failures do
render_component
- expect(page).to have_no_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
- expect(page).to have_no_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_backlog_bucket\z/)
- expect(page).to have_no_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_inbox\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_inbox\z/)
end
it "keeps the positional moves within its own list", :aggregate_failures do
@@ -435,7 +495,7 @@ def render_component(work_package: self.work_package, open_sprints_exist: true,
render_component
expect(page).to have_selector(:menuitem, text: "Move to position")
- expect(page).to have_element(:a, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
+ expect(page).to have_element(:button, id: /\Awork_package_#{work_package.id}_menu_move_to_sprint\z/)
end
end
end
diff --git a/modules/backlogs/spec/contracts/backlogs/work_packages/collection_ids_contract_spec.rb b/modules/backlogs/spec/contracts/backlogs/work_packages/collection_ids_contract_spec.rb
new file mode 100644
index 000000000000..fd0884c0e37d
--- /dev/null
+++ b/modules/backlogs/spec/contracts/backlogs/work_packages/collection_ids_contract_spec.rb
@@ -0,0 +1,61 @@
+# 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"
+
+RSpec.describe Backlogs::WorkPackages::CollectionIdsContract, type: :model do
+ shared_let(:project) { create(:project) }
+ shared_let(:user) { create(:user) }
+
+ it "accepts the full cap without requiring a destination" do
+ ids = Array.new(Backlogs::WorkPackages::BatchUpdateService::MAX_BATCH_SIZE) { |i| (i + 1).to_s }
+ contract = described_class.new(project, user, params: { ids: })
+ expect(contract).to be_valid
+ end
+
+ [[], [""], %w[1 1]].each do |ids|
+ it "rejects invalid membership of size #{ids.size}" do
+ contract = described_class.new(project, user, params: { ids: })
+ expect(contract).not_to be_valid
+ expect(contract.errors).not_to be_empty
+ expect(project.errors).to be_empty
+ end
+ end
+
+ it "rejects one more than the cap" do
+ ids = Array.new(Backlogs::WorkPackages::BatchUpdateService::MAX_BATCH_SIZE + 1) { |i| (i + 1).to_s }
+ contract = described_class.new(project, user, params: { ids: })
+
+ expect(contract).not_to be_valid
+ expect(contract.errors[:base]).to include(I18n.t("backlogs.work_packages.move_collection.too_many_work_packages",
+ max: Backlogs::WorkPackages::BatchUpdateService::MAX_BATCH_SIZE))
+ expect(project.errors).to be_empty
+ end
+end
diff --git a/modules/backlogs/spec/controllers/backlogs/work_packages_controller_spec.rb b/modules/backlogs/spec/controllers/backlogs/work_packages_controller_spec.rb
index 308ef477d023..ea294509fb89 100644
--- a/modules/backlogs/spec/controllers/backlogs/work_packages_controller_spec.rb
+++ b/modules/backlogs/spec/controllers/backlogs/work_packages_controller_spec.rb
@@ -680,55 +680,31 @@ def turbo_stream_template(action:)
expect(body).to include(I18n.t(:"js.button_open_details"))
end
- context "when another open sprint exists" do
+ context "when assignable sprints exist" do
let!(:other_open_sprint) { create(:sprint, name: "Sprint 2", project:) }
before { allow(Backlogs::WorkPackageCardMenuComponent).to receive(:new).and_call_original }
- it "passes open_sprints_exist: true to the menu component" do
+ it "passes ordered project sprint candidates to the menu component" do
response
expect(Backlogs::WorkPackageCardMenuComponent)
.to have_received(:new)
- .with(hash_including(open_sprints_exist: true))
+ .with(hash_including(sprint_ids: Sprint.assignable(project:, user:).order_by_date.ids))
end
end
- context "when no other open sprints exist" do
- before { allow(Backlogs::WorkPackageCardMenuComponent).to receive(:new).and_call_original }
-
- it "passes open_sprints_exist: false to the menu component" do
- response
-
- expect(Backlogs::WorkPackageCardMenuComponent)
- .to have_received(:new)
- .with(hash_including(open_sprints_exist: false))
- end
- end
-
- context "when other backlog buckets exist" do
+ context "when backlog buckets exist" do
let!(:buckets) { create_list(:backlog_bucket, 2, project:) }
before { allow(Backlogs::WorkPackageCardMenuComponent).to receive(:new).and_call_original }
- it "passes other_buckets_exist: true to the menu component" do
- response
-
- expect(Backlogs::WorkPackageCardMenuComponent)
- .to have_received(:new)
- .with(hash_including(other_buckets_exist: true))
- end
- end
-
- context "when no backlog buckets exist" do
- before { allow(Backlogs::WorkPackageCardMenuComponent).to receive(:new).and_call_original }
-
- it "passes other_buckets_exist: false to the menu component" do
+ it "passes alphabetically ordered project bucket candidates to the menu component" do
response
expect(Backlogs::WorkPackageCardMenuComponent)
.to have_received(:new)
- .with(hash_including(other_buckets_exist: false))
+ .with(hash_including(bucket_ids: buckets.sort_by(&:name).map(&:id)))
end
end
@@ -1167,191 +1143,145 @@ def turbo_stream_template(action:)
end
end
- describe "GET #move_to_sprint_dialog" do
- let!(:other_sprint) { create(:sprint) }
- let!(:displayed_sprints) { create_list(:sprint, 2, project:) }
-
- let(:params) { { project_id: project.id, id: work_package.id } }
-
- subject(:response) { get :move_to_sprint_dialog, params:, format: :turbo_stream }
-
- context "with a Sprint source" do
- it "responds with a dialog turbo stream", :aggregate_failures do
- expect(response).to be_successful
- expect(response).to have_turbo_stream action: "dialog"
- end
-
- it "includes the existing sprints as list_id options" do
- subject
-
- displayed_sprints.each do |sprint|
- expect(body).to include(%(value="#{sprint.id}"))
- end
- end
-
- it "does not include the current sprint as a list_id option" do
- subject
-
- expect(body).not_to include(%(value="#{sprint.id}"))
- end
-
- it "does not include the other sprint" do
- subject
-
- expect(body).not_to include(%(value="#{other_sprint.id}"))
- end
+ describe "POST #move_to_sprint_dialog" do
+ let!(:available_sprint) { create(:sprint, project:) }
+ let!(:first) { create(:work_package, status:, project:) }
+ let!(:second) { create(:work_package, status:, project:) }
+ let(:ids) { [second.id, first.id] }
+ let(:params) { { project_id: project.id, ids: } }
+
+ subject(:response) { post :move_to_sprint_dialog, params:, format: :turbo_stream }
+
+ it "renders the ordered collection and authoritative sprints" do
+ response
+
+ expect(response).to have_turbo_stream action: "dialog"
+ document = Nokogiri::HTML.fragment(response.body)
+ expect(document.css('input[name="ids[]"]').pluck("value"))
+ .to eq [second.id.to_s, first.id.to_s]
+ expect(document.css("select option").pluck("value"))
+ .to eq [available_sprint.id.to_s]
+ expect(document.css('input[name="prev_id"]')).to be_empty
+ expect(document.css("form").pluck("action"))
+ .to include(move_project_backlogs_work_packages_path(project))
end
- context "with inbox source (no sprint_id)" do
- let(:inbox_work_package) { create(:work_package, status:, project:) }
- let(:params) { { project_id: project.id, id: inbox_work_package.id } }
-
- it "responds with a dialog turbo stream", :aggregate_failures do
- expect(response).to be_successful
- expect(response).to have_turbo_stream action: "dialog"
- end
-
- it "embeds the no-sprint work_packages path in the dialog form action URL" do
- expect(body).to include("backlogs/work_packages/#{inbox_work_package.id}/move")
- expect(body).not_to include("sprints")
- end
-
- it "includes the existing sprints as list_id options" do
- subject
-
- displayed_sprints.each do |sprint|
- expect(body).to include(%(value="#{sprint.id}"))
+ context "with several distinct card types and statuses" do
+ let!(:stories) do
+ Array.new(5) do |index|
+ create(:work_package, project:,
+ type: create(:type, name: "Dialog type #{index}"),
+ status: create(:status, name: "Dialog status #{index}"))
end
end
+ let(:ids) { stories.map(&:id).reverse }
- it "does not include the other sprint" do
- subject
-
- expect(body).not_to include(%(value="#{other_sprint.id}"))
- end
- end
-
- context "with a Backlog bucket source" do
- let(:bucket) { create(:backlog_bucket, project:) }
- let(:bucket_work_package) { create(:work_package, status:, project:, backlog_bucket: bucket) }
- let(:params) { { project_id: project.id, id: bucket_work_package.id } }
+ it "loads card type and policy status in one query each" do
+ recorder = ActiveRecord::QueryRecorder.new { response }
- it "responds with a dialog turbo stream", :aggregate_failures do
- expect(response).to be_successful
expect(response).to have_turbo_stream action: "dialog"
- end
-
- it "includes the available sprints in the dialog" do
- displayed_sprints.each do |sprint|
- expect(body).to include(%(value="#{sprint.id}"))
- end
- end
-
- it "does not include sprints from other projects" do
- subject
-
- expect(body).not_to include(%(value="#{other_sprint.id}"))
+ expect(recorder.log.grep(/FROM "types"/).size).to eq 1
+ expect(recorder.log.grep(/FROM "statuses"/).size).to eq 1
+ document = Nokogiri::HTML.fragment(response.body)
+ expect(document.css('input[name="ids[]"]').pluck("value")).to eq ids.map(&:to_s)
end
end
- context "when all=true is in params" do
- let(:params) { { project_id: project.id, id: work_package.id, all: "true" } }
-
- it "embeds the all query in the dialog form action URL" do
- expect(body).to include("all=true")
+ shared_examples "rejects invalid dialog ids" do
+ it "returns a 422 Turbo Stream without opening a dialog", :aggregate_failures do
+ expect(response).to have_http_status :unprocessable_entity
+ expect(response).to have_turbo_stream action: "flash", target: "op-primer-flash-component"
+ expect(response).not_to have_turbo_stream action: "dialog"
end
end
- context "with a user lacking manage_sprint_items permission" do
- let(:user) { create(:user, member_with_permissions: { project => %i[view_sprints view_work_packages] }) }
+ context "with a blank id" do
+ let(:ids) { [second.id, ""] }
- it "responds with 403" do
- expect(response).to have_http_status :forbidden
- end
+ it_behaves_like "rejects invalid dialog ids"
end
- context "with a user lacking project permission" do
- let(:user) { create(:user) }
+ context "with duplicate ids" do
+ let(:ids) { [second.id, second.id] }
- it "responds with 404" do
- expect(response).to have_http_status :not_found
- end
+ it_behaves_like "rejects invalid dialog ids"
end
- end
-
- describe "GET #move_to_bucket_dialog" do
- let!(:displayed_buckets) { create_list(:backlog_bucket, 2, project:) }
- let!(:other_bucket) { create(:backlog_bucket, project: create(:project)) }
-
- let(:params) { { project_id: project.id, id: work_package.id } }
- subject(:response) { get :move_to_bucket_dialog, params:, format: :turbo_stream }
-
- context "with a Sprint source" do
- it "responds with a dialog turbo stream", :aggregate_failures do
- expect(response).to be_successful
- expect(response).to have_turbo_stream action: "dialog"
+ context "with an id from another project" do
+ let(:foreign_project) do
+ create(:project, name: "Foreign dialog project", identifier: "foreign-dialog-project")
end
-
- it "includes the project buckets as list_id options" do
- subject
-
- displayed_buckets.each do |bucket|
- expect(body).to include(%(value="#{bucket.id}"))
- end
+ let(:foreign_work_package) do
+ create(:work_package, project: foreign_project, author: user, status:, type: type_feature)
end
+ let(:ids) { [second.id, foreign_work_package.id] }
- it "does not include buckets from other projects" do
- subject
-
- expect(body).not_to include(%(value="#{other_bucket.id}"))
- end
+ it_behaves_like "rejects invalid dialog ids"
end
- context "when the work package is in a bucket" do
- let(:current_bucket) { create(:backlog_bucket, project:) }
- let(:current_bucket_wp) { create(:work_package, status:, project:, backlog_bucket: current_bucket) }
- let(:params) { { project_id: project.id, id: current_bucket_wp.id } }
-
- it "responds with a dialog turbo stream" do
- expect(response).to be_successful
- expect(response).to have_turbo_stream action: "dialog"
+ context "with an id the user cannot see" do
+ let(:user) do
+ create(:user, member_with_permissions: {
+ project => %i[view_work_packages view_sprints manage_sprint_items]
+ })
end
-
- it "excludes the current bucket from the options" do
- subject
-
- expect(body).not_to include(%(value="#{current_bucket.id}"))
+ let(:invisible_project) do
+ create(:project, public: false, name: "Invisible dialog project", identifier: "invisible-dialog-project")
end
-
- it "includes the other project buckets" do
- displayed_buckets.each do |bucket|
- expect(body).to include(%(value="#{bucket.id}"))
- end
+ let(:invisible_work_package) do
+ create(:work_package, project: invisible_project, author: user, status:, type: type_feature)
end
+ let(:ids) { [second.id, invisible_work_package.id] }
+
+ it_behaves_like "rejects invalid dialog ids"
end
- context "when all=true is in params" do
- let(:params) { { project_id: project.id, id: work_package.id, all: "true" } }
+ context "when every sprint destination is omitted" do
+ let!(:available_sprint) { sprint }
+ let!(:first) { create(:work_package, status:, project:, sprint:) }
+ let!(:second) { create(:work_package, status:, project:, sprint:) }
- it "embeds the all query in the dialog form action URL" do
- expect(body).to include("all=true")
+ it "returns a 422 Turbo Stream without opening an empty dialog", :aggregate_failures do
+ expect(response).to have_http_status :unprocessable_entity
+ expect(response).to have_turbo_stream action: "flash", target: "op-primer-flash-component"
+ expect(body).to include(I18n.t("backlogs.work_packages.move_to_sprint_dialog.no_available_destinations"))
+ expect(response).not_to have_turbo_stream action: "dialog"
end
end
+ end
- context "with a user lacking manage_sprint_items permission" do
- let(:user) { create(:user, member_with_permissions: { project => %i[view_sprints view_work_packages] }) }
-
- it "responds with 403" do
- expect(response).to have_http_status :forbidden
- end
+ describe "POST #move_to_bucket_dialog" do
+ let!(:available_bucket) { create(:backlog_bucket, project:) }
+ let!(:first) { create(:work_package, status:, project:) }
+ let!(:second) { create(:work_package, status:, project:) }
+ let(:params) { { project_id: project.id, ids: [second.id, first.id] } }
+
+ subject(:response) { post :move_to_bucket_dialog, params:, format: :turbo_stream }
+
+ it "renders the ordered collection and authoritative buckets" do
+ response
+
+ expect(response).to have_turbo_stream action: "dialog"
+ document = Nokogiri::HTML.fragment(response.body)
+ expect(document.css('input[name="ids[]"]').pluck("value"))
+ .to eq [second.id.to_s, first.id.to_s]
+ expect(document.css("select option").pluck("value"))
+ .to eq [available_bucket.id.to_s]
+ expect(document.css('input[name="prev_id"]')).to be_empty
+ expect(document.css("form").pluck("action"))
+ .to include(move_project_backlogs_work_packages_path(project))
end
- context "with a user lacking project permission" do
- let(:user) { create(:user) }
+ context "when every bucket destination is omitted" do
+ let!(:available_bucket) { create(:backlog_bucket, project:) }
+ let!(:first) { create(:work_package, status:, project:, backlog_bucket: available_bucket) }
+ let!(:second) { create(:work_package, status:, project:, backlog_bucket: available_bucket) }
- it "responds with 404" do
- expect(response).to have_http_status :not_found
+ it "returns a 422 Turbo Stream without opening an empty dialog", :aggregate_failures do
+ expect(response).to have_http_status :unprocessable_entity
+ expect(response).to have_turbo_stream action: "flash", target: "op-primer-flash-component"
+ expect(body).to include(I18n.t("backlogs.work_packages.move_to_bucket_dialog.no_available_destinations"))
+ expect(response).not_to have_turbo_stream action: "dialog"
end
end
end
diff --git a/modules/backlogs/spec/features/inbox_column_spec.rb b/modules/backlogs/spec/features/inbox_column_spec.rb
index 67989c756311..101aa098ff25 100644
--- a/modules/backlogs/spec/features/inbox_column_spec.rb
+++ b/modules/backlogs/spec/features/inbox_column_spec.rb
@@ -292,10 +292,8 @@
click_button "Move"
end
- planning_page
- .expect_and_dismiss_error(
- "Update failed: Sprint is not assignable since it is either not shared with the project or already finished."
- )
+ expected_error = I18n.t("backlogs.work_packages.batch_update_service.unavailable_target")
+ planning_page.expect_and_dismiss_error("Update failed: #{expected_error}")
# Item was *not* moved:
planning_page.expect_inbox_items(items: inbox_wp1)
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
new file mode 100644
index 000000000000..8c98b90e7f7e
--- /dev/null
+++ b/modules/backlogs/spec/features/work_packages/batch_destination_menu_spec.rb
@@ -0,0 +1,360 @@
+# 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 "Backlogs batch destination menus",
+ :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
+
+ context "with selected members in different source lists" do
+ let!(:source_sprint) { create(:sprint, project:, name: "Source sprint") }
+ let!(:destination_sprint) { create(:sprint, project:, name: "Destination sprint") }
+ let!(:bucket) { create(:backlog_bucket, project:, name: "Source bucket") }
+ let!(:destination_story) { create(:work_package, project:, type:, sprint: destination_sprint) }
+ let!(:bucket_story) { create(:work_package, project:, type:, backlog_bucket: bucket) }
+ let!(:sprint_story) { create(:work_package, project:, type:, sprint: source_sprint) }
+
+ it "moves a selected cross-list batch in live document order and announces its appended range" do
+ backlogs_page.visit!
+
+ backlogs_page.select_cards(sprint_story, bucket_story)
+ backlogs_page.expect_selected_cards_in_order(bucket_story, sprint_story)
+
+ backlogs_page.open_destination_dialog(sprint_story, "Move to sprint")
+ backlogs_page.expect_destination_dialog(
+ "Move to sprint",
+ work_packages: [bucket_story, sprint_story]
+ )
+ backlogs_page.submit_destination_dialog(
+ "Move to sprint",
+ field_label: Sprint.human_model_name,
+ option: destination_sprint.name
+ )
+
+ backlogs_page.expect_sprint_items_in_order(
+ destination_sprint,
+ items: [destination_story, bucket_story, sprint_story]
+ )
+ backlogs_page.expect_polite_announcement(
+ I18n.t(
+ "backlogs.work_packages.move_collection.moved_announcement",
+ count: 2,
+ list: destination_sprint.name,
+ first: 2,
+ last: 3,
+ total: 3
+ )
+ )
+ backlogs_page.expect_no_selected_cards
+
+ backlogs_page.visit!
+ backlogs_page.expect_sprint_items_in_order(
+ destination_sprint,
+ items: [destination_story, bucket_story, sprint_story]
+ )
+ end
+ end
+
+ context "with an unselected destination invoker" do
+ let!(:first_sprint) { create(:sprint, project:, name: "First sprint") }
+ let!(:second_sprint) { create(:sprint, project:, name: "Second sprint") }
+ let!(:bucket) { create(:backlog_bucket, project:, name: "Source bucket") }
+ let!(:selected_bucket_story) { create(:work_package, project:, type:, backlog_bucket: bucket) }
+ let!(:selected_sprint_story) { create(:work_package, project:, type:, sprint: first_sprint) }
+ let!(:invoker) { create(:work_package, project:, type:, sprint: second_sprint) }
+
+ it "replaces the old selection when an unselected card invokes a destination action" do
+ backlogs_page.visit!
+
+ backlogs_page.select_cards(selected_sprint_story, selected_bucket_story)
+ backlogs_page.expect_selected_cards_in_order(selected_bucket_story, selected_sprint_story)
+
+ backlogs_page.move_to_backlog_inbox(invoker)
+
+ backlogs_page.expect_inbox_items_in_order(items: [invoker])
+ backlogs_page.expect_bucket_items_in_order(bucket, items: [selected_bucket_story])
+ backlogs_page.expect_sprint_items_in_order(first_sprint, items: [selected_sprint_story])
+ backlogs_page.expect_no_selected_cards
+
+ backlogs_page.visit!
+ backlogs_page.expect_inbox_items_in_order(items: [invoker])
+ backlogs_page.expect_bucket_items_in_order(bucket, items: [selected_bucket_story])
+ backlogs_page.expect_sprint_items_in_order(first_sprint, items: [selected_sprint_story])
+ end
+ end
+
+ context "with a partly occupied destination" do
+ let!(:destination_sprint) { create(:sprint, project:, name: "Destination sprint") }
+ let!(:bucket) { create(:backlog_bucket, project:, name: "Source bucket") }
+ let!(:destination_story) { create(:work_package, project:, type:, sprint: destination_sprint, position: 1) }
+ let!(:selected_destination_story) { create(:work_package, project:, type:, sprint: destination_sprint, position: 2) }
+ let!(:bucket_story) { create(:work_package, project:, type:, backlog_bucket: bucket) }
+
+ it "offers a partly occupied destination and gathers the whole batch at its end" do
+ backlogs_page.visit!
+
+ backlogs_page.select_cards(selected_destination_story, bucket_story)
+ backlogs_page.expect_selected_cards_in_order(bucket_story, selected_destination_story)
+ backlogs_page.expect_work_package_action(selected_destination_story, "Move to sprint")
+
+ backlogs_page.open_destination_dialog(selected_destination_story, "Move to sprint")
+ backlogs_page.expect_destination_dialog_options(
+ "Move to sprint",
+ field_label: Sprint.human_model_name,
+ options: [destination_sprint.name]
+ )
+ backlogs_page.submit_destination_dialog(
+ "Move to sprint",
+ field_label: Sprint.human_model_name,
+ option: destination_sprint.name
+ )
+
+ backlogs_page.expect_sprint_items_in_order(
+ destination_sprint,
+ items: [destination_story, bucket_story, selected_destination_story]
+ )
+
+ backlogs_page.visit!
+ backlogs_page.expect_sprint_items_in_order(
+ destination_sprint,
+ items: [destination_story, bucket_story, selected_destination_story]
+ )
+ end
+ end
+
+ context "with confined members" do
+ let!(:first_sprint) { create(:sprint, project:, name: "First sprint") }
+ let!(:second_sprint) { create(:sprint, project:, name: "Second sprint") }
+ let!(:bucket) { create(:backlog_bucket, project:, name: "Source bucket") }
+ let!(:readonly_status) { create(:status, is_readonly: true) }
+ let!(:confined_first) { create(:work_package, project:, type:, sprint: first_sprint, status: readonly_status) }
+ let!(:confined_second) { create(:work_package, project:, type:, sprint: second_sprint, status: readonly_status) }
+ let!(:free_story) { create(:work_package, project:, type:, backlog_bucket: bucket) }
+
+ it "intersects destinations for confined selections and offers none when their lists conflict" do
+ backlogs_page.visit!
+
+ backlogs_page.select_cards(confined_first, free_story)
+ backlogs_page.expect_destination_actions(
+ confined_first,
+ present: ["Move to sprint"],
+ absent: ["Move to backlog bucket", "Move to backlog inbox"]
+ )
+ backlogs_page.open_destination_dialog(confined_first, "Move to sprint")
+ backlogs_page.expect_destination_dialog_options(
+ "Move to sprint",
+ field_label: Sprint.human_model_name,
+ options: [first_sprint.name]
+ )
+ backlogs_page.cancel_destination_dialog("Move to sprint")
+ backlogs_page.clear_card_selection(confined_first)
+
+ backlogs_page.select_cards(confined_first, confined_second)
+ backlogs_page.within_work_package_menu(confined_first) do |menu|
+ ["Move to sprint", "Move to backlog bucket", "Move to backlog inbox"].each do |label|
+ expect(menu).to have_no_selector(:menuitem, text: label, exact_text: true)
+ end
+ expect(menu).to have_no_css('[data-sortable-lists--item-target="moveDivider"]', visible: :visible)
+ end
+ end
+ end
+
+ context "with a 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) }
+
+ it "omits Move to position from a 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_work_package_action(first_story, "Move to backlog bucket")
+ end
+ end
+
+ context "when the last destination disappears" do
+ let!(:sprint) { create(:sprint, project:, name: "Only sprint") }
+ let!(:bucket) { create(:backlog_bucket, project:, name: "Source bucket") }
+ let!(:first_story) { create(:work_package, project:, type:, backlog_bucket: bucket, position: 1) }
+ let!(:second_story) { create(:work_package, project:, type:, backlog_bucket: bucket, position: 2) }
+
+ it "shows feedback without an empty modal when the last destination disappears before loading" do
+ backlogs_page.visit!
+
+ backlogs_page.select_cards(first_story, second_story)
+ backlogs_page.invoke_destination_action_after_menu_load(first_story, "Move to sprint") do
+ sprint.update_columns(status: "completed")
+ end
+
+ backlogs_page.expect_move_error(
+ I18n.t("backlogs.work_packages.move_to_sprint_dialog.no_available_destinations")
+ )
+ backlogs_page.expect_no_destination_dialog
+ backlogs_page.expect_selected_cards_in_order(first_story, second_story)
+ backlogs_page.expect_bucket_items_in_order(bucket, items: [first_story, second_story])
+ end
+ end
+
+ context "when a batch member refuses the destination" do
+ let!(:sprint) { create(:sprint, project:, name: "Destination sprint") }
+ let!(:bucket) { create(:backlog_bucket, project:, name: "Source bucket") }
+ let!(:first_story) { create(:work_package, project:, type:, backlog_bucket: bucket, position: 1) }
+ let!(:second_story) { create(:work_package, project:, type:, backlog_bucket: bucket, position: 2) }
+
+ it "rejects the complete batch atomically and retains its selection" do
+ backlogs_page.visit!
+
+ backlogs_page.select_cards(first_story, second_story)
+ backlogs_page.open_destination_dialog(first_story, "Move to sprint")
+
+ second_story.update_columns(status_id: create(:status, is_readonly: true).id)
+ backlogs_page.submit_destination_dialog(
+ "Move to sprint",
+ field_label: Sprint.human_model_name,
+ option: sprint.name,
+ frame_reload: false
+ )
+
+ backlogs_page.expect_move_error(
+ I18n.t("backlogs.work_packages.move_collection.member_failed",
+ work_package: second_story.reload.to_fs(:caption),
+ reason: I18n.t("backlogs.work_packages.batch_update_service.unavailable_target"))
+ )
+ backlogs_page.expect_bucket_items_in_order(bucket, items: [first_story, second_story])
+ backlogs_page.expect_selected_cards_in_order(first_story, second_story)
+
+ backlogs_page.visit!
+ backlogs_page.expect_bucket_items_in_order(bucket, items: [first_story, second_story])
+ end
+ end
+
+ context "with a selected batch moving to a backlog bucket" do
+ let!(:sprint) { create(:sprint, project:) }
+ let!(:bucket) { create(:backlog_bucket, project:) }
+ let!(:existing) { create(:work_package, project:, type:, backlog_bucket: bucket) }
+ let!(:first) { create(:work_package, project:, type:, sprint:, position: 1) }
+ let!(:second) { create(:work_package, project:, type:, sprint:, position: 2) }
+
+ it "appends the ordered batch and announces its range" do
+ backlogs_page.visit!
+ backlogs_page.select_cards(first, second)
+ backlogs_page.open_destination_dialog(first, "Move to backlog bucket")
+ backlogs_page.expect_destination_dialog("Move to backlog bucket", work_packages: [first, second])
+ backlogs_page.submit_destination_dialog(
+ "Move to backlog bucket", field_label: BacklogBucket.human_attribute_name(:name), option: bucket.name
+ )
+
+ backlogs_page.expect_bucket_items_in_order(bucket, items: [existing, first, second])
+ backlogs_page.expect_polite_announcement(
+ I18n.t("backlogs.work_packages.move_collection.moved_announcement",
+ count: 2, list: bucket.name, first: 2, last: 3, total: 3)
+ )
+ backlogs_page.expect_no_selected_cards
+ backlogs_page.visit!
+ backlogs_page.expect_bucket_items_in_order(bucket, items: [existing, first, second])
+ end
+ end
+
+ context "with a selected batch moving to the inbox" do
+ let!(:sprint) { create(:sprint, project:) }
+ let!(:existing) { create(:work_package, project:, type:) }
+ let!(:first) { create(:work_package, project:, type:, sprint:, position: 1) }
+ let!(:second) { create(:work_package, project:, type:, sprint:, position: 2) }
+
+ it "appends the whole selection and announces its range" do
+ backlogs_page.visit!
+ backlogs_page.select_cards(first, second)
+ backlogs_page.move_to_backlog_inbox(first)
+
+ backlogs_page.expect_inbox_items_in_order(items: [existing, first, second])
+ backlogs_page.expect_polite_announcement(
+ I18n.t("backlogs.work_packages.move_collection.moved_announcement",
+ count: 2, list: I18n.t(:label_inbox), first: 2, last: 3, total: 3)
+ )
+ backlogs_page.expect_no_selected_cards
+ backlogs_page.visit!
+ backlogs_page.expect_inbox_items_in_order(items: [existing, first, second])
+ end
+ end
+
+ context "with additive selection followed by a shrinking Shift range" do
+ let!(:source) { create(:sprint, project:, name: "Source sprint") }
+ let!(:destination) { create(:sprint, project:, name: "Destination sprint") }
+ let!(:existing) { create(:work_package, project:, type:, sprint: destination, subject: "X") }
+ let!(:stories) do
+ %w[A B C D E].map.with_index(1) do |subject, position|
+ create(:work_package, project:, type:, sprint: source, subject:, position:)
+ end
+ end
+
+ it "submits the current range while preserving the earlier additive member" do
+ a, b, c, d, e = stories
+ backlogs_page.visit!
+ backlogs_page.toggle_card(a)
+ backlogs_page.toggle_card(c)
+ backlogs_page.extend_selection_to(e)
+ backlogs_page.expect_selected_cards_in_order(a, c, d, e)
+ backlogs_page.extend_selection_to(d)
+ backlogs_page.expect_selected_cards_in_order(a, c, d)
+ backlogs_page.open_destination_dialog(c, "Move to sprint")
+ backlogs_page.expect_destination_dialog("Move to sprint", work_packages: [a, c, d])
+ backlogs_page.submit_destination_dialog(
+ "Move to sprint", field_label: Sprint.human_model_name, option: destination.name
+ )
+
+ backlogs_page.expect_sprint_items_in_order(destination, items: [existing, a, c, d])
+ backlogs_page.expect_sprint_items_in_order(source, items: [b, e])
+ backlogs_page.expect_no_selected_cards
+ backlogs_page.visit!
+ backlogs_page.expect_sprint_items_in_order(destination, items: [existing, a, c, d])
+ backlogs_page.expect_sprint_items_in_order(source, items: [b, e])
+ end
+ end
+end
diff --git a/modules/backlogs/spec/requests/work_packages/move_collection_spec.rb b/modules/backlogs/spec/requests/work_packages/move_collection_spec.rb
index 7e085fed013a..a972c8543272 100644
--- a/modules/backlogs/spec/requests/work_packages/move_collection_spec.rb
+++ b/modules/backlogs/spec/requests/work_packages/move_collection_spec.rb
@@ -53,6 +53,22 @@ def move_collection(ids:, **params)
headers: { "Accept" => "text/vnd.turbo-stream.html" }
end
+ def load_move_to_sprint_dialog(ids:)
+ post move_to_sprint_dialog_project_backlogs_work_packages_path(project),
+ params: { ids: },
+ headers: { "Accept" => "text/vnd.turbo-stream.html" }
+ end
+
+ def expect_moved_event(ids)
+ expect(response.body).to have_turbo_stream(action: "dispatchEvent") do |streams|
+ expect(streams.size).to eq(1)
+ expect(streams.first["event-name"])
+ .to eq(Backlogs::WorkPackagesController::WORK_PACKAGE_MOVED_EVENT)
+ expect(JSON.parse(streams.first["detail"]))
+ .to eq("work_package_ids" => ids)
+ end
+ end
+
context "without the manage_sprint_items permission" do
let(:permissions) { %i[view_work_packages edit_work_packages view_sprints] }
@@ -135,6 +151,65 @@ def move_collection(ids:, **params)
end
describe "successful moves" do
+ context "with one visible member" do
+ it "reuses the singular move announcement", :aggregate_failures do
+ move_collection(ids: [bucket_wp1.id], list_type: "sprint", list_id: sprint.id)
+
+ expect(response).to have_http_status(:ok)
+ expect(response.body).to include(
+ ERB::Util.html_escape(
+ I18n.t("backlogs.work_packages.move.moved_announcement",
+ label: bucket_wp1.to_fs(:caption), list: sprint.name, position: 2, total: 2)
+ )
+ )
+ expect_moved_event([bucket_wp1.id])
+ end
+ end
+
+ context "with two visible members" do
+ it "announces their count, destination, contiguous range, and total", :aggregate_failures do
+ moved_ids = [bucket_wp2.id, bucket_wp1.id]
+
+ move_collection(ids: moved_ids, list_type: "sprint", list_id: sprint.id)
+
+ expect(response).to have_http_status(:ok)
+ expect(response.body).to include(
+ ERB::Util.html_escape(
+ I18n.t("backlogs.work_packages.move_collection.moved_announcement",
+ count: 2, list: sprint.name, first: 2, last: 3, total: 3)
+ )
+ )
+ expect(sprint.work_packages_for(project).pluck(:id)).to eq [sprint_wp1.id, *moved_ids]
+ expect_moved_event(moved_ids)
+ end
+ end
+
+ context "with members already persisted at the requested append placement" do
+ it "treats the stale-state submission as success and announces the ordered batch", :aggregate_failures do
+ frozen_ids = [bucket_wp2.id, bucket_wp1.id]
+ load_move_to_sprint_dialog(ids: frozen_ids)
+ expect(response).to have_http_status(:ok)
+
+ concurrently_moved = Backlogs::WorkPackages::BatchUpdateService
+ .new(user:, work_packages: frozen_ids.map { |id| WorkPackage.find(id) })
+ .call(list_type: "sprint", list_id: sprint.id)
+ expect(concurrently_moved).to be_success
+ expect(sprint.work_packages_for(project).pluck(:id)).to eq [sprint_wp1.id, *frozen_ids]
+
+ move_collection(ids: frozen_ids, list_type: "sprint", list_id: sprint.id)
+
+ expect(response).to have_http_status(:ok)
+ expect(response.body).to include(
+ ERB::Util.html_escape(
+ I18n.t("backlogs.work_packages.move_collection.moved_announcement",
+ count: 2, list: sprint.name, first: 2, last: 3, total: 3)
+ )
+ )
+ expect(sprint.work_packages_for(project).pluck(:id)).to eq [sprint_wp1.id, *frozen_ids]
+ expect_moved_event(frozen_ids)
+ end
+ end
+
context "with an optimistic same-list reorder whose block is honored" do
it "responds with the moved event only, no frame reload", :aggregate_failures do
move_collection(ids: [sprint_wp1.id], list_type: "sprint", list_id: sprint.id,
@@ -228,6 +303,25 @@ def move_collection(ids:, **params)
expect(response).to have_http_status(:unprocessable_entity)
expect(response.body).to include(ERB::Util.html_escape(sprint_wp3.reload.to_fs(:caption)))
end
+
+ it "rejects a destination that becomes unavailable after the dialog loads", :aggregate_failures do
+ frozen_ids = [bucket_wp2.id, bucket_wp1.id]
+ load_move_to_sprint_dialog(ids: frozen_ids)
+ expect(response).to have_http_status(:ok)
+
+ positions_before = WorkPackage.order(:id).pluck(:id, :sprint_id, :backlog_bucket_id, :position)
+ sprint.update!(status: "completed")
+
+ move_collection(ids: frozen_ids, list_type: "sprint", list_id: sprint.id)
+
+ expect(response).to have_http_status(:unprocessable_entity)
+ expect(response.body).to include(
+ ERB::Util.html_escape(I18n.t("backlogs.work_packages.batch_update_service.unavailable_target"))
+ )
+ expect(WorkPackage.order(:id).pluck(:id, :sprint_id, :backlog_bucket_id, :position))
+ .to eq(positions_before)
+ expect(response.body).not_to have_turbo_stream(action: "dispatchEvent")
+ end
end
describe "invisibility after move" do
@@ -252,6 +346,7 @@ def move_collection(ids:, **params)
expect(response.body).to include(
ERB::Util.html_escape(I18n.t(:notice_work_package_invisible_after_move, count: 1, backlog: bucket.name))
)
+ expect(response.body).not_to have_turbo_stream(action: "liveRegion")
end
end
@@ -283,6 +378,7 @@ def move_collection(ids:, **params)
I18n.t(:notice_work_package_invisible_after_move, count: 2, backlog: bucket.name)
)
)
+ expect(response.body).not_to have_turbo_stream(action: "liveRegion")
end
end
end
diff --git a/modules/backlogs/spec/routing/backlogs/work_packages_routing_spec.rb b/modules/backlogs/spec/routing/backlogs/work_packages_routing_spec.rb
index 044ae50908fc..83982e293ccc 100644
--- a/modules/backlogs/spec/routing/backlogs/work_packages_routing_spec.rb
+++ b/modules/backlogs/spec/routing/backlogs/work_packages_routing_spec.rb
@@ -51,20 +51,18 @@
}
it {
- expect(get("/projects/project_42/backlogs/work_packages/85/move_to_sprint_dialog")).to route_to(
+ expect(post("/projects/project_42/backlogs/work_packages/move_to_sprint_dialog")).to route_to(
controller: "backlogs/work_packages",
action: "move_to_sprint_dialog",
- project_id: "project_42",
- id: "85"
+ project_id: "project_42"
)
}
it {
- expect(get("/projects/project_42/backlogs/work_packages/85/move_to_bucket_dialog")).to route_to(
+ expect(post("/projects/project_42/backlogs/work_packages/move_to_bucket_dialog")).to route_to(
controller: "backlogs/work_packages",
action: "move_to_bucket_dialog",
- project_id: "project_42",
- id: "85"
+ project_id: "project_42"
)
}
diff --git a/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_concurrency_spec.rb b/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_concurrency_spec.rb
index f589b612065c..00aaa9a3b207 100644
--- a/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_concurrency_spec.rb
+++ b/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_concurrency_spec.rb
@@ -131,6 +131,40 @@ def collect_cleanup_error(cleanup_errors)
cleanup_errors << e
end
+ # rubocop:disable-next RSpec/ExampleLength
+ it "rejects an empty append when an independent writer fills the inbox", retry: 0 do
+ service = described_class.new(user:, work_packages: sprint_work_packages)
+ placement_resolved = Concurrent::Event.new
+ release_batch = Concurrent::Event.new
+ batch_result = Queue.new
+ original_ids = source_sprint.work_packages_for(project).order_by_position.ids
+
+ allow(service).to receive(:resolve_placement).and_wrap_original do |method, *args|
+ placement = method.call(*args)
+ placement_resolved.set
+ raise "timed out waiting for inbox insertion" unless release_batch.wait(5)
+
+ placement
+ end
+ batch_thread = Thread.new do
+ ActiveRecord::Base.connection_pool.with_connection do
+ batch_result << service.call(list_type: "inbox")
+ end
+ end
+ raise "append did not resolve its placement" unless placement_resolved.wait(5)
+
+ inserted = create(:work_package, project:, type:)
+ release_batch.set
+ result = Timeout.timeout(5) { batch_result.pop }
+
+ expect(result).to be_failure
+ expect(result.message).to eq I18n.t("backlogs.work_packages.batch_update_service.stale_predecessor")
+ expect(source_sprint.work_packages_for(project).order_by_position.ids).to eq original_ids
+ expect(WorkPackage.where(project:, sprint_id: nil, backlog_bucket_id: nil).ids).to eq [inserted.id]
+ ensure
+ cleanup_concurrency_threads(release_events: [release_batch], threads: [batch_thread])
+ end
+
# rubocop:disable-next RSpec/ExampleLength
it "serializes disjoint batches before resolving append placement in an empty target", retry: 0 do
first_service = described_class.new(user:, work_packages: sprint_work_packages)
diff --git a/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_spec.rb b/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_spec.rb
index 893e570b1d60..ea427fbef45a 100644
--- a/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_spec.rb
+++ b/modules/backlogs/spec/services/backlogs/work_packages/batch_update_service_spec.rb
@@ -57,6 +57,27 @@ def sprint_order
sprint.work_packages_for(project).pluck(:id)
end
+ it "rejects a foreign destination before taking any movement locks" do
+ foreign = create(:sprint, project: create(:project))
+ allow(OpenProject::Mutex).to receive(:with_advisory_lock_transaction).and_call_original
+
+ result = service([bucket_wp1]).call(list_type: "sprint", list_id: foreign.id.to_s)
+
+ expect(result).to be_failure
+ expect(OpenProject::Mutex).not_to have_received(:with_advisory_lock_transaction)
+ end
+
+ it "rejects a user without movement permission before taking locks" do
+ unauthorized = create(:user, member_with_permissions: { project => %i[view_work_packages view_sprints] })
+ allow(OpenProject::Mutex).to receive(:with_advisory_lock_transaction).and_call_original
+
+ result = described_class.new(user: unauthorized, work_packages: [bucket_wp1])
+ .call(list_type: "inbox")
+
+ expect(result).to be_failure
+ expect(OpenProject::Mutex).not_to have_received(:with_advisory_lock_transaction)
+ end
+
it "moves a cross-list batch as one contiguous block after the predecessor" do
result = service([bucket_wp1, sprint_wp3])
.call(list_type: "sprint", list_id: sprint.id.to_s, prev_id: sprint_wp1.id.to_s)
@@ -410,10 +431,8 @@ def work_package_lock_ids(locked)
batch = service([bucket_wp1])
lock_names = []
- allow(batch).to receive(:raw_destination).and_wrap_original do |method, target|
- destination = method.call(target)
- destination.is_a?(Sprint) && destination.id == sprint.id ? concrete_sprint : destination
- end
+ allow(Sprint).to receive(:find_by).and_call_original
+ allow(Sprint).to receive(:find_by).with(id: sprint.id).and_return(concrete_sprint)
allow(OpenProject::Mutex)
.to receive(:with_advisory_lock)
.and_wrap_original do |method, resource_class, lock_name, *args, &block|
@@ -452,6 +471,33 @@ def work_package_lock_ids(locked)
end
describe "target availability" do
+ it "revalidates a member that became confined after destination projection", with_ee: %i[readonly_work_packages] do
+ projected = Backlogs::WorkPackages::DestinationAvailability.new(
+ project:,
+ user:,
+ work_packages: [bucket_wp1, bucket_wp2]
+ )
+ expect(projected.sprints).to include(sprint)
+
+ readonly_status = create(:status, :readonly)
+ WorkPackage.where(id: bucket_wp2.id).update_all(status_id: readonly_status.id)
+ allow(Backlogs::WorkPackages::UpdateService).to receive(:new).and_call_original
+
+ result = service([bucket_wp1, bucket_wp2])
+ .call(list_type: "sprint", list_id: sprint.id.to_s, prev_id: "")
+
+ expect(result).to be_failure
+ expect(result.message)
+ .to eq I18n.t("backlogs.work_packages.batch_update_service.unavailable_target")
+ expect(Backlogs::WorkPackages::UpdateService).not_to have_received(:new)
+ expect([bucket_wp1.reload, bucket_wp2.reload])
+ .to match [
+ have_attributes(backlog_bucket_id: bucket.id, sprint_id: nil, position: 1),
+ have_attributes(backlog_bucket_id: bucket.id, sprint_id: nil, position: 2)
+ ]
+ expect(sprint_order).to eq [sprint_wp1.id, sprint_wp2.id, sprint_wp3.id]
+ end
+
it "rejects a same-list reorder inside a sprint that completed after load" do
sprint.update!(status: "completed")
diff --git a/modules/backlogs/spec/services/backlogs/work_packages/destination_availability_spec.rb b/modules/backlogs/spec/services/backlogs/work_packages/destination_availability_spec.rb
new file mode 100644
index 000000000000..30892c96c2ae
--- /dev/null
+++ b/modules/backlogs/spec/services/backlogs/work_packages/destination_availability_spec.rb
@@ -0,0 +1,174 @@
+# 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"
+
+RSpec.describe Backlogs::WorkPackages::DestinationAvailability, type: :model do
+ shared_let(:type) { create(:type) }
+ shared_let(:project) do
+ create(:project, types: [type], enabled_module_names: %i[backlogs work_package_tracking])
+ end
+ shared_let(:user) do
+ create(:user, member_with_permissions: {
+ project => %i[view_work_packages view_sprints manage_sprint_items]
+ })
+ end
+ shared_let(:readonly_status) { create(:status, :readonly) }
+
+ let!(:later_sprint) do
+ create(:sprint, project:, name: "Later", start_date: 2.weeks.from_now, finish_date: 3.weeks.from_now)
+ end
+ let!(:earlier_sprint) do
+ create(:sprint, project:, name: "Earlier", start_date: 1.week.from_now, finish_date: 2.weeks.from_now)
+ end
+ let!(:completed_sprint) { create(:sprint, project:, status: :completed) }
+ let!(:zulu_bucket) { create(:backlog_bucket, project:, name: "Zulu") }
+ let!(:alpha_bucket) { create(:backlog_bucket, project:, name: "Alpha") }
+ let!(:foreign_bucket) { create(:backlog_bucket, project: create(:project), name: "Foreign") }
+
+ def availability(work_packages, for_user: user)
+ described_class.new(project:, user: for_user, work_packages:)
+ end
+
+ describe "destination intersection" do
+ it "offers every available project destination to free members in record order" do
+ first = create(:work_package, project:, type:)
+ second = create(:work_package, project:, type:, sprint: earlier_sprint)
+
+ result = availability([first, second])
+
+ expect(result.sprints).to eq [earlier_sprint, later_sprint]
+ expect(result.buckets).to eq [alpha_bucket, zulu_bucket]
+ expect(result.refusing(Backlogs::Target::InboxId)).to be_empty
+ end
+
+ it "intersects a confined member's current list with a free member", with_ee: %i[readonly_work_packages] do
+ confined = create(:work_package, project:, type:, status: readonly_status, sprint: earlier_sprint)
+ free = create(:work_package, project:, type:, backlog_bucket: alpha_bucket)
+
+ result = availability([confined, free])
+
+ expect(result.sprints).to eq [earlier_sprint]
+ expect(result.buckets).to be_empty
+ expect(result.refusing(Backlogs::Target::InboxId)).not_to be_empty
+ end
+
+ it "offers nothing when confined members are in different lists", with_ee: %i[readonly_work_packages] do
+ first = create(:work_package, project:, type:, status: readonly_status, sprint: earlier_sprint)
+ second = create(:work_package, project:, type:, status: readonly_status, backlog_bucket: alpha_bucket)
+
+ result = availability([first, second])
+
+ expect(result.sprints).to be_empty
+ expect(result.buckets).to be_empty
+ expect(result.refusing(Backlogs::Target::InboxId)).not_to be_empty
+ end
+
+ it "omits an all-members-current option without revoking positional reuse" do
+ work_packages = create_list(:work_package, 2, project:, type:, sprint: earlier_sprint)
+ current_target = Backlogs::Target.for(earlier_sprint)
+
+ result = availability(work_packages)
+
+ expect(result.sprints).to eq [later_sprint]
+ expect(result.candidate?(current_target)).to be true
+ expect(result.refusing(current_target)).to be_empty
+ end
+
+ it "loads read-only status identity in one query for the whole batch",
+ with_ee: %i[readonly_work_packages] do
+ work_packages = create_list(:work_package, 5, project:, type:, status: readonly_status)
+ recorder = ActiveRecord::QueryRecorder.new do
+ loaded = WorkPackage.where(id: work_packages.map(&:id)).includes(:status).to_a
+ expect(availability(loaded).refusing(Backlogs::Target::InboxId)).to be_empty
+ end
+
+ status_queries = recorder.log.grep(/FROM "statuses"/)
+ expect(status_queries.size).to eq 1
+ end
+
+ it "ignores persisted read-only flags when the feature is unavailable" do
+ persisted_readonly_status = create(:status)
+ persisted_readonly_status.update_column(:is_readonly, true)
+ work_package = create(:work_package, project:, type:, status: persisted_readonly_status)
+
+ expect(availability([work_package]).refusing(Backlogs::Target.for(alpha_bucket))).to be_empty
+ end
+ end
+
+ it "keeps the invoking member's current destination in the candidate universe" do
+ member = create(:work_package, project:, type:, sprint: earlier_sprint)
+ result = availability([member])
+
+ expect(result.candidate_sprints).to eq [earlier_sprint, later_sprint]
+ expect(result.candidate_buckets).to eq [alpha_bucket, zulu_bucket]
+ expect(result.sprints).to eq [later_sprint]
+ end
+
+ it "checks management permission once while offering both destination kinds" do
+ member = create(:work_package, project:, type:)
+ result = availability([member])
+ allow(user).to receive(:allowed_in_project?).and_call_original
+
+ result.sprints
+ result.buckets
+
+ expect(user).to have_received(:allowed_in_project?).with(:manage_sprint_items, project).once
+ end
+
+ describe "authoritative candidates" do
+ let(:work_package) { create(:work_package, project:, type:) }
+
+ it "rejects completed or otherwise unassignable sprints" do
+ result = availability([work_package])
+
+ expect(result.sprints).not_to include(completed_sprint)
+ expect(result.candidate?(Backlogs::Target.for(completed_sprint))).to be false
+ end
+
+ it "rejects buckets outside the batch project" do
+ result = availability([work_package])
+
+ expect(result.buckets).not_to include(foreign_bucket)
+ expect(result.candidate?(Backlogs::Target.for(foreign_bucket))).to be false
+ end
+
+ it "rejects every destination without manage_sprint_items permission" do
+ unauthorized = create(:user, member_with_permissions: {
+ project => %i[view_work_packages view_sprints]
+ })
+ result = availability([work_package], for_user: unauthorized)
+
+ expect(result.sprints).to be_empty
+ expect(result.buckets).to be_empty
+ expect(result.manage_permission?).to be false
+ end
+ end
+end
diff --git a/modules/backlogs/spec/support/pages/backlog.rb b/modules/backlogs/spec/support/pages/backlog.rb
index ecbce1b28d67..227a370c151c 100644
--- a/modules/backlogs/spec/support/pages/backlog.rb
+++ b/modules/backlogs/spec/support/pages/backlog.rb
@@ -743,6 +743,115 @@ def selected_card_ids
all("[data-batch-selected]").pluck("data-sortable-lists--item-id-value")
end
+ def select_cards(*work_packages)
+ work_packages.each { |work_package| toggle_card(work_package) }
+ end
+
+ def expect_selected_cards_in_order(*work_packages)
+ expect(selected_card_ids).to eq(work_packages.map { |work_package| work_package.id.to_s })
+ end
+
+ def expect_no_selected_cards
+ expect(selected_card_ids).to be_empty
+ end
+
+ def clear_card_selection(work_package)
+ work_package_card(work_package).send_keys(:escape)
+ expect_no_selected_cards
+ end
+
+ def expect_work_package_action(work_package, action_label)
+ within_work_package_menu(work_package) do |menu|
+ expect(menu).to have_selector(:menuitem, text: action_label, exact_text: true)
+ end
+ end
+
+ def expect_no_work_package_action(work_package, action_label)
+ within_work_package_menu(work_package) do |menu|
+ expect(menu).to have_no_selector(:menuitem, text: action_label, exact_text: true)
+ end
+ end
+
+ def expect_destination_actions(work_package, present:, absent:)
+ within_work_package_menu(work_package) do |menu|
+ present.each do |label|
+ expect(menu).to have_selector(:menuitem, text: label, exact_text: true)
+ end
+ absent.each do |label|
+ expect(menu).to have_no_selector(:menuitem, text: label, exact_text: true)
+ end
+ end
+ end
+
+ def open_destination_dialog(work_package, action_label, dialog_title: action_label)
+ click_in_work_package_menu(work_package, action_label, wait: false)
+ expect(page).to have_selector(:modal, text: dialog_title)
+ end
+
+ def invoke_destination_action_after_menu_load(work_package, action_label, &before_click)
+ within_work_package_menu(work_package) do |menu|
+ item = menu.find(:menuitem, text: action_label, exact_text: true)
+ before_click&.call
+ wait_for_backlogs_turbo_stream do
+ item.click
+ end
+ end
+ end
+
+ def move_to_backlog_inbox(work_package)
+ within_work_package_menu(work_package) do |menu|
+ wait_for_backlogs_turbo_stream(frame_reload: true) do
+ menu.find(:menuitem, text: "Move to backlog inbox", exact_text: true).click
+ end
+ end
+ end
+
+ def expect_destination_dialog(dialog_title, work_packages:)
+ within_modal dialog_title do
+ work_packages.each { |work_package| expect(page).to have_text(work_package.subject) }
+ expect(all("input[name='ids[]']", visible: :all).map(&:value))
+ .to eq(work_packages.map { |work_package| work_package.id.to_s })
+ end
+ end
+
+ def expect_destination_dialog_options(dialog_title, field_label:, options:)
+ within_modal dialog_title do
+ select = find(:select, field_label)
+ expect(select.all(:option).map(&:text)).to eq(options)
+ end
+ end
+
+ def submit_destination_dialog(dialog_title, field_label:, option:, frame_reload: true)
+ within_modal dialog_title do
+ select option, from: field_label
+ wait_for_backlogs_turbo_stream(frame_reload:) { click_button I18n.t(:button_move) }
+ end
+ end
+
+ def cancel_destination_dialog(dialog_title)
+ within_modal dialog_title do
+ click_button I18n.t(:button_cancel)
+ end
+ expect(page).to have_no_selector(:modal, text: dialog_title)
+ end
+
+ def expect_no_destination_dialog
+ expect(page).to have_no_selector(:modal)
+ end
+
+ def expect_move_error(reason)
+ expect_flash(
+ type: :error,
+ message: I18n.t(:notice_unsuccessful_update_with_reason, reason:)
+ )
+ end
+
+ def expect_polite_announcement(message)
+ wait_for do
+ page.evaluate_script("document.querySelector('live-region')?.getMessage('polite')")
+ end.to eq(message)
+ end
+
# The shared description every selected card's `aria-describedby` points
# at. Rendered once, permanently `hidden` — screen readers still reach it
# through the reference despite that — so `visible: :all` is required.