Files
sure/app/views/transactions/_form.html.erb
T
b62f6035ea fix(transactions): prevent duplicate creation on double-submit (#3338)
* fix(transactions): prevent duplicate creation on double-submit

TransactionsController#create had no protection against a repeated
form submission - a double-click, a browser retry, or two
near-simultaneous requests could all create a separate identical
transaction. Adds a per-form idempotency key (a UUID hidden field,
generated fresh on page load) that reuses the existing
entries(account_id, source, external_id) partial unique index, with a
pre-check for the sequential case and a RecordNotUnique rescue as the
authoritative backstop for genuine concurrent requests - the same
pattern already used by mark_as_recurring and the public API's
idempotency-key support.

Fixes #3334.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(transactions): store the idempotency key in its own column, not external_id

Codex review finding: reusing external_id/source for the web-form
idempotency token made every manually-created transaction satisfy
Entry#linked? (external_id.present?), since the form always supplies
a key. That incorrectly made manual entries look provider-synced -
disabling their date/nature/amount/currency fields in the editor
(app/views/transactions/show.html.erb), and hiding them from future
provider dedup matching (which filters to external_id: nil).

Adds a dedicated entries.idempotency_key column with its own partial
unique index scoped by account_id, used only for this de-duplication
and with no meaning anywhere else in the app, so it can't collide with
provider-linkage semantics. TransactionsController now tags/looks up
entries by this column instead of source/external_id.

Added a regression test asserting a transaction created via this path
is not linked? and has no external_id/source set.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(migration): rebuild an invalid index left by an interrupted CONCURRENTLY build

Codex review finding: index_exists? alone doesn't distinguish a valid
index from an INVALID one left behind by an interrupted CREATE INDEX
CONCURRENTLY (e.g. a deploy killed mid-build). A retry after such a
failure would short-circuit on the early-return and record this
migration as applied, while the actual uniqueness constraint stays
missing/broken. Checks pg_index.indisvalid directly before deciding
whether to skip the rebuild.

Co-Authored-By: Claude <noreply@anthropic.com>

* fix(transactions): rotate idempotency token on bfcache/Turbo restore, keep index removal concurrent

Codex flagged that a page restored from the browser bfcache or Turbo's
snapshot cache (back button, duplicated tab) keeps the already-consumed
idempotency token in the hidden field. Submitting a different, edited
transaction from that restored page would then match the old committed
entry and silently redirect onto it instead of creating the new one.
transaction_form_controller now rotates the token on turbo:before-cache
so any later restore starts from a fresh, unconsumed value.

Also address CodeRabbit's note that the migration's down block did a
blocking DROP INDEX instead of DROP INDEX CONCURRENTLY.

* fix(transactions): also rotate idempotency token on native bfcache restore

CodeRabbit noted turbo:before-cache only covers Turbo's own snapshot
cache, not the browser's native bfcache (e.g. a full navigation away
and back, not through Turbo drive). Add a persisted-pageshow handler
alongside it, and wire both through declarative data-action bindings
on the form per this repo's Stimulus convention instead of manual
addEventListener/connect/disconnect.

* fix(transactions): fall back to manual UUID when crypto.randomUUID is unavailable

crypto.randomUUID() requires a secure context, but this app's self-hosted
mode is commonly reached over plain HTTP (LAN, reverse proxy without TLS).
On such a deployment, calling it inside the cache-restore rotation handlers
throws, leaving the stale, already-consumed idempotency token in the hidden
field — a later edited resubmission would then silently match the old entry
via find_duplicate_manual_entry and drop the user's edits. Build a v4 UUID
manually from crypto.getRandomValues (which has no secure-context
restriction) when randomUUID is missing.

Also drops a stale comment reference to a MANUAL_FORM_SOURCE constant that
doesn't exist anywhere in the codebase, and corrects a rescue comment that
still described the old (account_id, source, external_id) index instead of
the (account_id, idempotency_key) index actually backing this constraint.

---------

Co-authored-by: Gerald <248542187+gfr-free@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
2026-09-04 06:04:12 +02:00

130 lines
5.8 KiB
ERB

