Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions api/v2/controllers/DraftController.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,20 @@ public function __construct()
{
$this->storageRoot = rtrim(getenv('ELMO_DRAFT_STORAGE') ?: (__DIR__ . '/../../../storage/drafts'), DIRECTORY_SEPARATOR);
$this->retentionDays = (int) (getenv('ELMO_DRAFT_RETENTION_DAYS') ?: 30);
}

/**
* Ensures the storage root directory exists and is writable.
*
* @throws \RuntimeException When the directory cannot be created.
*/
private function ensureStorageRoot(): void
{
if (!is_dir($this->storageRoot)) {
mkdir($this->storageRoot, 0775, true);
// Suppress warning from race when another process creates the dir concurrently
if (!@mkdir($this->storageRoot, 0775, true) && !is_dir($this->storageRoot)) {
throw new \RuntimeException('DraftController: cannot create storage directory: ' . $this->storageRoot);
}
}
}

Expand Down Expand Up @@ -126,7 +137,7 @@ public function get(array $vars = [], ?array $body = null): void
}

if (!$record) {
$this->respond(404, ['error' => 'Draft not found']);
$this->respond(204, null);
return;
}

Expand Down Expand Up @@ -287,12 +298,20 @@ private function createRecord(string $draftId, string $sessionId, array $payload
*/
private function persistRecord(array $record): void
{
$this->ensureStorageRoot();

$dir = $this->sessionDirectory($record['sessionId']);
if (!is_dir($dir)) {
mkdir($dir, 0775, true);
if (!@mkdir($dir, 0775, true) && !is_dir($dir)) {
throw new \RuntimeException('DraftController: cannot create session directory: ' . $dir);
}
}

file_put_contents($this->recordPath($record['sessionId'], $record['id']), json_encode($record, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
$path = $this->recordPath($record['sessionId'], $record['id']);
$bytes = file_put_contents($path, json_encode($record, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES));
if ($bytes === false) {
throw new \RuntimeException('DraftController: failed to write draft file: ' . $path);
}
}

/**
Expand Down
8 changes: 2 additions & 6 deletions api/v2/docs/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1189,12 +1189,8 @@ paths:
application/json:
schema:
$ref: "#/components/schemas/Error"
"404":
description: Draft not found for the current session
content:
application/json:
schema:
$ref: "#/components/schemas/Error"
"204":
description: Draft not found for the current session (no body)
"500":
description: Internal server error
content:
Expand Down
5 changes: 5 additions & 0 deletions doc/changelog.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ <h2 class="accordion-header" id="heading-changelog-version13">
<li>ORCID checksum validation using ISO 7064 Mod 11-2 algorithm.</li>
</ul>
</li>
<li><strong>Fixes:</strong>
<ul>
<li>Upload of XML files not working correctly.</li>
</ul>
</li>
</ul>
</div>
</div>
Expand Down
20 changes: 15 additions & 5 deletions js/descriptionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion js/freekeywordTags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
6 changes: 6 additions & 0 deletions js/language.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
15 changes: 14 additions & 1 deletion js/services/autosaveService.js
Original file line number Diff line number Diff line change
Expand Up @@ -294,11 +294,24 @@ class AutosaveService {
credentials: 'include'
});

if (response.status === 204 || response.status === 404) {
if (response.status === 204) {
// Clear stale draft ID so the next save creates a fresh draft
if (this.draftId) {
this.draftId = null;
this.removeStoredDraftId();
}
this.updateStatus('idle');
return;
}

if (response.status === 404) {
// Since the API now returns 204 for "not found", a 404 always
// indicates a misconfigured route or unavailable endpoint.
const errorMessage = await this.extractErrorMessage(response);
this.updateStatus('error', errorMessage || 'Draft endpoint not found');
return;
}

if (!response.ok) {
const errorMessage = await this.extractErrorMessage(response);
this.updateStatus('error', errorMessage || 'Unable to load autosaved draft');
Expand Down
4 changes: 2 additions & 2 deletions modals.html
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
</form>
</main>
<!-- Upload Modal -->
<div class="modal fade" id="modal-uploadxml" tabindex="-1" aria-labelledby="modal-uploadxml" aria-hidden="true">
<div class="modal fade" id="modal-uploadxml" tabindex="-1" aria-labelledby="modal-uploadxml-label" aria-hidden="true">
<div class="modal-dialog modal-lg modal-dialog-centered">
<div class="modal-content">
<div class="modal-header">
<h5 class="modal-title" id="modal-uploadxml">XML hochladen</h5>
<h5 class="modal-title" id="modal-uploadxml-label">XML hochladen</h5>
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="Close"></button>
</div>
<div class="modal-body">
Expand Down
4 changes: 2 additions & 2 deletions tests/ApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,9 @@ public function testDraftLifecycleEndpoints(): void

$getDeletedResponse = $this->client->get($getUrl);
$this->assertEquals(
404,
204,
$getDeletedResponse->getStatusCode(),
'Expected deleted draft to return 404. Response: ' . $getDeletedResponse->getBody()
'Expected deleted draft to return 204. Response: ' . $getDeletedResponse->getBody()
);
}
}
11 changes: 11 additions & 0 deletions tests/DraftControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
110 changes: 110 additions & 0 deletions tests/js/autosaveService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -429,4 +429,114 @@ 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('surfaces error when checkForExistingDraft gets 404 with stored draftId', 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' }),
text: () => 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();
await Promise.resolve();
await Promise.resolve();

// 404 should surface as error (API returns 204 for missing drafts)
const statusEl = document.getElementById('autosave-status');
expect(statusEl.dataset.state).toBe('error');
// draftId should NOT be silently cleared
expect(window.localStorage.getItem('elmo.autosave.draftId')).toBe('old-draft-id');
});

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');
});

test('surfaces error when 404 on session/latest (misconfigured route)', async () => {
// No stored draftId => service queries session/latest
const fetchMock = jest.fn().mockResolvedValue({
ok: false,
status: 404,
json: () => Promise.resolve({ error: 'Route not found' }),
text: () => Promise.resolve('Route 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();
await Promise.resolve();
await Promise.resolve();

// Should surface as error, not silently ignored
expect(fetchMock.mock.calls[0][0]).toBe('./api/v2/drafts/session/latest');
const statusEl = document.getElementById('autosave-status');
expect(statusEl.dataset.state).toBe('error');
});
});
33 changes: 33 additions & 0 deletions tests/js/descriptionTypes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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([
Expand Down
4 changes: 2 additions & 2 deletions tests/js/freekeywordTags.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ describe('freekeywordTags.js', () => {
errSpy.mockRestore();
});

test('logs when no keywords returned', () => {
test('does not log curated keywords message when empty array 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.toHaveBeenCalled();
const input = document.getElementById('input-freekeyword');
expect(input._tagify.settings.whitelist).toEqual([]);
logSpy.mockRestore();
Expand Down
Loading
Loading