From acf4cb20100db9058a8531cd078e6226c8dda2ba Mon Sep 17 00:00:00 2001 From: Guillem Arias Fauste Date: Sun, 16 Aug 2026 08:12:45 +0200 Subject: [PATCH] fix(goals): allow deleting a goal without archiving it first (#2963) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(goals): allow deleting a goal without archiving it first Goals could only be deleted after being archived. `GoalsController#destroy` redirected with "Archive the goal before deleting it." unless the goal was already archived, and the Delete item in the show-page kebab was wrapped in `if @goal.archived?`. Nothing in the archive confirm copy hinted that archiving was the prerequisite, so in practice an active goal had no delete affordance anywhere in the UI. The gate bought no safety. Destroying a goal cascades only to its own `goal_accounts` and `goal_pledges`, and `GoalPledge#clear_matched_transaction_extra` unstamps `extra["goal"]["pledge_id"]` from any transaction a matched pledge claimed. No account, balance, entry or transaction is touched. Every other resource in Sure (accounts, categories, rules, family merchants) deletes in one step. Drop the gate, render Delete unconditionally, and shorten the label from "Delete permanently" to "Delete" now that it no longer needs to contrast with an archive-first step. The confirm copy moves to `Goal#deletion_confirm` and spells out what survives. The generic `CustomConfirm.for_resource_deletion` only says "This is not reversible", which overstates it for a goal. Index cards deliberately keep no actions — the card stays a single click target, and the show-page kebab is one click away. * fix(goals): escape the goal name in the delete confirmation `confirm_dialog_controller` assigns the confirm `body` to `innerHTML` — bodies such as the accounts' `confirm_body_html` legitimately carry markup — so a goal named "" ran as soon as a family member opened the delete confirmation. Verified in a browser: parsing the rendered `data-turbo-confirm` and assigning its body produced a live `` element with a working `onerror` handler. Escape the interpolated name. Only `body` needs it; the dialog sets its title and button label with `textContent`. `CustomConfirm.for_resource_deletion` interpolates a record name into the same HTML-rendered body and was already reachable from accounts, categories, rules and family merchants, so it is escaped here too rather than left as a known hole next to the fixed one. Also add the three `confirm_delete_*` keys to every locale that ships goal translations. Fallbacks meant these silently rendered English rather than breaking, so this is untranslated copy rather than a fault — ru is included, which the review list omitted. * i18n(confirm): move the resource-deletion copy to locale keys `for_resource_deletion` built its title, body and button label as English string interpolation, against the project's rule that user-facing strings go through `t()`. It backs ~39 call sites — accounts, rules, tags, chats, every provider item — so all of them were English-only. Moved to `shared.custom_confirm.resource_deletion_*`, alongside the `default_*` keys the same class already used. `titleize` / `downcase` stay applied to the record name so the English output is byte-identical to what the hardcoded strings produced; a locale needing different casing can absorb it in its own string. Pinned by a test, along with the escaping of the one field the dialog renders as HTML. * i18n(confirm): translate the resource-deletion copy The keys added when this copy moved out of hardcoded English only landed in en.yml, leaving ~40 call sites falling back to English in every other locale. Added to the eight other shared locale files that already carry the sibling `custom_confirm.default_*` strings: ca, fr, hu, it, ru, tr, vi, zh-CN. Each body reuses that locale's own "this is not reversible" sentence, so the generic and resource-specific confirmations read the same, and each follows the register its `default_title` already set (vous / siz / Вы, tu for ca). The remaining shared locale files (de, es, nb, nl, pl, pt-BR, ro, zh-TW) have no `custom_confirm` block at all, so they are left alone — adding one would invent structure they have not adopted, and fallbacks already cover them. The test derives its locale list from which files define the sibling key rather than hardcoding it, so it follows that set as it grows. * test(goals): restore the active-goal destroy test lost in the merge Merging main into this branch hit a conflict in `test/controllers/goals_controller_test.rb`: main had added two tests immediately above the destroy block, and the resolution took main's side wholesale for that hunk. That resurrected `destroy on non-archived is rejected` — the test this PR replaces — and dropped its replacement. The resurrected test failed against the new controller, since destroy no longer gates on `archived?`: GoalsControllerTest#test_destroy_on_non-archived_is_rejected `Goal.count` didn't change by 0, but by -1. Swap it back for `destroy deletes an active goal and cascades to its links and pledges`. Main's two new tests stay. * i18n(goals): finish the delete copy in de and zh-TW Nine locales ship goals translations, not seven. `de.yml` and `zh-TW.yml` were left behind: both still carried the dead `goals.destroy.archive_first` key, still labelled the kebab item "Delete permanently" (Endgültig löschen / 永久刪除) after it was shortened elsewhere, and had none of the `confirm_delete_*` keys, so a German or Traditional Chinese family saw the new delete dialog in English. Add the three confirm keys using each file's existing vocabulary — Zusagen for pledges in German (informal du, matching the rest of the file), 投入 in Traditional Chinese — drop `archive_first`, and shorten the label. `confirm_delete copy resolves in every locale that ships goal translations` could not have caught this. It hardcoded the seven locales, and its assertions went through plain `I18n.t`: the backend has I18n::Backend::Fallbacks mixed in, so a missing German key resolved to the English string and `.present?` passed anyway. Verified — deleting `confirm_delete_title` from `de.yml` left the test green. Derive the locale list from the goals YAMLs and look the keys up with `fallback: false, default: nil`. The same deletion now fails with "de is missing goals.show.confirm_delete_title". --------- Signed-off-by: Juan José Mata Co-authored-by: Juan José Mata --- app/controllers/goals_controller.rb | 11 ++--- app/helpers/custom_confirm.rb | 17 ++++++-- app/models/goal.rb | 21 ++++++++++ app/views/goals/show.html.erb | 22 +++++----- config/locales/views/goals/de.yml | 6 ++- config/locales/views/goals/en.yml | 6 ++- config/locales/views/goals/fr.yml | 6 ++- config/locales/views/goals/it.yml | 6 ++- config/locales/views/goals/pl.yml | 6 ++- config/locales/views/goals/ru.yml | 4 +- config/locales/views/goals/tr.yml | 6 ++- config/locales/views/goals/zh-CN.yml | 6 ++- config/locales/views/goals/zh-TW.yml | 6 ++- config/locales/views/shared/ca.yml | 3 ++ config/locales/views/shared/en.yml | 3 ++ config/locales/views/shared/fr.yml | 3 ++ config/locales/views/shared/hu.yml | 3 ++ config/locales/views/shared/it.yml | 3 ++ config/locales/views/shared/ru.yml | 3 ++ config/locales/views/shared/tr.yml | 3 ++ config/locales/views/shared/vi.yml | 3 ++ config/locales/views/shared/zh-CN.yml | 3 ++ docs/llm-guides/goals.md | 13 +++++- test/controllers/goals_controller_test.rb | 46 +++++++++++++++++++-- test/helpers/custom_confirm_test.rb | 50 +++++++++++++++++++++++ test/models/goal_test.rb | 32 +++++++++++++++ 26 files changed, 250 insertions(+), 41 deletions(-) create mode 100644 test/helpers/custom_confirm_test.rb diff --git a/app/controllers/goals_controller.rb b/app/controllers/goals_controller.rb index 5de8aa24a..684b5ce0a 100644 --- a/app/controllers/goals_controller.rb +++ b/app/controllers/goals_controller.rb @@ -122,12 +122,13 @@ class GoalsController < ApplicationController render :edit, status: :unprocessable_entity end + # Deletable from any state. Destroying a goal cascades only to its own + # goal_accounts / goal_pledges — and GoalPledge#clear_matched_transaction_extra + # unstamps the pledge id it wrote onto a matched transaction. No account, + # balance, entry or transaction is removed, so the archive-first gate this + # used to enforce bought no safety; it only hid the action behind a two-step + # flow no other Sure resource requires. def destroy - unless @goal.archived? - redirect_to goal_path(@goal), alert: t(".archive_first") - return - end - @goal.destroy! redirect_to goals_path, notice: t(".success") end diff --git a/app/helpers/custom_confirm.rb b/app/helpers/custom_confirm.rb index f2d59a009..b4bc52be6 100644 --- a/app/helpers/custom_confirm.rb +++ b/app/helpers/custom_confirm.rb @@ -2,13 +2,24 @@ # default browser confirm API via Turbo. class CustomConfirm class << self + # `body` is the one field the dialog renders as HTML — confirm_dialog_controller + # assigns it to innerHTML so bodies such as accounts' `confirm_body_html` can + # carry markup, while title and button label go through textContent. Every + # caller passes a user-named record here, so the name is escaped on its way + # into the body: without it a record named "" executes + # as soon as someone opens the confirmation. + # `titleize` / `downcase` are English-shaped and stay applied to the record + # name so the English copy is unchanged; a locale that needs different + # casing can absorb it in its own string. def for_resource_deletion(resource_name, high_severity: false) new( destructive: true, high_severity: high_severity, - title: "Delete #{resource_name.titleize}?", - body: "Are you sure you want to delete #{resource_name.downcase}? This is not reversible.", - btn_text: "Delete #{resource_name.titleize}" + title: I18n.t("shared.custom_confirm.resource_deletion_title", resource: resource_name.titleize), + # Escaped, unlike the other two: this is the only field the dialog + # renders as HTML. + body: I18n.t("shared.custom_confirm.resource_deletion_body", resource: ERB::Util.html_escape(resource_name.downcase)), + btn_text: I18n.t("shared.custom_confirm.resource_deletion_btn_text", resource: resource_name.titleize) ) end end diff --git a/app/models/goal.rb b/app/models/goal.rb index e0a2598ed..bc0142035 100644 --- a/app/models/goal.rb +++ b/app/models/goal.rb @@ -476,6 +476,27 @@ class Goal < ApplicationRecord end end + # Confirm dialog for deleting this goal, shared by the show-page kebab and + # the index card kebab so the two can't drift. Spelled out rather than using + # CustomConfirm.for_resource_deletion, whose generic "This is not reversible" + # reads scarier than a goal delete is: only the goal, its account links and + # its pledge history go — the accounts, balances and transactions behind it + # are untouched. + def deletion_confirm + CustomConfirm.new( + destructive: true, + high_severity: true, + title: I18n.t("goals.show.confirm_delete_title"), + # Escaped: the dialog assigns `body` to innerHTML (so bodies like the + # accounts' confirm_body_html can carry

), so a goal named + # "" would otherwise run when a family member opens + # the confirmation. Only `body` needs this — the dialog sets its title and + # button label with textContent. + body: I18n.t("goals.show.confirm_delete_body", name: ERB::Util.html_escape(name)), + btn_text: I18n.t("goals.show.confirm_delete_cta") + ) + end + # Single-line state summary rendered between the header and the ring on # the show page. Replaces the stacked catch-up alert + inline status pill; # carries the same actionable copy without owning a CTA. Returns nil when diff --git a/app/views/goals/show.html.erb b/app/views/goals/show.html.erb index 5a79115a2..36c46d99f 100644 --- a/app/views/goals/show.html.erb +++ b/app/views/goals/show.html.erb @@ -79,17 +79,17 @@ <% if @goal.may_reopen? %> <% menu.with_item(variant: "button", text: t(".reopen"), icon: "rotate-ccw", href: reopen_goal_path(@goal), method: :patch) %> <% end %> - <% if @goal.archived? %> - <% menu.with_item( - variant: "button", - text: t(".delete"), - icon: "trash-2", - href: goal_path(@goal), - method: :delete, - destructive: true, - confirm: CustomConfirm.for_resource_deletion(@goal.name, high_severity: true) - ) %> - <% end %> + <%# Available in any state — see GoalsController#destroy for why there's + no archive-first gate. %> + <% menu.with_item( + variant: "button", + text: t(".delete"), + icon: "trash-2", + href: goal_path(@goal), + method: :delete, + destructive: true, + confirm: @goal.deletion_confirm + ) %> <% end %> diff --git a/config/locales/views/goals/de.yml b/config/locales/views/goals/de.yml index a00cdca1d..261af0cfb 100644 --- a/config/locales/views/goals/de.yml +++ b/config/locales/views/goals/de.yml @@ -16,7 +16,6 @@ de: create: success: Ziel erstellt. destroy: - archive_first: Archivier das Ziel, bevor du es löschst. success: Ziel gelöscht. edit: heading: Ziel bearbeiten @@ -185,7 +184,10 @@ de: confirm_complete_body_short: Du stehst bei %{progress} %, also %{saved} von %{target}. Wenn du abschließt, wird dieser Stand als dein Ergebnis festgehalten statt des ursprünglichen Ziels. Weiter, oder lieber schließen und das Ziel anpassen? confirm_complete_cta: Als erreicht markieren confirm_complete_title: Dieses Ziel als erreicht markieren? - delete: Endgültig löschen + confirm_delete_body: "%{name} und der zugehörige Verlauf der Zusagen werden endgültig entfernt. Deine verknüpften Konten, Salden und Transaktionen sind davon nicht betroffen. Wenn du es stattdessen als Beleg behalten willst, archivier es." + confirm_delete_cta: Ziel löschen + confirm_delete_title: Dieses Ziel löschen? + delete: Löschen edit: Bearbeiten empty: body: Überweis Geld auf dein verknüpftes Konto. Sure erkennt es beim nächsten Sync. Oder aktualisier den Saldo deines manuellen Kontos. diff --git a/config/locales/views/goals/en.yml b/config/locales/views/goals/en.yml index cfe336a6c..e3524b78f 100644 --- a/config/locales/views/goals/en.yml +++ b/config/locales/views/goals/en.yml @@ -77,7 +77,6 @@ en: success: Goal updated. destroy: success: Goal deleted. - archive_first: Archive the goal before deleting it. pause: success: Goal paused. invalid_transition: Goal can't be paused from its current state. @@ -104,7 +103,7 @@ en: archive: Archive unarchive: Restore reopen: Reopen goal - delete: Delete permanently + delete: Delete record_pledge_cta: Record pledge pledge_just_transferred: Log a transfer you made pledge_just_saved: Log money you set aside @@ -172,6 +171,9 @@ en: confirm_archive_title: Archive this goal? confirm_archive_body: Archived goals disappear from the main list. You can restore them later. confirm_archive_cta: Archive + confirm_delete_title: Delete this goal? + confirm_delete_body: "%{name} and its pledge history are removed for good. Your linked accounts, balances and transactions are not affected. To keep it as a record instead, archive it." + confirm_delete_cta: Delete goal paused_banner: title: This goal is paused body: Resume it to keep tracking your progress. diff --git a/config/locales/views/goals/fr.yml b/config/locales/views/goals/fr.yml index 840659ef9..e7d44ea6b 100644 --- a/config/locales/views/goals/fr.yml +++ b/config/locales/views/goals/fr.yml @@ -16,7 +16,6 @@ fr: create: success: Objectif créé. destroy: - archive_first: Archivez l'objectif avant de le supprimer. success: Objectif supprimé. edit: heading: Modifier l'objectif @@ -176,11 +175,14 @@ fr: confirm_archive_body: Les objectifs archivés disparaissent de la liste principale. Vous pourrez les restaurer plus tard. confirm_archive_cta: Archiver confirm_archive_title: Archiver cet objectif ? + confirm_delete_title: "Supprimer cet objectif ?" + confirm_delete_body: "%{name} et son historique de versements sont supprimés définitivement. Vos comptes liés, soldes et transactions ne sont pas affectés. Pour en garder une trace, archivez-le plutôt." + confirm_delete_cta: "Supprimer l'objectif" confirm_complete_body: Il quittera la liste En cours. Vous pourrez toujours l'archiver ou le restaurer plus tard. confirm_complete_body_short: Vous êtes à %{progress}%, %{saved} sur %{target}. Marquer comme terminé enregistrera cela comme votre réussite à la place de l'objectif initial. Continuer, ou fermer ceci et ajuster l'objectif à la place ? confirm_complete_cta: Marquer comme terminé confirm_complete_title: Marquer cet objectif comme terminé ? - delete: Supprimer définitivement + delete: Supprimer edit: Modifier empty: body: Effectuez un virement sur votre compte lié. Sure le détectera lors de la prochaine synchronisation. Ou mettez à jour manuellement le solde du compte. diff --git a/config/locales/views/goals/it.yml b/config/locales/views/goals/it.yml index 71b8531fd..09beb57b8 100644 --- a/config/locales/views/goals/it.yml +++ b/config/locales/views/goals/it.yml @@ -76,7 +76,6 @@ it: success: Obiettivo aggiornato. destroy: success: Obiettivo eliminato. - archive_first: Archivia l'obiettivo prima di eliminarlo. pause: success: Obiettivo messo in pausa. invalid_transition: L'obiettivo non può essere messo in pausa dal suo stato attuale. @@ -103,7 +102,7 @@ it: archive: Archivia unarchive: Ripristina reopen: Riapri obiettivo - delete: Elimina definitivamente + delete: Elimina record_pledge_cta: Registra impegno pledge_just_transferred: Registra un trasferimento effettuato pledge_just_saved: Registra denaro messo da parte @@ -171,6 +170,9 @@ it: confirm_archive_title: Archiviare questo obiettivo? confirm_archive_body: Gli obiettivi archiviati scompaiono dalla lista principale. Puoi ripristinarli in seguito. confirm_archive_cta: Archivia + confirm_delete_title: "Eliminare questo obiettivo?" + confirm_delete_body: "%{name} e la sua cronologia dei versamenti vengono eliminati definitivamente. I conti collegati, i saldi e le transazioni non vengono toccati. Per conservarlo come traccia, archivialo invece." + confirm_delete_cta: "Elimina obiettivo" paused_banner: title: Questo obiettivo è in pausa body: Riprendi per continuare a monitorare i tuoi progressi. diff --git a/config/locales/views/goals/pl.yml b/config/locales/views/goals/pl.yml index 4e7e87acd..4106dbd42 100644 --- a/config/locales/views/goals/pl.yml +++ b/config/locales/views/goals/pl.yml @@ -86,7 +86,6 @@ pl: success: Cel zaktualizowany. destroy: success: Cel usunięty. - archive_first: Zarchiwizuj cel przed jego usunięciem. pause: success: Cel wstrzymany. invalid_transition: Cel nie może być wstrzymany z bieżącego stanu. @@ -113,7 +112,7 @@ pl: archive: Archiwizuj unarchive: Przywróć reopen: Otwórz ponownie - delete: Usuń na stałe + delete: Usuń record_pledge_cta: Zapisz zobowiązanie pledge_just_transferred: Zarejestruj wykonany przelew pledge_just_saved: Zarejestruj odłożone pieniądze @@ -183,6 +182,9 @@ pl: confirm_archive_title: Zarchiwizować ten cel? confirm_archive_body: Zarchiwizowane cele znikną z głównej listy. Możesz je później przywrócić. confirm_archive_cta: Archiwizuj + confirm_delete_title: "Usunąć ten cel?" + confirm_delete_body: "%{name} i jego historia wpłat zostaną trwale usunięte. Powiązane konta, salda i transakcje pozostaną bez zmian. Aby zachować go jako zapis, zarchiwizuj go zamiast usuwać." + confirm_delete_cta: "Usuń cel" paused_banner: title: Ten cel jest wstrzymany body: Wznów go, aby kontynuować śledzenie postępów. diff --git a/config/locales/views/goals/ru.yml b/config/locales/views/goals/ru.yml index 5ab84f351..42a1323d1 100644 --- a/config/locales/views/goals/ru.yml +++ b/config/locales/views/goals/ru.yml @@ -16,7 +16,6 @@ ru: create: success: Цель создана. destroy: - archive_first: Сначала архивируйте цель перед удалением. success: Цель удалена. edit: heading: Редактировать цель @@ -194,6 +193,9 @@ ru: confirm_archive_body: Архивные цели скрываются из списка. Их можно восстановить позже. confirm_archive_cta: Архивировать confirm_archive_title: Архивировать цель? + confirm_delete_title: "Удалить эту цель?" + confirm_delete_body: "%{name} и история взносов будут удалены безвозвратно. Связанные счета, балансы и транзакции не затрагиваются. Чтобы сохранить её как запись, заархивируйте её вместо удаления." + confirm_delete_cta: "Удалить цель" confirm_complete_body: Цель исчезнет из активных. confirm_complete_body_short: Вы достигли %{progress}%, %{saved} из %{target}. confirm_complete_cta: Отметить выполненной diff --git a/config/locales/views/goals/tr.yml b/config/locales/views/goals/tr.yml index 961d60fd0..b44516e19 100644 --- a/config/locales/views/goals/tr.yml +++ b/config/locales/views/goals/tr.yml @@ -16,7 +16,6 @@ tr: create: success: Hedef oluşturuldu. destroy: - archive_first: Hedefi silmeden önce arşivleyin. success: Hedef silindi. edit: heading: Hedefi düzenle @@ -183,6 +182,9 @@ tr: geri yükleyebilirsiniz. confirm_archive_cta: Arşivle confirm_archive_title: Bu hedef arşivlensin mi? + confirm_delete_title: "Bu hedef silinsin mi?" + confirm_delete_body: "%{name} ve taahhüt geçmişi kalıcı olarak silinir. Bağlı hesaplarınız, bakiyeleriniz ve işlemleriniz etkilenmez. Kayıt olarak saklamak için bunun yerine arşivleyin." + confirm_delete_cta: "Hedefi sil" confirm_complete_body: Devam Eden listesinden çıkar. Daha sonra yine de arşivleyebilir veya geri yükleyebilirsiniz. confirm_complete_body_short: "%{progress}%'desiniz, %{target} hedefinin %{saved} @@ -190,7 +192,7 @@ tr: yerine başarınız olarak kaydeder. Devam edin ya da bunu kapatıp hedefi ayarlayın." confirm_complete_cta: Tamamlandı olarak işaretle confirm_complete_title: Bu hedef tamamlandı olarak işaretlensin mi? - delete: Kalıcı olarak sil + delete: Sil edit: Düzenle empty: body: Bağlı hesabınıza bir transfer yapın. Sure bir sonraki eşitlemede bunu diff --git a/config/locales/views/goals/zh-CN.yml b/config/locales/views/goals/zh-CN.yml index 6727a0f71..fedb67925 100644 --- a/config/locales/views/goals/zh-CN.yml +++ b/config/locales/views/goals/zh-CN.yml @@ -76,7 +76,6 @@ zh-CN: success: 目标已更新。 destroy: success: 目标已删除。 - archive_first: 删除前请先归档该目标。 pause: success: 目标已暂停。 invalid_transition: 目标当前状态不能暂停。 @@ -103,7 +102,7 @@ zh-CN: archive: 归档 unarchive: 恢复 reopen: 重新开启目标 - delete: 永久删除 + delete: 删除 record_pledge_cta: 记录承诺 pledge_just_transferred: 记录你完成的转账 pledge_just_saved: 记录你留出的资金 @@ -171,6 +170,9 @@ zh-CN: confirm_archive_title: 归档此目标？ confirm_archive_body: 归档后的目标会从主列表隐藏。之后可以恢复。 confirm_archive_cta: 归档 + confirm_delete_title: "删除此目标？" + confirm_delete_body: "%{name} 及其存款记录将被永久删除。你的关联账户、余额和交易不受影响。如果想保留记录，请改为归档。" + confirm_delete_cta: "删除目标" paused_banner: title: 此目标已暂停 body: 恢复它以继续跟踪进度。 diff --git a/config/locales/views/goals/zh-TW.yml b/config/locales/views/goals/zh-TW.yml index c79a6ee22..ba2d0c8a6 100644 --- a/config/locales/views/goals/zh-TW.yml +++ b/config/locales/views/goals/zh-TW.yml @@ -76,7 +76,6 @@ zh-TW: success: 目標已更新。 destroy: success: 目標已刪除。 - archive_first: 請先封存目標再刪除。 pause: success: 目標已暫停。 invalid_transition: 目標在目前的狀態下無法暫停。 @@ -103,7 +102,7 @@ zh-TW: archive: 封存 unarchive: 還原 reopen: 重新開啟目標 - delete: 永久刪除 + delete: 刪除 record_pledge_cta: 記錄投入 pledge_just_transferred: 記錄一筆您已完成的轉帳 pledge_just_saved: 記錄一筆您已存起來的錢 @@ -171,6 +170,9 @@ zh-TW: confirm_archive_title: 要封存這個目標嗎？ confirm_archive_body: 封存的目標不會出現在主清單中，您之後可以再還原。 confirm_archive_cta: 封存 + confirm_delete_title: 要刪除這個目標嗎？ + confirm_delete_body: "%{name} 和它的投入紀錄會被永久移除。您已連結的帳戶、餘額與交易都不會受到影響。若想保留成紀錄，請改為封存。" + confirm_delete_cta: 刪除目標 paused_banner: title: 這個目標已暫停 body: 恢復它就能繼續追蹤您的進度。 diff --git a/config/locales/views/shared/ca.yml b/config/locales/views/shared/ca.yml index 66e3f7bd6..3cd96803b 100644 --- a/config/locales/views/shared/ca.yml +++ b/config/locales/views/shared/ca.yml @@ -16,6 +16,9 @@ ca: default_body: Aquesta acció no es pot desfer. default_btn_text: Confirma default_title: Segur que vols continuar? + resource_deletion_title: "Voleu eliminar %{resource}?" + resource_deletion_body: "Segur que voleu eliminar %{resource}? Aquesta acció no es pot desfer." + resource_deletion_btn_text: "Elimina %{resource}" exchange_rate_tabs: calculate_rate_tab: Calcular la taxa FX convert_tab: Convertir amb la taxa FX diff --git a/config/locales/views/shared/en.yml b/config/locales/views/shared/en.yml index 056cb521e..9d7606cf2 100644 --- a/config/locales/views/shared/en.yml +++ b/config/locales/views/shared/en.yml @@ -29,6 +29,9 @@ en: default_title: "Are you sure?" default_body: "This is not reversible." default_btn_text: "Confirm" + resource_deletion_title: "Delete %{resource}?" + resource_deletion_body: "Are you sure you want to delete %{resource}? This is not reversible." + resource_deletion_btn_text: "Delete %{resource}" family_moniker: group_plural: Groups group_singular: Group diff --git a/config/locales/views/shared/fr.yml b/config/locales/views/shared/fr.yml index ed1652a13..06d54e075 100644 --- a/config/locales/views/shared/fr.yml +++ b/config/locales/views/shared/fr.yml @@ -15,6 +15,9 @@ fr: default_body: Ceci n’est pas réversible. default_btn_text: Confirmer default_title: Êtes-vous sûr? + resource_deletion_title: "Supprimer %{resource} ?" + resource_deletion_body: "Êtes-vous sûr de vouloir supprimer %{resource} ? Ceci n’est pas réversible." + resource_deletion_btn_text: "Supprimer %{resource}" exchange_rate_tabs: calculate_rate_tab: Calculer le taux FX convert_tab: Convertir avec le taux FX diff --git a/config/locales/views/shared/hu.yml b/config/locales/views/shared/hu.yml index 33191380c..50d7fd000 100644 --- a/config/locales/views/shared/hu.yml +++ b/config/locales/views/shared/hu.yml @@ -25,6 +25,9 @@ hu: default_title: "Biztosan?" default_body: "Ez a művelet nem vonható vissza." default_btn_text: "Megerősítés" + resource_deletion_title: "Törli a következőt: %{resource}?" + resource_deletion_body: "Biztosan törli a következőt: %{resource}? Ez a művelet nem vonható vissza." + resource_deletion_btn_text: "%{resource} törlése" family_moniker: singular: Háztartás plural: Háztartások diff --git a/config/locales/views/shared/it.yml b/config/locales/views/shared/it.yml index c67223cb2..aef40f7e0 100644 --- a/config/locales/views/shared/it.yml +++ b/config/locales/views/shared/it.yml @@ -29,6 +29,9 @@ it: default_title: "Sei sicuro?" default_body: "Questa operazione non è reversibile." default_btn_text: "Conferma" + resource_deletion_title: "Eliminare %{resource}?" + resource_deletion_body: "Sei sicuro di voler eliminare %{resource}? Questa operazione non è reversibile." + resource_deletion_btn_text: "Elimina %{resource}" family_moniker: group_plural: Gruppi group_singular: Gruppo diff --git a/config/locales/views/shared/ru.yml b/config/locales/views/shared/ru.yml index c4c3f9cb8..bb42f4337 100644 --- a/config/locales/views/shared/ru.yml +++ b/config/locales/views/shared/ru.yml @@ -16,6 +16,9 @@ ru: default_body: Это действие необратимо. default_btn_text: Подтвердить default_title: Вы уверены? + resource_deletion_title: "Удалить %{resource}?" + resource_deletion_body: "Вы уверены, что хотите удалить %{resource}? Это действие необратимо." + resource_deletion_btn_text: "Удалить %{resource}" exchange_rate_tabs: calculate_rate_tab: Рассчитать курс convert_tab: Конвертировать по курсу diff --git a/config/locales/views/shared/tr.yml b/config/locales/views/shared/tr.yml index 09579e0dd..03a069a30 100644 --- a/config/locales/views/shared/tr.yml +++ b/config/locales/views/shared/tr.yml @@ -16,6 +16,9 @@ tr: default_body: Bu işlem geri alınamaz. default_btn_text: Onayla default_title: Emin misiniz? + resource_deletion_title: "%{resource} silinsin mi?" + resource_deletion_body: "%{resource} ögesini silmek istediğinize emin misiniz? Bu işlem geri alınamaz." + resource_deletion_btn_text: "%{resource} sil" exchange_rate_tabs: calculate_rate_tab: Döviz Kuru Hesapla convert_tab: Döviz Kuru ile Dönüştür diff --git a/config/locales/views/shared/vi.yml b/config/locales/views/shared/vi.yml index db485cb38..cb27d9428 100644 --- a/config/locales/views/shared/vi.yml +++ b/config/locales/views/shared/vi.yml @@ -25,6 +25,9 @@ vi: default_title: "Bạn có chắc không?" default_body: "Thao tác này không thể hoàn tác." default_btn_text: "Xác nhận" + resource_deletion_title: "Xóa %{resource}?" + resource_deletion_body: "Bạn có chắc muốn xóa %{resource} không? Thao tác này không thể hoàn tác." + resource_deletion_btn_text: "Xóa %{resource}" family_moniker: group_plural: Nhóm group_singular: Nhóm diff --git a/config/locales/views/shared/zh-CN.yml b/config/locales/views/shared/zh-CN.yml index 5f2d3499f..1e285dac4 100644 --- a/config/locales/views/shared/zh-CN.yml +++ b/config/locales/views/shared/zh-CN.yml @@ -25,6 +25,9 @@ zh-CN: default_title: 确定吗？ default_body: 此操作不可逆。 default_btn_text: 确认 + resource_deletion_title: "删除%{resource}？" + resource_deletion_body: "确定要删除%{resource}吗？此操作不可逆。" + resource_deletion_btn_text: "删除%{resource}" family_moniker: group_plural: 组 group_singular: 组 diff --git a/docs/llm-guides/goals.md b/docs/llm-guides/goals.md index 7592d917d..ccd6966c3 100644 --- a/docs/llm-guides/goals.md +++ b/docs/llm-guides/goals.md @@ -44,7 +44,7 @@ Model layer: Controllers / routes: -- `app/controllers/goals_controller.rb` — index / show / new / create / edit / update / destroy / pause / resume / complete / archive / unarchive. +- `app/controllers/goals_controller.rb` — index / show / new / create / edit / update / destroy (any state) / pause / resume / complete / archive / unarchive. - `app/controllers/goal_pledges_controller.rb` — new / create / renew / destroy. - `config/routes.rb` — `resources :goals do resources :pledges ... member { patch :renew } end`. @@ -117,6 +117,17 @@ The AASM `state` is independent. Read `Goal#display_status` (not `#status`) to get the right pill label: it returns the AASM state when it's not `:active`, otherwise falls through to `#status`. +`archived` is a "keep it as a record" state, **not** a prerequisite for +deleting. `GoalsController#destroy` accepts a goal in any state. The only +affordance is the kebab on the show page — index cards deliberately carry +no actions, so the card stays a single click target. A delete cascades +only to `goal_accounts` and `goal_pledges`; +`GoalPledge#clear_matched_transaction_extra` unstamps +`extra["goal"]["pledge_id"]` from any transaction a matched pledge +claimed. No account, balance, entry or transaction is removed. Reuse +`Goal#deletion_confirm` rather than building a second `CustomConfirm` at +a new call site. + `Goal#pace` is the rolling 90-day net inflow into the linked accounts, divided by three. The query joins `entries` with `transactions` (valuations excluded by join shape), drops excluded entries, and drops diff --git a/test/controllers/goals_controller_test.rb b/test/controllers/goals_controller_test.rb index cfc5c6d5b..cd2023d62 100644 --- a/test/controllers/goals_controller_test.rb +++ b/test/controllers/goals_controller_test.rb @@ -1,6 +1,8 @@ require "test_helper" class GoalsControllerTest < ActionDispatch::IntegrationTest + include EntriesTestHelper + setup do @user = users(:family_admin) @user.update!(preferences: (@user.preferences || {}).merge("preview_features_enabled" => true)) @@ -210,6 +212,23 @@ class GoalsControllerTest < ActionDispatch::IntegrationTest assert fresh.reload.active? end + # The reported bug: Delete rendered only when the goal was archived, so an + # active goal had no delete affordance anywhere in the UI. The kebab is the + # only route to it, so assert the form is actually in the markup per state — + # a 200 alone would not have caught the original miss. + test "show exposes delete for a goal in every state" do + delete_form = "form[action='#{goal_path(@goal)}'] input[name='_method'][value='delete']" + + %w[active paused completed archived].each do |state| + @goal.update_column(:state, state) + + get goal_url(@goal) + + assert_response :success + assert_select delete_form, 1, "no delete affordance on a #{state} goal" + end + end + # A goal whose last funding account is deleted survives with zero links and # fails `must_have_at_least_one_linked_account` from then on. Editing is the # only way back, so update must validate the accounts the user SUBMITTED, @@ -240,14 +259,16 @@ class GoalsControllerTest < ActionDispatch::IntegrationTest assert_equal "active", orphan.reload.state end - test "destroy on non-archived is rejected" do - assert_no_difference "Goal.count" do + test "destroy deletes an active goal and cascades to its links and pledges" do + assert_difference -> { Goal.count } => -1, + -> { GoalAccount.count } => -2, + -> { GoalPledge.count } => -2 do delete goal_url(@goal) end - assert_redirected_to goal_path(@goal) + assert_redirected_to goals_path end - test "destroy on archived deletes" do + test "destroy deletes an archived goal" do @goal.archive! assert_difference "Goal.count", -1 do delete goal_url(@goal) @@ -255,6 +276,23 @@ class GoalsControllerTest < ActionDispatch::IntegrationTest assert_redirected_to goals_path end + # The one thing a goal delete reaches outside its own tables: a matched + # pledge stamps `extra["goal"]["pledge_id"]` onto the transaction it claimed, + # and GoalPledge#clear_matched_transaction_extra must unstamp it on the way + # out. The transaction itself must survive untouched. + test "destroy unstamps the transaction a matched pledge claimed" do + txn = create_transaction(account: @connected, amount: -300).entryable + pledge = goal_pledges(:matched_transfer) + txn.update!(extra: { "goal" => { "pledge_id" => pledge.id } }) + pledge.update_column(:matched_transaction_id, txn.id) + + delete goal_url(@goal) + + assert_redirected_to goals_path + assert Transaction.exists?(txn.id), "deleting a goal must not delete the transaction" + assert_nil txn.reload.extra.dig("goal", "pledge_id") + end + test "index KPI swaps to 'All caught up' when every tracked goal is reached" do family = users(:family_admin).family family.goals.destroy_all diff --git a/test/helpers/custom_confirm_test.rb b/test/helpers/custom_confirm_test.rb new file mode 100644 index 000000000..7fc7c6be1 --- /dev/null +++ b/test/helpers/custom_confirm_test.rb @@ -0,0 +1,50 @@ +require "test_helper" + +class CustomConfirmTest < ActiveSupport::TestCase + # `confirm_dialog_controller` assigns `body` to innerHTML so bodies like + # accounts' `confirm_body_html` can carry markup. Every caller passes a + # user-named record, so an account named "" would run + # when someone opened the confirmation. + test "escapes the resource name in the HTML-rendered body" do + data = CustomConfirm.for_resource_deletion("").to_data_attribute + + assert_includes data[:body], "<img src=x onerror=alert(1)>" + assert_no_match(/, 1, "expected more than just English to define this block" + + localized.each do |locale| + body = I18n.t("shared.custom_confirm.resource_deletion_body", resource: "Wedding", locale: locale) + assert_includes body, "Wedding", "#{locale} dropped the %{resource} interpolation" + assert I18n.t("shared.custom_confirm.resource_deletion_title", resource: "x", locale: locale).present? + assert I18n.t("shared.custom_confirm.resource_deletion_btn_text", resource: "x", locale: locale).present? + end + end + + test "high severity picks the destructive button variant" do + assert_equal "destructive", CustomConfirm.for_resource_deletion("rule", high_severity: true).to_data_attribute[:variant] + assert_equal "outline-destructive", CustomConfirm.for_resource_deletion("rule").to_data_attribute[:variant] + end +end diff --git a/test/models/goal_test.rb b/test/models/goal_test.rb index e6af35e84..3c9da94cf 100644 --- a/test/models/goal_test.rb +++ b/test/models/goal_test.rb @@ -14,6 +14,38 @@ class GoalTest < ActiveSupport::TestCase assert @goal.valid? end + # The confirm dialog assigns `body` to innerHTML, so an unescaped goal name + # would execute when a family member opens the delete confirmation. + test "deletion_confirm escapes the goal name in the dialog body" do + @goal.name = "" + + body = @goal.deletion_confirm.to_data_attribute[:body] + + assert_includes body, "<img src=x onerror=alert(1)>" + assert_no_match(/=, 7, "goal locale files disappeared — check the glob" + + locales.each do |locale| + body = I18n.t("goals.show.confirm_delete_body", name: "Wedding", locale: locale, fallback: false, default: nil) + assert body, "#{locale} is missing goals.show.confirm_delete_body" + assert_includes body, "Wedding", "#{locale} dropped the %{name} interpolation" + + %w[confirm_delete_title confirm_delete_cta].each do |key| + assert I18n.t("goals.show.#{key}", locale: locale, fallback: false, default: nil).present?, + "#{locale} is missing goals.show.#{key}" + end + end + end + # The days-left segment is returned separately so the view can keep it # unbroken; joined into one string it wrapped after "days" on a phone. test "header_summary_parts keeps the days-left phrase in its own segment" do