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

fix(unique-jobs): Unique jobs were not enqueuing jobs with keyword args #2857

Closed
wants to merge 1 commit into from
Closed
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
12 changes: 8 additions & 4 deletions app/jobs/fees/create_pay_in_advance_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ module Fees
class CreatePayInAdvanceJob < ApplicationJob
queue_as :default

def perform(charge:, event:, billing_at: nil)
unique :until_executed, on_conflict: :log

def perform(charge, event, billing_at = nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

@ivannovosad why is this needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

(the removal of keyword arguments that is)

result = Fees::CreatePayInAdvanceService.call(charge:, event:, billing_at:)

return if !result.success? && tax_error?(result)
Expand All @@ -13,9 +15,11 @@ def perform(charge:, event:, billing_at: nil)
end

def lock_key_arguments
args = arguments.first
event = Events::CommonFactory.new_instance(source: args[:event])
[args[:charge], event.organization_id, event.external_subscription_id, event.transaction_id]
charge = arguments.first
arg_event = arguments.second

event = Events::CommonFactory.new_instance(source: arg_event)
[charge, event.organization_id, event.external_subscription_id, event.transaction_id]
end

private
Expand Down
6 changes: 5 additions & 1 deletion app/jobs/integrations/aggregator/credit_notes/create_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ module CreditNotes
class CreateJob < ApplicationJob
queue_as 'integrations'

# https://github.com/veeqo/activejob-uniqueness/issues/75
# retry_on does not work with until_executed strategy
unique :until_executed_patch, on_conflict: :log

retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 3
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(credit_note:)
def perform(credit_note)
result = Integrations::Aggregator::CreditNotes::CreateService.call(credit_note:)
result.raise_if_error!
end
Expand Down
6 changes: 5 additions & 1 deletion app/jobs/integrations/aggregator/invoices/create_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,14 @@ module Invoices
class CreateJob < ApplicationJob
queue_as 'integrations'

# https://github.com/veeqo/activejob-uniqueness/issues/75
# retry_on does not work with until_executed strategy
unique :until_executed_patch, on_conflict: :log

retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 3
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(invoice:)
def perform(invoice)
result = Integrations::Aggregator::Invoices::CreateService.call(invoice:)
result.raise_if_error!
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class CreateCustomerAssociationJob < ApplicationJob
retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 10
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(invoice:)
def perform(invoice)
result = Integrations::Aggregator::Invoices::Crm::CreateCustomerAssociationService.call(invoice:)
result.raise_if_error!
end
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/integrations/aggregator/invoices/crm/create_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ class CreateJob < ApplicationJob
retry_on Integrations::Aggregator::BasePayload::Failure, wait: :polynomially_longer, attempts: 10
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(invoice:)
def perform(invoice)
result = Integrations::Aggregator::Invoices::Crm::CreateService.call(invoice:)

if result.success?
Integrations::Aggregator::Invoices::Crm::CreateCustomerAssociationJob.perform_later(invoice:)
Integrations::Aggregator::Invoices::Crm::CreateCustomerAssociationJob.perform_later(invoice)
end

result.raise_if_error!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class UpdateJob < ApplicationJob
retry_on Integrations::Aggregator::BasePayload::Failure, wait: :polynomially_longer, attempts: 10
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(invoice:)
def perform(invoice)
result = Integrations::Aggregator::Invoices::Crm::UpdateService.call(invoice:)
result.raise_if_error!
end
Expand Down
6 changes: 5 additions & 1 deletion app/jobs/integrations/aggregator/payments/create_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,15 @@ module Payments
class CreateJob < ApplicationJob
queue_as 'integrations'

# https://github.com/veeqo/activejob-uniqueness/issues/75
# retry_on does not work with until_executed strategy
unique :until_executed_patch, on_conflict: :log

retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 5
retry_on Integrations::Aggregator::BasePayload::Failure, wait: :polynomially_longer, attempts: 10
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(payment:)
def perform(payment)
result = Integrations::Aggregator::Payments::CreateService.call(payment:)
result.raise_if_error!
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class CreateCustomerAssociationJob < ApplicationJob
retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 10
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(subscription:)
def perform(subscription)
result = Integrations::Aggregator::Subscriptions::Crm::CreateCustomerAssociationService.call(subscription:)
result.raise_if_error!
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ class CreateJob < ApplicationJob
retry_on Integrations::Aggregator::BasePayload::Failure, wait: :polynomially_longer, attempts: 10
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(subscription:)
def perform(subscription)
result = Integrations::Aggregator::Subscriptions::Crm::CreateService.call(subscription:)

if result.success?
Integrations::Aggregator::Subscriptions::Crm::CreateCustomerAssociationJob.perform_later(subscription:)
Integrations::Aggregator::Subscriptions::Crm::CreateCustomerAssociationJob.perform_later(subscription)
end

result.raise_if_error!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class UpdateJob < ApplicationJob
retry_on Integrations::Aggregator::BasePayload::Failure, wait: :polynomially_longer, attempts: 10
retry_on RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(subscription:)
def perform(subscription)
result = Integrations::Aggregator::Subscriptions::Crm::UpdateService.call(subscription:)
result.raise_if_error!
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class DeployPropertiesJob < ApplicationJob
retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 3
retry_on Integrations::Aggregator::RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(integration:)
def perform(integration)
result = Integrations::Hubspot::Companies::DeployPropertiesService.call(integration:)
result.raise_if_error!
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class DeployPropertiesJob < ApplicationJob
retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 3
retry_on Integrations::Aggregator::RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(integration:)
def perform(integration)
result = Integrations::Hubspot::Contacts::DeployPropertiesService.call(integration:)
result.raise_if_error!
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class DeployObjectJob < ApplicationJob
retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 3
retry_on Integrations::Aggregator::RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(integration:)
def perform(integration)
result = Integrations::Hubspot::Invoices::DeployObjectService.call(integration:)
result.raise_if_error!
end
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/integrations/hubspot/save_portal_id_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class SavePortalIdJob < ApplicationJob
retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 3
retry_on Integrations::Aggregator::RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(integration:)
def perform(integration)
result = Integrations::Hubspot::SavePortalIdService.call(integration:)
result.raise_if_error!
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class DeployObjectJob < ApplicationJob
retry_on LagoHttpClient::HttpError, wait: :polynomially_longer, attempts: 3
retry_on Integrations::Aggregator::RequestLimitError, wait: :polynomially_longer, attempts: 100

def perform(integration:)
def perform(integration)
result = Integrations::Hubspot::Subscriptions::DeployObjectService.call(integration:)
result.raise_if_error!
end
Expand Down
20 changes: 12 additions & 8 deletions app/jobs/invoices/create_pay_in_advance_charge_job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ module Invoices
class CreatePayInAdvanceChargeJob < ApplicationJob
queue_as 'billing'

unique :until_executed, on_conflict: :log

retry_on Sequenced::SequenceError

def perform(charge:, event:, timestamp:, invoice: nil)
def perform(charge, event, timestamp, invoice = nil)
result = Invoices::CreatePayInAdvanceChargeService.call(charge:, event:, timestamp:, invoice:)
return if result.success?
# NOTE: We don't want a dead job for failed invoice due to the tax reason.
Expand All @@ -17,17 +19,19 @@ def perform(charge:, event:, timestamp:, invoice: nil)

# NOTE: retry the job with the already created invoice in a previous failed attempt
self.class.set(wait: 3.seconds).perform_later(
charge:,
event:,
timestamp:,
invoice: result.invoice
charge,
event,
timestamp,
result.invoice
)
end

def lock_key_arguments
args = arguments.first
event = Events::CommonFactory.new_instance(source: args[:event])
[args[:charge], event.organization_id, event.external_subscription_id, event.transaction_id]
charge = arguments.first
arg_event = arguments.second

event = Events::CommonFactory.new_instance(source: arg_event)
[charge, event.organization_id, event.external_subscription_id, event.transaction_id]
end

private
Expand Down
4 changes: 2 additions & 2 deletions app/services/events/pay_in_advance_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ def call
end

charges.where(invoiceable: false).find_each do |charge|
Fees::CreatePayInAdvanceJob.perform_later(charge:, event: event.as_json)
Fees::CreatePayInAdvanceJob.perform_later(charge, event.as_json)
end

charges.where(invoiceable: true).find_each do |charge|
Invoices::CreatePayInAdvanceChargeJob.perform_later(charge:, event: event.as_json, timestamp: event.timestamp)
Invoices::CreatePayInAdvanceChargeJob.perform_later(charge, event.as_json, event.timestamp)
end

result.event = event
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def call
def call_async
return result.not_found_failure!(resource: 'invoice') unless invoice

::Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:)
::Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice)

