From 9dccbec67236b116434f92c5c566491bef6f4492 Mon Sep 17 00:00:00 2001 From: Oleksandr Chebotarov Date: Fri, 10 Jan 2025 11:42:12 +0200 Subject: [PATCH 1/4] Add invoices amount filtering to the API, graphql and export --- app/controllers/api/v1/invoices_controller.rb | 2 + app/graphql/resolvers/invoices_resolver.rb | 6 +++ .../data_exports/invoices/filters_input.rb | 2 + app/queries/invoices_query.rb | 7 +++ schema.graphql | 4 ++ schema.json | 48 +++++++++++++++++++ .../data_exports/invoices/create_spec.rb | 2 + .../resolvers/invoices_resolver_spec.rb | 38 +++++++++++++++ .../invoices/filters_input_spec.rb | 2 + spec/queries/invoices_query_spec.rb | 40 ++++++++++++++++ .../api/v1/invoices_controller_spec.rb | 22 +++++++++ 11 files changed, 173 insertions(+) diff --git a/app/controllers/api/v1/invoices_controller.rb b/app/controllers/api/v1/invoices_controller.rb index a203051cdd3..b0070a06020 100644 --- a/app/controllers/api/v1/invoices_controller.rb +++ b/app/controllers/api/v1/invoices_controller.rb @@ -51,6 +51,8 @@ def index }, search_term: params[:search_term], filters: { + amount_from: params[:amount_from], + amount_to: params[:amount_to], payment_status: (params[:payment_status] if valid_payment_status?(params[:payment_status])), payment_dispute_lost: params[:payment_dispute_lost], payment_overdue: (params[:payment_overdue] if %w[true false].include?(params[:payment_overdue])), diff --git a/app/graphql/resolvers/invoices_resolver.rb b/app/graphql/resolvers/invoices_resolver.rb index bab1a53c1c5..a6a235e1d4a 100644 --- a/app/graphql/resolvers/invoices_resolver.rb +++ b/app/graphql/resolvers/invoices_resolver.rb @@ -9,6 +9,8 @@ class InvoicesResolver < Resolvers::BaseResolver description 'Query invoices' + argument :amount_from, Integer, required: false + argument :amount_to, Integer, required: false argument :currency, Types::CurrencyEnum, required: false argument :customer_external_id, String, required: false argument :customer_id, ID, required: false, description: 'Uniq ID of the customer' @@ -26,6 +28,8 @@ class InvoicesResolver < Resolvers::BaseResolver type Types::Invoices::Object.collection_type, null: false def resolve( # rubocop:disable Metrics/ParameterLists + amount_from: nil, + amount_to: nil, currency: nil, customer_external_id: nil, customer_id: nil, @@ -45,6 +49,8 @@ def resolve( # rubocop:disable Metrics/ParameterLists pagination: {page:, limit:}, search_term:, filters: { + amount_from:, + amount_to:, payment_status:, payment_dispute_lost:, payment_overdue:, diff --git a/app/graphql/types/data_exports/invoices/filters_input.rb b/app/graphql/types/data_exports/invoices/filters_input.rb index 06555552ea8..08ee6183201 100644 --- a/app/graphql/types/data_exports/invoices/filters_input.rb +++ b/app/graphql/types/data_exports/invoices/filters_input.rb @@ -7,6 +7,8 @@ class FiltersInput < BaseInputObject graphql_name 'DataExportInvoiceFiltersInput' description 'Export Invoices search query and filters input argument' + argument :amount_from, Integer, required: false + argument :amount_to, Integer, required: false argument :currency, Types::CurrencyEnum, required: false argument :customer_external_id, String, required: false argument :invoice_type, [Types::Invoices::InvoiceTypeEnum], required: false diff --git a/app/queries/invoices_query.rb b/app/queries/invoices_query.rb index 99ad557a36c..6954d4e0eb2 100644 --- a/app/queries/invoices_query.rb +++ b/app/queries/invoices_query.rb @@ -18,6 +18,7 @@ def call invoices = with_payment_status(invoices) if filters.payment_status.present? invoices = with_payment_dispute_lost(invoices) unless filters.payment_dispute_lost.nil? invoices = with_payment_overdue(invoices) unless filters.payment_overdue.nil? + invoices = with_amount_range(invoices) if filters.amount_from.present? || filters.amount_to.present? result.invoices = invoices result @@ -92,6 +93,12 @@ def with_issuing_date_range(scope) scope end + def with_amount_range(scope) + scope = scope.where("invoices.total_amount_cents >= ?", filters.amount_from) if filters.amount_from + scope = scope.where("invoices.total_amount_cents <= ?", filters.amount_to) if filters.amount_to + scope + end + def issuing_date_from @issuing_date_from ||= parse_datetime_filter(:issuing_date_from) end diff --git a/schema.graphql b/schema.graphql index aaa04f430e3..a4b81474c5f 100644 --- a/schema.graphql +++ b/schema.graphql @@ -3507,6 +3507,8 @@ enum DataExportFormatTypeEnum { Export Invoices search query and filters input argument """ input DataExportInvoiceFiltersInput { + amountFrom: Int + amountTo: Int currency: CurrencyEnum customerExternalId: String invoiceType: [InvoiceTypeEnum!] @@ -6771,6 +6773,8 @@ type Query { Query invoices """ invoices( + amountFrom: Int + amountTo: Int currency: CurrencyEnum customerExternalId: String diff --git a/schema.json b/schema.json index d7effd74a2e..f453990baaf 100644 --- a/schema.json +++ b/schema.json @@ -14936,6 +14936,30 @@ "possibleTypes": null, "fields": null, "inputFields": [ + { + "name": "amountFrom", + "description": null, + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "amountTo", + "description": null, + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "currency", "description": null, @@ -32931,6 +32955,30 @@ "isDeprecated": false, "deprecationReason": null, "args": [ + { + "name": "amountFrom", + "description": null, + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, + { + "name": "amountTo", + "description": null, + "type": { + "kind": "SCALAR", + "name": "Int", + "ofType": null + }, + "defaultValue": null, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "currency", "description": null, diff --git a/spec/graphql/mutations/data_exports/invoices/create_spec.rb b/spec/graphql/mutations/data_exports/invoices/create_spec.rb index ead616cabf7..1ecdcd2c95f 100644 --- a/spec/graphql/mutations/data_exports/invoices/create_spec.rb +++ b/spec/graphql/mutations/data_exports/invoices/create_spec.rb @@ -35,6 +35,8 @@ format: 'csv', resourceType: 'invoices', filters: { + amountFrom: 0, + amountTo: 10000, currency: 'USD', customerExternalId: 'abc123', invoiceType: ['one_off'], diff --git a/spec/graphql/resolvers/invoices_resolver_spec.rb b/spec/graphql/resolvers/invoices_resolver_spec.rb index 7d5b698cfd2..63aec0e7f54 100644 --- a/spec/graphql/resolvers/invoices_resolver_spec.rb +++ b/spec/graphql/resolvers/invoices_resolver_spec.rb @@ -365,4 +365,42 @@ end end end + + context 'with both amount_from and amount_to' do + subject(:result) do + execute_graphql( + current_user: membership.user, + current_organization: organization, + permissions: required_permission, + query: + ) + end + + let(:query) do + <<~GQL + query { + invoices( + limit: 5, + amountFrom: #{invoices.second.total_amount_cents}, + amountTo: #{invoices.fourth.total_amount_cents} + ) { + collection { id } + metadata { currentPage, totalCount } + } + } + GQL + end + + let!(:invoices) do + (1..5).to_a.map do |i| + create(:invoice, total_amount_cents: i.succ * 1_000, organization:) + end # from smallest to biggest + end + + it 'returns visible invoices total cents amount in provided range' do + collection = result['data']['invoices']['collection'] + + expect(collection.pluck('id')).to match_array invoices[1..3].pluck(:id) + end + end end diff --git a/spec/graphql/types/data_exports/invoices/filters_input_spec.rb b/spec/graphql/types/data_exports/invoices/filters_input_spec.rb index 39949f0f950..9429d563c4b 100644 --- a/spec/graphql/types/data_exports/invoices/filters_input_spec.rb +++ b/spec/graphql/types/data_exports/invoices/filters_input_spec.rb @@ -5,6 +5,8 @@ RSpec.describe Types::DataExports::Invoices::FiltersInput do subject { described_class } + it { is_expected.to accept_argument(:amount_from).of_type('Int') } + it { is_expected.to accept_argument(:amount_to).of_type('Int') } it { is_expected.to accept_argument(:currency).of_type('CurrencyEnum') } it { is_expected.to accept_argument(:customer_external_id).of_type('String') } it { is_expected.to accept_argument(:invoice_type).of_type('[InvoiceTypeEnum!]') } diff --git a/spec/queries/invoices_query_spec.rb b/spec/queries/invoices_query_spec.rb index 4c5d0d8ebe1..b059fb375c4 100644 --- a/spec/queries/invoices_query_spec.rb +++ b/spec/queries/invoices_query_spec.rb @@ -504,4 +504,44 @@ end end end + + context "when amount filters applied" do + let(:filters) { {amount_from:, amount_to:} } + + let!(:invoices) do + (1..5).to_a.map do |i| + create(:invoice, total_amount_cents: i.succ * 1_000, organization:) + end # from smallest to biggest + end + + context "when only amount from provided" do + let(:amount_from) { invoices.second.total_amount_cents } + let(:amount_to) { nil } + + it "returns invoices with total cents amount bigger or equal to provided value" do + expect(result).to be_success + expect(result.invoices.pluck(:id)).to match_array invoices[1..].pluck(:id) + end + end + + context "when only amount to provided" do + let(:amount_from) { 100 } + let(:amount_to) { invoices.fourth.total_amount_cents } + + it "returns invoices with total cents amount lower or equal to provided value" do + expect(result).to be_success + expect(result.invoices.pluck(:id)).to match_array invoices[..3].pluck(:id) + end + end + + context "when both amount from and amount to provided" do + let(:amount_from) { invoices.second.total_amount_cents } + let(:amount_to) { invoices.fourth.total_amount_cents } + + it "returns invoices with total cents amount in provided range" do + expect(result).to be_success + expect(result.invoices.pluck(:id)).to match_array invoices[1..3].pluck(:id) + end + end + end end diff --git a/spec/requests/api/v1/invoices_controller_spec.rb b/spec/requests/api/v1/invoices_controller_spec.rb index e781277fc37..f6ebfe6bcf0 100644 --- a/spec/requests/api/v1/invoices_controller_spec.rb +++ b/spec/requests/api/v1/invoices_controller_spec.rb @@ -482,6 +482,28 @@ expect(json[:invoices].first[:lago_id]).to eq(matching_invoice.id) end end + + context 'with amount filters' do + let(:params) do + { + amount_from: invoices.second.total_amount_cents, + amount_to: invoices.fourth.total_amount_cents + } + end + + let!(:invoices) do + (1..5).to_a.map do |i| + create(:invoice, total_amount_cents: i.succ * 1_000, organization:) + end # from smallest to biggest + end + + it 'returns invoices with total cents amount in provided range' do + subject + + expect(response).to have_http_status(:success) + expect(json[:invoices].pluck(:lago_id)).to match_array invoices[1..3].pluck(:id) + end + end end describe 'PUT /api/v1/invoices/:id/refresh' do From 14c174394ef68b62503ede17ba0e83f10917436f Mon Sep 17 00:00:00 2001 From: Oleksandr Chebotarov Date: Fri, 10 Jan 2025 14:46:39 +0200 Subject: [PATCH 2/4] Add invoices metadata filtering to the API --- app/controllers/api/v1/invoices_controller.rb | 3 +- app/queries/invoices_query.rb | 15 ++++++ spec/factories/invoice_metadata.rb | 4 +- spec/queries/invoices_query_spec.rb | 54 +++++++++++++++++++ .../api/v1/invoices_controller_spec.rb | 26 +++++++++ 5 files changed, 99 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/invoices_controller.rb b/app/controllers/api/v1/invoices_controller.rb index b0070a06020..de491c3279d 100644 --- a/app/controllers/api/v1/invoices_controller.rb +++ b/app/controllers/api/v1/invoices_controller.rb @@ -61,7 +61,8 @@ def index customer_external_id: params[:external_customer_id], invoice_type: params[:invoice_type], issuing_date_from: (Date.strptime(params[:issuing_date_from]) if valid_date?(params[:issuing_date_from])), - issuing_date_to: (Date.strptime(params[:issuing_date_to]) if valid_date?(params[:issuing_date_to])) + issuing_date_to: (Date.strptime(params[:issuing_date_to]) if valid_date?(params[:issuing_date_to])), + metadata: params[:metadata]&.permit!.to_h } ) diff --git a/app/queries/invoices_query.rb b/app/queries/invoices_query.rb index 6954d4e0eb2..103b31c8d4a 100644 --- a/app/queries/invoices_query.rb +++ b/app/queries/invoices_query.rb @@ -19,6 +19,7 @@ def call invoices = with_payment_dispute_lost(invoices) unless filters.payment_dispute_lost.nil? invoices = with_payment_overdue(invoices) unless filters.payment_overdue.nil? invoices = with_amount_range(invoices) if filters.amount_from.present? || filters.amount_to.present? + invoices = with_metadata(invoices) if filters.metadata.present? result.invoices = invoices result @@ -99,6 +100,20 @@ def with_amount_range(scope) scope end + def with_metadata(scope) + subquery = scope.joins(:metadata) + + filters.metadata.each do |key, value| + subquery = subquery.or(subquery.where(metadata: {key:, value:})) + end + + subquery = subquery + .group("invoices.id") + .having("COUNT(DISTINCT metadata.key) = ?", filters.metadata.size) + + scope.where(id: subquery.select(:id)) + end + def issuing_date_from @issuing_date_from ||= parse_datetime_filter(:issuing_date_from) end diff --git a/spec/factories/invoice_metadata.rb b/spec/factories/invoice_metadata.rb index 626833a4576..15207d2d641 100644 --- a/spec/factories/invoice_metadata.rb +++ b/spec/factories/invoice_metadata.rb @@ -4,7 +4,7 @@ factory :invoice_metadata, class: 'Metadata::InvoiceMetadata' do invoice - key { 'lead_name' } - value { 'John Doe' } + key { Faker::Commerce.color } + value { rand(100) } end end diff --git a/spec/queries/invoices_query_spec.rb b/spec/queries/invoices_query_spec.rb index b059fb375c4..42dd0bf4180 100644 --- a/spec/queries/invoices_query_spec.rb +++ b/spec/queries/invoices_query_spec.rb @@ -544,4 +544,58 @@ end end end + + context "when metadata filters applied" do + let(:filters) do + { + metadata: matching_invoice.metadata.to_h { |item| [item.key, item.value] } + } + end + + let!(:matching_invoice) { create(:invoice, organization:) } + + before do + metadata = create_pair(:invoice_metadata, invoice: matching_invoice) + + create(:invoice, organization:) do |invoice| + create(:invoice_metadata, invoice:, key: metadata.first.key, value: metadata.first.value) + end + end + + it "returns invoices with matching metadata filters" do + expect(result).to be_success + expect(result.invoices.pluck(:id)).to contain_exactly matching_invoice.id + end + end + + context "with multiple filters applied at the same time" do + let(:search_term) { invoice.number.first(5) } + + let(:filters) do + { + currency: invoice.currency, + customer_external_id: invoice.customer.external_id, + customer_id: invoice.customer.id, + invoice_type: invoice.invoice_type, + issuing_date_from: invoice.issuing_date, + issuing_date_to: invoice.issuing_date, + status: invoice.status, + payment_status: invoice.payment_status, + payment_dispute_lost: invoice.payment_dispute_lost_at.present?, + payment_overdue: invoice.payment_overdue, + amount_from: invoice.total_amount_cents, + amount_to: invoice.total_amount_cents, + metadata: invoice.metadata.to_h { |item| [item.key, item.value] } + } + end + + let!(:invoice) { create(:invoice, currency: "EUR", organization:) } + + before { create(:invoice, currency: "USD", organization:) } + + it "returns invoices matching all provided filters" do + expect(result).to be_success + expect(result.invoices.pluck(:id)).to contain_exactly invoice.id + end + end end diff --git a/spec/requests/api/v1/invoices_controller_spec.rb b/spec/requests/api/v1/invoices_controller_spec.rb index f6ebfe6bcf0..067e7393fe0 100644 --- a/spec/requests/api/v1/invoices_controller_spec.rb +++ b/spec/requests/api/v1/invoices_controller_spec.rb @@ -504,6 +504,32 @@ expect(json[:invoices].pluck(:lago_id)).to match_array invoices[1..3].pluck(:id) end end + + context 'with metadata filters' do + let(:params) do + metadata = matching_invoice.metadata.first + + { + metadata: { + metadata.key => metadata.value + } + } + end + + let(:matching_invoice) { create(:invoice, organization:) } + + before do + create(:invoice_metadata, invoice: matching_invoice) + create(:invoice, organization:) + end + + it 'returns invoices with matching metadata filters' do + subject + + expect(response).to have_http_status(:success) + expect(json[:invoices].pluck(:lago_id)).to contain_exactly matching_invoice.id + end + end end describe 'PUT /api/v1/invoices/:id/refresh' do From d8954565dbb07a3e3da584da6c4754247b407166 Mon Sep 17 00:00:00 2001 From: Oleksandr Chebotarov Date: Fri, 10 Jan 2025 17:01:15 +0200 Subject: [PATCH 3/4] Fix bug with invalid filtering if only one pair of metadata filters provided --- app/queries/invoices_query.rb | 13 ++++--- spec/queries/invoices_query_spec.rb | 54 ++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/app/queries/invoices_query.rb b/app/queries/invoices_query.rb index 103b31c8d4a..757cfea9ba1 100644 --- a/app/queries/invoices_query.rb +++ b/app/queries/invoices_query.rb @@ -101,10 +101,15 @@ def with_amount_range(scope) end def with_metadata(scope) - subquery = scope.joins(:metadata) - - filters.metadata.each do |key, value| - subquery = subquery.or(subquery.where(metadata: {key:, value:})) + base_scope = scope.joins(:metadata) + subquery = base_scope + + filters.metadata.each_with_index do |(key, value), index| + if index.zero? + subquery = base_scope.where(metadata: {key:, value:}) + else + subquery = subquery.or(base_scope.where(metadata: {key:, value:})) + end end subquery = subquery diff --git a/spec/queries/invoices_query_spec.rb b/spec/queries/invoices_query_spec.rb index 42dd0bf4180..be4dd40aa87 100644 --- a/spec/queries/invoices_query_spec.rb +++ b/spec/queries/invoices_query_spec.rb @@ -546,25 +546,53 @@ end context "when metadata filters applied" do - let(:filters) do - { - metadata: matching_invoice.metadata.to_h { |item| [item.key, item.value] } - } - end + let(:filters) { {metadata:} } + + context "when single filter provided" do + let(:metadata) { {red: 5} } - let!(:matching_invoice) { create(:invoice, organization:) } + let!(:matching_invoice) { create(:invoice, organization:) } - before do - metadata = create_pair(:invoice_metadata, invoice: matching_invoice) + before do + create(:invoice_metadata, invoice: matching_invoice, key: :red, value: 5) - create(:invoice, organization:) do |invoice| - create(:invoice_metadata, invoice:, key: metadata.first.key, value: metadata.first.value) + create(:invoice, organization:) do |invoice| + create(:invoice_metadata, invoice:) + end + end + + it "returns invoices with matching metadata filters" do + expect(result).to be_success + expect(result.invoices.pluck(:id)).to contain_exactly matching_invoice.id end end - it "returns invoices with matching metadata filters" do - expect(result).to be_success - expect(result.invoices.pluck(:id)).to contain_exactly matching_invoice.id + context "when multiple filters provided" do + let(:metadata) do + { + red: 5, + orange: 3 + } + end + + let!(:matching_invoices) { create_pair(:invoice, organization:) } + + before do + matching_invoices.each do |invoice| + metadata.each do |key, value| + create(:invoice_metadata, invoice:, key:, value:) + end + end + + create(:invoice, organization:) do |invoice| + create(:invoice_metadata, invoice:, key: :red, value: 5) + end + end + + it "returns invoices with matching metadata filters" do + expect(result).to be_success + expect(result.invoices.pluck(:id)).to match_array matching_invoices.pluck(:id) + end end end From 467f16836e54779cf5e36c778f5def854a31bcc1 Mon Sep 17 00:00:00 2001 From: Oleksandr Chebotarov Date: Fri, 10 Jan 2025 17:54:15 +0200 Subject: [PATCH 4/4] Fix rubocop warning --- app/queries/invoices_query.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/queries/invoices_query.rb b/app/queries/invoices_query.rb index 757cfea9ba1..0568f289dc6 100644 --- a/app/queries/invoices_query.rb +++ b/app/queries/invoices_query.rb @@ -105,10 +105,10 @@ def with_metadata(scope) subquery = base_scope filters.metadata.each_with_index do |(key, value), index| - if index.zero? - subquery = base_scope.where(metadata: {key:, value:}) + subquery = if index.zero? + base_scope.where(metadata: {key:, value:}) else - subquery = subquery.or(base_scope.where(metadata: {key:, value:})) + subquery.or(base_scope.where(metadata: {key:, value:})) end end