Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(anrok): make api calls async in dedicated service #3002

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/graphql/types/invoices/object.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class Object < Types::BaseObject
field :payment_dispute_lost_at, GraphQL::Types::ISO8601DateTime
field :payment_status, Types::Invoices::PaymentStatusTypeEnum, null: false
field :status, Types::Invoices::StatusTypeEnum, null: false
field :tax_status, Types::Invoices::TaxStatusTypeEnum, null: true
field :voidable, Boolean, null: false, method: :voidable?

field :currency, Types::CurrencyEnum
Expand Down
13 changes: 13 additions & 0 deletions app/graphql/types/invoices/tax_status_type_enum.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# frozen_string_literal: true

module Types
module Invoices
class TaxStatusTypeEnum < Types::BaseEnum
graphql_name 'InvoiceTaxStatusTypeEnum'

Invoice::TAX_STATUSES.keys.each do |type|
value type
end
end
end
end
11 changes: 0 additions & 11 deletions app/jobs/bill_subscription_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ def perform(subscriptions, timestamp, invoicing_reason:, invoice: nil, skip_char
skip_charges:
)
return if result.success?
# NOTE: We don't want a dead job for failed invoice due to the tax reason.
# This invoice should be in failed status and can be retried.
return if tax_error?(result)

# If the invoice was passed as an argument, it means the job was already retried (see end of function)
if invoice || !result.invoice&.generating?
Expand All @@ -41,12 +38,4 @@ def perform(subscriptions, timestamp, invoicing_reason:, invoice: nil, skip_char
skip_charges:
)
end

private

def tax_error?(result)
return false unless result.error.is_a?(BaseService::ValidationFailure)

result.error&.messages&.dig(:tax_error)&.present?
end
end
4 changes: 4 additions & 0 deletions app/jobs/fees/create_pay_in_advance_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

module Fees
class CreatePayInAdvanceJob < ApplicationJob
include ConcurrencyThrottlable

queue_as :default

retry_on BaseService::ThrottlingError, wait: :polynomially_longer, attempts: 25

unique :until_executed, on_conflict: :log

def perform(charge:, event:, billing_at: nil)
Expand Down
3 changes: 3 additions & 0 deletions app/jobs/invoices/create_pay_in_advance_charge_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

module Invoices
class CreatePayInAdvanceChargeJob < ApplicationJob
include ConcurrencyThrottlable

queue_as do
if ActiveModel::Type::Boolean.new.cast(ENV['SIDEKIQ_BILLING'])
:billing
Expand All @@ -11,6 +13,7 @@ class CreatePayInAdvanceChargeJob < ApplicationJob
end

retry_on Sequenced::SequenceError
retry_on BaseService::ThrottlingError, wait: :polynomially_longer, attempts: 25

unique :until_executed, on_conflict: :log

Expand Down
4 changes: 4 additions & 0 deletions app/jobs/invoices/provider_taxes/pull_taxes_and_apply_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,12 @@
module Invoices
module ProviderTaxes
class PullTaxesAndApplyJob < ApplicationJob
include ConcurrencyThrottlable

queue_as 'integrations'

retry_on BaseService::ThrottlingError, wait: :polynomially_longer, attempts: 25

def perform(invoice:)
Invoices::ProviderTaxes::PullTaxesAndApplyService.call(invoice:)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ def call
return result unless integration
return result unless integration.type == 'Integrations::AnrokIntegration'

throttle!(:anrok)

response = http_client.post_with_response(payload, headers)
body = JSON.parse(response.body)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ def call
return result unless integration
return result unless integration.type == 'Integrations::AnrokIntegration'

throttle!(:anrok)

response = http_client.post_with_response(payload, headers)
body = JSON.parse(response.body)

Expand Down
46 changes: 3 additions & 43 deletions app/services/invoices/calculate_fees_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,23 +47,10 @@ def call
Credits::ProgressiveBillingService.call(invoice:)
Credits::AppliedCouponsService.call(invoice:) if should_create_coupon_credit?

if customer_provider_taxation?
taxes_result = fetch_taxes_for_invoice
totals_result = Invoices::ComputeTaxesAndTotalsService.call(invoice:, finalizing: finalizing_invoice?)
return totals_result if !totals_result.success? && totals_result.error.is_a?(BaseService::UnknownTaxFailure)

unless taxes_result.success?
create_error_detail(taxes_result.error)

# only fail invoices that are finalizing
invoice.failed! if finalizing_invoice?

invoice.save!
return result.service_failure!(code: 'tax_error', message: taxes_result.error.code)
end