<%# locals: (entry:, account_currencies:, manual_accounts:, categories:, merchants:, tags:) %>
<%= styled_form_with model: entry, url: transactions_path, class: "space-y-4", data: { controller: "transaction-form", action: "turbo:before-cache@document->transaction-form#refreshIdempotencyKey pageshow@window->transaction-form#refreshIdempotencyKeyIfPersisted", transaction_form_exchange_rate_url_value: exchange_rate_path, transaction_form_account_currencies_value: account_currencies.to_json } do |f| %>
<% if entry.errors.any? %>
<%= render "shared/form_errors", model: entry %>
<% end %>
<section data-controller="transaction-type-tabs">
<%= render "shared/transaction_type_tabs", active_tab: params[:nature] == "inflow" ? "income" : "expense", account_id: params[:account_id] %>
<%= f.hidden_field :nature, value: params[:nature] || "outflow", data: { "transaction-type-tabs-target": "natureField" } %>
<%= f.hidden_field :entryable_type, value: "Transaction" %>
<%# Anti-double-submit token: a fresh UUID per page load, echoed back on
submit. TransactionsController#create uses it to recognize a repeat
submission (double-click, retry, or a genuine concurrent race) and
avoid creating a duplicate transaction.
transaction_form_controller#refreshIdempotencyKey replaces this value
on turbo:before-cache (Turbo's own snapshot cache) and on a
persisted pageshow (the browser's native bfcache), so a page
restored via either path - browser back, duplicated tab - can't
replay a token that already committed an entry and silently
redirect a distinct submission onto that old one. %>
<%= hidden_field_tag "entry[idempotency_key]", new_transaction_idempotency_key, data: { "transaction-form-target": "idempotencyKey" } %>
</section>
<section class="space-y-2">
<%= f.text_field :name, label: t(".description"), placeholder: t(".description_placeholder"), required: true %>
<% if @entry.account_id %>
<%= f.hidden_field :account_id, data: { transaction_form_target: "account" } %>
<% else %>
<%= f.collection_select :account_id, manual_accounts, :id, :name, { prompt: t(".account_prompt"), label: t(".account"), selected: Current.user.default_account_for_transactions&.id, variant: :logo, searchable: true }, required: true, class: "form-field__input text-ellipsis", data: { transaction_form_target: "account", action: "change->transaction-form#checkCurrencyDifference" } %>
<% end %>
<%= f.money_field :amount,
label: t(".amount"),
required: true,
container_class: "money-field-wrapper",
amount_data: { transaction_form_target: "amount", action: "input->transaction-form#onAmountChange" },
currency_data: { transaction_form_target: "currency", action: "change->transaction-form#onCurrencyChange" } %>
<%= f.fields_for :entryable do |ef| %>
<%= render DS::CategorySelect.new(
form: ef,
categories: categories,
selected_id: ef.object.category_id,
blank_label: t(".category_prompt")
) %>
<% end %>
<%= f.date_field :date,
label: t(".date"),
required: true,
min: Entry.min_supported_date,
max: Date.current,
value: f.object.date || Date.current,
data: { transaction_form_target: "date", action: "change->transaction-form#checkCurrencyDifference" } %>
<% convert_input = capture do %>
<%= f.fields_for :entryable do |ef| %>
<%= ef.number_field :exchange_rate,
label: t("shared.exchange_rate_tabs.exchange_rate"),
min: "0.00000000000001",
step: "0.00000000000001",
placeholder: "1.0",
class: "form-field__input",
data: {
transaction_form_target: "exchangeRateField",
action: "input->transaction-form#onConvertExchangeRateChange"
} %>
<% end %>
<% end %>
<% destination_input = capture do %>
<%= number_field_tag :destination_amount,
nil,
id: "transaction_form_destination_amount",
class: "form-field__input",
autocomplete: "off",
min: "0",
step: "0.00000001",
placeholder: "92",
data: {
transaction_form_target: "destinationAmount",
action: "input->transaction-form#onCalculateRateDestinationAmountChange"
} %>
<% end %>
<%= render "shared/exchange_rate_tabs",
controller_id: "transaction-form",
controller_key: "transaction_form",
help_text: t("shared.exchange_rate_tabs.exchange_rate_help"),
convert_tab_label: t("shared.exchange_rate_tabs.convert_tab"),
calculate_rate_tab_label: t("shared.exchange_rate_tabs.calculate_rate_tab"),
destination_amount_label: t("shared.exchange_rate_tabs.destination_amount"),
exchange_rate_label: t("shared.exchange_rate_tabs.exchange_rate"),
convert_input: convert_input,
destination_input: destination_input %>
</section>
<%= render DS::Disclosure.new(title: t(".details")) do %>
<section class="space-y-2">
<%= f.fields_for :entryable do |ef| %>
<%= render DS::MerchantSelect.new(
form: ef,
method: :merchant_id,
merchants: merchants,
selected_id: ef.object.merchant_id,
include_blank: t(".none"),
label: t(".merchant_label")
) %>
<%= render DS::TagSelect.new(
form: ef,
tags: tags,
selected_ids: ef.object.tag_ids
) %>
<% end %>
<%= f.text_area :notes,
label: t(".note_label"),
placeholder: t(".note_placeholder"),
rows: 5,
"data-auto-submit-form-target": "auto" %>
</section>
<% end %>
<section>
<%= f.submit t(".submit") %>
</section>
<% end %>