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 <noreply@anthropic.com>
This commit is contained in:
parent
c5ee817ebb
commit
1a3f294b31
|
|
@ -36,6 +36,8 @@ export const suffixRepository = {
|
||||||
/**
|
/**
|
||||||
* Renames a suffix across all entity title/label columns.
|
* Renames a suffix across all entity title/label columns.
|
||||||
* Updates suffix table entry + all titles containing [OLD_SUFFIX] to [NEW_SUFFIX].
|
* 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) {
|
async renameSuffix(userUuid, newSuffix) {
|
||||||
const existing = await this.getByUuid(userUuid)
|
const existing = await this.getByUuid(userUuid)
|
||||||
|
|
@ -43,27 +45,29 @@ export const suffixRepository = {
|
||||||
|
|
||||||
const oldPattern = `[${existing.suffix}]`
|
const oldPattern = `[${existing.suffix}]`
|
||||||
const newPattern = `[${newSuffix}]`
|
const newPattern = `[${newSuffix}]`
|
||||||
|
const params = [oldPattern, newPattern, oldPattern]
|
||||||
|
|
||||||
// Update suffix table
|
await db.batch([
|
||||||
await db.run('UPDATE suffix SET suffix = ? WHERE user_uuid = ?', [newSuffix, userUuid])
|
{
|
||||||
|
sql: 'UPDATE suffix SET suffix = ? WHERE user_uuid = ?',
|
||||||
// Update titles across entity tables
|
params: [newSuffix, userUuid],
|
||||||
await db.run(
|
},
|
||||||
"UPDATE exercise SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'",
|
{
|
||||||
[oldPattern, newPattern, oldPattern],
|
sql: "UPDATE exercise SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'",
|
||||||
)
|
params,
|
||||||
await db.run("UPDATE combo SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'", [
|
},
|
||||||
oldPattern,
|
{
|
||||||
newPattern,
|
sql: "UPDATE combo SET title = REPLACE(title, ?, ?) WHERE title LIKE '%' || ? || '%'",
|
||||||
oldPattern,
|
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],
|
|
||||||
)
|
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -347,6 +347,7 @@
|
||||||
"suffixPreview": "Titler vil se sådan ud: \"{{title}} [{{suffix}}]\"",
|
"suffixPreview": "Titler vil se sådan ud: \"{{title}} [{{suffix}}]\"",
|
||||||
"suffixRequired": "Suffiks er påkrævet.",
|
"suffixRequired": "Suffiks er påkrævet.",
|
||||||
"suffixTaken": "Dette suffiks bruges allerede af en anden bruger.",
|
"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:",
|
"collisionExplanation": "{{count}} element(er) er i konflikt med eksisterende data. Vælg hvad der skal gøres for hvert:",
|
||||||
"replace": "Erstat",
|
"replace": "Erstat",
|
||||||
"skip": "Spring over",
|
"skip": "Spring over",
|
||||||
|
|
|
||||||
|
|
@ -347,6 +347,7 @@
|
||||||
"suffixPreview": "Titles will look like: \"{{title}} [{{suffix}}]\"",
|
"suffixPreview": "Titles will look like: \"{{title}} [{{suffix}}]\"",
|
||||||
"suffixRequired": "Suffix is required.",
|
"suffixRequired": "Suffix is required.",
|
||||||
"suffixTaken": "This suffix is already used by another user.",
|
"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:",
|
"collisionExplanation": "{{count}} item(s) conflict with existing data. Choose what to do for each:",
|
||||||
"replace": "Replace",
|
"replace": "Replace",
|
||||||
"skip": "Skip",
|
"skip": "Skip",
|
||||||
|
|
|
||||||
|
|
@ -347,6 +347,7 @@
|
||||||
"suffixPreview": "Les titres ressembleront à : « {{title}} [{{suffix}}] »",
|
"suffixPreview": "Les titres ressembleront à : « {{title}} [{{suffix}}] »",
|
||||||
"suffixRequired": "Le suffixe est requis.",
|
"suffixRequired": "Le suffixe est requis.",
|
||||||
"suffixTaken": "Ce suffixe est déjà utilisé par un autre utilisateur.",
|
"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 :",
|
"collisionExplanation": "{{count}} élément(s) sont en conflit avec des données existantes. Choisissez quoi faire pour chacun :",
|
||||||
"replace": "Remplacer",
|
"replace": "Remplacer",
|
||||||
"skip": "Ignorer",
|
"skip": "Ignorer",
|
||||||
|
|
|
||||||
|
|
@ -274,7 +274,8 @@ async function saveEditSuffix(entry) {
|
||||||
await loadSuffixes()
|
await loadSuffixes()
|
||||||
cancelEditSuffix()
|
cancelEditSuffix()
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
editSuffixError.value = e.message
|
console.error('Suffix rename failed:', e)
|
||||||
|
editSuffixError.value = t('import.suffixRenameFailed')
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,7 @@ Step 3b — Exercise JSON Export/Import Tests
|
||||||
- test_settings_import_button: Settings import button works
|
- test_settings_import_button: Settings import button works
|
||||||
- test_suffix_management_visible: Suffix section visible after import from different user
|
- 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_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_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_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
|
- 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)
|
_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):
|
def test_suffix_delete(page: Page, app_url: str):
|
||||||
"""Suffix can be deleted from settings."""
|
"""Suffix can be deleted from settings."""
|
||||||
_go_to_exercises(page, app_url)
|
_go_to_exercises(page, app_url)
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue