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(dunning): Remove dunning_campaign_completed flag #2890

Merged
merged 2 commits into from
Nov 28, 2024
Merged
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
4 changes: 0 additions & 4 deletions app/models/customer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ class Customer < ApplicationRecord
scope :falling_back_to_default_dunning_campaign, -> {
where(applied_dunning_campaign_id: nil, exclude_from_dunning_campaign: false)
}
scope :with_dunning_campaign_completed, -> { where(dunning_campaign_completed: true) }
scope :with_dunning_campaign_not_completed, -> { where(dunning_campaign_completed: false) }

validates :country, :shipping_country, country_code: true, allow_nil: true
validates :document_locale, language_code: true, unless: -> { document_locale.nil? }
Expand Down Expand Up @@ -192,7 +190,6 @@ def overdue_balance_cents

def reset_dunning_campaign!
update!(
dunning_campaign_completed: false,
last_dunning_campaign_attempt: 0,
last_dunning_campaign_attempt_at: nil
)
Expand Down Expand Up @@ -222,7 +219,6 @@ def ensure_slug
# customer_type :enum
# deleted_at :datetime
# document_locale :string
# dunning_campaign_completed :boolean default(FALSE)
# email :string
# exclude_from_dunning_campaign :boolean default(FALSE), not null
# finalize_zero_amount_invoice :integer default("inherit"), not null
Expand Down
2 changes: 1 addition & 1 deletion app/models/dunning_campaign.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ def self.ransackable_attributes(_auth_object = nil)

