From cf68a86f7b3061efd46ae118db79344b7eb0b1fe Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sat, 21 Oct 2023 22:02:46 +0500 Subject: [PATCH 1/2] Optimize Pull Movements Logic (#39) --- app/jobs/pull_account_movements_job.rb | 2 +- app/services/open_banking_connector.rb | 31 +++++++++----- test/services/open_banking_connector_spec.rb | 44 ++++++++++++++++++-- 3 files changed, 62 insertions(+), 15 deletions(-) diff --git a/app/jobs/pull_account_movements_job.rb b/app/jobs/pull_account_movements_job.rb index ac8ea3c..ec1006f 100644 --- a/app/jobs/pull_account_movements_job.rb +++ b/app/jobs/pull_account_movements_job.rb @@ -6,7 +6,7 @@ def perform(account_id:) ## https://github.com/EduardoSimon/budget.io/issues/29 result = OpenBankingConnector.new.fetch_movements( - external_account_id: account.external_account_id + account: account ) Rails.logger.info({balance: result.balance}) diff --git a/app/services/open_banking_connector.rb b/app/services/open_banking_connector.rb index 11e4454..eab96c7 100644 --- a/app/services/open_banking_connector.rb +++ b/app/services/open_banking_connector.rb @@ -33,10 +33,11 @@ def fetch_session_result(id) AuthSession.new(id: id, url: response[:link], accounts: response[:accounts], status: :succeeded) end - def fetch_movements(external_account_id:) - account = client.account(external_account_id) + def fetch_movements(account:) + external_account_id = account.external_account_id + external_account = client.account(external_account_id) - balances_response = account.get_balances.deep_symbolize_keys + balances_response = external_account.get_balances.deep_symbolize_keys raise StandardError.new(balances_response.to_json) if balances_response[:status_code] && balances_response[:status_code] >= 400 @@ -54,14 +55,9 @@ def fetch_movements(external_account_id:) raise StandardError.new({message: "balances object was empty", params: main_balance}) if main_balance.nil? - seconds_in_day = 60 * 60 * 24 - previous_month = Time.now - (seconds_in_day * 30 * 4) - get_transactions_response = account.get_transactions( - date_from: previous_month.strftime("%F"), - date_to: Time.now.strftime("%F") - ).deep_symbolize_keys + transactions_response = get_transactions_response(external_account:, account:) - transactions = get_transactions_response[:transactions] + transactions = transactions_response[:transactions] raise StandardError.new({message: "No transactions object found"}) unless transactions booked_transactions = transactions[:booked] @@ -100,4 +96,19 @@ def auth_token_invalid? return true if @token_response.nil? @token_response[:last_fetch_timestamp] + @token_response.dig(:token_response, :access_expires) <= Time.now.to_i end + + def get_transactions_response(external_account:, account:) + last_movement_date = account + .movements + .order(:created_at) + .last&.created_at + + date_from = (last_movement_date || 4.months.ago).strftime("%F") + date_to = Time.now.strftime("%F") + + external_account.get_transactions( + date_from:, + date_to: + ).deep_symbolize_keys + end end diff --git a/test/services/open_banking_connector_spec.rb b/test/services/open_banking_connector_spec.rb index 5c39f4a..26676d4 100644 --- a/test/services/open_banking_connector_spec.rb +++ b/test/services/open_banking_connector_spec.rb @@ -6,6 +6,8 @@ class FetchMovementsTest < ActiveSupport::TestCase setup do @client_mock = Minitest::Mock.new @client_mock.expect(:generate_token, "wadus") + @account_instance = create(:account, external_account_id: "1") + @current_date = DateTime.now.strftime("%F") freeze_time end @@ -21,25 +23,59 @@ class FetchMovementsTest < ActiveSupport::TestCase ::Nordigen::NordigenClient.stub(:new, @client_mock) do exception = assert_raises(StandardError) do |e| - OpenBankingConnector.new.fetch_movements(external_account_id: "1") + OpenBankingConnector.new.fetch_movements(account: @account_instance) end assert_equal exception.message, error.to_json end end - test "returns the main balance of the given existent account as a float number, the currency and the transactions" do + test "When no movement is present, then returns the main balance of the given existent account as a float number, the currency and the transactions for last 4 months" do + date_from = 4.months.ago.strftime("%F") + date_to = @current_date + balances_response = {"balances" => [{"balanceAmount" => {"amount" => "896.13", "currency" => "EUR"}, "balanceType" => "information", "lastChangeDateTime" => "2023-10-12T00:00:00Z"}, {"balanceAmount" => {"amount" => "935.37", "currency" => "EUR"}, "balanceType" => "interimBooked", "lastChangeDateTime" => "2023-10-12T00:00:00Z"}]} + account_mock = Minitest::Mock.new + account_mock.expect(:get_balances, balances_response.deep_symbolize_keys) + account_mock.expect(:get_transactions, {"transactions" => {"booked" => MockClient::Stubs.transactions}}, date_from:, date_to:) + + @client_mock.expect(:account, account_mock, ["1"]) + + ::Nordigen::NordigenClient.stub(:new, @client_mock) do + response = OpenBankingConnector.new.fetch_movements(account: @account_instance) + + assert_equal @account_instance.movements.exists?, false + assert_equal response.balance, 896.13 + assert_equal response.currency, "EUR" + + assert_equal response.transactions.first[:id], 0 + assert_equal response.transactions.first[:amount], 100.0 + assert_equal response.transactions.first[:date], "2023-07-24" + assert_equal response.transactions.first[:description], "Pago en EL CORTE INGLES" + + assert_equal response.transactions.second[:id], 1 + assert_equal response.transactions.second[:amount], 200.0 + assert_equal response.transactions.second[:date], "2023-07-24" + assert_equal response.transactions.second[:description], "Pago en EL CORTE INGLES" + end + end + + test "When movement is present, then returns the main balance of the given existent account as a float number, the currency and the transactions from last moment onwards" do + create(:movement, account_id: @account_instance.id, created_at: 2.days.ago) + create(:movement, account_id: @account_instance.id, created_at: 1.days.ago) + last_movement_date = @account_instance.movements.last.created_at.strftime("%F") + balances_response = {"balances" => [{"balanceAmount" => {"amount" => "896.13", "currency" => "EUR"}, "balanceType" => "information", "lastChangeDateTime" => "2023-10-12T00:00:00Z"}, {"balanceAmount" => {"amount" => "935.37", "currency" => "EUR"}, "balanceType" => "interimBooked", "lastChangeDateTime" => "2023-10-12T00:00:00Z"}]} account_mock = Minitest::Mock.new account_mock.expect(:get_balances, balances_response.deep_symbolize_keys) - account_mock.expect(:get_transactions, {"transactions" => {"booked" => MockClient::Stubs.transactions}}, date_from: String, date_to: String) + account_mock.expect(:get_transactions, {"transactions" => {"booked" => MockClient::Stubs.transactions}}, date_from: last_movement_date, date_to: @current_date) @client_mock.expect(:account, account_mock, ["1"]) ::Nordigen::NordigenClient.stub(:new, @client_mock) do - response = OpenBankingConnector.new.fetch_movements(external_account_id: "1") + response = OpenBankingConnector.new.fetch_movements(account: @account_instance) + assert_equal @account_instance.movements.exists?, true assert_equal response.balance, 896.13 assert_equal response.currency, "EUR" From d468075293e321ed7ae729d298dfbd92ddefde9d Mon Sep 17 00:00:00 2001 From: Ahmed Ejaz Date: Sat, 21 Oct 2023 22:09:02 +0500 Subject: [PATCH 2/2] Fixes the Institutions migration isssue (#47) Resolves #36 This PR fixes the migration failure due to the invalid GoCardless credentials by the following changes: - Remove the migration in which we are creating the institutions - Add rake task to sync all the GoCardless institutions in the app - Update the README about the newly added rake task It's been observed that we are always having the failing Github tests workflow, that's because of the same issue. This PR will fix this workflow as well. --- README.md | 6 ++++- ...institutions_from_open_banking_provider.rb | 17 ------------ db/seeds.rb | 9 +++++-- lib/tasks/institutions_data.rake | 26 +++++++++++++++++++ 4 files changed, 38 insertions(+), 20 deletions(-) delete mode 100644 db/migrate/20221229143942_add_institutions_from_open_banking_provider.rb create mode 100644 lib/tasks/institutions_data.rake diff --git a/README.md b/README.md index 2576375..86138e0 100644 --- a/README.md +++ b/README.md @@ -29,11 +29,15 @@ Use a ruby dependency manager like rbenv or asdf. The supported ruby version is `make prepare` - 5. Run the server `bin/rails s` +## Useful Tasks +1. Sync the Institutions from GoCardless + +`bundle exec rake institutions_data:sync` + ## Installing git hooks You might install the provided git hooks to prevent committing changes that do not conform to the established style guide. diff --git a/db/migrate/20221229143942_add_institutions_from_open_banking_provider.rb b/db/migrate/20221229143942_add_institutions_from_open_banking_provider.rb deleted file mode 100644 index 3d9ad7c..0000000 --- a/db/migrate/20221229143942_add_institutions_from_open_banking_provider.rb +++ /dev/null @@ -1,17 +0,0 @@ -class AddInstitutionsFromOpenBankingProvider < ActiveRecord::Migration[7.0] - def up - institutions = OpenBankingConnector.new.fetch_institutions_by_country("ES") - - institutions.each do |institution| - Institution.create!( - institution_id: institution[:id], - name: institution[:name], - thumbnail_url: institution[:logo] - ) - end - end - - def down - Institution.destroy_all - end -end diff --git a/db/seeds.rb b/db/seeds.rb index a847ad4..fd21a12 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -12,10 +12,15 @@ budget = Budget.create!(title: "Test budget") +institution = Institution.create!( + institution_id: "04533925-cd07-473d-9b3b-f47a52184b14", + name: "Test institution" +) + account = Account.create!(name: "Test account", - institution: Institution.first, + institution: institution, budget: budget, - institution_id: Institution.first.id, + institution_id: institution.id, external_account_id: "48883f05-bfe1-46fb-818c-d272ace6a069") AuthSession.create!(account: account, diff --git a/lib/tasks/institutions_data.rake b/lib/tasks/institutions_data.rake new file mode 100644 index 0000000..6d62ab0 --- /dev/null +++ b/lib/tasks/institutions_data.rake @@ -0,0 +1,26 @@ +namespace :institutions_data do + desc "Sync all the institutions data from GoCardless" + task sync: :environment do + puts "Syncing institutions data..." + institutions = OpenBankingConnector.new.fetch_institutions_by_country("ES") + + institutions.each do |institution| + temp_institution = Institution.find_by(institution_id: institution[:id]) + + if temp_institution + temp_institution.update!( + name: institution[:name], + thumbnail_url: institution[:logo] + ) + else + Institution.create!( + institution_id: institution[:id], + name: institution[:name], + thumbnail_url: institution[:logo] + ) + end + end + + puts "Successfully synced #{institutions.count} institutions" + end +end