fix(goals): allow deleting a goal without archiving it first (#2963)

* 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 "<img src=x onerror=…>" 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 `<img>` 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 <juanjo.mata@gmail.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
This commit is contained in:
Guillem Arias Fauste
2026-08-16 08:12:45 +02:00
committed by GitHub
co-authored by Juan José Mata
parent 47525d7a73
commit acf4cb2010
26 changed files with 250 additions and 41 deletions
+6 -5
View File
@@ -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
+14 -3
View File
@@ -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 "<img src=x onerror=…>" 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
+21
View File
@@ -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 <p>), so a goal named
# "<img src=x onerror=…>" 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
+11 -11
View File
@@ -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 %>
</div>
</header>
+4 -2
View File
@@ -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.
+4 -2
View File
@@ -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.
+4 -2
View File
@@ -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.
+4 -2
View File
@@ -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.
+4 -2
View File
@@ -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.
+3 -1
View File
@@ -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: Отметить выполненной
+4 -2
View File
@@ -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
+4 -2
View File
@@ -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: 恢复它以继续跟踪进度。
+4 -2
View File
@@ -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: 恢復它就能繼續追蹤您的進度。
+3
View File
@@ -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
+3
View File
@@ -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
+3
View File
@@ -15,6 +15,9 @@ fr:
default_body: Ceci nest 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 nest 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
+3
View File
@@ -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
+3
View File
@@ -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
+3
View File
@@ -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: Конвертировать по курсу
+3
View File
@@ -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
+3
View File
@@ -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
+3
View File
@@ -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:
+12 -1
View File
@@ -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
+42 -4
View File
@@ -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
+50
View File
@@ -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 "<img src=x onerror=…>" would run
# when someone opened the confirmation.
test "escapes the resource name in the HTML-rendered body" do
data = CustomConfirm.for_resource_deletion("<img src=x onerror=alert(1)>").to_data_attribute
assert_includes data[:body], "&lt;img src=x onerror=alert(1)&gt;"
assert_no_match(/<img/, data[:body])
end
# Title and button label reach the dialog through textContent, so they are
# inert — pinned here so a future move to innerHTML doesn't pass silently.
test "keeps the English copy the hardcoded strings produced" do
data = CustomConfirm.for_resource_deletion("rule").to_data_attribute
assert_equal "Delete Rule?", data[:title]
assert_equal "Are you sure you want to delete rule? This is not reversible.", data[:body]
assert_equal "Delete Rule", data[:confirmText]
end
# These back ~40 call sites across the app, so a locale that ships the
# sibling `default_*` copy should ship these too. Locales without a
# `custom_confirm` block at all are left out on purpose — adding one would
# invent structure they have not adopted, and fallbacks cover them.
test "resource-deletion copy resolves wherever the sibling default copy lives" do
localized = Dir["config/locales/views/shared/*.yml"].filter_map do |path|
locale = File.basename(path, ".yml")
next unless I18n.exists?("shared.custom_confirm.default_title", locale)
locale
end
assert_operator localized.size, :>, 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
+32
View File
@@ -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 = "<img src=x onerror=alert(1)>"
body = @goal.deletion_confirm.to_data_attribute[:body]
assert_includes body, "&lt;img src=x onerror=alert(1)&gt;"
assert_no_match(/<img/, body)
end
# Derived, not a hardcoded list: a locale added later ships a goals YAML
# without these keys otherwise. `fallback: false` carries as much weight —
# the backend has I18n::Backend::Fallbacks mixed in, so a missing German key
# resolves to the English string and a plain `.present?` assertion passes
# while a German family reads English in the dialog.
test "confirm_delete copy resolves in every locale that ships goal translations" do
locales = Rails.root.glob("config/locales/views/goals/*.yml").map { |f| f.basename(".yml").to_s.to_sym }
assert_operator locales.size, :>=, 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