diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 1806205870..ccc766ac6d 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -19,6 +19,7 @@ Changelog ~~~~~~~~~~~~~~~~~~ * Fix: Handle `child_block` being passed as a kwarg in ListBlock migrations (Matt Westcott) + * Fix: Fix broken task type filter in workflow task chooser modal (Sage Abdullah) 6.2 (01.08.2024) diff --git a/client/src/controllers/SwapController.test.js b/client/src/controllers/SwapController.test.js index 8bbaaa9d74..4e7f0d4d9e 100644 --- a/client/src/controllers/SwapController.test.js +++ b/client/src/controllers/SwapController.test.js @@ -558,6 +558,71 @@ describe('SwapController', () => { }); }); + describe('performing a content update via actions on a controlled button without a form', () => { + beforeEach(() => { + document.body.innerHTML = ` + +
+ `; + }); + + it('should default the request method to GET', async () => { + const button = document.getElementById('clear'); + const targetElement = document.getElementById('results'); + + const results = getMockResults(); + + const onSuccess = new Promise((resolve) => { + document.addEventListener('w-swap:success', resolve); + }); + + fetch.mockResponseSuccessText(results); + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + expect(targetElement.getAttribute('aria-busy')).toBeNull(); + + button.click(); + + jest.runAllTimers(); // update is debounced + + // the content should be marked as busy + await Promise.resolve(); // trigger next rendering + expect(targetElement.getAttribute('aria-busy')).toEqual('true'); + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + '/admin/custom/results/?type=bar', + expect.objectContaining({ + method: 'GET', + body: undefined, + }), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: '/admin/custom/results/?type=bar', + results, + }); + + // should update HTML + expect(targetElement.querySelectorAll('li')).toHaveLength(3); + + await flushPromises(); + + // should reset the busy state + expect(targetElement.getAttribute('aria-busy')).toBeNull(); + }); + }); + describe('performing a content update via actions on a controlled form without using form values', () => { let beginEventHandler; let formElement; @@ -731,7 +796,7 @@ describe('SwapController', () => { 'x-requested-with': 'XMLHttpRequest', 'x-xsrf-token': 'potato', }, - method: 'post', + method: 'POST', }), ); // We are using #replace, not #submit, so we should not have a body @@ -1029,6 +1094,11 @@ describe('SwapController', () => { document.addEventListener('w-swap:success', resolve); }); + // Even if the attribute and the property use lowercase + const formElement = document.querySelector('form'); + expect(formElement.getAttribute('method')).toEqual('get'); + expect(formElement.method).toEqual('get'); + const beginEventHandler = jest.fn(); document.addEventListener('w-swap:begin', beginEventHandler); @@ -1059,7 +1129,11 @@ describe('SwapController', () => { expect(handleError).not.toHaveBeenCalled(); expect(global.fetch).toHaveBeenCalledWith( '/path/to/form/action/?q=alpha&type=some-type&other=something+on+other', - expect.any(Object), + expect.objectContaining({ + // Should normalize the method name to uppercase and not send a body + method: 'GET', + body: undefined, + }), ); const successEvent = await onSuccess; @@ -1133,7 +1207,7 @@ describe('SwapController', () => { 'x-requested-with': 'XMLHttpRequest', 'x-xsrf-token': 'potato', }, - method: 'post', + method: 'POST', body: expect.any(FormData), }), ); @@ -1166,6 +1240,86 @@ describe('SwapController', () => { expect(window.location.search).toEqual(''); }); + it('should use the normalized method name and not send a body in a GET request', async () => { + const input = document.getElementById('search'); + const formElement = document.querySelector('form'); + + // Use a non-standard casing for the method + formElement.setAttribute('method', 'Get'); + expect(formElement.getAttribute('method')).toEqual('Get'); + + // The method property is an enum that always uses lowercase + // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#attr-fs-method + expect(formElement.method).toEqual('get'); + + const results = getMockResults({ total: 5 }); + + const onSuccess = new Promise((resolve) => { + document.addEventListener('w-swap:success', resolve); + }); + + const beginEventHandler = jest.fn(); + document.addEventListener('w-swap:begin', beginEventHandler); + + fetch.mockResponseSuccessText(results); + + expect(window.location.search).toEqual(''); + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).not.toHaveBeenCalled(); + + input.value = 'alpha'; + document.querySelector('[name="other"]').value = 'something on other'; + input.dispatchEvent(new CustomEvent('change', { bubbles: true })); + + expect(beginEventHandler).not.toHaveBeenCalled(); + + jest.runAllTimers(); // search is debounced + + // should fire a begin event before the request is made + const expectedRequestUrl = + '/path/to/form/action/?q=alpha&type=some-type&other=something+on+other'; + expect(beginEventHandler).toHaveBeenCalledTimes(1); + expect(beginEventHandler.mock.calls[0][0].detail).toEqual({ + requestUrl: expectedRequestUrl, + }); + + // visual loading state should be active + await Promise.resolve(); // trigger next rendering + + expect(handleError).not.toHaveBeenCalled(); + expect(global.fetch).toHaveBeenCalledWith( + // The form data should be sent as query params, without the body + expectedRequestUrl, + expect.objectContaining({ + headers: { + 'x-requested-with': 'XMLHttpRequest', + 'x-xsrf-token': 'potato', + }, + method: 'GET', // normalized method name should be in uppercase + body: undefined, + }), + ); + + const successEvent = await onSuccess; + + // should dispatch success event + expect(successEvent.detail).toEqual({ + requestUrl: expectedRequestUrl, + results: expect.any(String), + }); + + // should update HTML + expect( + document.getElementById('task-results').querySelectorAll('li').length, + ).toBeTruthy(); + + await flushPromises(); + + // should NOT update the current URL + // as the reflect-value attribute is not set + expect(window.location.search).toEqual(''); + }); + it('should reflect the query params of the request URL if reflect-value is true', async () => { const formElement = document.querySelector('form'); formElement.setAttribute('data-w-swap-reflect-value', 'true'); diff --git a/client/src/controllers/SwapController.ts b/client/src/controllers/SwapController.ts index 3b2c5bf5e2..0085295093 100644 --- a/client/src/controllers/SwapController.ts +++ b/client/src/controllers/SwapController.ts @@ -5,8 +5,8 @@ import { WAGTAIL_CONFIG } from '../config/wagtailConfig'; /** * Allow for an element to trigger an async query that will - * patch the results into a results DOM container. The query - * input can be the controlled element or the containing form. + * patch the results into a results DOM container. The controlled + * element can be the query input, the containing form, or a button. * It supports the ability to update the URL with the query * when processed or simply make a query based on a form's * values. @@ -35,9 +35,21 @@ import { WAGTAIL_CONFIG } from '../config/wagtailConfig'; * data-w-swap-target-value="#listing-results" * /> * + * @example - A single button that will update the results + * + * + * */ export class SwapController extends Controller< - HTMLFormElement | HTMLInputElement + HTMLFormElement | HTMLInputElement | HTMLButtonElement > { static defaultClearParam = 'p'; @@ -220,13 +232,14 @@ export class SwapController extends Controller< */ submit() { const form = this.formElement; - const data = new FormData(form); + let data: FormData | undefined = new FormData(form); let url = this.srcValue; // serialise the form to a query string if it's a GET request - if (form.method === 'get') { + if (form.getAttribute('method')?.toUpperCase() === 'GET') { // cast as any to avoid https://github.com/microsoft/TypeScript/issues/43797 url += '?' + new URLSearchParams(data as any).toString(); + data = undefined; } this.replace(url, data); @@ -279,7 +292,8 @@ export class SwapController extends Controller< }) as CustomEvent<{ requestUrl: string }>; if (beginEvent.defaultPrevented) return Promise.resolve(); - const formMethod = this.formElement.getAttribute('method') || undefined; + const formMethod = + this.formElement.getAttribute('method')?.toUpperCase() || 'GET'; return fetch(requestUrl, { headers: { 'x-requested-with': 'XMLHttpRequest', @@ -287,7 +301,7 @@ export class SwapController extends Controller< }, signal, method: formMethod, - body: formMethod !== 'get' ? data : undefined, + body: formMethod !== 'GET' ? data : undefined, }) .then(async (response) => { if (!response.ok) { diff --git a/docs/releases/6.2.1.md b/docs/releases/6.2.1.md index 502dc0e679..b976927fa5 100644 --- a/docs/releases/6.2.1.md +++ b/docs/releases/6.2.1.md @@ -15,3 +15,4 @@ depth: 1 ### Bug fixes * Handle `child_block` being passed as a kwarg in ListBlock migrations (Matt Westcott) + * Fix broken task type filter in workflow task chooser modal (Sage Abdullah)