From a7a9195dd18da34a39893e6f306a4857b5a696f0 Mon Sep 17 00:00:00 2001 From: Holger Ehrmann Date: Thu, 2 Apr 2026 19:51:15 +0200 Subject: [PATCH 01/15] Added tests for TDD --- tests/DraftControllerTest.php | 11 ++ tests/js/autosaveService.test.js | 75 +++++++++ tests/js/descriptionTypes.test.js | 33 ++++ tests/js/freekeywordTags.test.js | 4 +- tests/js/language.module.test.js | 21 +++ .../playwright/flows/stage-diagnosis.spec.ts | 159 ++++++++++++++++++ 6 files changed, 301 insertions(+), 2 deletions(-) create mode 100644 tests/playwright/flows/stage-diagnosis.spec.ts diff --git a/tests/DraftControllerTest.php b/tests/DraftControllerTest.php index 91a129d55..f3024793f 100644 --- a/tests/DraftControllerTest.php +++ b/tests/DraftControllerTest.php @@ -274,4 +274,15 @@ public function testDifferentSessionCannotReadDraft(): void $this->assertSame(403, $forbiddenStatus); } + + public function testGetNonExistentDraftReturns204(): void + { + $controller = new DraftController(); + [$status, $data] = $this->captureResponse(function () use ($controller) { + $controller->get(['id' => 'aaaabbbbccccdddd1111222233334444']); + }); + + $this->assertSame(204, $status); + $this->assertNull($data); + } } \ No newline at end of file diff --git a/tests/js/autosaveService.test.js b/tests/js/autosaveService.test.js index e1d42780b..7df2c4d45 100644 --- a/tests/js/autosaveService.test.js +++ b/tests/js/autosaveService.test.js @@ -429,4 +429,79 @@ describe('autosaveService', () => { expect(fetchMock.mock.calls[0][0]).toBe('/mde-msl/api/v2/drafts'); expect(service.apiBaseUrl).toBe('/mde-msl/api/v2'); }); + + test('clears stale draftId from localStorage when checkForExistingDraft gets 204', async () => { + window.localStorage.setItem('elmo.autosave.draftId', 'stale-draft-id'); + + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 204, + json: () => Promise.resolve(null) + }); + + const service = new AutosaveService('form-mde', { + fetch: fetchMock, + throttleMs: 0, + statusElementId: 'autosave-status', + statusTextId: 'autosave-status-text', + restoreModalId: 'modal-restore-draft' + }); + + service.start(); + await Promise.resolve(); + await Promise.resolve(); + + expect(window.localStorage.getItem('elmo.autosave.draftId')).toBeNull(); + expect(service.draftId).toBeNull(); + }); + + test('clears stale draftId from localStorage when checkForExistingDraft gets 404', async () => { + window.localStorage.setItem('elmo.autosave.draftId', 'old-draft-id'); + + const fetchMock = jest.fn().mockResolvedValue({ + ok: false, + status: 404, + json: () => Promise.resolve({ error: 'Draft not found' }) + }); + + const service = new AutosaveService('form-mde', { + fetch: fetchMock, + throttleMs: 0, + statusElementId: 'autosave-status', + statusTextId: 'autosave-status-text', + restoreModalId: 'modal-restore-draft' + }); + + service.start(); + await Promise.resolve(); + await Promise.resolve(); + + expect(window.localStorage.getItem('elmo.autosave.draftId')).toBeNull(); + expect(service.draftId).toBeNull(); + }); + + test('does not clear draftId when 204 response is for session/latest (no stored draft)', async () => { + // No stored draftId => service queries session/latest + const fetchMock = jest.fn().mockResolvedValue({ + ok: true, + status: 204, + json: () => Promise.resolve(null) + }); + + const service = new AutosaveService('form-mde', { + fetch: fetchMock, + throttleMs: 0, + statusElementId: 'autosave-status', + statusTextId: 'autosave-status-text', + restoreModalId: 'modal-restore-draft' + }); + + service.start(); + await Promise.resolve(); + await Promise.resolve(); + + // draftId should remain null (was never set) + expect(service.draftId).toBeNull(); + expect(fetchMock.mock.calls[0][0]).toBe('./api/v2/drafts/session/latest'); + }); }); \ No newline at end of file diff --git a/tests/js/descriptionTypes.test.js b/tests/js/descriptionTypes.test.js index 2a6ae33ce..5519af528 100644 --- a/tests/js/descriptionTypes.test.js +++ b/tests/js/descriptionTypes.test.js @@ -214,6 +214,39 @@ describe('descriptionTypes.js', () => { expect(slugs).toEqual([]); }); + test('resolves even when applyTranslations throws', async () => { + window.applyTranslations = jest.fn(() => { + throw new TypeError("Cannot read properties of undefined (reading 'logoTitle')"); + }); + + $.ajax.mockImplementation(function (options) { + options.success([ + { id: 1, name: 'Abstract', slug: 'Abstract' }, + { id: 2, name: 'Methods', slug: 'Methods' } + ]); + }); + + const slugs = await module.initDescriptionTypes(); + + expect(slugs).toEqual(['Methods']); + expect(window.ELMO_ACTIVE_DESCRIPTION_TYPES).toEqual(['Methods']); + }); + + test('resolves even when updateHelpStatus throws', async () => { + window.updateHelpStatus = undefined; + global.updateHelpStatus = jest.fn(() => { + throw new Error('updateHelpStatus explosion'); + }); + + $.ajax.mockImplementation(function (options) { + options.success([{ id: 2, name: 'Methods', slug: 'Methods' }]); + }); + + const slugs = await module.initDescriptionTypes(); + + expect(slugs).toEqual(['Methods']); + }); + test('renders all 5 dynamic types correctly', async () => { $.ajax.mockImplementation(function (options) { options.success([ diff --git a/tests/js/freekeywordTags.test.js b/tests/js/freekeywordTags.test.js index d8492d5b6..bcc782e1d 100644 --- a/tests/js/freekeywordTags.test.js +++ b/tests/js/freekeywordTags.test.js @@ -81,13 +81,13 @@ describe('freekeywordTags.js', () => { errSpy.mockRestore(); }); - test('logs when no keywords returned', () => { + test('does not log when no keywords returned', () => { const logSpy = jest.spyOn(console, 'log').mockImplementation(() => {}); loadScript(() => ({ done(cb) { cb([]); return { fail: jest.fn() }; }, fail: jest.fn() })); - expect(logSpy).toHaveBeenCalledWith('ELMO currently has no curated keywords.'); + expect(logSpy).not.toHaveBeenCalledWith('ELMO currently has no curated keywords.'); const input = document.getElementById('input-freekeyword'); expect(input._tagify.settings.whitelist).toEqual([]); logSpy.mockRestore(); diff --git a/tests/js/language.module.test.js b/tests/js/language.module.test.js index ae94560e4..d2143ed07 100644 --- a/tests/js/language.module.test.js +++ b/tests/js/language.module.test.js @@ -207,6 +207,27 @@ describe('language module coverage', () => { // applyTranslations should work without throwing expect(() => languageModule.applyTranslations()).not.toThrow(); }); + + test('returns early without errors when translations are empty object', () => { + window.setTranslations({}); + const originalTitle = document.title; + expect(() => languageModule.applyTranslations()).not.toThrow(); + expect(document.title).toBe(originalTitle); + expect(window.resizeTitle).not.toHaveBeenCalled(); + }); + + test('returns early without errors when translations.general is undefined', () => { + window.setTranslations({ other: { key: 'value' } }); + const originalTitle = document.title; + expect(() => languageModule.applyTranslations()).not.toThrow(); + expect(document.title).toBe(originalTitle); + expect(window.adjustButtons).not.toHaveBeenCalled(); + }); + + test('returns early without errors when translations are null', () => { + window.setTranslations(null); + expect(() => languageModule.applyTranslations()).not.toThrow(); + }); }); describe('loadTranslations', () => { diff --git a/tests/playwright/flows/stage-diagnosis.spec.ts b/tests/playwright/flows/stage-diagnosis.spec.ts new file mode 100644 index 000000000..d59794a74 --- /dev/null +++ b/tests/playwright/flows/stage-diagnosis.spec.ts @@ -0,0 +1,159 @@ +/** + * Regression test for console errors and XML upload field population. + * + * Verifies that the following issues do not regress: + * - Race condition in applyTranslations() when descriptionTypes AJAX resolves first + * - Abstract and Funder fields not populated during XML upload + * - Duplicate modal IDs in HTML + * + * Run against Stage: + * APP_BASE_URL=https://env.rz-vm182.gfz.de/elmo/ npx playwright test stage-diagnosis --project=chromium + * Run locally: + * npx playwright test stage-diagnosis --project=chromium + */ +import { test, expect } from '@playwright/test'; + +const SAMPLE_XML = ` + + 10.1234/elmo.test + 2024 + en + + Stage Diagnosis Test + + + + Doe, Jane + Jane + Doe + + + + This is a test abstract for diagnosis. + + Dataset + + CC BY 4.0 + + + + Deutsche Forschungsgemeinschaft + 501100001659 + TEST123 + Test Grant Title + + +`; + +test.describe('Console errors regression', () => { + test('no JavaScript errors on initial page load', async ({ page, baseURL }) => { + const jsErrors: string[] = []; + + page.on('pageerror', (err) => { + jsErrors.push(err.message); + }); + + await page.goto(baseURL!, { waitUntil: 'domcontentloaded' }); + + // Wait for async initialization (description types, translations, etc.) + await page.waitForTimeout(8000); + + // Filter out known external/non-critical warnings + const criticalErrors = jsErrors.filter( + (e) => !e.includes('google.maps') && !e.includes('installHook') + ); + + expect(criticalErrors).toEqual([]); + }); + + test('descriptionTypesReady promise resolves', async ({ page, baseURL }) => { + await page.goto(baseURL!, { waitUntil: 'domcontentloaded' }); + await page.waitForTimeout(5000); + + const promiseState = await page.evaluate(() => { + return new Promise((resolve) => { + if (!(window as any).descriptionTypesReady) { + resolve('NOT_SET'); + return; + } + let resolved = false; + (window as any).descriptionTypesReady.then(() => { + resolved = true; + resolve('RESOLVED'); + }); + setTimeout(() => { + if (!resolved) resolve('STUCK'); + }, 5000); + }); + }); + + expect(promiseState).toBe('RESOLVED'); + }); + + test('no duplicate HTML IDs on upload modal', async ({ page, baseURL }) => { + await page.goto(baseURL!, { waitUntil: 'domcontentloaded' }); + await page.waitForTimeout(3000); + + // Verify the upload modal ID is unique (was duplicated on both div and h5) + const uploadModalCount = await page.evaluate(() => { + return document.querySelectorAll('#modal-uploadxml').length; + }); + + expect(uploadModalCount).toBe(1); + + // Verify the label element now has a distinct ID + const labelExists = await page.evaluate(() => { + return document.querySelector('#modal-uploadxml-label') !== null; + }); + + expect(labelExists).toBe(true); + }); +}); + +test.describe('XML Upload field population regression', () => { + test('Abstract and Funder fields are populated after XML upload', async ({ page, baseURL }) => { + await page.goto(baseURL!, { waitUntil: 'domcontentloaded' }); + await page.waitForTimeout(5000); + + // Click Load button + const loadButton = page.locator('#button-form-load'); + await expect(loadButton).toBeVisible({ timeout: 10000 }); + await loadButton.click(); + + // Wait for upload modal + const modal = page.locator('div#modal-uploadxml'); + await expect(modal).toBeVisible({ timeout: 5000 }); + + // Upload XML file + await page.setInputFiles('#input-uploadxml-file', { + name: 'regression-test.xml', + mimeType: 'text/xml', + buffer: Buffer.from(SAMPLE_XML, 'utf-8'), + }); + + // Wait for title to be populated (indicates loadXmlToForm completed initial steps) + await expect(page.locator('#input-resourceinformation-title')).toHaveValue( + 'Stage Diagnosis Test', + { timeout: 15000 } + ); + + // Wait for async processing (descriptionTypesReady + processFunders) + await page.waitForTimeout(3000); + + // Assert Abstract was populated + await expect(page.locator('#input-abstract')).toHaveValue( + 'This is a test abstract for diagnosis.' + ); + + // Assert Funder fields were populated + await expect(page.locator('input[name="funder[]"]').first()).toHaveValue( + 'Deutsche Forschungsgemeinschaft' + ); + await expect(page.locator('input[name="grantNummer[]"]').first()).toHaveValue( + 'TEST123' + ); + await expect(page.locator('input[name="grantName[]"]').first()).toHaveValue( + 'Test Grant Title' + ); + }); +}); From 8db9480248fce28eefa4755c7e4382f26f4b5b81 Mon Sep 17 00:00:00 2001 From: Holger Ehrmann Date: Thu, 2 Apr 2026 19:54:40 +0200 Subject: [PATCH 02/15] Several fixes aginst race conditions --- api/v2/controllers/DraftController.php | 2 +- js/descriptionTypes.js | 20 +++++++++++++++----- js/freekeywordTags.js | 1 - js/language.js | 6 ++++++ js/services/autosaveService.js | 5 +++++ modals.html | 4 ++-- 6 files changed, 29 insertions(+), 9 deletions(-) diff --git a/api/v2/controllers/DraftController.php b/api/v2/controllers/DraftController.php index e82be3fc4..3473669fc 100644 --- a/api/v2/controllers/DraftController.php +++ b/api/v2/controllers/DraftController.php @@ -126,7 +126,7 @@ public function get(array $vars = [], ?array $body = null): void } if (!$record) { - $this->respond(404, ['error' => 'Draft not found']); + $this->respond(204, null); return; } diff --git a/js/descriptionTypes.js b/js/descriptionTypes.js index 050b6c1fa..462ec99f7 100644 --- a/js/descriptionTypes.js +++ b/js/descriptionTypes.js @@ -77,14 +77,24 @@ function initDescriptionTypes() { // Store active slugs globally for the help system window.ELMO_ACTIVE_DESCRIPTION_TYPES = activeSlugs; - // Re-apply translations to newly added elements - if (typeof window.applyTranslations === 'function') { - window.applyTranslations(); + // Re-apply translations to newly added elements. + // Wrapped in try/catch so the promise always resolves – translations + // will be re-applied when language.js finishes loading anyway. + try { + if (typeof window.applyTranslations === 'function') { + window.applyTranslations(); + } + } catch (_ignored) { + // Non-critical: translations will be applied later } // Sync help icon visibility with current help status - if (typeof updateHelpStatus === 'function') { - updateHelpStatus(); + try { + if (typeof updateHelpStatus === 'function') { + updateHelpStatus(); + } + } catch (_ignored) { + // Non-critical } resolve(activeSlugs); diff --git a/js/freekeywordTags.js b/js/freekeywordTags.js index caef82584..53ff3bfdb 100644 --- a/js/freekeywordTags.js +++ b/js/freekeywordTags.js @@ -146,7 +146,6 @@ document.addEventListener('DOMContentLoaded', function () { // Check if the array is empty if (data.length === 0) { - console.log("ELMO currently has no curated keywords."); return; } diff --git a/js/language.js b/js/language.js index a358becb0..ff869a076 100644 --- a/js/language.js +++ b/js/language.js @@ -69,6 +69,12 @@ function getNestedValue(obj, path) { * Applies the loaded translations to all UI elements */ function applyTranslations() { + // Guard: skip if translations have not been loaded yet (race condition + // when descriptionTypes.js AJAX resolves before the language file). + if (!translations || !translations.general) { + return; + } + // Set document title document.title = translations.general.logoTitle; diff --git a/js/services/autosaveService.js b/js/services/autosaveService.js index 9d3a0de70..c584efa0a 100644 --- a/js/services/autosaveService.js +++ b/js/services/autosaveService.js @@ -295,6 +295,11 @@ class AutosaveService { }); if (response.status === 204 || response.status === 404) { + // Clear stale draft ID so the next save creates a fresh draft + if (this.draftId) { + this.draftId = null; + this.removeStoredDraftId(); + } this.updateStatus('idle'); return; } diff --git a/modals.html b/modals.html index 13dcef650..4271f1e9b 100644 --- a/modals.html +++ b/modals.html @@ -1,11 +1,11 @@ -