From a6b4723ac6d7a19b67d96528d1b4a3a732488007 Mon Sep 17 00:00:00 2001 From: kaivalya Date: Thu, 9 Jul 2026 15:30:52 +0200 Subject: [PATCH] F4: Delete confirms show usage; no more session_item orphans session_item.item_id is polymorphic (no FK), so deleting an exercise or combo used in a session left orphaned rows that the INNER JOIN in getItems silently hid. Exercise/combo deletes now also remove their session_item rows in one transaction. Delete confirmations on the exercise/combo/session detail views now include usage counts (sessions/ combos using an exercise, sessions using a combo, collections containing a session) via new count helpers and deleteConfirmUsage keys (EN/FR/DA). Confirm stays informative, not blocking, consistent with the existing history warning. Co-Authored-By: Claude Fable 5 --- docs/decisions-log.md | 6 ++ src/db/repositories/collectionRepository.js | 8 +++ src/db/repositories/comboRepository.js | 18 ++++- src/db/repositories/exerciseRepository.js | 10 ++- src/db/repositories/sessionRepository.js | 8 +++ src/i18n/locales/da.json | 3 + src/i18n/locales/en.json | 3 + src/i18n/locales/fr.json | 3 + src/views/ComboDetailView.vue | 8 ++- src/views/ExerciseDetailView.vue | 15 ++++- src/views/SessionDetailView.vue | 11 ++- tests/test_step3_exercises.py | 74 +++++++++++++++++++++ tests/test_step4_combos.py | 59 ++++++++++++++++ 13 files changed, 219 insertions(+), 7 deletions(-) diff --git a/docs/decisions-log.md b/docs/decisions-log.md index 3a8d195..94d9b9b 100644 --- a/docs/decisions-log.md +++ b/docs/decisions-log.md @@ -2,6 +2,12 @@ Record of architectural and technical decisions made during development. +## 2026-07-09 — Deleting an exercise/combo cleans session usage and warns about it (code review F4) + +**Decision:** `session_item.item_id` is polymorphic (exercise or combo) and carries no FK, so deleting an exercise or combo used in a session silently left orphaned `session_item` rows that the INNER JOIN in `sessionRepository.getItems` simply hid — the entry vanished from the session with no trace. Deletes in `exerciseRepository`/`comboRepository` now also remove the matching `session_item` rows (in one `db.batch` transaction), and the delete confirmations on the exercise/combo/session detail views show usage counts before the user confirms: sessions+combos using the exercise, sessions using the combo, collections containing the session (new repository count helpers, new `deleteConfirmUsage` i18n keys in EN/FR/DA). + +**Rationale:** The confirmation stays **informative, not blocking** — consistent with the existing history-cascade warning UX; the user can still delete, they just see the blast radius first. Collection/session junctions already have FK CASCADE so they need no cleanup, only the usage warning. (Chosen from the remediation plan's proposal without live validation — flagged for review.) + ## 2026-07-09 — DB restore no longer copies the backup's schema_version stamp (code review F3) **Decision:** `handleImportDb` in `src/db/worker.js` now excludes `schema_version` from the tables copied out of a restored backup. The live DB keeps its own version stamp, which is correct by construction: any adapt/migration scripts run against the temp DB *before* the copy, so the restored data always matches the current schema. diff --git a/src/db/repositories/collectionRepository.js b/src/db/repositories/collectionRepository.js index 193745e..7ca3d69 100644 --- a/src/db/repositories/collectionRepository.js +++ b/src/db/repositories/collectionRepository.js @@ -88,6 +88,14 @@ export const collectionRepository = { ]) }, + async countCollectionsUsingSession(sessionId) { + const row = await db.selectOne( + 'SELECT COUNT(DISTINCT collection_id) AS count FROM collection_session WHERE session_id = ?', + [sessionId], + ) + return row ? row.count : 0 + }, + async getSessions(collectionId) { return db.selectAll( `SELECT cs.position, cs.session_id, s.title diff --git a/src/db/repositories/comboRepository.js b/src/db/repositories/comboRepository.js index 0702677..7ae0a6d 100644 --- a/src/db/repositories/comboRepository.js +++ b/src/db/repositories/comboRepository.js @@ -64,7 +64,23 @@ export const comboRepository = { }, async delete(id) { - return db.run('DELETE FROM combo WHERE id = ?', [id]) + // session_item.item_id is polymorphic (no FK), so combo entries in + // sessions must be removed explicitly or they'd linger as orphans + return db.batch([ + { + sql: "DELETE FROM session_item WHERE item_id = ? AND item_type = 'combo'", + params: [id], + }, + { sql: 'DELETE FROM combo WHERE id = ?', params: [id] }, + ]) + }, + + async countCombosUsingExercise(exerciseId) { + const row = await db.selectOne( + 'SELECT COUNT(DISTINCT combo_id) AS count FROM combo_exercise WHERE exercise_id = ?', + [exerciseId], + ) + return row ? row.count : 0 }, async createWithId(id, combo) { diff --git a/src/db/repositories/exerciseRepository.js b/src/db/repositories/exerciseRepository.js index bd1112a..73ba72f 100644 --- a/src/db/repositories/exerciseRepository.js +++ b/src/db/repositories/exerciseRepository.js @@ -113,7 +113,15 @@ export const exerciseRepository = { }, async delete(id) { - return db.run('DELETE FROM exercise WHERE id = ?', [id]) + // session_item.item_id is polymorphic (no FK), so exercise entries in + // sessions must be removed explicitly or they'd linger as orphans + return db.batch([ + { + sql: "DELETE FROM session_item WHERE item_id = ? AND item_type = 'exercise'", + params: [id], + }, + { sql: 'DELETE FROM exercise WHERE id = ?', params: [id] }, + ]) }, async search(query) { diff --git a/src/db/repositories/sessionRepository.js b/src/db/repositories/sessionRepository.js index 46c4163..5318c3a 100644 --- a/src/db/repositories/sessionRepository.js +++ b/src/db/repositories/sessionRepository.js @@ -74,6 +74,14 @@ export const sessionRepository = { ]) }, + async countSessionsUsingItem(itemId, itemType) { + const row = await db.selectOne( + 'SELECT COUNT(DISTINCT session_id) AS count FROM session_item WHERE item_id = ? AND item_type = ?', + [itemId, itemType], + ) + return row ? row.count : 0 + }, + async getItems(sessionId) { const exerciseRows = await db.selectAll( `SELECT si.position, si.item_id, si.item_type, si.repetitions, si.weight, e.title diff --git a/src/i18n/locales/da.json b/src/i18n/locales/da.json index 77df4d4..d5c7305 100644 --- a/src/i18n/locales/da.json +++ b/src/i18n/locales/da.json @@ -54,6 +54,7 @@ "detail": "Øvelsesdetaljer", "deleteConfirm": "Er du sikker på, at du vil slette \"{{title}}\"? Dette kan ikke fortrydes.", "deleteConfirmWithHistory": "Er du sikker på, at du vil slette \"{{title}}\"? Dette sletter også {{count}} journalpost(er). Dette kan ikke fortrydes.", + "deleteConfirmUsage": "Den bruges i {{sessions}} session(er) og {{combos}} kombo(er) og vil blive fjernet derfra.", "deleted": "Øvelse slettet.", "form": { "title": "Titel", @@ -90,6 +91,7 @@ "edit": "Rediger kombo", "detail": "Kombodetailer", "deleteConfirm": "Er du sikker på, at du vil slette \"{{title}}\"? Dette kan ikke fortrydes.", + "deleteConfirmUsage": "Den bruges i {{sessions}} session(er) og vil blive fjernet derfra.", "deleted": "Kombo slettet.", "exercises": "øvelser", "timerConfig": "Timer-konfiguration", @@ -158,6 +160,7 @@ "detail": "Sessionsdetaljer", "deleteConfirm": "Er du sikker på, at du vil slette \"{{title}}\"? Dette kan ikke fortrydes.", "deleteConfirmWithHistory": "Er du sikker på, at du vil slette \"{{title}}\"? Dette sletter også {{count}} udførelseslog(ge). Dette kan ikke fortrydes.", + "deleteConfirmUsage": "Den er en del af {{collections}} samling(er) og vil blive fjernet derfra.", "deleted": "Session slettet.", "items": "elementer", "delete": "Slet session", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index dd12a20..9d5397e 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -54,6 +54,7 @@ "detail": "Exercise Details", "deleteConfirm": "Are you sure you want to delete \"{{title}}\"? This cannot be undone.", "deleteConfirmWithHistory": "Are you sure you want to delete \"{{title}}\"? This will also delete {{count}} journal entry/entries. This cannot be undone.", + "deleteConfirmUsage": "It is used in {{sessions}} session(s) and {{combos}} combo(s) and will be removed from them.", "deleted": "Exercise deleted.", "form": { "title": "Title", @@ -90,6 +91,7 @@ "edit": "Edit Combo", "detail": "Combo Details", "deleteConfirm": "Are you sure you want to delete \"{{title}}\"? This cannot be undone.", + "deleteConfirmUsage": "It is used in {{sessions}} session(s) and will be removed from them.", "deleted": "Combo deleted.", "exercises": "exercises", "timerConfig": "Timer Configuration", @@ -158,6 +160,7 @@ "detail": "Session Details", "deleteConfirm": "Are you sure you want to delete \"{{title}}\"? This cannot be undone.", "deleteConfirmWithHistory": "Are you sure you want to delete \"{{title}}\"? This will also delete {{count}} journal log(s). This cannot be undone.", + "deleteConfirmUsage": "It is part of {{collections}} collection(s) and will be removed from them.", "deleted": "Session deleted.", "items": "items", "delete": "Delete session", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index 555f4f2..960f8bd 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -54,6 +54,7 @@ "detail": "Détails de l'exercice", "deleteConfirm": "Êtes-vous sûr de vouloir supprimer « {{title}} » ? Cette action est irréversible.", "deleteConfirmWithHistory": "Êtes-vous sûr de vouloir supprimer « {{title}} » ? Cela supprimera également {{count}} entrée(s) du journal. Cette action est irréversible.", + "deleteConfirmUsage": "Il est utilisé dans {{sessions}} séance(s) et {{combos}} combo(s), et en sera retiré.", "deleted": "Exercice supprimé.", "form": { "title": "Titre", @@ -90,6 +91,7 @@ "edit": "Modifier le Combo", "detail": "Détails du Combo", "deleteConfirm": "Êtes-vous sûr de vouloir supprimer « {{title}} » ? Cette action est irréversible.", + "deleteConfirmUsage": "Il est utilisé dans {{sessions}} séance(s) et en sera retiré.", "deleted": "Combo supprimé.", "exercises": "exercices", "timerConfig": "Configuration du minuteur", @@ -158,6 +160,7 @@ "detail": "Détails de la séance", "deleteConfirm": "Êtes-vous sûr de vouloir supprimer « {{title}} » ? Cette action est irréversible.", "deleteConfirmWithHistory": "Êtes-vous sûr de vouloir supprimer « {{title}} » ? Cela supprimera également {{count}} journal(aux) d'exécution. Cette action est irréversible.", + "deleteConfirmUsage": "Elle fait partie de {{collections}} collection(s) et en sera retirée.", "deleted": "Séance supprimée.", "items": "éléments", "delete": "Supprimer la séance", diff --git a/src/views/ComboDetailView.vue b/src/views/ComboDetailView.vue index 45b2fd4..4482613 100644 --- a/src/views/ComboDetailView.vue +++ b/src/views/ComboDetailView.vue @@ -6,6 +6,7 @@ import { useCombos } from '../composables/useCombos.js' import { useJsonExport } from '../composables/useJsonExport.js' import { useKeyboardShortcut } from '../composables/useKeyboardShortcut.js' import { renderMarkdown } from '../utils/markdown.js' +import { sessionRepository } from '../db/repositories/sessionRepository.js' const { t } = useTranslation() const router = useRouter() @@ -45,7 +46,12 @@ function goToExercise(exerciseId) { async function onDelete() { if (!combo.value) return - const confirmed = window.confirm(t('combos.deleteConfirm', { title: combo.value.title })) + const sessionCount = await sessionRepository.countSessionsUsingItem(comboId.value, 'combo') + let msg = t('combos.deleteConfirm', { title: combo.value.title }) + if (sessionCount > 0) { + msg += '\n\n' + t('combos.deleteConfirmUsage', { sessions: sessionCount }) + } + const confirmed = window.confirm(msg) if (!confirmed) return await remove(comboId.value) router.push({ name: 'combos' }) diff --git a/src/views/ExerciseDetailView.vue b/src/views/ExerciseDetailView.vue index 9419f59..966f6e7 100644 --- a/src/views/ExerciseDetailView.vue +++ b/src/views/ExerciseDetailView.vue @@ -9,6 +9,8 @@ import { useOnlineStatus } from '../composables/useOnlineStatus.js' import { renderMarkdown } from '../utils/markdown.js' import { getYouTubeId } from '../utils/youtube.js' import { executionLogRepository } from '../db/repositories/executionLogRepository.js' +import { sessionRepository } from '../db/repositories/sessionRepository.js' +import { comboRepository } from '../db/repositories/comboRepository.js' const { t } = useTranslation() const router = useRouter() @@ -45,11 +47,20 @@ function goBack() { async function onDelete() { if (!exercise.value) return - const count = await executionLogRepository.countItemsByExercise(exerciseId.value) - const msg = + const [count, sessionCount, comboCount] = await Promise.all([ + executionLogRepository.countItemsByExercise(exerciseId.value), + sessionRepository.countSessionsUsingItem(exerciseId.value, 'exercise'), + comboRepository.countCombosUsingExercise(exerciseId.value), + ]) + let msg = count > 0 ? t('exercises.deleteConfirmWithHistory', { title: exercise.value.title, count }) : t('exercises.deleteConfirm', { title: exercise.value.title }) + if (sessionCount > 0 || comboCount > 0) { + msg += + '\n\n' + + t('exercises.deleteConfirmUsage', { sessions: sessionCount, combos: comboCount }) + } const confirmed = window.confirm(msg) if (!confirmed) return await remove(exerciseId.value) diff --git a/src/views/SessionDetailView.vue b/src/views/SessionDetailView.vue index dd4d09c..0e0dc0b 100644 --- a/src/views/SessionDetailView.vue +++ b/src/views/SessionDetailView.vue @@ -7,6 +7,7 @@ import { useJsonExport } from '../composables/useJsonExport.js' import { useKeyboardShortcut } from '../composables/useKeyboardShortcut.js' import { renderMarkdown } from '../utils/markdown.js' import { executionLogRepository } from '../db/repositories/executionLogRepository.js' +import { collectionRepository } from '../db/repositories/collectionRepository.js' const { t } = useTranslation() const router = useRouter() @@ -58,11 +59,17 @@ function getItemTypeLabel(itemType) { async function onDelete() { if (!session.value) return - const count = await executionLogRepository.countLogsBySession(sessionId.value) - const msg = + const [count, collectionCount] = await Promise.all([ + executionLogRepository.countLogsBySession(sessionId.value), + collectionRepository.countCollectionsUsingSession(sessionId.value), + ]) + let msg = count > 0 ? t('sessions.deleteConfirmWithHistory', { title: session.value.title, count }) : t('sessions.deleteConfirm', { title: session.value.title }) + if (collectionCount > 0) { + msg += '\n\n' + t('sessions.deleteConfirmUsage', { collections: collectionCount }) + } const confirmed = window.confirm(msg) if (!confirmed) return await remove(sessionId.value) diff --git a/tests/test_step3_exercises.py b/tests/test_step3_exercises.py index b7aa6fa..2933b0f 100644 --- a/tests/test_step3_exercises.py +++ b/tests/test_step3_exercises.py @@ -8,6 +8,7 @@ Step 3 — Exercise Management Tests - test_edit_exercise: Edit an exercise and verify changes - test_delete_exercise: Delete an exercise with confirmation - test_delete_exercise_cancel: Cancelling delete does not remove exercise +- test_delete_exercise_used_in_session: Delete confirm mentions usage; no session_item orphans - test_search_exercises: Search filters exercises by title - test_search_no_results: Search with no matches shows empty message - test_search_clear_button: Clear button empties the search field and restores the full list @@ -330,6 +331,79 @@ def test_delete_exercise_cancel(page: Page, app_url: str): _clean_exercises(page) +def test_delete_exercise_used_in_session(page: Page, app_url: str): + """Deleting an exercise used in a session and a combo warns about the + usage, and after deletion no session_item rows reference it (no orphans).""" + _go_to_exercises(page, app_url) + _clean_exercises(page) + + exercise_id = page.evaluate( + """async () => { + const uuid = await window.__repos.settings.get('user_uuid'); + return window.__repos.exercises.create({ + title: 'Used Everywhere', + description: '', + asymmetric: false, + alternate: false, + image_urls: [], + video_urls: [], + default_reps: 10, + default_weight: 0, + created_by: uuid + }); + }""" + ) + page.evaluate( + f"""async () => {{ + const uuid = await window.__repos.settings.get('user_uuid'); + const sessionId = await window.__repos.sessions.create({{ + title: 'Session Using Exercise', description: '', created_by: uuid + }}); + await window.__repos.sessions.setItems(sessionId, [ + {{ item_id: '{exercise_id}', item_type: 'exercise', repetitions: 10, weight: 0 }}, + ]); + const comboId = await window.__repos.combos.create({{ + title: 'Combo Using Exercise', description: '', type: 'NONE', created_by: uuid + }}); + await window.__repos.combos.setExercises(comboId, ['{exercise_id}']); + }}""" + ) + + dialog_message = {} + + def on_dialog(dialog): + dialog_message["text"] = dialog.message + dialog.accept() + + page.on("dialog", on_dialog) + + page.goto(f"{app_url}/#/exercises/{exercise_id}") + page.wait_for_selector('[data-testid="exercise-delete-btn"]', timeout=5000) + page.click('[data-testid="exercise-delete-btn"]') + page.wait_for_timeout(500) + + msg = dialog_message.get("text", "") + assert "1 session(s)" in msg, f"Confirm must mention session usage, got: {msg}" + assert "1 combo(s)" in msg, f"Confirm must mention combo usage, got: {msg}" + + orphans = page.evaluate( + f"""async () => (await window.__db.selectAll( + "SELECT * FROM session_item WHERE item_id = '{exercise_id}'" + )).length""" + ) + assert orphans == 0, f"Expected no orphaned session_item rows, got {orphans}" + + # Clean up the seeded session/combo + page.evaluate( + """async () => { + for (const s of await window.__repos.sessions.getAll()) + await window.__repos.sessions.delete(s.id) + for (const c of await window.__repos.combos.getAll()) + await window.__repos.combos.delete(c.id) + }""" + ) + + def test_search_exercises(page: Page, app_url: str): """Search filters exercises by title.""" _go_to_exercises(page, app_url) diff --git a/tests/test_step4_combos.py b/tests/test_step4_combos.py index fd9507e..86f19c0 100644 --- a/tests/test_step4_combos.py +++ b/tests/test_step4_combos.py @@ -8,6 +8,7 @@ Step 4 — Combo Management Tests - test_edit_combo: Edit a combo and verify changes - test_delete_combo: Delete a combo with confirmation - test_delete_combo_cancel: Cancelling delete does not remove combo +- test_delete_combo_used_in_session: Delete confirm mentions usage; no session_item orphans - test_search_combos: Search filters combos by title - test_search_no_results: Search with no matches shows empty message - test_search_clear_button: Clear button empties the search field and restores the full list @@ -298,6 +299,64 @@ def test_delete_combo_cancel(page: Page, app_url: str): _clean_combos(page) +def test_delete_combo_used_in_session(page: Page, app_url: str): + """Deleting a combo used in a session warns about the usage, and after + deletion no session_item rows reference it (no orphans).""" + _go_to_combos(page, app_url) + _clean_combos(page) + + ex_id = _create_exercise_via_repo(page, "Combo Member Exercise") + combo_id = page.evaluate( + f"""async () => {{ + const uuid = await window.__repos.settings.get('user_uuid'); + const comboId = await window.__repos.combos.create({{ + title: 'Combo In Session', description: '', type: 'NONE', created_by: uuid + }}); + await window.__repos.combos.setExercises(comboId, ['{ex_id}']); + const sessionId = await window.__repos.sessions.create({{ + title: 'Session Using Combo', description: '', created_by: uuid + }}); + await window.__repos.sessions.setItems(sessionId, [ + {{ item_id: comboId, item_type: 'combo', repetitions: 1, weight: 0 }}, + ]); + return comboId; + }}""" + ) + + dialog_message = {} + + def on_dialog(dialog): + dialog_message["text"] = dialog.message + dialog.accept() + + page.on("dialog", on_dialog) + + page.goto(f"{app_url}/#/combos/{combo_id}") + page.wait_for_selector('[data-testid="combo-delete-btn"]', timeout=5000) + page.click('[data-testid="combo-delete-btn"]') + page.wait_for_timeout(500) + + msg = dialog_message.get("text", "") + assert "1 session(s)" in msg, f"Confirm must mention session usage, got: {msg}" + + orphans = page.evaluate( + f"""async () => (await window.__db.selectAll( + "SELECT * FROM session_item WHERE item_id = '{combo_id}'" + )).length""" + ) + assert orphans == 0, f"Expected no orphaned session_item rows, got {orphans}" + + # Clean up the seeded session and exercise + page.evaluate( + """async () => { + for (const s of await window.__repos.sessions.getAll()) + await window.__repos.sessions.delete(s.id) + for (const e of await window.__repos.exercises.getAll()) + await window.__repos.exercises.delete(e.id) + }""" + ) + + def test_search_combos(page: Page, app_url: str): """Search filters combos by title.""" _go_to_combos(page, app_url)