Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class AccountsController < ApplicationController
include StreamExtensions

before_action :set_account, only: %i[show sparkline sync set_default remove_default]
before_action :set_manageable_account, only: %i[toggle_active destroy unlink confirm_unlink select_provider]
before_action :set_manageable_account, only: %i[toggle_active toggle_category_matcher destroy unlink confirm_unlink select_provider]
include Periodable

def index
Expand Down Expand Up @@ -96,6 +96,12 @@ def toggle_active
redirect_to accounts_path
end

def toggle_category_matcher
value = ActiveModel::Type::Boolean.new.cast(params.dig(:account, :enable_category_matcher))
@account.update!(enable_category_matcher: value)
Comment on lines +99 to +101
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Require the toggle param before casting to avoid accidental disable

At Line 99, cast(params.dig(...)) treats a missing key as false, so malformed/partial requests can silently disable category matching. Require + permit this field and fail fast when absent.

Proposed fix
 def toggle_category_matcher
-  value = ActiveModel::Type::Boolean.new.cast(params.dig(:account, :enable_category_matcher))
+  value = ActiveModel::Type::Boolean.new.cast(
+    params.require(:account).permit(:enable_category_matcher).fetch(:enable_category_matcher)
+  )
   `@account.update`!(enable_category_matcher: value)
   head :no_content
 end

As per coding guidelines: app/controllers/**/*.rb: “Implement strong parameters and CSRF protection throughout the application”.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def toggle_category_matcher
value = ActiveModel::Type::Boolean.new.cast(params.dig(:account, :enable_category_matcher))
@account.update!(enable_category_matcher: value)
def toggle_category_matcher
value = ActiveModel::Type::Boolean.new.cast(
params.require(:account).permit(:enable_category_matcher).fetch(:enable_category_matcher)
)
`@account.update`!(enable_category_matcher: value)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/controllers/accounts_controller.rb` around lines 98 - 100, The
toggle_category_matcher action currently casts params.dig(...) which treats a
missing key as false; instead require and permit the param so missing requests
fail fast. Update toggle_category_matcher to pull params via something like
params.require(:account).permit(:enable_category_matcher) (or use require +
fetch on :enable_category_matcher) and then cast the permitted value with
ActiveModel::Type::Boolean before calling `@account.update`! so malformed/partial
requests raise ParameterMissing rather than silently disabling the feature.

head :no_content
end

def set_default
unless @account.eligible_for_transaction_default?
redirect_to accounts_path, alert: t("accounts.set_default.depository_only")
Expand Down
1 change: 1 addition & 0 deletions app/controllers/concerns/accountable_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ def account_params
:name, :balance, :subtype, :currency, :accountable_type, :return_to,
:opening_balance_date,
:institution_name, :institution_domain, :notes,
:enable_category_matcher,
accountable_attributes: self.class.permitted_accountable_attributes
)
end
Expand Down
1 change: 1 addition & 0 deletions app/models/plaid_entry/processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ def detailed_category

def matched_category
return nil unless detailed_category
return nil unless account&.enable_category_matcher?
@matched_category ||= category_matcher.match(detailed_category)
end

Expand Down
10 changes: 10 additions & 0 deletions app/views/accounts/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@

<%= yield form %>

<% if account.linked? && account.persisted? %>
<div class="flex items-center justify-between gap-3 py-2">
<div class="space-y-1">
<p class="text-sm text-primary"><%= t(".enable_category_matcher_label") %></p>
<p class="text-secondary text-sm"><%= t(".enable_category_matcher_hint") %></p>
</div>
<%= form.toggle :enable_category_matcher %>
</div>
<% end %>

<details class="group">
<summary class="cursor-pointer text-sm text-secondary hover:text-primary flex items-center gap-1 py-2">
<%= icon "chevron-right", size: "sm", class: "group-open:rotate-90 transition-transform" %>
Expand Down
8 changes: 6 additions & 2 deletions config/locales/views/accounts/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ en:
institution_domain_placeholder: e.g., chase.com
notes_label: Notes
notes_placeholder: Store additional information like account numbers, sort codes, IBAN, routing numbers, etc.
enable_category_matcher_label: Enable category matcher
enable_category_matcher_hint: When enabled, the provider's suggested category is applied to imported transactions. Disable to leave new transactions uncategorized.
index:
accounts: Accounts
manual_accounts:
Expand Down Expand Up @@ -87,7 +89,8 @@ en:
owed: Amount owed
menu:
confirm_accept: Delete "%{name}"
confirm_body_html: "<p>By deleting this account, you will erase its value
confirm_body_html:
"<p>By deleting this account, you will erase its value
history, affecting various aspects of your overall account. This action
will have a direct impact on your net worth calculations and the account
graphs.</p><br /> <p>After deletion, there is no way you'll be able to restore
Expand Down Expand Up @@ -163,5 +166,6 @@ en:
email_confirmations:
new:
invalid_token: Invalid or expired confirmation link.
success_login: Your email has been confirmed. Please log in with your new email
success_login:
Your email has been confirmed. Please log in with your new email
address.
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@
post :sync
get :sparkline
patch :toggle_active
patch :toggle_category_matcher
patch :set_default
patch :remove_default
get :select_provider
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddEnableCategoryMatcherToAccounts < ActiveRecord::Migration[7.2]
def change
add_column :accounts, :enable_category_matcher, :boolean, default: true, null: false
end
end
1 change: 1 addition & 0 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 31 additions & 0 deletions test/models/plaid_entry/processor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,35 @@ class PlaidEntry::ProcessorTest < ActiveSupport::TestCase
assert_equal "Amazon", entry.name
assert_equal categories(:food_and_drink).id, entry.transaction.category_id
end

test "skips category matcher when account.enable_category_matcher is false" do
@plaid_account.current_account.update!(enable_category_matcher: false)

plaid_transaction = {
"transaction_id" => "456",
"merchant_name" => "Amazon",
"amount" => 100,
"date" => Date.current,
"iso_currency_code" => "USD",
"personal_finance_category" => {
"detailed" => "Food"
},
"merchant_entity_id" => "456"
}

@category_matcher.expects(:match).never

processor = PlaidEntry::Processor.new(
plaid_transaction,
plaid_account: @plaid_account,
category_matcher: @category_matcher
)

assert_difference [ "Entry.count", "Transaction.count" ], 1 do
processor.process
end

entry = Entry.order(created_at: :desc).first
assert_nil entry.transaction.category_id
end
end
2 changes: 1 addition & 1 deletion test/system/accounts_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def assert_account_created(accountable_type, &block)
fill_in "Institution domain", with: updated_institution_domain
fill_in "Notes", with: updated_notes
click_button "Update Account"
assert_selector "h2", text: "Updated account name"
assert_selector "h3", text: "Updated account name"

created_account.reload
assert_equal updated_institution_name, created_account[:institution_name]
Expand Down
Loading