def reset_customers_last_attempt
# NOTE: Reset last attempt on customers with the campaign applied explicitly
customers.with_dunning_campaign_not_completed.update_all( # rubocop:disable Rails/SkipsModelValidations
customers.update_all( # rubocop:disable Rails/SkipsModelValidations
last_dunning_campaign_attempt: 0,
last_dunning_campaign_attempt_at: nil
)
Expand Down
1 change: 0 additions & 1 deletion app/models/organization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ def auto_dunning_enabled?
def reset_customers_last_dunning_campaign_attempt
customers
.falling_back_to_default_dunning_campaign
.with_dunning_campaign_not_completed
.update_all( # rubocop:disable Rails/SkipsModelValidations
last_dunning_campaign_attempt: 0,
last_dunning_campaign_attempt_at: nil
Expand Down
2 changes: 0 additions & 2 deletions app/services/dunning_campaigns/bulk_process_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ def call

def eligible_customers
Customer
.with_dunning_campaign_not_completed
.joins(:organization)
.where(exclude_from_dunning_campaign: false)
.where("organizations.premium_integrations @> ARRAY[?]::varchar[]", ['auto_dunning'])
Expand All @@ -31,7 +30,6 @@ def initialize(customer)
end

def call
return result if customer.dunning_campaign_completed?
return result unless threshold
return result if max_attempts_reached?
return result unless days_between_attempts_satisfied?
Expand Down
5 changes: 2 additions & 3 deletions app/services/dunning_campaigns/process_attempt_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def call
return result unless applicable_dunning_campaign?
return result unless dunning_campaign_threshold_reached?
return result unless days_between_attempts_passed?
return result if customer.dunning_campaign_completed?
ancorcruz marked this conversation as resolved.
Show resolved Hide resolved
return result if max_attempts_reached?

ActiveRecord::Base.transaction do
payment_request_result = PaymentRequests::CreateService.call(
Expand All @@ -30,7 +30,6 @@ def call

customer.increment(:last_dunning_campaign_attempt)
customer.last_dunning_campaign_attempt_at = Time.zone.now
customer.dunning_campaign_completed = last_dunning_campaign_attempt?
customer.save!

result.customer = customer
Expand Down Expand Up @@ -65,7 +64,7 @@ def days_between_attempts_passed?
(customer.last_dunning_campaign_attempt_at + dunning_campaign.days_between_attempts.days).past?
end

def last_dunning_campaign_attempt?
def max_attempts_reached?
customer.last_dunning_campaign_attempt >= dunning_campaign.max_attempts
end

Expand Down
13 changes: 1 addition & 12 deletions app/services/dunning_campaigns/update_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ def call
dunning_campaign.assign_attributes(permitted_attributes)
handle_thresholds if params.key?(:thresholds)
handle_applied_to_organization_update if params.key?(:applied_to_organization)
handle_max_attempts_change if dunning_campaign.max_attempts_changed?

dunning_campaign.save!
end
Expand Down Expand Up @@ -84,9 +83,7 @@ def reset_customers_if_no_threshold_match
end

def customers_to_reset
@customers_to_reset ||= customers_applied_campaign
.or(customers_fallback_campaign)
.with_dunning_campaign_not_completed
@customers_to_reset ||= customers_applied_campaign.or(customers_fallback_campaign)
end

def customers_applied_campaign
Expand All @@ -110,17 +107,9 @@ def handle_applied_to_organization_update
organization.reset_customers_last_dunning_campaign_attempt

customers_fallback_campaign.update_all( # rubocop:disable Rails/SkipsModelValidations
dunning_campaign_completed: false,
last_dunning_campaign_attempt_at: nil,
last_dunning_campaign_attempt: 0
)
end

def handle_max_attempts_change
# we assume there is matching threshold at this point or it was reseted
customers_to_reset
.where("last_dunning_campaign_attempt >= ?", dunning_campaign.max_attempts)
.update_all(dunning_campaign_completed: true) # rubocop:disable Rails/SkipsModelValidations
end
end
end
6 changes: 1 addition & 5 deletions app/services/payment_requests/payments/stripe_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,7 @@ def reset_customer_dunning_campaign_status(payment_status)
return unless payment_status_succeeded?(payment_status)
return unless payable.try(:dunning_campaign)

customer.update!(
dunning_campaign_completed: false,
last_dunning_campaign_attempt: 0,
last_dunning_campaign_attempt_at: nil
)
customer.reset_dunning_campaign!
end
end
end
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

class RemoveDunningCampaignCompletedFromCustomers < ActiveRecord::Migration[7.1]
def change
safety_assured do
remove_column :customers, :dunning_campaign_completed, :boolean, default: false
end
end
end
3 changes: 1 addition & 2 deletions db/schema.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 1 addition & 3 deletions spec/models/customer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -599,16 +599,14 @@
let(:customer) do
create(
:customer,
dunning_campaign_completed: true,
last_dunning_campaign_attempt: 5,
last_dunning_campaign_attempt_at: 1.day.ago
)
end

it "changes dunning campaign status counters" do
expect { customer.reset_dunning_campaign! && customer.reload }
.to change(customer, :dunning_campaign_completed).to(false)
.and change(customer, :last_dunning_campaign_attempt).to(0)
.to change(customer, :last_dunning_campaign_attempt).to(0)
.and change(customer, :last_dunning_campaign_attempt_at).to(nil)
end
end
Expand Down
25 changes: 0 additions & 25 deletions spec/models/dunning_campaign_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,19 +65,6 @@
.and change { customer.last_dunning_campaign_attempt_at }.from(last_dunning_campaign_attempt_at).to(nil)
end

it "does not reset last attempt on customers with dunning campaign already completed" do
customer = create(
:customer,
organization:,
applied_dunning_campaign: dunning_campaign,
last_dunning_campaign_attempt: 1,
dunning_campaign_completed: true
)

expect { dunning_campaign.reset_customers_last_attempt }
.not_to change { customer.reload.last_dunning_campaign_attempt }.from(1)
end

context "when applied to organization" do
subject(:dunning_campaign) { create(:dunning_campaign, applied_to_organization: true) }

Expand All @@ -93,18 +80,6 @@
.to change { customer.reload.last_dunning_campaign_attempt }.from(2).to(0)
.and change { customer.last_dunning_campaign_attempt_at }.from(last_dunning_campaign_attempt_at).to(nil)
end

it "does not reset last attempt on customers with dunning campaign already completed" do
customer = create(
:customer,
organization:,
last_dunning_campaign_attempt: 2,
dunning_campaign_completed: true
)

expect { dunning_campaign.reset_customers_last_attempt }
.not_to change { customer.reload.last_dunning_campaign_attempt }.from(2)
end
end
end
end
2 changes: 0 additions & 2 deletions spec/models/organization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -245,13 +245,11 @@
it "resets the last dunning campaign attempt for customers" do
customer1 = create(:customer, organization:, last_dunning_campaign_attempt: 1, last_dunning_campaign_attempt_at:)
customer2 = create(:customer, organization:, last_dunning_campaign_attempt: 1, last_dunning_campaign_attempt_at:, applied_dunning_campaign: campaign)
customer3 = create(:customer, organization:, last_dunning_campaign_attempt: 1, last_dunning_campaign_attempt_at:, dunning_campaign_completed: true)

expect { organization.reset_customers_last_dunning_campaign_attempt }
.to change { customer1.reload.last_dunning_campaign_attempt }.from(1).to(0)
.and change(customer1, :last_dunning_campaign_attempt_at).from(last_dunning_campaign_attempt_at).to(nil)
expect(customer2.reload.last_dunning_campaign_attempt).to eq(1)
expect(customer3.reload.last_dunning_campaign_attempt).to eq(1)
end
end

Expand Down
17 changes: 4 additions & 13 deletions spec/services/customers/update_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,6 @@
expect { customers_service.call }
.to not_change(customer, :applied_dunning_campaign_id)
.and not_change(customer, :exclude_from_dunning_campaign)
.and not_change(customer, :dunning_campaign_completed)
.and not_change(customer, :last_dunning_campaign_attempt)
.and not_change { customer.last_dunning_campaign_attempt_at.iso8601 }

Expand All @@ -370,8 +369,7 @@
organization:,
exclude_from_dunning_campaign: true,
last_dunning_campaign_attempt: 3,
last_dunning_campaign_attempt_at: 2.days.ago,
dunning_campaign_completed: true
last_dunning_campaign_attempt_at: 2.days.ago
)
end

Expand All @@ -391,7 +389,6 @@
expect { customers_service.call }
.to change(customer, :applied_dunning_campaign_id).to(dunning_campaign.id)
.and change(customer, :exclude_from_dunning_campaign).to(false)
.and change(customer, :dunning_campaign_completed).to(false)
.and change(customer, :last_dunning_campaign_attempt).to(0)
.and change(customer, :last_dunning_campaign_attempt_at).to(nil)

Expand All @@ -405,8 +402,7 @@
organization:,
applied_dunning_campaign: dunning_campaign,
last_dunning_campaign_attempt: 3,
last_dunning_campaign_attempt_at: 2.days.ago,
dunning_campaign_completed: true
last_dunning_campaign_attempt_at: 2.days.ago
)
end

Expand All @@ -418,7 +414,6 @@
expect { customers_service.call }
.to change(customer, :applied_dunning_campaign_id).to(nil)
.and change(customer, :exclude_from_dunning_campaign).to(true)
.and change(customer, :dunning_campaign_completed).to(false)
.and change(customer, :last_dunning_campaign_attempt).to(0)
.and change(customer, :last_dunning_campaign_attempt_at).to(nil)

Expand All @@ -434,8 +429,7 @@
applied_dunning_campaign: dunning_campaign,
exclude_from_dunning_campaign: false,
last_dunning_campaign_attempt: 3,
last_dunning_campaign_attempt_at: 2.days.ago,
dunning_campaign_completed: true
last_dunning_campaign_attempt_at: 2.days.ago
)
end

