Skip to content

Commit

Permalink
Remove dummy_order and dummy_line_item helpers
Browse files Browse the repository at this point in the history
These helpers are dangerous: they provide a false sense of assurance
by making you think that the order and the line item they return can
be used to infer the correct total value of future subscription orders.

In reality, order calculation in Solidus is an extremely complex process
that may take a ton of different parameters into account, and each store
is better off calculating the subscription total with their custom logic
rather than this extension trying to provide a solution that works for
everyone.

In the future, we may provide a way to compute a subscription's total,
but for the time being it's better to remove the helpers altogether.
  • Loading branch information
aldesantis committed Jan 30, 2021
1 parent ce4edc0 commit 4717d24
Show file tree
Hide file tree
Showing 10 changed files with 1 addition and 222 deletions.
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,7 @@ This will associate a `SolidusSubscriptions::LineItem` to the line item being ad

The customer will not be charged for the subscription until it is processed. The subscription line
items should be shown to the user on the cart page by looping over
`Spree::Order#subscription_line_items`. `SolidusSubscriptions::LineItem#dummy_line_item` may be
useful to help you display the subscription line item with your existing cart infrastructure.
`Spree::Order#subscription_line_items`.

When the order is finalized, a `SolidusSubscriptions::Subscription` will be created for each group
of subscription line items which can be fulfilled by a single subscription.
Expand Down
7 changes: 0 additions & 7 deletions app/models/solidus_subscriptions/installment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,6 @@ class Installment < ApplicationRecord
unfulfilled.where("#{table_name}.actionable_date <= ?", Time.zone.today)
end)

# Get the builder for the subscription_line_item. This will be an
# object that can generate the appropriate line item for the subscribable
# object
#
# @return [SolidusSubscriptions::LineItemBuilder]
delegate :line_item_builder, to: :subscription

# Mark this installment as out of stock.
#
# @return [SolidusSubscriptions::InstallmentDetail] The record of the failed
Expand Down
29 changes: 0 additions & 29 deletions app/models/solidus_subscriptions/line_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,34 +38,5 @@ class LineItem < ApplicationRecord
validates :subscribable_id, presence: true
validates :quantity, numericality: { greater_than: 0 }
validates :interval_length, numericality: { greater_than: 0 }, unless: -> { subscription }

def as_json(**options)
options[:methods] ||= [:dummy_line_item]
super(options)
end

# Get a placeholder line item for calculating the values of future
# subscription orders. It is frozen and cannot be saved
def dummy_line_item
li = LineItemBuilder.new([self]).spree_line_items.first
return unless li

li.order = dummy_order
li.validate
li.freeze
end

private

# Get a placeholder order for calculating the values of future
# subscription orders. It is a frozen duplicate of the current order and
# cannot be saved
def dummy_order
order = spree_line_item ? spree_line_item.order.dup : ::Spree::Order.create
order.ship_address = subscription.shipping_address || subscription.user.ship_address if subscription
order.bill_address = subscription.billing_address || subscription.user.bill_address if subscription

order.freeze
end
end
end
9 changes: 0 additions & 9 deletions app/models/solidus_subscriptions/subscription.rb
Original file line number Diff line number Diff line change
Expand Up @@ -203,15 +203,6 @@ def advance_actionable_date
actionable_date
end

# Get the builder for the subscription_line_item. This will be an
# object that can generate the appropriate line item for the subscribable
# object
#
# @return [SolidusSubscriptions::LineItemBuilder]
def line_item_builder
LineItemBuilder.new(line_items)
end

# The state of the last attempt to process an installment associated to
# this subscription
#
Expand Down
1 change: 0 additions & 1 deletion lib/solidus_subscriptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
require 'solidus_subscriptions/churn_buster/subscription_serializer'
require 'solidus_subscriptions/churn_buster/order_serializer'
require 'solidus_subscriptions/checkout'
require 'solidus_subscriptions/line_item_builder'
require 'solidus_subscriptions/subscription_generator'
require 'solidus_subscriptions/subscription_line_item_builder'
require 'solidus_subscriptions/dispatcher/base'
Expand Down
36 changes: 0 additions & 36 deletions lib/solidus_subscriptions/line_item_builder.rb

