diff options
12 files changed, 239 insertions, 148 deletions
@@ -141,6 +141,10 @@ SolidusSubscriptions.configure do |config| end ``` +### Subscription product deletion +When a product is soft deleted, its subscription line items need to be deleted as well, in order to avoid error on subscription processing. +If the product class is `Spree::Variant`, this corner case is handled automatically on the variant soft deletion, otherwise it should be handled manually. + ## Development ### Testing the extension diff --git a/app/decorators/models/solidus_subscriptions/spree/variant/auto_delete_from_subscriptions.rb b/app/decorators/models/solidus_subscriptions/spree/variant/auto_delete_from_subscriptions.rb new file mode 100644 index 0000000..bba173e --- /dev/null +++ b/app/decorators/models/solidus_subscriptions/spree/variant/auto_delete_from_subscriptions.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module SolidusSubscriptions + module Spree + module Variant + module AutoDeleteFromSubscriptions + def self.prepended(base) + base.after_discard(:remove_from_subscriptions) + base.after_destroy(:remove_from_subscriptions) + end + + def remove_from_subscriptions + SolidusSubscriptions::LineItem.where(subscribable: self).delete_all + end + end + end + end +end + +Spree::Variant.prepend(SolidusSubscriptions::Spree::Variant::AutoDeleteFromSubscriptions) diff --git a/app/jobs/solidus_subscriptions/process_subscription_job.rb b/app/jobs/solidus_subscriptions/process_subscription_job.rb new file mode 100644 index 0000000..569fa6f --- /dev/null +++ b/app/jobs/solidus_subscriptions/process_subscription_job.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module SolidusSubscriptions + class ProcessSubscriptionJob < ApplicationJob + queue_as { SolidusSubscriptions.configuration.processing_queue } + + def perform(subscription) + ActiveRecord::Base.transaction do + if SolidusSubscriptions.configuration.clear_past_installments + subscription.installments.unfulfilled.actionable.each do |installment| + installment.update!(actionable_date: nil) + end + end + + if subscription.actionable? + subscription.successive_skip_count = 0 + subscription.advance_actionable_date + + subscription.installments.create!(actionable_date: Time.zone.now) + end + + subscription.cancel! if subscription.pending_cancellation? + subscription.deactivate! if subscription.can_be_deactivated? + + subscription.installments.actionable.find_each do |installment| + SolidusSubscriptions::ProcessInstallmentJob.perform_later(installment) + end + end + end + end +end diff --git a/app/models/solidus_subscriptions/subscription.rb b/app/models/solidus_subscriptions/subscription.rb index 9b3ff68..df82447 100644 --- a/app/models/solidus_subscriptions/subscription.rb +++ b/app/models/solidus_subscriptions/subscription.rb @@ -267,6 +267,10 @@ module SolidusSubscriptions Time.zone.now > (failing_since + SolidusSubscriptions.configuration.maximum_reprocessing_time) end + def actionable? + actionable_date && actionable_date <= Time.zone.today && ["canceled", "inactive"].exclude?(state) + end + private def check_successive_skips_exceeded diff --git a/lib/solidus_subscriptions/permission_sets/default_customer.rb b/lib/solidus_subscriptions/permission_sets/default_customer.rb index ebe888e..1fc42d2 100644 --- a/lib/solidus_subscriptions/permission_sets/default_customer.rb +++ b/lib/solidus_subscriptions/permission_sets/default_customer.rb @@ -4,12 +4,12 @@ module SolidusSubscriptions module PermissionSets class DefaultCustomer < ::Spree::PermissionSets::Base def activate! - can [:display, :update, :skip, :cancel], Subscription, ['user_id = ?', user.id] do |subscription, guest_token| + can [:show, :display, :update, :skip, :cancel], Subscription, ['user_id = ?', user.id] do |subscription, guest_token| (subscription.guest_token.present? && subscription.guest_token == guest_token) || (subscription.user && subscription.user == user) end - can [:display, :update, :destroy], LineItem do |line_item, guest_token| + can [:show, :display, :update, :destroy], LineItem do |line_item, guest_token| (line_item.subscription&.guest_token.present? && line_item.subscription.guest_token == guest_token) || (line_item.subscription&.user && line_item.subscription.user == user) end diff --git a/lib/solidus_subscriptions/processor.rb b/lib/solidus_subscriptions/processor.rb index ffc274b..04b84e8 100644 --- a/lib/solidus_subscriptions/processor.rb +++ b/lib/solidus_subscriptions/processor.rb @@ -4,32 +4,13 @@ module SolidusSubscriptions class Processor class << self def run - SolidusSubscriptions::Subscription.actionable.find_each(&method(:process_subscription)) - SolidusSubscriptions::Installment.actionable.find_each(&method(:process_installment)) - end - - private - - def process_subscription(subscription) - ActiveRecord::Base.transaction do - subscription.successive_skip_count = 0 - subscription.advance_actionable_date - - subscription.cancel! if subscription.pending_cancellation? - subscription.deactivate! if subscription.can_be_deactivated? - - if SolidusSubscriptions.configuration.clear_past_installments - subscription.installments.unfulfilled.actionable.each do |installment| - installment.update!(actionable_date: nil) - end + SolidusSubscriptions::Subscription + .where(installments: SolidusSubscriptions::Installment.actionable) + .or(SolidusSubscriptions::Subscription.actionable) + .distinct + .find_each do |subscription| + ProcessSubscriptionJob.perform_later(subscription) end - - subscription.installments.create!(actionable_date: Time.zone.now) - end - end - - def process_installment(installment) - ProcessInstallmentJob.perform_later(installment) end end end diff --git a/solidus_subscriptions.gemspec b/solidus_subscriptions.gemspec index 46fec6e..e5f01a6 100644 --- a/solidus_subscriptions.gemspec +++ b/solidus_subscriptions.gemspec @@ -33,7 +33,7 @@ Gem::Specification.new do |spec| spec.add_dependency 'httparty', '~> 0.18' spec.add_dependency 'i18n' spec.add_dependency 'solidus_core', ['>= 2.0.0', '< 3'] - spec.add_dependency 'solidus_support', '~> 0.5' + spec.add_dependency 'solidus_support', '~> 0.7' spec.add_dependency 'state_machines' spec.add_development_dependency 'rspec-activemodel-mocks' diff --git a/spec/decorators/models/solidus_subscriptions/spree/variant/auto_delete_from_subscriptions_spec.rb b/spec/decorators/models/solidus_subscriptions/spree/variant/auto_delete_from_subscriptions_spec.rb new file mode 100644 index 0000000..422e992 --- /dev/null +++ b/spec/decorators/models/solidus_subscriptions/spree/variant/auto_delete_from_subscriptions_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +RSpec.describe SolidusSubscriptions::Spree::Variant::AutoDeleteFromSubscriptions, type: :model do + subject { create(:variant, subscribable: true) } + + describe '.discard' do + it 'deletes itself from subscriptions' do + subscription = create(:subscription) + create(:subscription_line_item, subscription: subscription, subscribable: subject) + + expect { subject.discard }.to change { SolidusSubscriptions::LineItem.count }.by(-1) + end + end + + describe '.destroy' do + it 'deletes itself from subscriptions' do + subscription = create(:subscription) + create(:subscription_line_item, subscription: subscription, subscribable: subject) + + expect { subject.destroy }.to change { SolidusSubscriptions::LineItem.count }.by(-1) + end + end +end diff --git a/spec/jobs/solidus_subscriptions/process_subscription_job_spec.rb b/spec/jobs/solidus_subscriptions/process_subscription_job_spec.rb new file mode 100644 index 0000000..1a3cf6f --- /dev/null +++ b/spec/jobs/solidus_subscriptions/process_subscription_job_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +RSpec.describe SolidusSubscriptions::ProcessSubscriptionJob do + context 'when clear_past_installments is enabled' do + it 'voids the actionable date of the unfulfilled installments' do + stub_config(clear_past_installments: true) + subscription = create(:subscription) + unfulfilled_installment = create(:installment, :failed, subscription: subscription) + + described_class.perform_now(subscription) + + expect(unfulfilled_installment.reload.actionable_date).to eq(nil) + end + end + + context 'when the subscription is actionable' do + it 'resets the successive_skip_count' do + subscription = create(:subscription, :actionable, successive_skip_count: 3) + + described_class.perform_now(subscription) + + expect(subscription.reload.successive_skip_count).to eq(0) + end + + it 'creates a new installment' do + subscription = create(:subscription, :actionable) + + described_class.perform_now(subscription) + + expect(subscription.installments.count).to eq(1) + end + + it 'advances the actionable date' do + subscription = create(:subscription, :actionable) + subscription.update_columns(interval_length: 1, interval_units: 'week') + old_actionable_date = subscription.reload.actionable_date + + described_class.perform_now(subscription) + + expect(subscription.reload.actionable_date.to_date).to eq((old_actionable_date + 1.week).to_date) + end + end + + context 'when the subscription is pending cancellation' do + it 'cancels the subscription' do + subscription = create( + :subscription, + :actionable, + :pending_cancellation, + ) + described_class.perform_now(subscription) + + expect(subscription.reload.state).to eq('canceled') + end + end + + context 'when the subscription is pending of deactivation' do + it 'deactivates the subscription' do + subscription = create( + :subscription, + :actionable, + interval_units: 'week', + interval_length: 2, + end_date: 3.days.from_now, + ) + described_class.perform_now(subscription) + + expect(subscription.reload.state).to eq('inactive') + end + end + + it 'schedules all the subscription actionable installments for processing' do + subscription = create(:subscription, :actionable) + unfulfilled_installment = create(:installment, :failed, subscription: subscription) + + described_class.perform_now(subscription) + + new_installment = subscription.reload.installments.last + [unfulfilled_installment, new_installment].each do |installment| + expect(SolidusSubscriptions::ProcessInstallmentJob).to have_been_enqueued.with(installment) + end + end +end diff --git a/spec/lib/solidus_subscriptions/permission_sets/default_customer_spec.rb b/spec/lib/solidus_subscriptions/permission_sets/default_customer_spec.rb index 7865aab..febe268 100644 --- a/spec/lib/solidus_subscriptions/permission_sets/default_customer_spec.rb +++ b/spec/lib/solidus_subscriptions/permission_sets/default_customer_spec.rb @@ -10,7 +10,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).to be_able_to([:display, :update], subscription) + expect(ability).to be_able_to([:show, :display, :update], subscription) end it "is not allowed to display or update someone else's subscriptions" do @@ -21,7 +21,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).not_to be_able_to([:display, :update], subscription) + expect(ability).not_to be_able_to([:show, :display, :update], subscription) end it 'is allowed to display and update line items on their subscriptions' do @@ -33,7 +33,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).to be_able_to([:display, :update], line_item) + expect(ability).to be_able_to([:show, :display, :update], line_item) end it "is not allowed to display or update line items on someone else's subscriptions" do @@ -45,7 +45,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).not_to be_able_to([:display, :update], line_item) + expect(ability).not_to be_able_to([:show, :display, :update], line_item) end end @@ -57,7 +57,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).to be_able_to([:display, :update], subscription, subscription.guest_token) + expect(ability).to be_able_to([:show, :display, :update], subscription, subscription.guest_token) end it "is not allowed to display or update someone else's subscriptions" do @@ -67,7 +67,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).not_to be_able_to([:display, :update], subscription, 'invalid') + expect(ability).not_to be_able_to([:show, :display, :update], subscription, 'invalid') end it 'is allowed to display and update line items on their subscriptions' do @@ -78,7 +78,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).to be_able_to([:display, :update], line_item, subscription.guest_token) + expect(ability).to be_able_to([:show, :display, :update], line_item, subscription.guest_token) end it "is not allowed to display or update line items on someone else's subscriptions" do @@ -89,7 +89,7 @@ RSpec.describe SolidusSubscriptions::PermissionSets::DefaultCustomer do permission_set = described_class.new(ability) permission_set.activate! - expect(ability).not_to be_able_to([:display, :update], line_item, 'invalid') + expect(ability).not_to be_able_to([:show, :display, :update], line_item, 'invalid') end end end diff --git a/spec/lib/solidus_subscriptions/processor_spec.rb b/spec/lib/solidus_subscriptions/processor_spec.rb index e82c0e9..5deaba4 100644 --- a/spec/lib/solidus_subscriptions/processor_spec.rb +++ b/spec/lib/solidus_subscriptions/processor_spec.rb @@ -1,124 +1,32 @@ -RSpec.describe SolidusSubscriptions::Processor, :checkout do - shared_examples 'processes the subscription' do - it 'resets the successive_skip_count' do - subscription - subscription.update_columns(successive_skip_count: 3) +RSpec.describe SolidusSubscriptions::Processor do + it 'schedules the processing of actionable subscriptions' do + actionable_subscription = create(:subscription, :actionable) - described_class.run + described_class.run - expect(subscription.reload.successive_skip_count).to eq(0) - end - - context 'with clear_past_installments set to true' do - it 'clears any past unfulfilled installments' do - stub_config(clear_past_installments: true) - subscription - installment = create(:installment, :actionable, subscription: subscription) - - described_class.run - - expect(installment.reload.actionable_date).to eq(nil) - end - end - - context 'with clear_past_installments set to false' do - it 'does not clear any past unfulfilled installments' do - stub_config(clear_past_installments: false) - subscription - installment = create(:installment, :actionable, subscription: subscription) - - described_class.run - - expect(installment.reload.actionable_date).not_to be_nil - end - end - - it 'creates a new installment' do - subscription - - described_class.run - - expect(subscription.installments.count).to eq(1) - end - - it 'schedules the newly created installment for processing' do - subscription - - described_class.run - - expect(SolidusSubscriptions::ProcessInstallmentJob).to have_been_enqueued - .with(subscription.installments.last) - end - - it 'schedules other actionable installments for processing' do - actionable_installment = create(:installment, :actionable) - - described_class.run - - expect(SolidusSubscriptions::ProcessInstallmentJob).to have_been_enqueued - .with(actionable_installment) - end + expect(SolidusSubscriptions::ProcessSubscriptionJob).to have_been_enqueued + .with(actionable_subscription) end - shared_examples 'schedules the subscription for reprocessing' do - it 'advances the actionable_date' do - subscription - subscription.update_columns(interval_length: 1, interval_units: 'week') - old_actionable_date = subscription.actionable_date + it 'schedules the processing of non actionable subscriptions with actionable installments' do + subscription_with_actionable_installment = create( + :subscription, + actionable_date: Time.zone.today + 7.days, + installments: [create(:installment, :actionable)] + ) - described_class.run + described_class.run - expect(subscription.reload.actionable_date.to_date).to eq((old_actionable_date + 1.week).to_date) - end + expect(SolidusSubscriptions::ProcessSubscriptionJob).to have_been_enqueued + .with(subscription_with_actionable_installment) end - context 'with a regular subscription' do - let(:subscription) { create(:subscription, :actionable) } - - include_examples 'processes the subscription' - include_examples 'schedules the subscription for reprocessing' - end - - context 'with a subscription that is pending deactivation' do - let(:subscription) do - create( - :subscription, - :actionable, - interval_units: 'week', - interval_length: 2, - end_date: 3.days.from_now, - ) - end - - include_examples 'processes the subscription' - include_examples 'schedules the subscription for reprocessing' - - it 'deactivates the subscription' do - subscription - - described_class.run - - expect(subscription.reload.state).to eq('inactive') - end - end - - context 'with a subscription that is pending cancellation' do - let(:subscription) do - create( - :subscription, - :actionable, - :pending_cancellation, - ) - end - - include_examples 'processes the subscription' - - it 'cancels the subscription' do - subscription + it 'does not schedule the processing of non actionable subscriptions' do + non_actionable_subscription = create(:subscription, actionable_date: Time.zone.today + 14.days) - described_class.run + described_class.run - expect(subscription.reload.state).to eq('canceled') - end + expect(SolidusSubscriptions::ProcessSubscriptionJob).not_to have_been_enqueued + .with(non_actionable_subscription) end end diff --git a/spec/models/solidus_subscriptions/subscription_spec.rb b/spec/models/solidus_subscriptions/subscription_spec.rb index b9bfe56..c2fd4a4 100644 --- a/spec/models/solidus_subscriptions/subscription_spec.rb +++ b/spec/models/solidus_subscriptions/subscription_spec.rb @@ -762,4 +762,41 @@ RSpec.describe SolidusSubscriptions::Subscription, type: :model do end end end + + describe '#actionable?' do + context 'when the actionable date is nil' do + it 'is not actionable' do + subscription = build_stubbed(:subscription, actionable_date: nil) + + expect(subscription).not_to be_actionable + end + end + + context 'when the actionable date is in the future' do + it 'is not actionable' do + subscription = build_stubbed(:subscription, actionable_date: Time.zone.today + 5.days) + + expect(subscription).not_to be_actionable + end + end + + context 'when the state is either canceled or inactive' do + it 'is not actionable' do + canceled_subscription = build_stubbed(:subscription, :canceled) + inactive_subscription = build_stubbed(:subscription, :inactive) + + [canceled_subscription, inactive_subscription].each do |subscription| + expect(subscription).not_to be_actionable + end + end + end + + context 'when the active subscription actionable date is today or in the past' do + it 'is actionable' do + subscription = build_stubbed(:subscription, actionable_date: Time.zone.today) + + expect(subscription).to be_actionable + end + end + end end |