Expand All @@ -445,7 +439,6 @@
expect { customers_service.call }
.to change(customer, :applied_dunning_campaign_id).to(nil)
.and not_change(customer, :exclude_from_dunning_campaign)
.and change(customer, :dunning_campaign_completed).to(false)
.and change(customer, :last_dunning_campaign_attempt).to(0)
.and change(customer, :last_dunning_campaign_attempt_at).to(nil)

Expand All @@ -461,8 +454,7 @@
applied_dunning_campaign: dunning_campaign,
exclude_from_dunning_campaign: false,
last_dunning_campaign_attempt: 3,
last_dunning_campaign_attempt_at: 2.days.ago,
dunning_campaign_completed: true
last_dunning_campaign_attempt_at: 2.days.ago
)
end

Expand All @@ -472,7 +464,6 @@
expect { customers_service.call }
.to not_change(customer, :applied_dunning_campaign_id)
.and not_change(customer, :exclude_from_dunning_campaign)
.and not_change(customer, :dunning_campaign_completed)
.and not_change(customer, :last_dunning_campaign_attempt)
.and not_change(customer, :last_dunning_campaign_attempt_at)

Expand Down
11 changes: 0 additions & 11 deletions spec/services/dunning_campaigns/bulk_process_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,6 @@
.with(customer:, dunning_campaign_threshold:)
end

context "when the customer has completed the dunning campaign" do
let(:customer) do
create :customer, organization:, currency:, dunning_campaign_completed: true
end

it "does not queue a job for the customer" do
result
expect(DunningCampaigns::ProcessAttemptJob).not_to have_been_enqueued
end
end

context "when organization does not have auto_dunning feature enabled" do
let(:organization) { create(:organization, premium_integrations: []) }

Expand Down
30 changes: 6 additions & 24 deletions spec/services/dunning_campaigns/process_attempt_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,25 +71,23 @@
expect { result && customer.reload }
.to change(customer, :last_dunning_campaign_attempt).by(1)
.and change(customer, :last_dunning_campaign_attempt_at).to(Time.zone.now)
.and not_change(customer, :dunning_campaign_completed).from(false)
end
end

context "when dunning campaign max attemp is reached" do
context "when dunning campaign max attempt is reached" do
let(:customer) do
create(
:customer,
organization:,
currency:,
last_dunning_campaign_attempt: dunning_campaign.max_attempts - 1,
last_dunning_campaign_attempt_at: dunning_campaign.days_between_attempts.days.ago,
dunning_campaign_completed: false
last_dunning_campaign_attempt: dunning_campaign.max_attempts,
last_dunning_campaign_attempt_at: dunning_campaign.days_between_attempts.days.ago
)
end

it "updates customer's dunning campaign completed flag" do
expect { result && customer.reload }
.to change(customer, :dunning_campaign_completed).to(true)
it "does nothing" do
result
expect(PaymentRequests::CreateService).not_to have_received(:call)
end
end

Expand Down Expand Up @@ -136,22 +134,6 @@
end
end

context "when the customer has completed the dunning campaign" do
let(:customer) do
create(
:customer,
organization:,
currency:,
dunning_campaign_completed: true
)
end

it "does nothing" do
result
expect(PaymentRequests::CreateService).not_to have_received(:call)
end
end

context "when days between attempts has not passed" do
let(:customer) do
create(
Expand Down
Loading