From c77cc11594502effd435058ce244925f6474552f Mon Sep 17 00:00:00 2001 From: Ancor Cruz Date: Fri, 22 Nov 2024 14:56:55 +0000 Subject: [PATCH] fix: Plans::UpdateAmountService handle pending subscription upgrades (#2827) ## Context in `cascade_subscription_fee_update` we call `Plans::UpdateAmountJob`. This job should deal with pending subscriptions similar to how the `Plans::UpdateService` does it. (see `process_downgrade`|`pending_subscriptions`) ## Description This changes handles subscription upgrades when plan amount_cents changes because of cascading from parent plan and there are pending subscriptions. --- app/services/plans/update_amount_service.rb | 24 ++++++++- .../plans/update_amount_service_spec.rb | 51 +++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/app/services/plans/update_amount_service.rb b/app/services/plans/update_amount_service.rb index 07ed82f602b..5b0f9c8645f 100644 --- a/app/services/plans/update_amount_service.rb +++ b/app/services/plans/update_amount_service.rb @@ -17,13 +17,35 @@ def call return result if plan.amount_cents != expected_amount_cents plan.amount_cents = amount_cents - plan.save! + + ActiveRecord::Base.transaction do + plan.save! + process_pending_subscriptions + end result + rescue ActiveRecord::RecordInvalid => e + result.record_validation_failure!(record: e.record) + rescue BaseService::FailedResult => e + e.result end private attr_reader :plan, :amount_cents, :expected_amount_cents + + def process_pending_subscriptions + Subscription.where(plan:, status: :pending).find_each do |subscription| + next unless subscription.previous_subscription + + if plan.yearly_amount_cents >= subscription.previous_subscription.plan.yearly_amount_cents + Subscriptions::PlanUpgradeService.call( + current_subscription: subscription.previous_subscription, + plan: plan, + params: {name: subscription.name} + ).raise_if_error! + end + end + end end end diff --git a/spec/services/plans/update_amount_service_spec.rb b/spec/services/plans/update_amount_service_spec.rb index b27485ae3de..b65995db477 100644 --- a/spec/services/plans/update_amount_service_spec.rb +++ b/spec/services/plans/update_amount_service_spec.rb @@ -45,5 +45,56 @@ end end end + + context "when there are pending subscriptions which are not relevant after the amount cents increase", :aggregate_failures do + let(:original_plan) { create(:plan, organization:, amount_cents: expected_amount_cents) } + let(:subscription) { create(:subscription, plan: original_plan) } + let(:pending_subscription) do + create(:subscription, plan:, status: :pending, previous_subscription_id: subscription.id) + end + let(:plan_upgrade_result) { BaseService::Result.new } + + before do + allow(Subscriptions::PlanUpgradeService) + .to receive(:call) + .and_return(plan_upgrade_result) + + pending_subscription + end + + it "upgrades subscription plan" do + update_service.call + + expect(Subscriptions::PlanUpgradeService).to have_received(:call) + end + + context "when pending subscription does not have a previous one" do + let(:pending_subscription) do + create(:subscription, plan:, status: :pending, previous_subscription_id: nil) + end + + it "does not upgrade it" do + update_service.call + + expect(Subscriptions::PlanUpgradeService).not_to have_received(:call) + end + end + + context "when subscription upgrade fails" do + let(:plan_upgrade_result) do + BaseService::Result.new.validation_failure!( + errors: {billing_time: ["value_is_invalid"]} + ) + end + + it "returns an error" do + result = update_service.call + + expect(result).not_to be_success + expect(result.error).to be_a(BaseService::ValidationFailure) + expect(result.error.messages).to eq({billing_time: ["value_is_invalid"]}) + end + end + end end end