result.invoice_id = invoice.id
result
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def call
def call_async
return result.not_found_failure!(resource: 'payment') unless payment

::Integrations::Aggregator::Payments::CreateJob.perform_later(payment:)
::Integrations::Aggregator::Payments::CreateJob.perform_later(payment)

result.payment_id = payment.id
result
Expand Down
2 changes: 1 addition & 1 deletion app/services/integrations/hubspot/create_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def call

if integration.type == 'Integrations::HubspotIntegration'
Integrations::Aggregator::SyncCustomObjectsAndPropertiesJob.perform_later(integration:)
Integrations::Hubspot::SavePortalIdJob.perform_later(integration:)
Integrations::Hubspot::SavePortalIdJob.perform_later(integration)
end

result.integration = integration
Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/add_on_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ def create
GeneratePdfAndNotifyJob.perform_later(invoice: result.invoice, email: should_deliver_email?)

if result.invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice: result.invoice)
Integrations::Aggregator::Invoices::CreateJob.perform_later(result.invoice)
end

if result.invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice: result.invoice)
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(result.invoice)
end

create_payment(result.invoice)
Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/advance_charges_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ def call
if invoice && !invoice.closed?
SendWebhookJob.perform_later('invoice.created', invoice)
Invoices::GeneratePdfAndNotifyJob.perform_later(invoice:, email: false)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
Utils::SegmentTrack.invoice_created(invoice)
end

Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/create_one_off_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ def call
Utils::SegmentTrack.invoice_created(invoice)
SendWebhookJob.perform_later('invoice.one_off_created', invoice)
GeneratePdfAndNotifyJob.perform_later(invoice:, email: should_deliver_email?)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
Invoices::Payments::CreateService.new(invoice).call
end

Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/create_pay_in_advance_charge_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ def call
Utils::SegmentTrack.invoice_created(invoice)
deliver_webhooks
GeneratePdfAndNotifyJob.perform_later(invoice:, email: should_deliver_email?)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
Invoices::Payments::CreateService.new(invoice).call
end

Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/finalize_open_credit_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ def call

