mirror of
https://github.com/opf/openproject.git
synced 2026-06-14 03:30:14 +00:00
Remove per-item move URL from sortable lists
No template ever sets the per-item `moveUrl` Stimulus value, so the root controller's URL template is the only move URL source. Drops the defunct payload field, the resolver precedence branch, and the spec fixtures and absence assertions that referenced the removed attribute.
This commit is contained in:
@@ -86,26 +86,21 @@ describe('Sortable lists controller', () => {
|
||||
};
|
||||
}
|
||||
|
||||
function itemRow(id:string, { moveUrl = '/move' }:{ moveUrl?:string|null } = {}):HTMLLIElement {
|
||||
function itemRow(id:string):HTMLLIElement {
|
||||
const row = document.createElement('li');
|
||||
|
||||
row.setAttribute('data-sortable-lists--item-id-value', id);
|
||||
row.setAttribute('data-sortable-lists--item-type-value', 'work_package');
|
||||
if (moveUrl) {
|
||||
row.setAttribute('data-sortable-lists--item-move-url-value', moveUrl);
|
||||
}
|
||||
|
||||
return row;
|
||||
}
|
||||
|
||||
function renderFixture({
|
||||
acceptedType = null,
|
||||
moveUrlTemplate = null,
|
||||
itemMoveUrl = '/move',
|
||||
moveUrlTemplate = '/move/{id}',
|
||||
}:{
|
||||
acceptedType?:string|null;
|
||||
moveUrlTemplate?:string|null;
|
||||
itemMoveUrl?:string|null;
|
||||
} = {}) {
|
||||
fixture.innerHTML = `
|
||||
<div
|
||||
@@ -121,8 +116,8 @@ describe('Sortable lists controller', () => {
|
||||
const [sourceList, targetList] = Array.from(fixture.querySelectorAll<HTMLElement>('[data-sortable-lists-target="list"]'));
|
||||
const root = fixture.querySelector<HTMLElement>('[data-controller="sortable-lists"]')!;
|
||||
|
||||
sourceList.append(itemRow('1', { moveUrl: itemMoveUrl }), itemRow('2', { moveUrl: itemMoveUrl }), itemRow('3', { moveUrl: itemMoveUrl }));
|
||||
targetList.append(itemRow('4', { moveUrl: itemMoveUrl }), itemRow('5', { moveUrl: itemMoveUrl }));
|
||||
sourceList.append(itemRow('1'), itemRow('2'), itemRow('3'));
|
||||
targetList.append(itemRow('4'), itemRow('5'));
|
||||
|
||||
return {
|
||||
root,
|
||||
@@ -158,11 +153,7 @@ describe('Sortable lists controller', () => {
|
||||
await monitorOptions?.onDrop?.({
|
||||
source: sourcePayload(
|
||||
sourceElement,
|
||||
itemData(
|
||||
sourceElement.getAttribute('data-sortable-lists--item-id-value')!,
|
||||
'work_package',
|
||||
sourceElement.getAttribute('data-sortable-lists--item-move-url-value') ?? undefined,
|
||||
),
|
||||
itemData(sourceElement.getAttribute('data-sortable-lists--item-id-value')!),
|
||||
),
|
||||
location: {
|
||||
initial: {
|
||||
@@ -188,8 +179,8 @@ describe('Sortable lists controller', () => {
|
||||
});
|
||||
}
|
||||
|
||||
function itemData(itemId = '1', type = 'work_package', moveUrl?:string) {
|
||||
return sortableItemData({ itemId, moveUrl, type });
|
||||
function itemData(itemId = '1', type = 'work_package') {
|
||||
return sortableItemData({ itemId, type });
|
||||
}
|
||||
|
||||
function sourcePayload(element:HTMLElement, data:Record<string|symbol, unknown> = itemData()) {
|
||||
@@ -259,18 +250,22 @@ describe('Sortable lists controller', () => {
|
||||
|
||||
it('ignores drops that belong to another sortable lists root', async () => {
|
||||
fixture.innerHTML = `
|
||||
<div data-controller="sortable-lists" data-sortable-lists-accepted-type-value="work_package">
|
||||
<div
|
||||
data-controller="sortable-lists"
|
||||
data-sortable-lists-accepted-type-value="work_package"
|
||||
data-sortable-lists-move-url-template-value="/move/{id}"
|
||||
>
|
||||
<ul data-sortable-lists-target="list" data-sortable-lists-list-type="sprint" data-sortable-lists-list-id="1">
|
||||
<li data-sortable-lists--item-id-value="1" data-sortable-lists--item-type-value="work_package"></li>
|
||||
</ul>
|
||||
</div>
|
||||
<div data-controller="sortable-lists" data-sortable-lists-accepted-type-value="work_package">
|
||||
<div
|
||||
data-controller="sortable-lists"
|
||||
data-sortable-lists-accepted-type-value="work_package"
|
||||
data-sortable-lists-move-url-template-value="/move/{id}"
|
||||
>
|
||||
<ul data-sortable-lists-target="list" data-sortable-lists-list-type="sprint" data-sortable-lists-list-id="2">
|
||||
<li
|
||||
data-sortable-lists--item-id-value="10"
|
||||
data-sortable-lists--item-type-value="work_package"
|
||||
data-sortable-lists--item-move-url-value="/move-10"
|
||||
></li>
|
||||
<li data-sortable-lists--item-id-value="10" data-sortable-lists--item-type-value="work_package"></li>
|
||||
<li data-sortable-lists--item-id-value="11" data-sortable-lists--item-type-value="work_package"></li>
|
||||
</ul>
|
||||
</div>
|
||||
@@ -283,7 +278,7 @@ describe('Sortable lists controller', () => {
|
||||
const secondRootTarget = fixture.querySelector<HTMLElement>('[data-sortable-lists--item-id-value="11"]')!;
|
||||
|
||||
await firstRootMonitor?.onDrop?.({
|
||||
source: sourcePayload(secondRootSource, itemData('10', 'work_package', '/move-10')),
|
||||
source: sourcePayload(secondRootSource, itemData('10', 'work_package')),
|
||||
location: {
|
||||
initial: {
|
||||
dropTargets: [],
|
||||
@@ -311,7 +306,7 @@ describe('Sortable lists controller', () => {
|
||||
await ctx.nextFrame();
|
||||
|
||||
await vi.mocked(monitorForElements).mock.lastCall?.[0].onDrop?.({
|
||||
source: sourcePayload(firstSourceItem, itemData('1', 'meeting_agenda_item', '/move')),
|
||||
source: sourcePayload(firstSourceItem, itemData('1', 'meeting_agenda_item')),
|
||||
location: {
|
||||
initial: {
|
||||
dropTargets: [],
|
||||
@@ -350,7 +345,6 @@ describe('Sortable lists controller', () => {
|
||||
it('builds the move URL from the controller URI template', async () => {
|
||||
const { targetList, firstSourceItem } = renderFixture({
|
||||
moveUrlTemplate: '/projects/demo/backlogs/work_packages/{id}/move',
|
||||
itemMoveUrl: null,
|
||||
});
|
||||
|
||||
await ctx.nextFrame();
|
||||
@@ -362,31 +356,13 @@ describe('Sortable lists controller', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('uses the sortable item move URL before the controller URI template', async () => {
|
||||
const { targetList, firstSourceItem } = renderFixture({
|
||||
moveUrlTemplate: '/projects/demo/backlogs/work_packages/{id}/move',
|
||||
itemMoveUrl: '/custom/move',
|
||||
});
|
||||
it('does nothing when the controller has no move URL template', async () => {
|
||||
const { targetList, firstSourceItem } = renderFixture({ moveUrlTemplate: null });
|
||||
|
||||
await ctx.nextFrame();
|
||||
await dropCurrentItemOnList(firstSourceItem, targetList);
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledWith(
|
||||
'/custom/move',
|
||||
expect.objectContaining({ method: 'PUT' }),
|
||||
);
|
||||
});
|
||||
|
||||
it('falls back to the sortable item move URL while item-specific endpoints still exist', async () => {
|
||||
const { targetList, firstSourceItem } = renderFixture();
|
||||
|
||||
await ctx.nextFrame();
|
||||
await dropCurrentItemOnList(firstSourceItem, targetList);
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledWith(
|
||||
'/move',
|
||||
expect.objectContaining({ method: 'PUT' }),
|
||||
);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('marks the sortable lists root and global loading indicator while moving an item', async () => {
|
||||
|
||||
@@ -229,11 +229,7 @@ export default class SortableListsController extends Controller<HTMLElement> {
|
||||
}
|
||||
}
|
||||
|
||||
private resolveMoveUrl(data:{ itemId:string; moveUrl?:string }):string|null {
|
||||
if (data.moveUrl) {
|
||||
return data.moveUrl;
|
||||
}
|
||||
|
||||
private resolveMoveUrl(data:{ itemId:string }):string|null {
|
||||
if (this.hasMoveUrlTemplateValue) {
|
||||
return parseTemplate(this.moveUrlTemplateValue).expand({ id: data.itemId });
|
||||
}
|
||||
|
||||
@@ -131,14 +131,6 @@ describe('sortable lists drag and drop helpers', () => {
|
||||
expect(data.itemId).toEqual('42');
|
||||
expect(isSortableItemData(data)).toBe(true);
|
||||
});
|
||||
|
||||
it('includes a move URL when the sortable item has one', () => {
|
||||
expect(sortableItemData({ type: 'work_package', itemId: '42', moveUrl: '/move' })).toEqual(expect.objectContaining({
|
||||
itemId: '42',
|
||||
moveUrl: '/move',
|
||||
type: 'work_package',
|
||||
}));
|
||||
});
|
||||
});
|
||||
|
||||
describe('acceptsSortableItemType', () => {
|
||||
|
||||
@@ -48,7 +48,6 @@ export interface SortableItemData extends Record<string|symbol, unknown> {
|
||||
[sortableItemDataKey]:true;
|
||||
type:string;
|
||||
itemId:string;
|
||||
moveUrl?:string;
|
||||
}
|
||||
|
||||
export interface SortableListData extends Record<string|symbol, unknown> {
|
||||
@@ -75,17 +74,14 @@ export function isSortableListData(data:Record<string|symbol, unknown>):data is
|
||||
export function sortableItemData({
|
||||
type,
|
||||
itemId,
|
||||
moveUrl,
|
||||
}:{
|
||||
type:string;
|
||||
itemId:string;
|
||||
moveUrl?:string;
|
||||
}):SortableItemData {
|
||||
return {
|
||||
[sortableItemDataKey]: true,
|
||||
type,
|
||||
itemId,
|
||||
...(moveUrl ? { moveUrl } : {}),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -401,7 +401,6 @@ describe('Sortable lists item controller', () => {
|
||||
class="Box-row"
|
||||
data-controller="sortable-lists--item"
|
||||
data-test-selector="work-package-${itemId}"
|
||||
data-sortable-lists--item-move-url-value="/move"
|
||||
data-sortable-lists--item-id-value="${itemId}"
|
||||
data-sortable-lists--item-type-value="work_package"
|
||||
>
|
||||
@@ -473,14 +472,13 @@ describe('Sortable lists item controller', () => {
|
||||
expect(row.hasAttribute('data-dragging')).toBe(false);
|
||||
});
|
||||
|
||||
it('includes the optional move URL in the sortable item data', async () => {
|
||||
it('builds the sortable item data for the drag payload', async () => {
|
||||
const { article } = renderBacklogsRow();
|
||||
|
||||
await ctx.nextFrame();
|
||||
|
||||
expect(vi.mocked(draggable).mock.lastCall?.[0].getInitialData?.(draggableArgs(article))).toEqual(expect.objectContaining({
|
||||
itemId: '123',
|
||||
moveUrl: '/move',
|
||||
type: 'work_package',
|
||||
}));
|
||||
});
|
||||
|
||||
@@ -70,12 +70,10 @@ export default class ItemController extends Controller<HTMLElement> {
|
||||
|
||||
static values = {
|
||||
id: String,
|
||||
moveUrl: String,
|
||||
type: { type: String, default: 'item' },
|
||||
};
|
||||
|
||||
declare idValue:string;
|
||||
declare moveUrlValue:string;
|
||||
declare typeValue:string;
|
||||
|
||||
declare readonly handleTarget:HTMLElement;
|
||||
@@ -186,7 +184,6 @@ export default class ItemController extends Controller<HTMLElement> {
|
||||
private getItemData():SortableItemData {
|
||||
return sortableItemData({
|
||||
itemId: this.idValue,
|
||||
moveUrl: this.moveUrlValue || undefined,
|
||||
type: this.typeValue,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -233,12 +233,9 @@ RSpec.describe Backlogs::BucketComponent, type: :component do
|
||||
expect(rendered_component).to have_no_css(".Box-row#work_package_#{work_package.id}.Box-row--draggable")
|
||||
expect(rendered_component)
|
||||
.to have_no_css(".Box-row#work_package_#{work_package.id}[data-sortable-lists--item-id-value]")
|
||||
expect(rendered_component)
|
||||
.to have_no_css(".Box-row#work_package_#{work_package.id}[data-sortable-lists--item-move-url-value]")
|
||||
expect(rendered_component).to have_no_css(".Box-row#work_package_#{work_package.id}[draggable='true']")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-id-value]")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-target]")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-move-url-value]")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[draggable='true']")
|
||||
end
|
||||
end
|
||||
|
||||
@@ -146,12 +146,9 @@ RSpec.describe Backlogs::SprintComponent, type: :component do
|
||||
expect(rendered_component).to have_no_css(".Box-row#work_package_#{work_package1.id}.Box-row--draggable")
|
||||
expect(rendered_component)
|
||||
.to have_no_css(".Box-row#work_package_#{work_package1.id}[data-sortable-lists--item-id-value]")
|
||||
expect(rendered_component)
|
||||
.to have_no_css(".Box-row#work_package_#{work_package1.id}[data-sortable-lists--item-move-url-value]")
|
||||
expect(rendered_component).to have_no_css(".Box-row#work_package_#{work_package1.id}[draggable='true']")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-id-value]")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-target]")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[data-sortable-lists--item-move-url-value]")
|
||||
expect(rendered_component).to have_no_css(".op-work-package-card[draggable='true']")
|
||||
end
|
||||
end
|
||||
|
||||
-2
@@ -95,7 +95,6 @@ RSpec.describe Backlogs::WorkPackageCardListItemComponent, type: :component do
|
||||
it "does not mark the row as draggable" do
|
||||
expect(item.row_args[:classes]).not_to include("Box-row--draggable")
|
||||
expect(item.row_args[:data]).not_to include(:sortable_lists_prev_item_id)
|
||||
expect(item.row_args[:data]).not_to include(:sortable_lists__item_move_url_value)
|
||||
expect(item.row_args).not_to include(:draggable)
|
||||
expect(item.row_args).not_to include(:tabindex)
|
||||
end
|
||||
@@ -156,7 +155,6 @@ RSpec.describe Backlogs::WorkPackageCardListItemComponent, type: :component do
|
||||
".op-work-package-card[data-controller~='backlogs--story']"
|
||||
)
|
||||
expect(rendered_card).to have_no_css(".op-work-package-card[data-controller~='sortable-lists--item']")
|
||||
expect(rendered_card).to have_no_css(".op-work-package-card[data-sortable-lists--item-move-url-value]")
|
||||
expect(rendered_card).to have_no_css(".op-work-package-card[draggable='true']")
|
||||
end
|
||||
|
||||
|
||||
Reference in New Issue
Block a user