From 4fa2a1c5a70d321613b8857901ef023ff9673bd8 Mon Sep 17 00:00:00 2001 From: jojo112628e Date: Mon, 8 Jun 2026 15:03:26 -0700 Subject: [PATCH 1/6] fix: store returned event id after check-in to enable same-page deletion (#12417) When a check-in is created and immediately deleted without refreshing the page, the DELETE request was sent to /check-ins/ with no ID, resulting in a 404. The POST response body containing the new event id was never parsed, so checkInForm.eventId remained null. Fix by awaiting resp.json() in both submit-check-in handlers and calling setEventId() with the returned id. Co-Authored-By: Claude Sonnet 4.5 --- .../js/my-books/MyBooksDropper/CheckInComponents.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js index 64f62ff4e0a..c29996fce80 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js +++ b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js @@ -95,10 +95,14 @@ export class CheckInComponents { const eventData = this.prepareEventRequest(year, month, day); this.postCheckIn(eventData, this.checkInForm.getFormAction()) - .then((resp) => { + .then(async (resp) => { if (!resp.ok) { throw Error(`Check-in request failed. Status: ${resp.status}`); } + const data = await resp.json(); + if (data.id) { + this.checkInForm.setEventId(data.id); + } this.updateDateAndShowDisplay(year, month, day); }) .catch(() => { @@ -147,10 +151,14 @@ export class CheckInComponents { const eventData = this.prepareEventRequest(year, month, day); this.postCheckIn(eventData, this.checkInForm.getFormAction()) - .then((resp) => { + .then(async (resp) => { if (!resp.ok) { throw Error(`Check-in request failed. Status: ${resp.status}`); } + const data = await resp.json(); + if (data.id) { + this.checkInForm.setEventId(data.id); + } this.updateDateAndShowDisplay(year, month, day); }) .catch(() => { From 582fd5968997db6fbc1617e7ee7155faf03da409 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 8 Jun 2026 22:04:36 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- .../js/my-books/MyBooksDropper/CheckInComponents.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js index c29996fce80..e53e89c3258 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js +++ b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js @@ -95,7 +95,7 @@ export class CheckInComponents { const eventData = this.prepareEventRequest(year, month, day); this.postCheckIn(eventData, this.checkInForm.getFormAction()) - .then(async (resp) => { + .then(async(resp) => { if (!resp.ok) { throw Error(`Check-in request failed. Status: ${resp.status}`); } @@ -151,7 +151,7 @@ export class CheckInComponents { const eventData = this.prepareEventRequest(year, month, day); this.postCheckIn(eventData, this.checkInForm.getFormAction()) - .then(async (resp) => { + .then(async(resp) => { if (!resp.ok) { throw Error(`Check-in request failed. Status: ${resp.status}`); } From 0adba16f7fab7b0298353b0dbad3a1fc80a6684a Mon Sep 17 00:00:00 2001 From: jojo112628e Date: Mon, 8 Jun 2026 15:09:40 -0700 Subject: [PATCH 3/6] test: add unit tests for CheckInComponents event id storage after check-in (#12417) Verify that the event id returned by the server after a successful POST is stored via setEventId() in both the prompt and form submit-check-in handlers, enabling same-page deletion without a page refresh. Co-Authored-By: Claude Sonnet 4.5 --- tests/unit/js/my-books.test.js | 89 ++++++++++++++++++- .../unit/js/sample-html/checkIns-test-data.js | 76 ++++++++++++++++ 2 files changed, 163 insertions(+), 2 deletions(-) diff --git a/tests/unit/js/my-books.test.js b/tests/unit/js/my-books.test.js index 4130f33c511..e361a1cb026 100644 --- a/tests/unit/js/my-books.test.js +++ b/tests/unit/js/my-books.test.js @@ -1,9 +1,94 @@ import { listCreationForm } from './sample-html/lists-test-data'; -import { checkInForm } from './sample-html/checkIns-test-data'; +import { checkInForm, checkInContainer, checkInFormModal } from './sample-html/checkIns-test-data'; import { CreateListForm } from '../../../openlibrary/plugins/openlibrary/js/my-books/CreateListForm'; -import { CheckInForm } from '../../../openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents'; +import { CheckInComponents, CheckInForm } from '../../../openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents'; jest.mock('jquery-ui/ui/widgets/dialog', () => {}); +jest.mock('../../../openlibrary/plugins/openlibrary/js/dialog', () => ({ + initDialogClosers: jest.fn(), +})); +jest.mock('../../../openlibrary/plugins/openlibrary/js/Toast', () => ({ + PersistentToast: jest.fn().mockImplementation(() => ({ show: jest.fn() })), +})); + +describe('CheckInComponents class', () => { + beforeEach(() => { + document.body.innerHTML = checkInContainer + checkInFormModal; + // Mock $.colorbox used by closeModal() + global.$ = { colorbox: { close: jest.fn() } }; + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('stores the event id from the server response after a successful check-in via prompt', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: jest.fn().mockResolvedValue({ status: 'ok', id: 789 }), + }); + + const containerElem = document.querySelector('.check-in-container'); + const components = new CheckInComponents(containerElem); + components.initialize(); + + // Simulate clicking "Today" shortcut in the check-in prompt + const submitEvent = new CustomEvent('submit-check-in', { + detail: { year: 2024, month: 6, day: 15 }, + }); + components.checkInPrompt.getRootElement().dispatchEvent(submitEvent); + + // Wait for the fetch promise and json() to resolve + await new Promise(resolve => setTimeout(resolve, 0)); + + // The event id returned by the server should now be stored in the form + // so that a subsequent DELETE request uses the correct URL + expect(components.checkInForm.getEventId()).toBe('789'); + }); + + it('stores the event id from the server response after a successful check-in via form submit button', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + json: jest.fn().mockResolvedValue({ status: 'ok', id: 456 }), + }); + + const containerElem = document.querySelector('.check-in-container'); + const components = new CheckInComponents(containerElem); + components.initialize(); + + // Simulate submitting via the modal form's submit button + const submitEvent = new CustomEvent('submit-check-in', { + detail: { year: 2024, month: 1, day: 1 }, + }); + components.checkInForm.getRootElement().dispatchEvent(submitEvent); + + await new Promise(resolve => setTimeout(resolve, 0)); + + expect(components.checkInForm.getEventId()).toBe('456'); + }); + + it('does not set event id when server response has no id field', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: true, + // Server returns ok but no id (edge case) + json: jest.fn().mockResolvedValue({ status: 'ok' }), + }); + + const containerElem = document.querySelector('.check-in-container'); + const components = new CheckInComponents(containerElem); + components.initialize(); + + const submitEvent = new CustomEvent('submit-check-in', { + detail: { year: 2024, month: 3, day: 10 }, + }); + components.checkInPrompt.getRootElement().dispatchEvent(submitEvent); + + await new Promise(resolve => setTimeout(resolve, 0)); + + // eventId should remain empty — no crash, no bad value stored + expect(components.checkInForm.getEventId()).toBe(''); + }); +}); describe('CreateListForm.js class', () => { let form; diff --git a/tests/unit/js/sample-html/checkIns-test-data.js b/tests/unit/js/sample-html/checkIns-test-data.js index a653c1b4db1..f8eedc91fdd 100644 --- a/tests/unit/js/sample-html/checkIns-test-data.js +++ b/tests/unit/js/sample-html/checkIns-test-data.js @@ -1,3 +1,79 @@ +/** + * HTML fixture for the CheckInComponents container element. + * Mirrors the structure expected by the CheckInComponents constructor. + */ +export const checkInContainer = ` +
+
+ This year + Today +
+ +
+`; + +/** + * HTML fixture for the check-in form modal template. + * Must be present in the DOM with id="check-in-form-modal" so that + * CheckInComponents.createModalContentFromTemplate() can clone it. + */ +export const checkInFormModal = ` +
+
+ + + +
+ + + + + + + Today +
+ + + + +
+
+`; + export const checkInForm = `
From 251a3e26dbbdca497a816ac6ddc1896e5ac3b080 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 8 Jun 2026 22:10:37 +0000 Subject: [PATCH 4/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/unit/js/my-books.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/js/my-books.test.js b/tests/unit/js/my-books.test.js index e361a1cb026..5177065de73 100644 --- a/tests/unit/js/my-books.test.js +++ b/tests/unit/js/my-books.test.js @@ -22,7 +22,7 @@ describe('CheckInComponents class', () => { jest.clearAllMocks(); }); - it('stores the event id from the server response after a successful check-in via prompt', async () => { + it('stores the event id from the server response after a successful check-in via prompt', async() => { global.fetch = jest.fn().mockResolvedValue({ ok: true, json: jest.fn().mockResolvedValue({ status: 'ok', id: 789 }), @@ -46,7 +46,7 @@ describe('CheckInComponents class', () => { expect(components.checkInForm.getEventId()).toBe('789'); }); - it('stores the event id from the server response after a successful check-in via form submit button', async () => { + it('stores the event id from the server response after a successful check-in via form submit button', async() => { global.fetch = jest.fn().mockResolvedValue({ ok: true, json: jest.fn().mockResolvedValue({ status: 'ok', id: 456 }), @@ -67,7 +67,7 @@ describe('CheckInComponents class', () => { expect(components.checkInForm.getEventId()).toBe('456'); }); - it('does not set event id when server response has no id field', async () => { + it('does not set event id when server response has no id field', async() => { global.fetch = jest.fn().mockResolvedValue({ ok: true, // Server returns ok but no id (edge case) From 0d5e35cd42410707e24ac989947eafa01b502411 Mon Sep 17 00:00:00 2001 From: RayBB Date: Wed, 17 Jun 2026 11:43:59 -0700 Subject: [PATCH 5/6] improve tests --- .../MyBooksDropper/CheckInComponents.js | 2 +- tests/unit/js/my-books.test.js | 107 +++++++++++++----- 2 files changed, 78 insertions(+), 31 deletions(-) diff --git a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js index e53e89c3258..cedfa9bbb60 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js +++ b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js @@ -236,7 +236,7 @@ export class CheckInComponents { return fetch(url, { method: 'POST', headers: { - 'content-type': 'application/x-www-form-urlencoded', + 'content-type': 'application/json', accept: 'application/json' }, body: JSON.stringify(eventData) diff --git a/tests/unit/js/my-books.test.js b/tests/unit/js/my-books.test.js index 5177065de73..5051911b831 100644 --- a/tests/unit/js/my-books.test.js +++ b/tests/unit/js/my-books.test.js @@ -11,6 +11,50 @@ jest.mock('../../../openlibrary/plugins/openlibrary/js/Toast', () => ({ PersistentToast: jest.fn().mockImplementation(() => ({ show: jest.fn() })), })); +/** + * Creates and initializes a CheckInComponents instance for testing. + * @returns {CheckInComponents} + */ +function createAndInitialize() { + const containerElem = document.querySelector('.check-in-container'); + const components = new CheckInComponents(containerElem); + components.initialize(); + return components; +} + +/** Shorthand to flush pending promises in async tests. */ +function flushPromises() { + return new Promise(resolve => setTimeout(resolve, 0)); +} + +/** + * Dispatches a submit-check-in event on the check-in prompt element + * and waits for async resolution. + * @param {CheckInComponents} components + * @param {{year: number, month: number, day: number}} detail + */ +async function dispatchSubmitOnPrompt(components, {year, month, day}) { + const submitEvent = new CustomEvent('submit-check-in', { + detail: {year, month, day}, + }); + components.checkInPrompt.getRootElement().dispatchEvent(submitEvent); + await flushPromises(); +} + +/** + * Dispatches a submit-check-in event on the form element + * and waits for async resolution. + * @param {CheckInComponents} components + * @param {{year: number, month: number, day: number}} detail + */ +async function dispatchSubmitOnForm(components, {year, month, day}) { + const submitEvent = new CustomEvent('submit-check-in', { + detail: {year, month, day}, + }); + components.checkInForm.getRootElement().dispatchEvent(submitEvent); + await flushPromises(); +} + describe('CheckInComponents class', () => { beforeEach(() => { document.body.innerHTML = checkInContainer + checkInFormModal; @@ -28,18 +72,9 @@ describe('CheckInComponents class', () => { json: jest.fn().mockResolvedValue({ status: 'ok', id: 789 }), }); - const containerElem = document.querySelector('.check-in-container'); - const components = new CheckInComponents(containerElem); - components.initialize(); + const components = createAndInitialize(); - // Simulate clicking "Today" shortcut in the check-in prompt - const submitEvent = new CustomEvent('submit-check-in', { - detail: { year: 2024, month: 6, day: 15 }, - }); - components.checkInPrompt.getRootElement().dispatchEvent(submitEvent); - - // Wait for the fetch promise and json() to resolve - await new Promise(resolve => setTimeout(resolve, 0)); + await dispatchSubmitOnPrompt(components, {year: 2024, month: 6, day: 15}); // The event id returned by the server should now be stored in the form // so that a subsequent DELETE request uses the correct URL @@ -52,17 +87,9 @@ describe('CheckInComponents class', () => { json: jest.fn().mockResolvedValue({ status: 'ok', id: 456 }), }); - const containerElem = document.querySelector('.check-in-container'); - const components = new CheckInComponents(containerElem); - components.initialize(); - - // Simulate submitting via the modal form's submit button - const submitEvent = new CustomEvent('submit-check-in', { - detail: { year: 2024, month: 1, day: 1 }, - }); - components.checkInForm.getRootElement().dispatchEvent(submitEvent); + const components = createAndInitialize(); - await new Promise(resolve => setTimeout(resolve, 0)); + await dispatchSubmitOnForm(components, {year: 2024, month: 1, day: 1}); expect(components.checkInForm.getEventId()).toBe('456'); }); @@ -74,20 +101,40 @@ describe('CheckInComponents class', () => { json: jest.fn().mockResolvedValue({ status: 'ok' }), }); - const containerElem = document.querySelector('.check-in-container'); - const components = new CheckInComponents(containerElem); - components.initialize(); + const components = createAndInitialize(); - const submitEvent = new CustomEvent('submit-check-in', { - detail: { year: 2024, month: 3, day: 10 }, - }); - components.checkInPrompt.getRootElement().dispatchEvent(submitEvent); - - await new Promise(resolve => setTimeout(resolve, 0)); + await dispatchSubmitOnPrompt(components, {year: 2024, month: 3, day: 10}); // eventId should remain empty — no crash, no bad value stored expect(components.checkInForm.getEventId()).toBe(''); }); + + it('performs a DELETE request with the stored event id after a check-in then delete', async() => { + const mockFetch = jest.fn() + .mockResolvedValueOnce({ + ok: true, + json: jest.fn().mockResolvedValue({ status: 'ok', id: 789 }), + }) + .mockResolvedValueOnce({ + ok: true, + json: jest.fn().mockResolvedValue({ status: 'ok' }), + }); + global.fetch = mockFetch; + + const components = createAndInitialize(); + + // Step 1: Check in — stores the event id from the response + await dispatchSubmitOnPrompt(components, {year: 2024, month: 6, day: 15}); + expect(components.checkInForm.getEventId()).toBe('789'); + + // Step 2: Dispatch delete event — triggers DELETE /check-ins/ + const deleteEvent = new CustomEvent('delete-check-in'); + components.checkInForm.getRootElement().dispatchEvent(deleteEvent); + await flushPromises(); + + // Step 3: Verify the DELETE was sent to the correct URL + expect(mockFetch).toHaveBeenNthCalledWith(2, '/check-ins/789', { method: 'DELETE' }); + }); }); describe('CreateListForm.js class', () => { From d40df64f321f30be21a6aa31c8303328af864f40 Mon Sep 17 00:00:00 2001 From: RayBB Date: Wed, 17 Jun 2026 11:46:02 -0700 Subject: [PATCH 6/6] remove extra change --- .../openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js index cedfa9bbb60..e53e89c3258 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js +++ b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js @@ -236,7 +236,7 @@ export class CheckInComponents { return fetch(url, { method: 'POST', headers: { - 'content-type': 'application/json', + 'content-type': 'application/x-www-form-urlencoded', accept: 'application/json' }, body: JSON.stringify(eventData)