Invoices::ComputeAmountsFromFees.call(invoice:, provider_taxes: taxes_result.fees)
else
Invoices::ComputeAmountsFromFees.call(invoice:)
end
totals_result.raise_if_error!

create_credit_note_credit if should_create_credit_note_credit?
create_applied_prepaid_credit if should_create_applied_prepaid_credit?
Expand Down Expand Up @@ -337,33 +324,6 @@ def in_trial_period_not_ending_today?(subscription, timestamp)
timestamp.in_time_zone(tz).to_date != subscription.trial_end_datetime.in_time_zone(tz).to_date
end

def customer_provider_taxation?
@customer_provider_taxation ||= invoice.customer.anrok_customer
end

def create_error_detail(error)
error_result = ErrorDetails::CreateService.call(
owner: invoice,
organization: invoice.organization,
params: {
error_code: :tax_error,
details: {
tax_error: error.code
}.tap do |details|
details[:tax_error_message] = error.error_message if error.code == 'validationError'
end
}
)
error_result.raise_if_error!
end

def fetch_taxes_for_invoice
if finalizing_invoice?
return Integrations::Aggregator::Taxes::Invoices::CreateService.call(invoice:, fees: invoice.fees)
end
Integrations::Aggregator::Taxes::Invoices::CreateDraftService.call(invoice:, fees: invoice.fees)
end

def finalizing_invoice?
context == :finalize || Invoice::GENERATED_INVOICE_STATUSES.include?(invoice.status)
end
Expand Down
38 changes: 38 additions & 0 deletions app/services/invoices/compute_taxes_and_totals_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# frozen_string_literal: true

module Invoices
class ComputeTaxesAndTotalsService < BaseService
def initialize(invoice:, finalizing: true)
@invoice = invoice
@finalizing = finalizing

super
end

def call
return result.not_found_failure!(resource: 'invoice') unless invoice

if customer_provider_taxation?
invoice.status = 'pending' if finalizing
invoice.tax_status = 'pending'
invoice.save!
after_commit { Invoices::ProviderTaxes::PullTaxesAndApplyJob.perform_later(invoice:) }

return result.unknown_tax_failure!(code: 'tax_error', message: 'unknown taxes')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we always return a failure if customer is on anrok? Why?

Copy link
Collaborator Author

@lovrocolic lovrocolic Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This specific error is returned to indicate the path where invoice is missing taxes and is not complete, which is always the case for Anrok, at least until the Invoices::ProviderTaxes::PullTaxesAndApplyJob process the invoice

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but then this is the expected behavior not a failure, isn't it?

Copy link
Collaborator Author

@lovrocolic lovrocolic Dec 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invoice is not complete without taxes so that's why I wanted to clearly indicate this state in the service. Probably it is just style preference. Other option could be that I check wherever needed if invoice.tax_status = 'pending' or something similar, but I wanted to return different type of response in the service level:)

else
Invoices::ComputeAmountsFromFees.call(invoice:)
end

result.invoice = invoice
result
end

private

attr_reader :invoice, :finalizing

def customer_provider_taxation?
@customer_provider_taxation ||= invoice.customer.anrok_customer
end
end
end
35 changes: 3 additions & 32 deletions app/services/invoices/progressive_billing_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,10 @@ def call
Credits::ProgressiveBillingService.call(invoice:)
Credits::AppliedCouponsService.call(invoice:)

if customer_provider_taxation?
taxes_result = Integrations::Aggregator::Taxes::Invoices::CreateService.call(invoice:, fees: invoice.fees)
totals_result = Invoices::ComputeTaxesAndTotalsService.call(invoice:)
return totals_result if !totals_result.success? && totals_result.error.is_a?(BaseService::UnknownTaxFailure)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why we don't raise the error if it is an UnknownTaxFailure?

Maybe the naming of the failure is not the best as I would expect to raise unknown failures more than known ones...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe naming is confusing, but I couldn't find better one:) The idea is:

  • if unknown error happens, we don't want to raise error since it would rollback all DB changes
  • instead we just want to catch this state and leave invoice in pending status
  • Async job for fetching taxes will process pending invoice and put it either in finalized or failed (if there are tax errors like invalid product mapping)


unless taxes_result.success?
create_error_detail(taxes_result.error)
invoice.failed!

return result.service_failure!(code: 'tax_error', message: taxes_result.error.code)
end
Invoices::ComputeAmountsFromFees.call(invoice:, provider_taxes: taxes_result.fees)
else
Invoices::ComputeAmountsFromFees.call(invoice:)
end
totals_result.raise_if_error!

