From 1a3f294b31daa943684cc381c0b8c7cf0659c9f2 Mon Sep 17 00:00:00 2001 From: kaivalya Date: Thu, 9 Jul 2026 15:23:31 +0200 Subject: [PATCH] F5: renameSuffix runs as a single transaction The 5 sequential UPDATEs could half-rename the database if a renamed title collided with an existing one (UNIQUE constraint) mid-way. They now go through db.batch(), which rolls back on error. SettingsView shows a translated error (new import.suffixRenameFailed key, EN/FR/DA) instead of the raw constraint message. Test seeds a colliding title and asserts the rename fails with nothing renamed. Co-Authored-By: Claude Fable 5 --- src/db/repositories/suffixRepository.js | 44 ++++++++------ src/i18n/locales/da.json | 1 + src/i18n/locales/en.json | 1 + src/i18n/locales/fr.json | 1 + src/views/SettingsView.vue | 3 +- tests/test_step3b_exercise_export_import.py | 67 +++++++++++++++++++++ 6 files changed, 96 insertions(+), 21 deletions(-) diff --git a/src/db/repositories/suffixRepository.js b/src/db/repositories/suffixRepository.js index 3cf8bf7..d4840b9 100644 --- a/src/db/repositories/suffixRepository.js +++ b/src/db/repositories/suffixRepository.js @@ -36,6 +36,8 @@ export const suffixRepository = { /** * Renames a suffix across all entity title/label columns. * Updates suffix table entry + all titles containing [OLD_SUFFIX] to [NEW_SUFFIX]. + * Runs as a single transaction: a failure (e.g. a UNIQUE-title collision on + * a renamed title) rolls everything back instead of leaving a half-rename. */ async renameSuffix(userUuid, newSuffix) { const existing = await this.getByUuid(userUuid) @@ -43,27 +45,29 @@ export const suffixRepository = { const oldPattern = `[${existing.suffix}]` const newPattern = `[${newSuffix}]` + const params = [oldPattern, newPattern, oldPattern] - // Update suffix table - await db.run('UPDATE suffix SET suffix = ? WHERE user_uuid = ?', [newSuffix, userUuid]) - - // Update titles across entity tables - await db.run( - "UPDATE exercise SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'", - [oldPattern, newPattern, oldPattern], - ) - await db.run("UPDATE combo SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'", [ - oldPattern, - newPattern, - oldPattern, + await db.batch([ + { + sql: 'UPDATE suffix SET suffix = ? WHERE user_uuid = ?', + params: [newSuffix, userUuid], + }, + { + sql: "UPDATE exercise SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'", + params, + }, + { + sql: "UPDATE combo SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'", + params, + }, + { + sql: "UPDATE session SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'", + params, + }, + { + sql: "UPDATE collection SET label = REPLACE(label, ?, ?) WHERE label LIKE '%' || ? || '%'", + params, + }, ]) - await db.run( - "UPDATE session SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'", - [oldPattern, newPattern, oldPattern], - ) - await db.run( - "UPDATE collection SET label = REPLACE(label, ?, ?) WHERE label LIKE '%' || ? || '%'", - [oldPattern, newPattern, oldPattern], - ) }, } diff --git a/src/i18n/locales/da.json b/src/i18n/locales/da.json index 635f6d3..77df4d4 100644 --- a/src/i18n/locales/da.json +++ b/src/i18n/locales/da.json @@ -347,6 +347,7 @@ "suffixPreview": "Titler vil se sådan ud: \"{{title}} [{{suffix}}]\"", "suffixRequired": "Suffiks er påkrævet.", "suffixTaken": "Dette suffiks bruges allerede af en anden bruger.", + "suffixRenameFailed": "Omdøbning mislykkedes: den ville skabe duplikerede titler. Intet blev omdøbt.", "collisionExplanation": "{{count}} element(er) er i konflikt med eksisterende data. Vælg hvad der skal gøres for hvert:", "replace": "Erstat", "skip": "Spring over", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index a1fbcbf..dd12a20 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -347,6 +347,7 @@ "suffixPreview": "Titles will look like: \"{{title}} [{{suffix}}]\"", "suffixRequired": "Suffix is required.", "suffixTaken": "This suffix is already used by another user.", + "suffixRenameFailed": "Rename failed: it would create duplicate titles. Nothing was renamed.", "collisionExplanation": "{{count}} item(s) conflict with existing data. Choose what to do for each:", "replace": "Replace", "skip": "Skip", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index e64ac8d..555f4f2 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -347,6 +347,7 @@ "suffixPreview": "Les titres ressembleront à : « {{title}} [{{suffix}}] »", "suffixRequired": "Le suffixe est requis.", "suffixTaken": "Ce suffixe est déjà utilisé par un autre utilisateur.", + "suffixRenameFailed": "Le renommage a échoué : il créerait des titres en double. Rien n'a été renommé.", "collisionExplanation": "{{count}} élément(s) sont en conflit avec des données existantes. Choisissez quoi faire pour chacun :", "replace": "Remplacer", "skip": "Ignorer", diff --git a/src/views/SettingsView.vue b/src/views/SettingsView.vue index d7ec2f5..8a25027 100644 --- a/src/views/SettingsView.vue +++ b/src/views/SettingsView.vue @@ -274,7 +274,8 @@ async function saveEditSuffix(entry) { await loadSuffixes() cancelEditSuffix() } catch (e) { - editSuffixError.value = e.message + console.error('Suffix rename failed:', e) + editSuffixError.value = t('import.suffixRenameFailed') } } diff --git a/tests/test_step3b_exercise_export_import.py b/tests/test_step3b_exercise_export_import.py index 48189a5..8fdb593 100644 --- a/tests/test_step3b_exercise_export_import.py +++ b/tests/test_step3b_exercise_export_import.py @@ -16,6 +16,7 @@ Step 3b — Exercise JSON Export/Import Tests - test_settings_import_button: Settings import button works - test_suffix_management_visible: Suffix section visible after import from different user - test_suffix_rename_cascade: Renaming suffix updates titles across exercises +- test_suffix_rename_collision_rolls_back: Colliding rename fails atomically, nothing renamed - test_suffix_delete: Suffix can be deleted from settings - test_import_as_mine_button_visible: Import-as-mine buttons visible on exercises and settings - test_import_as_mine_no_suffix_no_collision: Foreign exercises imported as own without suffix @@ -919,6 +920,72 @@ def test_suffix_rename_cascade(page: Page, app_url: str): _clean_all(page) +def test_suffix_rename_collision_rolls_back(page: Page, app_url: str): + """A rename that would create a duplicate title fails atomically: an error + is shown and nothing (suffix entry or titles) was renamed.""" + _go_to_exercises(page, app_url) + _clean_all(page) + + other_uuid = "rename-atomic-uuid-6666" + + # Suffixed exercise 'Squats [CH]' plus a local 'Squats [CW]' that the + # rename CH → CW would duplicate (exercise.title is UNIQUE) + page.evaluate( + f"""async () => {{ + await window.__repos.suffixes.create({{ + user_uuid: '{other_uuid}', + user_name: 'Charlie', + suffix: 'CH', + }}); + await window.__repos.exercises.createWithId('atomic-ex-1', {{ + title: 'Squats [CH]', + description: 'Test', + asymmetric: false, + alternate: false, + image_urls: [], + video_urls: [], + default_reps: 10, + default_weight: 0, + created_by: '{other_uuid}', + }}); + const userId = await window.__repos.settings.get('user_uuid') + await window.__repos.exercises.createWithId('atomic-ex-2', {{ + title: 'Squats [CW]', + description: 'Blocker', + asymmetric: false, + alternate: false, + image_urls: [], + video_urls: [], + default_reps: 10, + default_weight: 0, + created_by: userId, + }}); + }}""" + ) + + _go_to_settings(page, app_url) + + page.click('[data-testid="suffix-edit-btn"]') + page.fill('[data-testid="suffix-edit-input"]', "CW") + page.click('[data-testid="suffix-save-btn"]') + + # The rename must fail with a visible error + expect(page.locator('[data-testid="suffix-edit-error"]')).to_be_visible() + + # ... and nothing must have been renamed (transaction rolled back) + state = page.evaluate( + f"""async () => {{ + const suffix = await window.__repos.suffixes.getByUuid('{other_uuid}') + const ex = await window.__repos.exercises.getById('atomic-ex-1') + return {{ suffix: suffix.suffix, title: ex.title }} + }}""" + ) + assert state["suffix"] == "CH", f"Suffix entry must be unchanged, got {state['suffix']}" + assert state["title"] == "Squats [CH]", f"Exercise title must be unchanged, got {state['title']}" + + _clean_all(page) + + def test_suffix_delete(page: Page, app_url: str): """Suffix can be deleted from settings.""" _go_to_exercises(page, app_url)