SendWebhookJob.perform_later('invoice.paid_credit_added', result.invoice)
GeneratePdfAndNotifyJob.perform_later(invoice:, email: should_deliver_email?)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
Utils::SegmentTrack.invoice_created(result.invoice)

result
Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/paid_credit_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def call
Utils::SegmentTrack.invoice_created(result.invoice)
SendWebhookJob.perform_later('invoice.paid_credit_added', result.invoice)
GeneratePdfAndNotifyJob.perform_later(invoice:, email: should_deliver_email?)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
end

create_payment(result.invoice)
Expand Down
2 changes: 1 addition & 1 deletion app/services/invoices/payments/adyen_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def call
invoice_payment_status = invoice_payment_status(payment.status)
update_invoice_payment_status(payment_status: invoice_payment_status)

Integrations::Aggregator::Payments::CreateJob.perform_later(payment:) if payment.should_sync_payment?
Integrations::Aggregator::Payments::CreateJob.perform_later(payment) if payment.should_sync_payment?

result.payment = payment
result
Expand Down
2 changes: 1 addition & 1 deletion app/services/invoices/payments/gocardless_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def call
invoice_payment_status = invoice_payment_status(payment.status)
update_invoice_payment_status(payment_status: invoice_payment_status)

Integrations::Aggregator::Payments::CreateJob.perform_later(payment:) if payment.should_sync_payment?
Integrations::Aggregator::Payments::CreateJob.perform_later(payment) if payment.should_sync_payment?

result.payment = payment
result
Expand Down
2 changes: 1 addition & 1 deletion app/services/invoices/payments/stripe_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def call
processing: payment.status == 'processing'
)

Integrations::Aggregator::Payments::CreateJob.perform_later(payment:) if payment.should_sync_payment?
Integrations::Aggregator::Payments::CreateJob.perform_later(payment) if payment.should_sync_payment?

handle_requires_action(payment) if payment.status == 'requires_action'

Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/progressive_billing_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def call
Utils::SegmentTrack.invoice_created(invoice)
SendWebhookJob.perform_later('invoice.created', invoice)
Invoices::GeneratePdfAndNotifyJob.perform_later(invoice:, email: should_deliver_email?)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
Invoices::Payments::CreateService.call(invoice)

result.invoice = invoice
Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/refresh_draft_and_finalize_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ def call
unless invoice.closed?
SendWebhookJob.perform_later('invoice.created', invoice)
GeneratePdfAndNotifyJob.perform_later(invoice:, email: should_deliver_email?)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
Invoices::Payments::CreateService.new(invoice).call
Utils::SegmentTrack.invoice_created(invoice)
end
Expand Down
4 changes: 2 additions & 2 deletions app/services/invoices/retry_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ def call

SendWebhookJob.perform_later('invoice.created', invoice)
GeneratePdfAndNotifyJob.perform_later(invoice:, email: should_deliver_email?)
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice:) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice:) if invoice.should_sync_crm_invoice?
Integrations::Aggregator::Invoices::CreateJob.perform_later(invoice) if invoice.should_sync_invoice?
Integrations::Aggregator::Invoices::Crm::CreateJob.perform_later(invoice) if invoice.should_sync_crm_invoice?
Invoices::Payments::CreateService.new(invoice).call
Utils::SegmentTrack.invoice_created(invoice)

Expand Down
Loading