This file was deleted.

33 changes: 0 additions & 33 deletions spec/lib/solidus_subscriptions/line_item_builder_spec.rb

This file was deleted.

9 changes: 0 additions & 9 deletions spec/models/solidus_subscriptions/installment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,6 @@

it { is_expected.to validate_presence_of :subscription }

describe '#line_item_builder' do
subject { installment.line_item_builder }

let(:line_items) { installment.subscription.line_items }

it { is_expected.to be_a SolidusSubscriptions::LineItemBuilder }
it { is_expected.to have_attributes(subscription_line_items: line_items) }
end

describe '#out_of_stock' do
subject { installment.out_of_stock }

Expand Down
86 changes: 0 additions & 86 deletions spec/models/solidus_subscriptions/line_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,90 +24,4 @@
expect(subject.from_now).to eq Date.parse("2016-10-22")
end
end

describe '#as_json' do
subject { line_item.as_json }

around { |e| Timecop.freeze { e.run } }

let(:line_item) { create(:subscription_line_item, :with_subscription) }

let(:expected_hash) do
hash = {
"id" => line_item.id,
"spree_line_item_id" => line_item.spree_line_item.id,
"subscription_id" => line_item.subscription_id,
"quantity" => line_item.quantity,
"end_date" => line_item.end_date,
"subscribable_id" => line_item.subscribable_id,
"created_at" => line_item.created_at,
"updated_at" => line_item.updated_at,
"interval_units" => line_item.interval_units,
"interval_length" => line_item.interval_length
}
Rails.gem_version >= Gem::Version.new('6.0.0') ? hash.as_json : hash
end

it 'includes the attribute values' do
expect(subject).to match a_hash_including(expected_hash)
end

it 'includes the dummy lineitem' do
expect(subject).to have_key('dummy_line_item')
end
end

describe '#dummy_line_item' do
subject { line_item.dummy_line_item }

let(:line_item) { create(:subscription_line_item, :with_subscription) }

it { is_expected.to be_a Spree::LineItem }
it { is_expected.to be_frozen }

it 'has the correct variant' do
expect(subject.variant_id).to eq line_item.subscribable_id
end

context 'with no spree line item' do
let(:line_item) { create(:subscription_line_item, :with_subscription, spree_line_item: nil) }

it { is_expected.to be_a Spree::LineItem }
it { is_expected.to be_frozen }

it 'has the correct variant' do
expect(subject.variant_id).to eq line_item.subscribable_id
end
end

context 'with an associated subscription' do
context 'the associated subscription has a shipping address' do
let(:line_item) do
create(:subscription_line_item, :with_subscription, subscription_traits: [:with_shipping_address])
end

it 'uses the subscription shipping address' do
expect(subject.order.ship_address).to eq line_item.subscription.shipping_address
end

it 'uses the subscription users billing address' do
expect(subject.order.bill_address).to eq line_item.subscription.user.bill_address
end
end

context 'the associated subscription has a billing address' do
let(:line_item) do
create(:subscription_line_item, :with_subscription, subscription_traits: [:with_billing_address])
end

it 'uses the subscription users shipping address' do
expect(subject.order.ship_address).to eq line_item.subscription.user.ship_address
end

it 'uses the subscription billing address' do
expect(subject.order.bill_address).to eq line_item.subscription.billing_address
end
end
end
end
end
10 changes: 0 additions & 10 deletions spec/models/solidus_subscriptions/subscription_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -348,16 +348,6 @@
end
end

describe '#line_item_builder' do
subject { subscription.line_item_builder }

let(:subscription) { create :subscription, :with_line_item }
let(:line_items) { subscription.line_items }

it { is_expected.to be_a SolidusSubscriptions::LineItemBuilder }
it { is_expected.to have_attributes(subscription_line_items: line_items) }
end

describe '#processing_state' do
subject { subscription.processing_state }

Expand Down

0 comments on commit 4717d24

Please sign in to comment.