Skip to content

Hotfix: Upload of XML files to form not working#1033

Merged
McNamara84 merged 15 commits intomainfrom
fix/race-condidtions-at-upload
Apr 2, 2026
Merged

Hotfix: Upload of XML files to form not working#1033
McNamara84 merged 15 commits intomainfrom
fix/race-condidtions-at-upload

Conversation

@McNamara84
Copy link
Copy Markdown
Owner

@McNamara84 McNamara84 commented Apr 2, 2026

This pull request introduces several improvements and bug fixes across the backend, frontend, and test suites, with a focus on robustness, error handling, and regression prevention. The most significant changes are the switch of the draft "not found" response from HTTP 404 to 204 (No Content), improved handling of race conditions and error states in JavaScript, fixes for modal accessibility and duplicate IDs, and the addition of comprehensive regression and unit tests.

Changes

Draft API and Autosave Handling

  • Changed the Draft API to return HTTP 204 (No Content) instead of 404 when a draft is not found, updating both the PHP controller (DraftController.php) and OpenAPI docs (swagger.yaml). Autosave logic and tests were updated to handle this new behavior, ensuring stale draft IDs are cleared only on 204, and 404 is now treated as a configuration error. [1] [2] [3] [4] [5] [6]

Frontend Robustness and Error Handling

  • Added guards and try/catch blocks to prevent race conditions and uncaught exceptions in applyTranslations (language.js) and initDescriptionTypes (descriptionTypes.js), ensuring UI initialization does not fail if AJAX or translation loading order varies. [1] [2]
  • Removed unnecessary console logging when no curated keywords are returned, and updated tests to reflect this. [1] [2]

Accessibility and Modal Fixes

  • Fixed duplicate modal IDs in modals.html by separating the modal and label IDs, improving accessibility and preventing HTML validation issues.

Regression and Unit Testing

  • Added a Playwright regression test to verify that no JavaScript errors occur on page load, modal IDs are unique, and XML uploads correctly populate Abstract and Funder fields.
  • Expanded unit tests for JavaScript modules to cover new error handling and edge cases, including translation loading and description type initialization. [1] [2]

Documentation and Changelog

  • Updated the changelog to document the XML upload fix.

Notes for Reviewer

Please test if ELMO-generated XML files can be uploaded and will seed the input fields.

Checklist

  • My code follows the style guide.
  • I have self-reviewed my code.
  • I added comments for hard-to-understand code.
  • If applicable, PHP code is documented using PHPDoc.
  • If applicable, JavaScript code is documented using JSDoc.
  • If needed, the ELMO Guide has been updated.
  • If needed, the README has been updated.
  • If needed, the API documentation has been updated.
  • If a new feature was added or a bug fixed, the changelog has been updated.
  • My changes do not create new warnings in the test browser console.
  • I have added unit tests that cover my code.
  • All new and existing unit tests pass locally.
  • All new and existing automated unit tests pass in the pull request.
  • If applicable, Playwright tests have been updated and new tests added.
  • All new and existing automated Playwright tests pass in the pull request.
  • I ensured the changes meet accessibility guidelines.

Known Issues

None.

@McNamara84 McNamara84 self-assigned this Apr 2, 2026
@McNamara84 McNamara84 added the bug Etwas funktioniert nicht label Apr 2, 2026
@McNamara84 McNamara84 marked this pull request as ready for review April 2, 2026 19:25
@McNamara84 McNamara84 requested a review from Copilot April 2, 2026 19:26
@McNamara84 McNamara84 linked an issue Apr 2, 2026 that may be closed by this pull request
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses regressions in the XML upload flow (field population + JS initialization robustness) and aligns autosave draft handling between frontend and backend, while adding regression coverage to prevent recurrences.

Changes:

  • Backend: return 204 No Content when a draft is missing (instead of 404), and update PHP API tests accordingly.
  • Frontend JS: harden initialization against translation/help race conditions; clear stale autosave draft IDs when drafts are missing; remove noisy logging for empty curated keywords; fix duplicate upload-modal IDs.
  • Tests/docs: add Playwright regression flows and expand Jest coverage for translation guards and autosave edge cases; update changelog.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
api/v2/controllers/DraftController.php Changes missing-draft response to 204 and suppresses storage-root mkdir warnings.
js/services/autosaveService.js Clears stale draftId on 204/404 during draft lookup.
js/language.js Adds guard in applyTranslations() to avoid race-condition errors when translations aren’t loaded.
js/descriptionTypes.js Wraps translation/help updates in try/catch to ensure promise resolution even on JS errors.
js/freekeywordTags.js Removes console logging when curated keywords list is empty.
modals.html Fixes duplicate IDs by introducing modal-uploadxml-label and updating aria-labelledby.
doc/changelog.html Notes the XML upload fix in the changelog.
tests/DraftControllerTest.php Adds coverage for 204 on missing drafts.
tests/ApiTest.php Updates lifecycle expectations to match 204 for deleted/missing drafts.
tests/js/autosaveService.test.js Adds unit tests for stale draftId clearing on 204/404.
tests/js/language.module.test.js Adds tests for applyTranslations() early-return guards.
tests/js/descriptionTypes.test.js Adds tests ensuring initDescriptionTypes() resolves even if downstream calls throw.
tests/js/freekeywordTags.test.js Updates expectation to reflect removed logging behavior.
tests/playwright/flows/stage-diagnosis.spec.ts Adds regression checks for init errors, modal ID uniqueness, and XML upload population.
tests/playwright/flows/xml-upload-datacite47.spec.ts Adds an end-to-end XML upload flow covering many mapped fields and console error filtering.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

@McNamara84 McNamara84 merged commit 60a9e01 into main Apr 2, 2026
10 checks passed
@McNamara84 McNamara84 deleted the fix/race-condidtions-at-upload branch April 2, 2026 22:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

All ELMOs bug Etwas funktioniert nicht

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] data upload incomplete

2 participants