diff --git a/app/services/subscriptions/activate_service.rb b/app/services/subscriptions/activate_service.rb index feb191b18be..43b93fa06db 100644 --- a/app/services/subscriptions/activate_service.rb +++ b/app/services/subscriptions/activate_service.rb @@ -53,10 +53,7 @@ def activate_from_incomplete def gate_subscription subscription.mark_as_incomplete!(timestamp) - EmitFixedChargeEventsService.call!( - subscriptions: [subscription], - timestamp: subscription.started_at + 1.second - ) + emit_fixed_charge_events after_commit do bill_subscription(skip_charges: true) if subscription.payment_gated? @@ -67,12 +64,24 @@ def gate_subscription end def activate_with_side_effects + if upgrade? + activate_for_upgrade + else + activate_standalone + end + end + + def upgrade? + return false unless subscription.previous_subscription + return false if subscription.plan.id == subscription.previous_subscription.plan.id + + subscription.plan.yearly_amount_cents >= subscription.previous_subscription.plan.yearly_amount_cents + end + + def activate_standalone subscription.mark_as_active!(timestamp) - EmitFixedChargeEventsService.call!( - subscriptions: [subscription], - timestamp: subscription.started_at + 1.second - ) + emit_fixed_charge_events after_commit do bill_subscription(skip_charges: true) @@ -80,20 +89,63 @@ def activate_with_side_effects end end + def activate_for_upgrade + Subscriptions::TerminateService.call( + subscription: subscription.previous_subscription, + upgrade: true + ) + + subscription.mark_as_active!(timestamp) + + emit_fixed_charge_events + + after_commit { notify_started } + + bill_upgrade_subscriptions + end + + def billable_subscriptions + @billable_subscriptions ||= begin + billable = [subscription.previous_subscription] + bill_new_in_advance = subscription.fixed_charges.pay_in_advance.any? || + (subscription.plan.pay_in_advance? && !subscription.in_trial_period?) + billable << subscription if bill_new_in_advance + billable + end + end + + def bill_upgrade_subscriptions + after_commit do + billing_at = Time.current + 1.second + BillSubscriptionJob.perform_later(billable_subscriptions, billing_at.to_i, invoicing_reason: :upgrading) + BillNonInvoiceableFeesJob.perform_later(billable_subscriptions, billing_at) + end + end + def notify_started SendWebhookJob.perform_later("subscription.started", subscription) Utils::ActivityLog.produce(subscription, "subscription.started") - # Skip Hubspot UpdateJob when activating during subscription creation — - # CreateService fires Hubspot::CreateJob after this, which captures the - # active state and avoids a redundant Update that would race with Create. - return if during_creation + return unless subscription.should_sync_hubspot_subscription? - if subscription.should_sync_hubspot_subscription? + if upgrade? + # The new upgrade subscription has no Hubspot record yet. + Integrations::Aggregator::Subscriptions::Hubspot::CreateJob.perform_later(subscription:) + elsif !during_creation + # Skip when activating during subscription creation — CreateService + # fires Hubspot::CreateJob after this, which captures the active state + # and avoids a redundant Update that would race with Create. Integrations::Aggregator::Subscriptions::Hubspot::UpdateJob.perform_later(subscription:) end end + def emit_fixed_charge_events + EmitFixedChargeEventsService.call!( + subscriptions: [subscription], + timestamp: subscription.started_at + 1.second + ) + end + def bill_subscription(skip_charges: false) if subscription.plan.pay_in_advance? && !subscription.in_trial_period? BillSubscriptionJob.perform_later( diff --git a/app/services/subscriptions/plan_upgrade_service.rb b/app/services/subscriptions/plan_upgrade_service.rb index 7d80043270c..a832dec87c1 100644 --- a/app/services/subscriptions/plan_upgrade_service.rb +++ b/app/services/subscriptions/plan_upgrade_service.rb @@ -18,7 +18,7 @@ def initialize(current_subscription:, plan:, params:) def call ActiveRecord::Base.transaction do if current_subscription.starting_in_the_future? - apply_activation_rules if params[:activation_rules] + apply_activation_rules(current_subscription) if params[:activation_rules] update_pending_subscription result.subscription = current_subscription @@ -29,30 +29,13 @@ def call cancel_pending_subscription if pending_subscription? - # Group subscriptions for billing - billable_subscriptions = billable_subscriptions(new_subscription) + new_subscription.pending! - # Terminate current subscription as part of the upgrade process - Subscriptions::TerminateService.call( - subscription: current_subscription, - upgrade: true - ) + apply_activation_rules(new_subscription) if params[:activation_rules].present? - new_subscription.mark_as_active! - - EmitFixedChargeEventsService.call!( - subscriptions: [new_subscription], - timestamp: new_subscription.started_at + 1.second - ) + Subscriptions::ActivateService.call!(subscription: new_subscription) result.subscription = new_subscription - - after_commit do - SendWebhookJob.perform_later("subscription.started", new_subscription) - Utils::ActivityLog.produce(new_subscription, "subscription.started") - end - - bill_subscriptions(billable_subscriptions) if billable_subscriptions.any? end result @@ -100,9 +83,9 @@ def update_pending_subscription end end - def apply_activation_rules + def apply_activation_rules(subscription) Subscriptions::ActivationRules::ApplyService.call!( - subscription: current_subscription, + subscription:, activation_rules: params[:activation_rules] ) end @@ -120,30 +103,5 @@ def pending_subscription? current_subscription.next_subscription.pending? end - - def billable_subscriptions(new_subscription) - billable_subscriptions = if current_subscription.starting_in_the_future? - [] - elsif current_subscription.pending? - [] - elsif !current_subscription.terminated? - [current_subscription] - end.to_a - - has_billable_fixed_charges = new_subscription.fixed_charges.pay_in_advance.any? - plan_billable = new_subscription.plan.pay_in_advance? && !new_subscription.in_trial_period? - - billable_subscriptions << new_subscription if has_billable_fixed_charges || plan_billable - - billable_subscriptions - end - - def bill_subscriptions(billable_subscriptions) - after_commit do - billing_at = Time.current + 1.second - BillSubscriptionJob.perform_later(billable_subscriptions, billing_at.to_i, invoicing_reason: :upgrading) - BillNonInvoiceableFeesJob.perform_later(billable_subscriptions, billing_at) - end - end end end diff --git a/app/services/subscriptions/terminate_service.rb b/app/services/subscriptions/terminate_service.rb index eaf8cbb14b6..579e4588661 100644 --- a/app/services/subscriptions/terminate_service.rb +++ b/app/services/subscriptions/terminate_service.rb @@ -128,6 +128,9 @@ def terminate_and_start_next(timestamp:) attr_reader :subscription, :async, :upgrade, :on_termination_credit_note, :on_termination_invoice def cancel_next_subscription + # NOTE: Upgrade path: next_subscription is the new subscription we just persisted, not a stale scheduled change + return if upgrade + next_subscription = subscription.next_subscription return if next_subscription.nil? diff --git a/spec/services/subscriptions/activate_service_spec.rb b/spec/services/subscriptions/activate_service_spec.rb index db6e41d7719..0e4c02e598f 100644 --- a/spec/services/subscriptions/activate_service_spec.rb +++ b/spec/services/subscriptions/activate_service_spec.rb @@ -380,4 +380,144 @@ expect(SendWebhookJob).not_to have_been_enqueued end end + + context "when subscription comes from an upgrade" do + let(:customer) { create(:customer, :with_hubspot_integration, organization:) } + let(:previous_plan) { create(:plan, organization:, amount_cents: 50) } + let(:plan) { create(:plan, organization:, amount_cents: 100) } + let(:previous_subscription) do + create( + :subscription, + organization:, + customer:, + plan: previous_plan, + status: :active, + started_at: 1.day.ago, + subscription_at: 1.day.ago + ) + end + let(:subscription) do + create( + :subscription, + :pending, + organization:, + customer:, + plan:, + previous_subscription:, + subscription_at: Time.current + ) + end + + it "terminates the previous subscription" do + result + + expect(previous_subscription.reload).to be_terminated + end + + it "marks the new subscription as active" do + freeze_time do + expect(result.subscription).to be_active + expect(result.subscription.started_at).to eq(Time.current) + end + end + + it "enqueues Hubspot::CreateJob for the new subscription" do + result + + expect(Integrations::Aggregator::Subscriptions::Hubspot::CreateJob) + .to have_been_enqueued.with(subscription: subscription) + end + + context "when subscription should not sync with hubspot" do + let(:customer) { create(:customer, organization:) } + + it "does not enqueue Hubspot::CreateJob" do + result + + expect(Integrations::Aggregator::Subscriptions::Hubspot::CreateJob).not_to have_been_enqueued + end + end + + it "sends a subscription.started webhook for the new subscription" do + result + + expect(SendWebhookJob).to have_been_enqueued.with("subscription.started", subscription) + end + + it "produces a subscription.started activity log" do + result + + expect(Utils::ActivityLog).to have_produced("subscription.started").with(subscription) + end + + it "enqueues BillSubscriptionJob with invoicing_reason :upgrading" do + result + + expect(BillSubscriptionJob).to have_been_enqueued + .with([previous_subscription], anything, invoicing_reason: :upgrading) + end + + it "enqueues BillNonInvoiceableFeesJob" do + result + + expect(BillNonInvoiceableFeesJob).to have_been_enqueued + end + + context "when the new plan is pay in advance" do + let(:plan) { create(:plan, organization:, amount_cents: 100, pay_in_advance: true) } + + it "includes both previous and new subscription in the upgrade bill" do + result + + expect(BillSubscriptionJob).to have_been_enqueued + .with([previous_subscription, subscription], anything, invoicing_reason: :upgrading) + end + end + + context "when activation_rules gate the new subscription" do + let(:plan) { create(:plan, organization:, amount_cents: 100, pay_in_advance: true) } + let(:subscription) do + create( + :subscription, + :pending, + :with_activation_rules, + organization:, + customer:, + plan:, + previous_subscription:, + subscription_at: Time.current + ) + end + + it "marks the new subscription as incomplete" do + expect(result.subscription).to be_incomplete + end + + it "does not terminate the previous subscription" do + result + + expect(previous_subscription.reload).to be_active + end + + it "sends a subscription.incomplete webhook" do + result + + expect(SendWebhookJob).to have_been_enqueued.with("subscription.incomplete", subscription) + end + + it "enqueues BillSubscriptionJob for the incomplete subscription with skip_charges" do + result + + expect(BillSubscriptionJob).to have_been_enqueued + .with([subscription], anything, invoicing_reason: :subscription_starting, skip_charges: true) + end + + it "does not enqueue a BillSubscriptionJob with invoicing_reason :upgrading" do + result + + expect(BillSubscriptionJob).not_to have_been_enqueued + .with(anything, anything, invoicing_reason: :upgrading) + end + end + end end diff --git a/spec/services/subscriptions/plan_upgrade_service_spec.rb b/spec/services/subscriptions/plan_upgrade_service_spec.rb index 57ba78c9e84..4df8f9c0b6f 100644 --- a/spec/services/subscriptions/plan_upgrade_service_spec.rb +++ b/spec/services/subscriptions/plan_upgrade_service_spec.rb @@ -205,6 +205,81 @@ end end + context "with activation_rules in params" do + let(:params) do + { + name: subscription_name, + activation_rules: [{type: "payment", timeout_hours: 48}] + } + end + + context "when the plan is pay-in-advance" do + let(:plan) { create(:plan, amount_cents: 200, organization:, pay_in_advance: true) } + + it "keeps the current subscription active" do + result + expect(subscription.reload).to be_active + end + + it "creates the new subscription as incomplete" do + expect(result).to be_success + expect(result.subscription).to be_incomplete + expect(result.subscription.previous_subscription_id).to eq(subscription.id) + expect(result.subscription.activation_rules.pending.count).to eq(1) + end + + it "sends the subscription.incomplete webhook" do + result + expect(SendWebhookJob).to have_been_enqueued.with("subscription.incomplete", result.subscription) + end + + it "does not fire upgrade-completion webhooks" do + result + expect(SendWebhookJob).not_to have_been_enqueued.with("subscription.terminated", subscription) + expect(SendWebhookJob).not_to have_been_enqueued.with("subscription.started", result.subscription) + end + + it "enqueues BillSubscriptionJob for the incomplete subscription with skip_charges" do + new_subscription = result.subscription + + expect(BillSubscriptionJob).to have_been_enqueued + .with([new_subscription], anything, invoicing_reason: :subscription_starting, skip_charges: true) + end + + it "does not enqueue a BillSubscriptionJob with invoicing_reason :upgrading" do + expect { result }.not_to have_enqueued_job(BillSubscriptionJob) + .with(anything, anything, hash_including(invoicing_reason: :upgrading)) + end + + context "with a pending next_subscription" do + let(:pending_next) do + create(:subscription, status: :pending, previous_subscription: subscription, organization:, customer:) + end + + before { pending_next } + + it "cancels the pending next_subscription before gating" do + expect { result }.to change { pending_next.reload.status }.from("pending").to("canceled") + end + end + end + + context "when the plan has no upfront billing" do + let(:plan) { create(:plan, amount_cents: 100, organization:, pay_in_advance: false) } + + it "marks the activation rule as not_applicable" do + expect(result).to be_success + expect(result.subscription.activation_rules.not_applicable.count).to eq(1) + end + + it "runs the existing immediate-upgrade flow" do + result + expect(subscription.reload).to be_terminated + expect(result.subscription).to be_active + end + end + end + context "when old subscription is payed in arrear" do let(:old_plan) { create(:plan, amount_cents: 100, organization:, pay_in_advance: false) } diff --git a/spec/services/subscriptions/terminate_service_spec.rb b/spec/services/subscriptions/terminate_service_spec.rb index 9569c25baad..3226b102234 100644 --- a/spec/services/subscriptions/terminate_service_spec.rb +++ b/spec/services/subscriptions/terminate_service_spec.rb @@ -131,6 +131,17 @@ expect(result).to be_success expect(next_subscription.reload).to be_canceled end + + context "when called with upgrade: true" do + subject(:result) { described_class.call(subscription:, on_termination_credit_note:, upgrade: true) } + + it "does not cancel the next subscription" do + subject + + expect(result).to be_success + expect(next_subscription.reload).to be_pending + end + end end context "when subscription was paid in advance" do