create_credit_note_credit
create_applied_prepaid_credit
Expand Down Expand Up @@ -149,25 +140,5 @@ def create_applied_prepaid_credit

invoice.total_amount_cents -= prepaid_credit_result.prepaid_credit_amount_cents
end

def customer_provider_taxation?
@customer_provider_taxation ||= invoice.customer.anrok_customer
end

def create_error_detail(error)
error_result = ErrorDetails::CreateService.call(
owner: invoice,
organization: invoice.organization,
params: {
error_code: :tax_error,
details: {
tax_error: error.code
}.tap do |details|
details[:tax_error_message] = error.error_message if error.code == 'validationError'
end
}
)
error_result.raise_if_error!
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,19 @@ def call
provider_taxes = taxes_result.fees

ActiveRecord::Base.transaction do
unless invoice.draft?
invoice.issuing_date = issuing_date
invoice.payment_due_date = payment_due_date
Comment on lines +38 to +39
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we expect to overwrite invoice issuing_date and payment_due_date if it its not in draft status? why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are right, draft invoice can already have this informations. However, when we are in the finalize mode, we need to overwrite those informations since finalization can happen also before original issuing date (if it is triggered manually)

end

Invoices::ComputeAmountsFromFees.call(invoice:, provider_taxes:)

create_credit_note_credit if should_create_credit_note_credit?
create_applied_prepaid_credit if should_create_applied_prepaid_credit?

invoice.payment_status = invoice.total_amount_cents.positive? ? :pending : :succeeded
invoice.tax_status = 'succeeded'
if invoice.draft?
invoice.status = :draft
else
Invoices::TransitionToFinalStatusService.call(invoice:)
end
Invoices::TransitionToFinalStatusService.call(invoice:) unless invoice.draft?

invoice.save!
invoice.reload
Expand Down Expand Up @@ -115,6 +116,14 @@ def create_applied_prepaid_credit
invoice.total_amount_cents -= prepaid_credit_result.prepaid_credit_amount_cents
end

def issuing_date
@issuing_date ||= Time.current.in_time_zone(customer.applicable_timezone).to_date
end

def payment_due_date
@payment_due_date ||= issuing_date + customer.applicable_net_payment_term.days
end

def customer
@customer ||= invoice.customer
end
Expand Down
8 changes: 1 addition & 7 deletions app/services/invoices/refresh_draft_and_finalize_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def call
ActiveRecord::Base.transaction do
invoice.issuing_date = issuing_date
refresh_result = Invoices::RefreshDraftService.call(invoice:, context: :finalize)
if tax_error?(refresh_result.error)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tax_error? method is not being called and can be deleted from this service.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, will remove it

if invoice.tax_pending?
invoice.update!(issuing_date: drafted_issuing_date)
return refresh_result
end
Expand Down Expand Up @@ -78,11 +78,5 @@ def should_deliver_email?
License.premium? &&
invoice.organization.email_settings.include?('invoice.finalized')
end

def tax_error?(error)
return false unless error.is_a?(BaseService::ValidationFailure)

error&.messages&.dig(:tax_error).present?
end
end
end
11 changes: 4 additions & 7 deletions app/services/invoices/refresh_draft_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ def call
CreditNotes::RefreshDraftService.call(credit_note:, fee:, old_fee_values:)
end

if tax_error?(calculate_result.error)
return result.validation_failure!(errors: {tax_error: [calculate_result.error.error_message]})
end
calculate_result.raise_if_error!
calculate_result.raise_if_error! unless tax_error?(calculate_result.error)

if old_total_amount_cents != invoice.total_amount_cents
flag_lifetime_usage_for_refresh
Expand All @@ -76,6 +73,8 @@ def call
# NOTE: In case of a refresh the same day of the termination.
invoice.fees.update_all(created_at: invoice.created_at) # rubocop:disable Rails/SkipsModelValidations

return result if tax_error?(calculate_result.error)

if invoice.should_update_hubspot_invoice?
Integrations::Aggregator::Invoices::Hubspot::UpdateJob.perform_later(invoice: invoice.reload)
end
Expand Down Expand Up @@ -110,9 +109,7 @@ def flag_lifetime_usage_for_refresh
end

def tax_error?(error)
return false unless error.is_a?(BaseService::ServiceFailure)

error&.code == 'tax_error'
error&.is_a?(BaseService::UnknownTaxFailure)
end

def reset_invoice_values
Expand Down
Loading
Loading