gavin/maybe
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 28 Wiki Activity

Fix account transaction form resetting amount to 0 #1133

Merged
zachgoll merged 1 commits from zachgoll/fix-1132 into main 2024-08-27 07:10:17 +08:00
Conversation 2 Commits 1 Files Changed 1 +14 -11
1 changed files with 14 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
app/controllers/account/transactions_controller.rb
View File

@@ -12,8 +12,7 @@ class Account::TransactionsController < ApplicationController
end
def update
@entry.update!(entry_params.merge(amount: amount))
@entry.sync_account_later
@entry.update!(entry_params)
zachgoll commented 2024-08-27 03:53:38 +08:00 (Migrated from github.com)
Review
Copy Link

Merging in amount caused the scenario when we didn't include the param, it was calling nil.to_d, which resulted in 0

Merging in `amount` caused the scenario when we didn't include the param, it was calling `nil.to_d`, which resulted in 0
zachgoll commented 2024-08-27 03:53:00 +08:00 (Migrated from github.com)
Review
Copy Link

Removed the sync_later because this triggers a refresh of the entire page (via the Sync broadcasts) which gets annoying for updating transaction details. This will cause out-of-sync data if the user changes the amount or date, but will be updated on the next sync.

Eventually we'll want to get more granular with when we trigger the re-sync, but for now this feels like a decent tradeoff.

Removed the `sync_later` because this triggers a refresh of the entire page (via the Sync broadcasts) which gets annoying for updating transaction details. This will cause out-of-sync data if the user changes the amount or date, but will be updated on the next sync. Eventually we'll want to get more granular with _when_ we trigger the re-sync, but for now this feels like a decent tradeoff.
respond_to do |format|
format.html { redirect_to account_entry_path(@account, @entry), notice: t(".success") }
@@ -34,7 +33,7 @@ class Account::TransactionsController < ApplicationController
def entry_params
params.require(:account_entry)
.permit(
:name, :date, :amount, :currency, :entryable_type,
:name, :date, :amount, :currency, :entryable_type, :nature,
entryable_attributes: [
:id,
:notes,
@@ -43,14 +42,18 @@ class Account::TransactionsController < ApplicationController
:merchant_id,
{ tag_ids: [] }
]
)
end
).tap do |permitted_params|
nature = permitted_params.delete(:nature)
def amount
if params[:account_entry][:nature] == "income"
entry_params[:amount].to_d * -1
else
entry_params[:amount].to_d
end
if permitted_params[:amount]
amount_value = permitted_params[:amount].to_d
if nature == "income"
amount_value *= -1
end
permitted_params[:amount] = amount_value
end
end
end
end