From f3d17bfa913fb736724dc279cb8c0dbf3de9bfd6 Mon Sep 17 00:00:00 2001 From: Ivan Novosad Date: Fri, 19 Jul 2024 10:00:11 +0200 Subject: [PATCH] fix(xero): Fix syncing xero accounts (#2290) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Roadmap Task 👉 https://getlago.canny.io/feature-requests/p/integration-with-xero ## Context Currently Lago does not support accounting integrations ## Description This PR removes the accounts resolver and adds FetchAccounts mutation instead. --- .../integration_items/fetch_accounts.rb | 29 +++ .../integrations/accounts_resolver.rb | 26 -- app/graphql/types/mutation_type.rb | 1 + app/graphql/types/query_type.rb | 1 - app/models/integration_item.rb | 3 +- .../aggregator/accounts_service.rb | 47 +++- .../integrations/aggregator/sync_service.rb | 1 + schema.graphql | 38 +-- schema.json | 223 ++++++------------ .../integration_items/fetch_accounts_spec.rb | 65 +++++ .../integrations/accounts_resolver_spec.rb | 64 ----- spec/models/integration_item_spec.rb | 2 + .../aggregator/accounts_service_spec.rb | 9 +- 13 files changed, 246 insertions(+), 263 deletions(-) create mode 100644 app/graphql/mutations/integration_items/fetch_accounts.rb delete mode 100644 app/graphql/resolvers/integrations/accounts_resolver.rb create mode 100644 spec/graphql/mutations/integration_items/fetch_accounts_spec.rb delete mode 100644 spec/graphql/resolvers/integrations/accounts_resolver_spec.rb diff --git a/app/graphql/mutations/integration_items/fetch_accounts.rb b/app/graphql/mutations/integration_items/fetch_accounts.rb new file mode 100644 index 00000000000..d752a127013 --- /dev/null +++ b/app/graphql/mutations/integration_items/fetch_accounts.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Mutations + module IntegrationItems + class FetchAccounts < BaseMutation + include AuthenticableApiUser + include RequiredOrganization + + REQUIRED_PERMISSION = 'organization:integrations:update' + + graphql_name 'FetchIntegrationAccounts' + description 'Fetch integration accounts' + + argument :integration_id, ID, required: true + + type Types::IntegrationItems::Object.collection_type, null: false + + def resolve(**args) + integration = current_organization.integrations.find_by(id: args[:integration_id]) + + ::Integrations::Aggregator::SyncService.call(integration:, options: {only_accounts: true}) + + result = ::Integrations::Aggregator::AccountsService.call(integration:) + + result.success? ? result.accounts : result_error(result) + end + end + end +end diff --git a/app/graphql/resolvers/integrations/accounts_resolver.rb b/app/graphql/resolvers/integrations/accounts_resolver.rb deleted file mode 100644 index 1040a020cf0..00000000000 --- a/app/graphql/resolvers/integrations/accounts_resolver.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module Resolvers - module Integrations - class AccountsResolver < Resolvers::BaseResolver - include AuthenticableApiUser - include RequiredOrganization - - REQUIRED_PERMISSION = 'organization:integrations:view' - - description 'Query integration accounts' - - argument :integration_id, ID, required: false - - type Types::Integrations::Accounts::Object.collection_type, null: true - - def resolve(integration_id: nil) - integration = current_organization.integrations.find(integration_id) - - result = ::Integrations::Aggregator::AccountsService.call(integration:) - - result.accounts - end - end - end -end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index c6b6d340938..c7bb25c07cb 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -67,6 +67,7 @@ class MutationType < Types::BaseObject field :destroy_integration_collection_mapping, mutation: Mutations::IntegrationCollectionMappings::Destroy field :destroy_integration_mapping, mutation: Mutations::IntegrationMappings::Destroy + field :fetch_integration_accounts, mutation: Mutations::IntegrationItems::FetchAccounts field :fetch_integration_items, mutation: Mutations::IntegrationItems::FetchItems field :fetch_integration_tax_items, mutation: Mutations::IntegrationItems::FetchTaxItems diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 2af920930ef..4fac35695e7 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -32,7 +32,6 @@ class QueryType < Types::BaseObject field :google_auth_url, resolver: Resolvers::Auth::Google::AuthUrlResolver field :gross_revenues, resolver: Resolvers::Analytics::GrossRevenuesResolver field :integration, resolver: Resolvers::IntegrationResolver - field :integration_accounts, resolver: Resolvers::Integrations::AccountsResolver field :integration_collection_mapping, resolver: Resolvers::IntegrationCollectionMappingResolver field :integration_collection_mappings, resolver: Resolvers::IntegrationCollectionMappingsResolver field :integration_items, resolver: Resolvers::IntegrationItemsResolver diff --git a/app/models/integration_item.rb b/app/models/integration_item.rb index d7ac8c710b9..7bb1d98b3ec 100644 --- a/app/models/integration_item.rb +++ b/app/models/integration_item.rb @@ -7,7 +7,8 @@ class IntegrationItem < ApplicationRecord ITEM_TYPES = [ :standard, - :tax + :tax, + :account ].freeze enum item_type: ITEM_TYPES diff --git a/app/services/integrations/aggregator/accounts_service.rb b/app/services/integrations/aggregator/accounts_service.rb index 6b1f99d7bef..f603d325104 100644 --- a/app/services/integrations/aggregator/accounts_service.rb +++ b/app/services/integrations/aggregator/accounts_service.rb @@ -3,20 +3,38 @@ module Integrations module Aggregator class AccountsService < BaseService + LIMIT = 450 + MAX_SUBSEQUENT_REQUESTS = 15 + def action_path "v1/#{provider}/accounts" end def call - response = http_client.get(headers:) + @cursor = '' + @items = [] + + ActiveRecord::Base.transaction do + integration.integration_items.where(item_type: :account).destroy_all + + MAX_SUBSEQUENT_REQUESTS.times do |_i| + response = http_client.get(headers:, params:) - result.accounts = handle_accounts(response['records']) + handle_accounts(response['records']) + @cursor = response['next_cursor'] + + break if cursor.blank? + end + end + result.accounts = items result end private + attr_reader :cursor, :items + def headers { 'Connection-Id' => integration.connection_id, @@ -25,15 +43,28 @@ def headers } end - def handle_accounts(accounts) - accounts.map do |account| - OpenStruct.new( - external_id: account['id'], - external_account_code: account['code'], - external_name: account['name'] + def handle_accounts(new_items) + new_items.each do |item| + integration_item = IntegrationItem.new( + integration:, + external_id: item['id'], + external_account_code: item['code'], + external_name: item['name'], + item_type: :account ) + + integration_item.save! + + @items << integration_item end end + + def params + { + limit: LIMIT, + cursor: + } + end end end end diff --git a/app/services/integrations/aggregator/sync_service.rb b/app/services/integrations/aggregator/sync_service.rb index 1a264ef03fa..abac3172a6c 100644 --- a/app/services/integrations/aggregator/sync_service.rb +++ b/app/services/integrations/aggregator/sync_service.rb @@ -42,6 +42,7 @@ def sync_list return [list[:items]] if options[:only_items] return [list[:tax_items]] if options[:only_tax_items] + return [list[:accounts]] if options[:only_accounts] list.values end diff --git a/schema.graphql b/schema.graphql index 8ce01a9b859..826f86d30bb 100644 --- a/schema.graphql +++ b/schema.graphql @@ -15,17 +15,6 @@ input AcceptInviteInput { token: String! } -type Account { - externalAccountCode: String! - externalId: String! - externalName: String -} - -type AccountCollection { - collection: [Account!]! - metadata: CollectionMetadata! -} - """ Adyen input arguments """ @@ -3601,6 +3590,17 @@ input FetchDraftInvoiceTaxesInput { fees: [FeeInput!]! } +""" +Autogenerated input type of FetchIntegrationAccounts +""" +input FetchIntegrationAccountsInput { + """ + A unique identifier for the client performing the mutation. + """ + clientMutationId: String + integrationId: ID! +} + """ Autogenerated input type of FetchIntegrationItems """ @@ -3819,6 +3819,7 @@ type IntegrationItemCollection { } enum IntegrationItemTypeEnum { + account standard tax } @@ -4543,6 +4544,16 @@ type Mutation { input: FetchDraftInvoiceTaxesInput! ): AnrokFeeObjectCollection + """ + Fetch integration accounts + """ + fetchIntegrationAccounts( + """ + Parameters for FetchIntegrationAccounts + """ + input: FetchIntegrationAccountsInput! + ): IntegrationItemCollection! + """ Fetch integration items """ @@ -5561,11 +5572,6 @@ type Query { id: ID ): Integration - """ - Query integration accounts - """ - integrationAccounts(integrationId: ID): AccountCollection - """ Query a single integration collection mapping """ diff --git a/schema.json b/schema.json index 2e28668d324..7bc81525f94 100644 --- a/schema.json +++ b/schema.json @@ -80,126 +80,6 @@ ], "enumValues": null }, - { - "kind": "OBJECT", - "name": "Account", - "description": null, - "interfaces": [ - - ], - "possibleTypes": null, - "fields": [ - { - "name": "externalAccountCode", - "description": null, - "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "String", - "ofType": null - } - }, - "isDeprecated": false, - "deprecationReason": null, - "args": [ - - ] - }, - { - "name": "externalId", - "description": null, - "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "SCALAR", - "name": "String", - "ofType": null - } - }, - "isDeprecated": false, - "deprecationReason": null, - "args": [ - - ] - }, - { - "name": "externalName", - "description": null, - "type": { - "kind": "SCALAR", - "name": "String", - "ofType": null - }, - "isDeprecated": false, - "deprecationReason": null, - "args": [ - - ] - } - ], - "inputFields": null, - "enumValues": null - }, - { - "kind": "OBJECT", - "name": "AccountCollection", - "description": null, - "interfaces": [ - - ], - "possibleTypes": null, - "fields": [ - { - "name": "collection", - "description": null, - "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "LIST", - "name": null, - "ofType": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "OBJECT", - "name": "Account", - "ofType": null - } - } - } - }, - "isDeprecated": false, - "deprecationReason": null, - "args": [ - - ] - }, - { - "name": "metadata", - "description": null, - "type": { - "kind": "NON_NULL", - "name": null, - "ofType": { - "kind": "OBJECT", - "name": "CollectionMetadata", - "ofType": null - } - }, - "isDeprecated": false, - "deprecationReason": null, - "args": [ - - ] - } - ], - "inputFields": null, - "enumValues": null - }, { "kind": "INPUT_OBJECT", "name": "AddAdyenPaymentProviderInput", @@ -16350,6 +16230,45 @@ ], "enumValues": null }, + { + "kind": "INPUT_OBJECT", + "name": "FetchIntegrationAccountsInput", + "description": "Autogenerated input type of FetchIntegrationAccounts", + "interfaces": null, + "possibleTypes": null, + "fields": null, + "inputFields": [ + { + "name": "clientMutationId", + "description": "A unique identifier for the client performing the mutation.", + "type": { + "kind": "SCALAR", + "name": "String", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "integrationId", + "description": null, + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "SCALAR", + "name": "ID", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ], + "enumValues": null + }, { "kind": "INPUT_OBJECT", "name": "FetchIntegrationItemsInput", @@ -18064,6 +17983,12 @@ "description": null, "isDeprecated": false, "deprecationReason": null + }, + { + "name": "account", + "description": null, + "isDeprecated": false, + "deprecationReason": null } ] }, @@ -22318,6 +22243,39 @@ } ] }, + { + "name": "fetchIntegrationAccounts", + "description": "Fetch integration accounts", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "OBJECT", + "name": "IntegrationItemCollection", + "ofType": null + } + }, + "isDeprecated": false, + "deprecationReason": null, + "args": [ + { + "name": "input", + "description": "Parameters for FetchIntegrationAccounts", + "type": { + "kind": "NON_NULL", + "name": null, + "ofType": { + "kind": "INPUT_OBJECT", + "name": "FetchIntegrationAccountsInput", + "ofType": null + } + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + } + ] + }, { "name": "fetchIntegrationItems", "description": "Fetch integration items", @@ -28545,31 +28503,6 @@ } ] }, - { - "name": "integrationAccounts", - "description": "Query integration accounts", - "type": { - "kind": "OBJECT", - "name": "AccountCollection", - "ofType": null - }, - "isDeprecated": false, - "deprecationReason": null, - "args": [ - { - "name": "integrationId", - "description": null, - "type": { - "kind": "SCALAR", - "name": "ID", - "ofType": null - }, - "defaultValue": null, - "isDeprecated": false, - "deprecationReason": null - } - ] - }, { "name": "integrationCollectionMapping", "description": "Query a single integration collection mapping", diff --git a/spec/graphql/mutations/integration_items/fetch_accounts_spec.rb b/spec/graphql/mutations/integration_items/fetch_accounts_spec.rb new file mode 100644 index 00000000000..dc8bee5ea46 --- /dev/null +++ b/spec/graphql/mutations/integration_items/fetch_accounts_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Mutations::IntegrationItems::FetchAccounts, type: :graphql do + let(:required_permission) { 'organization:integrations:update' } + let(:membership) { create(:membership) } + let(:organization) { membership.organization } + let(:integration) { create(:netsuite_integration, organization:) } + let(:integration_item) { create(:integration_item, integration:) } + let(:sync_service) { instance_double(Integrations::Aggregator::SyncService) } + + let(:accounts_response) do + File.read(Rails.root.join('spec/fixtures/integration_aggregator/accounts_response.json')) + end + + let(:mutation) do + <<~GQL + mutation($input: FetchIntegrationAccountsInput!) { + fetchIntegrationAccounts(input: $input) { + collection { externalName, externalAccountCode, externalId } + } + } + GQL + end + + let(:account_ids) do + %w[12ec4c59-ad56-4a4f-93eb-fb0a7740f4e2 6317441d-6547-417c-89e2-6e43ece791d8 80701036-73b5-4468-a4b3-a139262035b4] + end + + before do + allow(Integrations::Aggregator::SyncService).to receive(:new).and_return(sync_service) + allow(sync_service).to receive(:call).and_return(true) + + stub_request(:get, 'https://api.nango.dev/v1/netsuite/accounts?cursor=&limit=450') + .to_return(status: 200, body: accounts_response, headers: {}) + + integration_item + end + + it_behaves_like 'requires current user' + it_behaves_like 'requires current organization' + it_behaves_like 'requires permission', 'organization:integrations:update' + + it 'fetches the integration accounts' do + result = execute_graphql( + current_user: membership.user, + current_organization: organization, + permissions: required_permission, + query: mutation, + variables: { + input: {integrationId: integration.id} + } + ) + + result_data = result['data']['fetchIntegrationAccounts'] + + ids = result_data['collection'].map { |value| value['externalId'] } + + aggregate_failures do + expect(ids).to eq(account_ids) + expect(integration.integration_items.where(item_type: :account).count).to eq(3) + end + end +end diff --git a/spec/graphql/resolvers/integrations/accounts_resolver_spec.rb b/spec/graphql/resolvers/integrations/accounts_resolver_spec.rb deleted file mode 100644 index 074484fd6e8..00000000000 --- a/spec/graphql/resolvers/integrations/accounts_resolver_spec.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Resolvers::Integrations::AccountsResolver, type: :graphql do - let(:required_permission) { 'organization:integrations:view' } - let(:query) do - <<~GQL - query($integrationId: ID!) { - integrationAccounts(integrationId: $integrationId) { - collection { externalAccountCode externalId externalName } - } - } - GQL - end - - let(:membership) { create(:membership) } - let(:organization) { membership.organization } - let(:integration) { create(:xero_integration, organization:) } - let(:lago_client) { instance_double(LagoHttpClient::Client) } - let(:accounts_endpoint) { 'https://api.nango.dev/v1/xero/accounts' } - - let(:headers) do - { - 'Connection-Id' => integration.connection_id, - 'Authorization' => "Bearer #{ENV["NANGO_SECRET_KEY"]}", - 'Provider-Config-Key' => 'xero' - } - end - - let(:aggregator_response) do - path = Rails.root.join('spec/fixtures/integration_aggregator/accounts_response.json') - JSON.parse(File.read(path)) - end - - before do - allow(LagoHttpClient::Client).to receive(:new).with(accounts_endpoint).and_return(lago_client) - allow(lago_client).to receive(:get).with(headers:).and_return(aggregator_response) - end - - it_behaves_like 'requires current user' - it_behaves_like 'requires current organization' - it_behaves_like 'requires permission', 'organization:integrations:view' - - it 'returns a list of accounts' do - result = execute_graphql( - current_user: membership.user, - current_organization: organization, - permissions: required_permission, - query:, - variables: {integrationId: integration.id} - ) - - accounts = result['data']['integrationAccounts'] - account = accounts['collection'].first - - aggregate_failures do - expect(accounts['collection'].count).to eq(3) - expect(account['externalAccountCode']).to eq('1111') - expect(account['externalId']).to eq('12ec4c59-ad56-4a4f-93eb-fb0a7740f4e2') - expect(account['externalName']).to eq('Accounts Payable') - end - end -end diff --git a/spec/models/integration_item_spec.rb b/spec/models/integration_item_spec.rb index 08b2df4aada..4317f7b7ead 100644 --- a/spec/models/integration_item_spec.rb +++ b/spec/models/integration_item_spec.rb @@ -9,6 +9,8 @@ it { is_expected.to validate_presence_of(:external_id) } + it { is_expected.to define_enum_for(:item_type).with_values(%i[standard tax account]) } + it 'validates uniqueness of external id' do expect(integration_item).to validate_uniqueness_of(:external_id).scoped_to([:integration_id, :item_type]) end diff --git a/spec/services/integrations/aggregator/accounts_service_spec.rb b/spec/services/integrations/aggregator/accounts_service_spec.rb index be675b4f8e2..bb3df0f36be 100644 --- a/spec/services/integrations/aggregator/accounts_service_spec.rb +++ b/spec/services/integrations/aggregator/accounts_service_spec.rb @@ -17,7 +17,12 @@ 'Provider-Config-Key' => 'netsuite-tba' } end - + let(:params) do + { + limit: 450, + cursor: '' + } + end let(:aggregator_response) do path = Rails.root.join('spec/fixtures/integration_aggregator/accounts_response.json') JSON.parse(File.read(path)) @@ -28,7 +33,7 @@ .with(accounts_endpoint) .and_return(lago_client) allow(lago_client).to receive(:get) - .with(headers:) + .with(headers:, params:) .and_return(aggregator_response) end