From 53f4b32c330b107f5295105a78cb660b74a8deee Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 31 Jan 2025 17:04:26 -0500 Subject: [PATCH] Fix EU plaid flow (#1761) * Fix EU plaid flow * Fix failing tests --- .../concerns/accountable_resource.rb | 13 ++++++++-- .../controllers/plaid_controller.js | 20 +++++++------- app/models/account/balance_calculator.rb | 2 +- app/models/account/syncer.rb | 14 ++++++---- app/models/concerns/plaidable.rb | 19 ++++++++------ app/models/family.rb | 20 +++++++------- app/models/plaid_item.rb | 9 +++---- app/models/provider/plaid.rb | 25 +++++++----------- .../accounts/new/_method_selector.html.erb | 26 +++++++++---------- app/views/credit_cards/new.html.erb | 2 +- app/views/cryptos/new.html.erb | 2 +- app/views/depositories/new.html.erb | 2 +- .../confirmation_email.html.erb | 2 +- .../confirmation_email.text.erb | 2 +- app/views/investments/new.html.erb | 2 +- app/views/loans/new.html.erb | 2 +- app/views/settings/profiles/show.html.erb | 4 +-- .../plaid_items_controller_test.rb | 7 +++-- .../accountable_resource_interface_test.rb | 4 +-- 19 files changed, 91 insertions(+), 86 deletions(-) diff --git a/app/controllers/concerns/accountable_resource.rb b/app/controllers/concerns/accountable_resource.rb index f149adc3..675c3d93 100644 --- a/app/controllers/concerns/accountable_resource.rb +++ b/app/controllers/concerns/accountable_resource.rb @@ -52,12 +52,21 @@ module AccountableResource private def set_link_token - @link_token = Current.family.get_link_token( + @us_link_token = Current.family.get_link_token( webhooks_url: webhooks_url, redirect_url: accounts_url, accountable_type: accountable_type.name, - region: Current.family.country.to_s.downcase == "us" ? :us : :eu + region: :us ) + + if Current.family.eu? + @eu_link_token = Current.family.get_link_token( + webhooks_url: webhooks_url, + redirect_url: accounts_url, + accountable_type: accountable_type.name, + region: :eu + ) + end end def webhooks_url diff --git a/app/javascript/controllers/plaid_controller.js b/app/javascript/controllers/plaid_controller.js index 5f4f1d29..cf1f71cb 100644 --- a/app/javascript/controllers/plaid_controller.js +++ b/app/javascript/controllers/plaid_controller.js @@ -4,7 +4,7 @@ import { Controller } from "@hotwired/stimulus"; export default class extends Controller { static values = { linkToken: String, - region: { type: String, default: "us" } + region: { type: String, default: "us" }, }; open() { @@ -19,7 +19,7 @@ export default class extends Controller { handler.open(); } - handleSuccess(public_token, metadata) { + handleSuccess = (public_token, metadata) => { window.location.href = "/accounts"; fetch("/plaid_items", { @@ -32,7 +32,7 @@ export default class extends Controller { plaid_item: { public_token: public_token, metadata: metadata, - region: this.regionValue + region: this.regionValue, }, }), }).then((response) => { @@ -40,17 +40,17 @@ export default class extends Controller { window.location.href = response.url; } }); - } + }; - handleExit(err, metadata) { + handleExit = (err, metadata) => { // no-op - } + }; - handleEvent(eventName, metadata) { + handleEvent = (eventName, metadata) => { // no-op - } + }; - handleLoad() { + handleLoad = () => { // no-op - } + }; } diff --git a/app/models/account/balance_calculator.rb b/app/models/account/balance_calculator.rb index 55e094ed..d5b3b3d1 100644 --- a/app/models/account/balance_calculator.rb +++ b/app/models/account/balance_calculator.rb @@ -11,7 +11,7 @@ class Account::BalanceCalculator holdings_value = converted_holdings.select { |h| h.date == balance.date }.sum(&:amount) balance.balance = balance.balance + holdings_value balance - end + end.compact end private diff --git a/app/models/account/syncer.rb b/app/models/account/syncer.rb index d5a5ec84..37074aa7 100644 --- a/app/models/account/syncer.rb +++ b/app/models/account/syncer.rb @@ -76,29 +76,33 @@ class Account::Syncer exchange_rates = ExchangeRate.find_rates( from: from_currency, to: to_currency, - start_date: balances.first.date + start_date: balances.min_by(&:date).date ) converted_balances = balances.map do |balance| exchange_rate = exchange_rates.find { |er| er.date == balance.date } + next unless exchange_rate.present? + account.balances.build( date: balance.date, balance: exchange_rate.rate * balance.balance, currency: to_currency - ) if exchange_rate.present? - end + ) + end.compact converted_holdings = holdings.map do |holding| exchange_rate = exchange_rates.find { |er| er.date == holding.date } + next unless exchange_rate.present? + account.holdings.build( security: holding.security, date: holding.date, amount: exchange_rate.rate * holding.amount, currency: to_currency - ) if exchange_rate.present? - end + ) + end.compact Account.transaction do load_balances(converted_balances) diff --git a/app/models/concerns/plaidable.rb b/app/models/concerns/plaidable.rb index 838b4837..062eab8e 100644 --- a/app/models/concerns/plaidable.rb +++ b/app/models/concerns/plaidable.rb @@ -2,22 +2,25 @@ module Plaidable extend ActiveSupport::Concern class_methods do - def plaid_provider - Provider::Plaid.new if Rails.application.config.plaid + def plaid_us_provider + Provider::Plaid.new(Rails.application.config.plaid, :us) if Rails.application.config.plaid end def plaid_eu_provider - Provider::Plaid.new if Rails.application.config.plaid_eu + Provider::Plaid.new(Rails.application.config.plaid_eu, :eu) if Rails.application.config.plaid_eu end - def plaid_provider_for(plaid_item) - return nil unless plaid_item - plaid_item.eu? ? plaid_eu_provider : plaid_provider + def plaid_provider_for_region(region) + region.to_sym == :eu ? plaid_eu_provider : plaid_us_provider end end private - def plaid_provider_for(plaid_item) - self.class.plaid_provider_for(plaid_item) + def eu? + raise "eu? is not implemented for #{self.class.name}" + end + + def plaid_provider + eu? ? self.class.plaid_eu_provider : self.class.plaid_us_provider end end diff --git a/app/models/family.rb b/app/models/family.rb index 4b36fcd8..7ac41f25 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -1,4 +1,3 @@ -# rubocop:disable Layout/ElseAlignment, Layout/IndentationWidth class Family < ApplicationRecord include Plaidable, Syncable @@ -48,22 +47,22 @@ class Family < ApplicationRecord super || accounts.manual.any?(&:syncing?) || plaid_items.any?(&:syncing?) end - def get_link_token(webhooks_url:, redirect_url:, accountable_type: nil, region: :us) - provider = case region - when :eu - self.class.plaid_eu_provider - else - self.class.plaid_provider - end + def eu? + country != "US" && country != "CA" + end - return nil unless provider + def get_link_token(webhooks_url:, redirect_url:, accountable_type: nil, region: :us) + provider = if region.to_sym == :eu + self.class.plaid_eu_provider + else + self.class.plaid_us_provider + end provider.get_link_token( user_id: id, webhooks_url: webhooks_url, redirect_url: redirect_url, accountable_type: accountable_type, - eu: region == :eu ).link_token end @@ -238,4 +237,3 @@ class Family < ApplicationRecord ) end end -# rubocop:enable Layout/ElseAlignment, Layout/IndentationWidth diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb index a41d96a9..9ac1e002 100644 --- a/app/models/plaid_item.rb +++ b/app/models/plaid_item.rb @@ -21,8 +21,8 @@ class PlaidItem < ApplicationRecord scope :ordered, -> { order(created_at: :desc) } class << self - def create_from_public_token(token, item_name:, region: "us") - response = plaid_provider.exchange_public_token(token) + def create_from_public_token(token, item_name:, region:) + response = plaid_provider_for_region(region).exchange_public_token(token) new_plaid_item = create!( name: item_name, @@ -59,11 +59,10 @@ class PlaidItem < ApplicationRecord private def fetch_and_load_plaid_data data = {} - provider = plaid_provider_for(self) - item = provider.get_item(access_token).item + item = plaid_provider.get_item(access_token).item update!(available_products: item.available_products, billed_products: item.billed_products) - fetched_accounts = provider.get_item_accounts(self).accounts + fetched_accounts = plaid_provider.get_item_accounts(self).accounts data[:accounts] = fetched_accounts || [] internal_plaid_accounts = fetched_accounts.map do |account| diff --git a/app/models/provider/plaid.rb b/app/models/provider/plaid.rb index 813102a0..877a955f 100644 --- a/app/models/provider/plaid.rb +++ b/app/models/provider/plaid.rb @@ -1,5 +1,5 @@ class Provider::Plaid - attr_reader :client + attr_reader :client, :region MAYBE_SUPPORTED_PLAID_PRODUCTS = %w[transactions investments liabilities].freeze MAX_HISTORY_DAYS = Rails.env.development? ? 90 : 730 @@ -54,27 +54,22 @@ class Provider::Plaid actual_hash = Digest::SHA256.hexdigest(raw_body) raise JWT::VerificationError, "Invalid webhook body hash" unless ActiveSupport::SecurityUtils.secure_compare(expected_hash, actual_hash) end - - def client - api_client = Plaid::ApiClient.new( - Rails.application.config.plaid - ) - - Plaid::PlaidApi.new(api_client) - end end - def initialize - @client = self.class.client + def initialize(config, region) + @client = Plaid::PlaidApi.new( + Plaid::ApiClient.new(config) + ) + @region = region end - def get_link_token(user_id:, webhooks_url:, redirect_url:, accountable_type: nil, eu: false) + def get_link_token(user_id:, webhooks_url:, redirect_url:, accountable_type: nil) request = Plaid::LinkTokenCreateRequest.new({ user: { client_user_id: user_id }, client_name: "Maybe Finance", products: [ get_primary_product(accountable_type) ], additional_consented_products: get_additional_consented_products(accountable_type), - country_codes: get_country_codes(eu), + country_codes: country_codes, language: "en", webhook: webhooks_url, redirect_uri: redirect_url, @@ -199,8 +194,8 @@ class Provider::Plaid MAYBE_SUPPORTED_PLAID_PRODUCTS - [ get_primary_product(accountable_type) ] end - def get_country_codes(eu) - if eu + def country_codes + if region.to_sym == :eu [ "ES", "NL", "FR", "IE", "DE", "IT", "PL", "DK", "NO", "SE", "EE", "LT", "LV", "PT", "BE" ] # EU supported countries else [ "US", "CA" ] # US + CA only diff --git a/app/views/accounts/new/_method_selector.html.erb b/app/views/accounts/new/_method_selector.html.erb index 0ec22cfd..13834f09 100644 --- a/app/views/accounts/new/_method_selector.html.erb +++ b/app/views/accounts/new/_method_selector.html.erb @@ -1,4 +1,4 @@ -<%# locals: (path:, link_token: nil) %> +<%# locals: (path:, us_link_token: nil, eu_link_token: nil) %> <%= render layout: "accounts/new/container", locals: { title: t(".title"), back_path: new_account_path } do %>
@@ -9,24 +9,24 @@ <%= t("accounts.new.method_selector.manual_entry") %> <% end %> - <% if link_token.present? %> + <% if us_link_token %> <%# Default US-only Link %> - + <% end %> - <%# EU Link %> - <% unless Current.family.country == "US" %> - - <% end %> + <%# EU Link %> + <% if eu_link_token %> + <% end %>
<% end %> diff --git a/app/views/credit_cards/new.html.erb b/app/views/credit_cards/new.html.erb index 9297f2ae..5b35dce1 100644 --- a/app/views/credit_cards/new.html.erb +++ b/app/views/credit_cards/new.html.erb @@ -1,5 +1,5 @@ <% if params[:step] == "method_select" %> - <%= render "accounts/new/method_selector", path: new_credit_card_path(return_to: params[:return_to]), link_token: @link_token %> + <%= render "accounts/new/method_selector", path: new_credit_card_path(return_to: params[:return_to]), us_link_token: @us_link_token, eu_link_token: @eu_link_token %> <% else %> <%= modal_form_wrapper title: t(".title") do %> <%= render "credit_cards/form", account: @account, url: credit_cards_path %> diff --git a/app/views/cryptos/new.html.erb b/app/views/cryptos/new.html.erb index ff9e1d07..f4979c6e 100644 --- a/app/views/cryptos/new.html.erb +++ b/app/views/cryptos/new.html.erb @@ -1,5 +1,5 @@ <% if params[:step] == "method_select" %> - <%= render "accounts/new/method_selector", path: new_crypto_path(return_to: params[:return_to]), link_token: @link_token %> + <%= render "accounts/new/method_selector", path: new_crypto_path(return_to: params[:return_to]), us_link_token: @us_link_token, eu_link_token: @eu_link_token %> <% else %> <%= modal_form_wrapper title: t(".title") do %> <%= render "cryptos/form", account: @account, url: cryptos_path %> diff --git a/app/views/depositories/new.html.erb b/app/views/depositories/new.html.erb index 9247ca2c..3ed099b3 100644 --- a/app/views/depositories/new.html.erb +++ b/app/views/depositories/new.html.erb @@ -1,5 +1,5 @@ <% if params[:step] == "method_select" %> - <%= render "accounts/new/method_selector", path: new_depository_path(return_to: params[:return_to]), link_token: @link_token %> + <%= render "accounts/new/method_selector", path: new_depository_path(return_to: params[:return_to]), us_link_token: @us_link_token, eu_link_token: @eu_link_token %> <% else %> <%= modal_form_wrapper title: t(".title") do %> <%= render "depositories/form", account: @account, url: depositories_path %> diff --git a/app/views/email_confirmation_mailer/confirmation_email.html.erb b/app/views/email_confirmation_mailer/confirmation_email.html.erb index be87ccd0..1501a00e 100644 --- a/app/views/email_confirmation_mailer/confirmation_email.html.erb +++ b/app/views/email_confirmation_mailer/confirmation_email.html.erb @@ -4,4 +4,4 @@ <%= link_to @cta, @confirmation_url, class: "button" %> -

<%= t(".expiry_notice", hours: 24) %>

\ No newline at end of file +

<%= t(".expiry_notice", hours: 24) %>

diff --git a/app/views/email_confirmation_mailer/confirmation_email.text.erb b/app/views/email_confirmation_mailer/confirmation_email.text.erb index 8f5fa14b..e315baac 100644 --- a/app/views/email_confirmation_mailer/confirmation_email.text.erb +++ b/app/views/email_confirmation_mailer/confirmation_email.text.erb @@ -6,4 +6,4 @@ EmailConfirmation#confirmation_email <%= t(".cta") %>: <%= @confirmation_url %> -<%= t(".expiry_notice", hours: 24) %> \ No newline at end of file +<%= t(".expiry_notice", hours: 24) %> diff --git a/app/views/investments/new.html.erb b/app/views/investments/new.html.erb index 532a168c..92dc1da7 100644 --- a/app/views/investments/new.html.erb +++ b/app/views/investments/new.html.erb @@ -1,5 +1,5 @@ <% if params[:step] == "method_select" %> - <%= render "accounts/new/method_selector", path: new_investment_path(return_to: params[:return_to]), link_token: @link_token %> + <%= render "accounts/new/method_selector", path: new_investment_path(return_to: params[:return_to]), us_link_token: @us_link_token, eu_link_token: @eu_link_token %> <% else %> <%= modal_form_wrapper title: t(".title") do %> <%= render "investments/form", account: @account, url: investments_path %> diff --git a/app/views/loans/new.html.erb b/app/views/loans/new.html.erb index 110a68bd..b09a6b32 100644 --- a/app/views/loans/new.html.erb +++ b/app/views/loans/new.html.erb @@ -1,5 +1,5 @@ <% if params[:step] == "method_select" %> - <%= render "accounts/new/method_selector", path: new_loan_path(return_to: params[:return_to]), link_token: @link_token %> + <%= render "accounts/new/method_selector", path: new_loan_path(return_to: params[:return_to]), us_link_token: @us_link_token, eu_link_token: @eu_link_token %> <% else %> <%= modal_form_wrapper title: t(".title") do %> <%= render "loans/form", account: @account, url: loans_path %> diff --git a/app/views/settings/profiles/show.html.erb b/app/views/settings/profiles/show.html.erb index d0ffb6da..7fa30dd1 100644 --- a/app/views/settings/profiles/show.html.erb +++ b/app/views/settings/profiles/show.html.erb @@ -51,7 +51,7 @@ <% if Current.user.admin? && user != Current.user %>
- <%= button_to settings_profile_path(user_id: user), + <%= button_to settings_profile_path(user_id: user), method: :delete, class: "text-red-500 hover:text-red-700", data: { turbo_confirm: { @@ -101,7 +101,7 @@
<% end %> <% if Current.user.admin? %> - <%= button_to invitation_path(invitation), + <%= button_to invitation_path(invitation), method: :delete, class: "text-red-500 hover:text-red-700", data: { turbo_confirm: { diff --git a/test/controllers/plaid_items_controller_test.rb b/test/controllers/plaid_items_controller_test.rb index 78636757..7aede85c 100644 --- a/test/controllers/plaid_items_controller_test.rb +++ b/test/controllers/plaid_items_controller_test.rb @@ -4,13 +4,12 @@ require "ostruct" class PlaidItemsControllerTest < ActionDispatch::IntegrationTest setup do sign_in @user = users(:family_admin) - - @plaid_provider = mock - - PlaidItem.stubs(:plaid_provider).returns(@plaid_provider) end test "create" do + @plaid_provider = mock + PlaidItem.expects(:plaid_provider_for_region).with("us").returns(@plaid_provider) + public_token = "public-sandbox-1234" @plaid_provider.expects(:exchange_public_token).with(public_token).returns( diff --git a/test/interfaces/accountable_resource_interface_test.rb b/test/interfaces/accountable_resource_interface_test.rb index 913854a3..1a99d63e 100644 --- a/test/interfaces/accountable_resource_interface_test.rb +++ b/test/interfaces/accountable_resource_interface_test.rb @@ -4,9 +4,7 @@ module AccountableResourceInterfaceTest extend ActiveSupport::Testing::Declarative test "shows new form" do - Plaid::PlaidApi.any_instance.stubs(:link_token_create).returns( - Plaid::LinkTokenCreateResponse.new(link_token: "test-link-token") - ) + Family.any_instance.stubs(:get_link_token).returns("test-link-token") get new_polymorphic_url(@account.accountable) assert_response :success