Skip to content

Commit

Permalink
fix(export-invoices): allow no pagination for Query objects (#2307)
Browse files Browse the repository at this point in the history
## Roadmap Task

👉
https://getlago.canny.io/feature-requests/p/ability-to-export-data-from-the-user-interface

## Context

When exporting data (eg: invoices) we use query objects to fetch the
records to export. However, the current implementation of query objects
paginates to first page and limit results to 25 or 100 (depends on
implementation) when no pagination information is provided.

We need the query object to allow no pagination and fetch all resources
matching the search / filter criteria when pagination args are not provided.

## Description

This change allows client code to call Query object with `pagination: nil`
arg to avoid pagination defaults (page: 1, limit: 25).

Also, it makes `BaseQuery::Pagination` internal knowledge of the query
objects. As query objects expect pagination as a hash, for example: `{page: 1, limit:
50}`. The same will be applied to `BaseQuery::Filters` in a coming PR.

Updates the Query objects implementing the latest approach and updates
`InvoicesQuery` object to allow no pagination and fix the CSV data
export issues.
  • Loading branch information
ancorcruz authored Jul 19, 2024
1 parent a3b6e56 commit d2ff043
Show file tree
Hide file tree
Showing 24 changed files with 167 additions and 167 deletions.
4 changes: 2 additions & 2 deletions app/controllers/api/v1/applied_coupons_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ def create
def index
result = AppliedCouponsQuery.call(
organization: current_organization,
pagination: BaseQuery::Pagination.new(
pagination: {
page: params[:page],
limit: params[:per_page] || PER_PAGE
),
},
filters: BaseQuery::Filters.new(index_filters)
)

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/customers/usage_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ def current
def past
result = PastUsageQuery.call(
organization: current_organization,
pagination: BaseQuery::Pagination.new(
pagination: {
page: params[:page],
limit: params[:per_page] || PER_PAGE
),
},
filters: BaseQuery::Filters.new(past_usage_filters)
)

Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/fees_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ def update
def index
result = FeesQuery.call(
organization: current_organization,
pagination: BaseQuery::Pagination.new(
pagination: {
page: params[:page],
limit: params[:per_page] || PER_PAGE
),
},
filters: BaseQuery::Filters.new(index_filters)
)

Expand Down
10 changes: 7 additions & 3 deletions app/controllers/api/v1/invoices_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,13 @@ def show
end

def index
result = InvoicesQuery.new(organization: current_organization).call(
page: params[:page],
limit: params[:per_page] || PER_PAGE,
result = InvoicesQuery.new(
organization: current_organization,
pagination: {
page: params[:page],
limit: params[:per_page] || PER_PAGE
}
).call(
search_term: params[:search_term],
payment_status: (params[:payment_status] if valid_payment_status?(params[:payment_status])),
payment_dispute_lost: params[:payment_dispute_lost],
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/subscriptions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ def show
def index
result = SubscriptionsQuery.call(
organization: current_organization,
pagination: BaseQuery::Pagination.new(
pagination: {
page: params[:page],
limit: params[:per_page] || PER_PAGE
),
},
filters: BaseQuery::Filters.new(index_filters)
)

Expand Down
8 changes: 5 additions & 3 deletions app/graphql/resolvers/customer_portal/invoices_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@ class InvoicesResolver < Resolvers::BaseResolver
type Types::Invoices::Object.collection_type, null: false

def resolve(status: nil, page: nil, limit: nil, search_term: nil)
query = InvoicesQuery.new(organization: context[:customer_portal_user].organization)
query = InvoicesQuery.new(
organization: context[:customer_portal_user],
pagination: {page:, limit:}
)

result = query.call(
customer_id: context[:customer_portal_user].id,
search_term:,
page:,
limit:,
status:
)

Expand Down
4 changes: 1 addition & 3 deletions app/graphql/resolvers/customers/invoices_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@ class InvoicesResolver < Resolvers::BaseResolver
type Types::Invoices::Object.collection_type, null: false

def resolve(customer_id: nil, status: nil, page: nil, limit: nil, search_term: nil)
query = InvoicesQuery.new(organization: current_organization)
query = InvoicesQuery.new(organization: current_organization, pagination: {page:, limit:})
result = query.call(
search_term:,
customer_id:,
page:,
limit:,
status:
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class IntegrationCollectionMappingsResolver < Resolvers::BaseResolver
def resolve(page: nil, limit: nil, integration_id: nil, mapping_type: nil)
result = ::IntegrationCollectionMappingsQuery.call(
organization: current_organization,
pagination: BaseQuery::Pagination.new(page:, limit:),
pagination: {page:, limit:},
filters: BaseQuery::Filters.new({integration_id:, mapping_type:})
)

Expand Down
2 changes: 1 addition & 1 deletion app/graphql/resolvers/integration_mappings_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class IntegrationMappingsResolver < Resolvers::BaseResolver
def resolve(page: nil, limit: nil, integration_id: nil, mappable_type: nil)
result = ::IntegrationMappingsQuery.call(
organization: current_organization,
pagination: BaseQuery::Pagination.new(page:, limit:),
pagination: {page:, limit:},
filters: BaseQuery::Filters.new({integration_id:, mappable_type:})
)

Expand Down
4 changes: 1 addition & 3 deletions app/graphql/resolvers/invoices_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,9 @@ def resolve( # rubocop:disable Metrics/ParameterLists
payment_dispute_lost: nil,
payment_overdue: nil
)
query = InvoicesQuery.new(organization: current_organization)
query = InvoicesQuery.new(organization: current_organization, pagination: {page:, limit:})
result = query.call(
search_term:,
page:,
limit:,
payment_status:,
payment_dispute_lost:,
payment_overdue:,
Expand Down
2 changes: 1 addition & 1 deletion app/graphql/resolvers/subscriptions_resolver.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class SubscriptionsResolver < Resolvers::BaseResolver
def resolve(page: nil, limit: nil, plan_code: nil, status: nil)
result = SubscriptionsQuery.call(
organization: current_organization,
pagination: BaseQuery::Pagination.new(page:, limit:),
pagination: {page:, limit:},
filters: BaseQuery::Filters.new({plan_code:, status:})
)

Expand Down
26 changes: 17 additions & 9 deletions app/queries/base_query.rb
Original file line number Diff line number Diff line change
@@ -1,29 +1,37 @@
# frozen_string_literal: true

class BaseQuery < BaseService
PER_PAGE = 100
# nil values force Kaminari to apply its default values for page and limit.
DEFAULT_PAGINATION_PARAMS = {page: nil, limit: nil}

Pagination = Struct.new(:page, :limit, keyword_init: true) do
def initialize(page: 0, limit: PER_PAGE)
super
end
end
Pagination = Struct.new(:page, :limit, keyword_init: true)

class Filters < OpenStruct; end

def initialize(organization:, pagination: Pagination.new, filters: Filters.new)
def initialize(organization:, pagination: DEFAULT_PAGINATION_PARAMS, filters: Filters.new)
@organization = organization
@pagination = pagination
@pagination_params = pagination
@filters = filters

super
end

private

attr_reader :organization, :pagination, :filters
attr_reader :organization, :pagination_params, :filters

def pagination
return if pagination_params.blank?

@pagination ||= Pagination.new(
page: pagination_params[:page],
limit: pagination_params[:limit]
)
end

def paginate(scope)
return scope unless pagination

scope.page(pagination.page).per(pagination.limit)
end

Expand Down
5 changes: 3 additions & 2 deletions app/queries/invoices_query.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# frozen_string_literal: true

class InvoicesQuery < BaseQuery
def call(search_term:, status:, page:, limit:, filters: {}, customer_id: nil, payment_status: nil, payment_dispute_lost: nil, payment_overdue: nil) # rubocop:disable Metrics/ParameterLists
def call(search_term: nil, status: nil, filters: {}, customer_id: nil, payment_status: nil, payment_dispute_lost: nil, payment_overdue: nil) # rubocop:disable Metrics/ParameterLists
@search_term = search_term
@customer_id = customer_id
@filters = filters
Expand All @@ -16,7 +16,8 @@ def call(search_term:, status:, page:, limit:, filters: {}, customer_id: nil, pa
invoices = invoices.where(payment_status:) if payment_status.present?
invoices = invoices.where.not(payment_dispute_lost_at: nil) unless payment_dispute_lost.nil?
invoices = invoices.where(payment_overdue:) if payment_overdue.present?
invoices = invoices.order(issuing_date: :desc, created_at: :desc).page(page).per(limit)
invoices = invoices.order(issuing_date: :desc, created_at: :desc)
invoices = paginate(invoices)

result.invoices = invoices
result
Expand Down
12 changes: 7 additions & 5 deletions app/queries/past_usage_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ def call
end

# NOTE: Pagination attributes
result.current_page = query_result.current_page
result.next_page = query_result.next_page
result.prev_page = query_result.prev_page
result.total_pages = query_result.total_pages
result.total_count = query_result.total_count
if pagination
result.current_page = query_result.current_page
result.next_page = query_result.next_page
result.prev_page = query_result.prev_page
result.total_pages = query_result.total_pages
result.total_count = query_result.total_count
end

result
end
Expand Down
4 changes: 1 addition & 3 deletions app/services/data_exports/csv/invoices.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,9 @@ def query
status = resource_query["status"]
filters = resource_query.except("search_term", "status")

InvoicesQuery.new(organization: organization).call(
InvoicesQuery.new(organization: organization, pagination: nil).call(
search_term:,
status:,
page: nil,
limit: nil,
filters:
)
end
Expand Down
86 changes: 38 additions & 48 deletions spec/queries/applied_coupons_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
require 'rails_helper'

RSpec.describe AppliedCouponsQuery, type: :query do
subject(:applied_coupons_query) { described_class.new(organization:, pagination:, filters:) }
subject(:result) do
described_class.call(organization:, pagination:, filters:)
end

let(:organization) { create(:organization) }
let(:pagination) { BaseQuery::Pagination.new }
let(:pagination) { nil }
let(:filters) { BaseQuery::Filters.new(query_filters) }

let(:query_filters) { {} }
Expand All @@ -18,67 +20,55 @@

before { applied_coupon }

describe 'call' do
it 'returns a list of applied_coupons' do
result = applied_coupons_query.call

aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(1)
expect(result.applied_coupons).to eq([applied_coupon])
end
it 'returns a list of applied_coupons' do
aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(1)
expect(result.applied_coupons).to eq([applied_coupon])
end
end

context 'when customer is deleted' do
let(:customer) { create(:customer, :deleted, organization:) }

it 'filters the applied_coupons' do
result = applied_coupons_query.call
context 'when customer is deleted' do
let(:customer) { create(:customer, :deleted, organization:) }

aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(0)
end
it 'filters the applied_coupons' do
aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(0)
end
end
end

context 'with pagination' do
let(:pagination) { BaseQuery::Pagination.new(page: 2, limit: 10) }

it 'applies the pagination' do
result = applied_coupons_query.call
context 'with pagination' do
let(:pagination) { {page: 2, limit: 10} }

aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(0)
expect(result.applied_coupons.current_page).to eq(2)
end
it 'applies the pagination' do
aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(0)
expect(result.applied_coupons.current_page).to eq(2)
end
end
end

context 'with customer filter' do
let(:query_filters) { {external_customer_id: customer.external_id} }

it 'applies the filter' do
result = applied_coupons_query.call
context 'with customer filter' do
let(:query_filters) { {external_customer_id: customer.external_id} }

aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(1)
end
it 'applies the filter' do
aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(1)
end
end
end

context 'with status filter' do
let(:query_filters) { {status: 'terminated'} }

it 'applies the filter' do
result = applied_coupons_query.call
context 'with status filter' do
let(:query_filters) { {status: 'terminated'} }

aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(0)
end
it 'applies the filter' do
aggregate_failures do
expect(result).to be_success
expect(result.applied_coupons.count).to eq(0)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions spec/queries/fees_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
subject(:fees_query) { described_class.new(organization:, pagination:, filters:) }

let(:organization) { create(:organization) }
let(:pagination) { BaseQuery::Pagination.new }
let(:pagination) { nil }
let(:filters) { BaseQuery::Filters.new(query_filters) }

let(:query_filters) { {} }
Expand All @@ -29,7 +29,7 @@
end

context 'with pagination' do
let(:pagination) { BaseQuery::Pagination.new(page: 2, limit: 10) }
let(:pagination) { {page: 2, limit: 10} }

it 'applies the pagination' do
result = fees_query.call
Expand Down
2 changes: 1 addition & 1 deletion spec/queries/integration_collection_mappings_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
RSpec.describe IntegrationCollectionMappingsQuery, type: :query do
subject(:collection_mappings_query) { described_class.new(organization:, pagination:, filters:) }

let(:pagination) { BaseQuery::Pagination.new }
let(:pagination) { nil }
let(:filters) { BaseQuery::Filters.new(query_filters) }
let(:query_filters) { {} }
let(:membership) { create(:membership) }
Expand Down
2 changes: 1 addition & 1 deletion spec/queries/integration_mappings_query_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
RSpec.describe IntegrationMappingsQuery, type: :query do
subject(:integration_mappings_query) { described_class.new(organization:, pagination:, filters:) }

let(:pagination) { BaseQuery::Pagination.new }
let(:pagination) { nil }
let(:filters) { BaseQuery::Filters.new(query_filters) }
let(:query_filters) { {} }
let(:membership) { create(:membership) }
Expand Down
Loading

0 comments on commit d2ff043

Please sign in to comment.