diff --git a/Gemfile b/Gemfile index 1a975795edd..79b8856db57 100644 --- a/Gemfile +++ b/Gemfile @@ -34,7 +34,6 @@ gem 'puma', '< 7', require: false gem 'i18n-tasks', '~> 0.9', require: false gem 'rspec_junit_formatter', require: false gem 'yard', require: false -gem 'db-query-matchers', require: false if ENV['GITHUB_ACTIONS'] gem "rspec-github", "~> 3.0", require: false diff --git a/admin/spec/spec_helper.rb b/admin/spec/spec_helper.rb index 66114310c67..5d0f76e7100 100644 --- a/admin/spec/spec_helper.rb +++ b/admin/spec/spec_helper.rb @@ -78,14 +78,6 @@ require 'axe-rspec' require 'axe-capybara' -# DB Query Matchers -require "db-query-matchers" -DBQueryMatchers.configure do |config| - config.ignores = [/SHOW TABLES LIKE/] - config.ignore_cached = true - config.schemaless = true -end - RSpec.configure do |config| if ENV["GITHUB_ACTIONS"] require "rspec/github" diff --git a/core/app/models/spree/in_memory_order_updater.rb b/core/app/models/spree/in_memory_order_updater.rb new file mode 100644 index 00000000000..57b1dac32ca --- /dev/null +++ b/core/app/models/spree/in_memory_order_updater.rb @@ -0,0 +1,262 @@ +# frozen_string_literal: true + +require 'spree/manipulative_query_monitor' + +module Spree + class InMemoryOrderUpdater + attr_reader :order + + # logs a warning when a manipulative query is made when the persist flag is set to false + class_attribute :log_manipulative_queries + self.log_manipulative_queries = true + + delegate :payments, :line_items, :adjustments, :all_adjustments, :shipments, :quantity, to: :order + + def initialize(order) + @order = order + end + + # This is a multi-purpose method for processing logic related to changes in the Order. + # It is meant to be called from various observers so that the Order is aware of changes + # that affect totals and other values stored in the Order. + # + # This method should never do anything to the Order that results in a save call on the + # object with callbacks (otherwise you will end up in an infinite recursion as the + # associations try to save and then in turn try to call +update!+ again.) + def recalculate(persist: true) + monitor = + if log_manipulative_queries + Spree::ManipulativeQueryMonitor + else + proc { |&block| block.call } + end + + order.transaction do + monitor.call do + recalculate_line_item_prices + recalculate_item_count + assign_shipment_amounts + end + + if persist + update_totals(persist:) + else + monitor.call do + update_totals(persist:) + end + end + + monitor.call do + if order.completed? + recalculate_payment_state + recalculate_shipment_state + end + end + + Spree::Bus.publish(:order_recalculated, order:) + + persist_totals if persist + end + end + alias_method :update, :recalculate + deprecate update: :recalculate, deprecator: Spree.deprecator + + # Recalculates the state on all of them shipments, then recalculates the + # +shipment_state+ attribute according to the following logic: + # + # shipped when all Shipments are in the "shipped" state + # partial when at least one Shipment has a state of "shipped" and there is another Shipment with a state other than "shipped" + # or there are InventoryUnits associated with the order that have a state of "sold" but are not associated with a Shipment. + # ready when all Shipments are in the "ready" state + # backorder when there is backordered inventory associated with an order + # pending when all Shipments are in the "pending" state + # + # The +shipment_state+ value helps with reporting, etc. since it provides a quick and easy way to locate Orders needing attention. + def recalculate_shipment_state + shipments.each(&:recalculate_state) + + order.shipment_state = determine_shipment_state + order.shipment_state + end + alias_method :update_shipment_state, :recalculate_shipment_state + deprecate update_shipment_state: :recalculate_shipment_state, deprecator: Spree.deprecator + + # Recalculates the +payment_state+ attribute according to the following logic: + # + # paid when +payment_total+ is equal to +total+ + # balance_due when +payment_total+ is less than +total+ + # credit_owed when +payment_total+ is greater than +total+ + # failed when most recent payment is in the failed state + # void when the order has been canceled and the payment total is 0 + # + # The +payment_state+ value helps with reporting, etc. since it provides a quick and easy way to locate Orders needing attention. + def recalculate_payment_state + order.payment_state = determine_payment_state + order.payment_state + end + alias_method :update_payment_state, :recalculate_payment_state + deprecate update_payment_state: :recalculate_payment_state, deprecator: Spree.deprecator + + private + + def determine_payment_state + if payments.present? && payments.valid.empty? && order.outstanding_balance != 0 + 'failed' + elsif order.state == 'canceled' && order.payment_total.zero? + 'void' + elsif order.outstanding_balance > 0 + 'balance_due' + elsif order.outstanding_balance < 0 + 'credit_owed' + else + # outstanding_balance == 0 + 'paid' + end + end + + def determine_shipment_state + if order.backordered? + 'backorder' + else + # get all the shipment states for this order + shipment_states = shipments.states + if shipment_states.size > 1 + # multiple shiment states means it's most likely partially shipped + 'partial' + else + # will return nil if no shipments are found + shipment_states.first + end + end + end + + # This will update and select the best promotion adjustment, update tax + # adjustments, update cancellation adjustments, and then update the total + # fields (promo_total, included_tax_total, additional_tax_total, and + # adjustment_total) on the item. + # @return [void] + def update_adjustments(persist:) + # Promotion adjustments must be applied first, then tax adjustments. + # This fits the criteria for VAT tax as outlined here: + # http://www.hmrc.gov.uk/vat/managing/charging/discounts-etc.htm#1 + # It also fits the criteria for sales tax as outlined here: + # http://www.boe.ca.gov/formspubs/pub113/ + update_promotions(persist:) + update_tax_adjustments + assign_item_totals + end + + # Updates the following Order total values: + # + # +payment_total+ The total value of all finalized Payments (NOTE: non-finalized Payments are excluded) + # +item_total+ The total value of all LineItems + # +adjustment_total+ The total value of all adjustments (promotions, credits, etc.) + # +promo_total+ The total value of all promotion adjustments + # +total+ The so-called "order total." This is equivalent to +item_total+ plus +adjustment_total+. + def update_totals(persist:) + recalculate_payment_total + recalculate_item_total + recalculate_shipment_total + update_adjustment_total(persist:) + end + + def assign_shipment_amounts + shipments.each(&:assign_amounts) + end + + def update_adjustment_total(persist:) + update_adjustments(persist:) + + all_items = line_items + shipments + # Ignore any adjustments that have been marked for destruction in our + # calculations. They'll get removed when/if we persist the order. + valid_adjustments = adjustments.reject(&:marked_for_destruction?) + order_tax_adjustments = valid_adjustments.select(&:tax?) + + order.adjustment_total = all_items.sum(&:adjustment_total) + valid_adjustments.sum(&:amount) + order.included_tax_total = all_items.sum(&:included_tax_total) + order_tax_adjustments.select(&:included?).sum(&:amount) + order.additional_tax_total = all_items.sum(&:additional_tax_total) + order_tax_adjustments.reject(&:included?).sum(&:amount) + + recalculate_order_total + end + + def update_promotions(persist:) + Spree::Config.promotions.order_adjuster_class.new(order).call(persist:) + end + + def update_tax_adjustments + Spree::Config.tax_adjuster_class.new(order).adjust! + end + + def update_cancellations + end + deprecate :update_cancellations, deprecator: Spree.deprecator + + def assign_item_totals + [*line_items, *shipments].each do |item| + Spree::Config.item_total_class.new(item).recalculate! + + next unless item.changed? + + item.assign_attributes( + promo_total: item.promo_total, + included_tax_total: item.included_tax_total, + additional_tax_total: item.additional_tax_total, + adjustment_total: item.adjustment_total + ) + end + end + + def recalculate_payment_total + order.payment_total = payments.completed.includes(:refunds).sum { |payment| payment.amount - payment.refunds.sum(:amount) } + end + + def recalculate_shipment_total + order.shipment_total = shipments.to_a.sum(&:cost) + recalculate_order_total + end + + def recalculate_order_total + order.total = order.item_total + order.shipment_total + order.adjustment_total + end + + def recalculate_item_count + order.item_count = line_items.to_a.sum(&:quantity) + end + + def recalculate_item_total + order.item_total = line_items.to_a.sum(&:amount) + recalculate_order_total + end + + def recalculate_line_item_prices + if Spree::Config.recalculate_cart_prices + line_items.each(&:recalculate_price) + end + end + + def persist_totals + shipments.each(&:persist_amounts) + assign_item_totals + log_state_change("payment") + log_state_change("shipment") + order.save! + end + + def log_state_change(name) + state = "#{name}_state" + previous_state, current_state = order.changes[state] + + if previous_state != current_state + # Enqueue the job to track this state change + StateChangeTrackingJob.perform_later( + order, + previous_state, + current_state, + Time.current, + name + ) + end + end + end +end diff --git a/core/app/models/spree/null_promotion_adjuster.rb b/core/app/models/spree/null_promotion_adjuster.rb index daebd91f08e..dfb4f25e3ff 100644 --- a/core/app/models/spree/null_promotion_adjuster.rb +++ b/core/app/models/spree/null_promotion_adjuster.rb @@ -6,7 +6,7 @@ def initialize(order) @order = order end - def call + def call(persist: true) # rubocop:disable Lint/UnusedMethodArgument @order end end diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 37656b070db..d1863430ed4 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -804,7 +804,7 @@ def ensure_inventory_units end def ensure_promotions_eligible - Spree::Config.promotions.order_adjuster_class.new(self).call + Spree::Config.promotions.order_adjuster_class.new(self).call(persist: false) if promo_total_changed? restart_checkout_flow diff --git a/core/app/models/spree/shipment.rb b/core/app/models/spree/shipment.rb index 9f4758782ad..9d2bca151a0 100644 --- a/core/app/models/spree/shipment.rb +++ b/core/app/models/spree/shipment.rb @@ -278,14 +278,21 @@ def tracking_url end def update_amounts - if selected_shipping_rate - self.cost = selected_shipping_rate.cost - if changed? - update_columns( - cost:, - updated_at: Time.current - ) - end + assign_amounts + persist_amounts + end + + def assign_amounts + return unless selected_shipping_rate + self.cost = selected_shipping_rate.cost + end + + def persist_amounts + if cost_changed? + update_columns( + cost:, + updated_at: Time.current + ) end end diff --git a/core/config/initializers/db_query_matchers.rb b/core/config/initializers/db_query_matchers.rb new file mode 100644 index 00000000000..093cd1b168c --- /dev/null +++ b/core/config/initializers/db_query_matchers.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require "db-query-matchers" + +DBQueryMatchers.configure do |config| + config.ignores = [/SHOW TABLES LIKE/] + config.ignore_cached = true + config.schemaless = true +end diff --git a/core/lib/spree/manipulative_query_monitor.rb b/core/lib/spree/manipulative_query_monitor.rb new file mode 100644 index 00000000000..b41eef36ad1 --- /dev/null +++ b/core/lib/spree/manipulative_query_monitor.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Spree + class ManipulativeQueryMonitor + def self.call(&block) + counter = ::DBQueryMatchers::QueryCounter.new({ matches: [/^\ *(INSERT|UPDATE|DELETE\ FROM)/] }) + ActiveSupport::Notifications.subscribed(counter.to_proc, + "sql.active_record", + &block) + if counter.count > 0 + message = "Detected #{counter.count} manipulative queries. #{counter.log.join(', ')}\n" + + message += caller.select{ |line| line.include?(Rails.root.to_s) || line.include?('solidus') }.join("\n") + + Rails.logger.warn(message) + end + end + end +end diff --git a/core/solidus_core.gemspec b/core/solidus_core.gemspec index b61e5a85144..260d8e8fcac 100644 --- a/core/solidus_core.gemspec +++ b/core/solidus_core.gemspec @@ -38,6 +38,7 @@ Gem::Specification.new do |s| s.add_dependency 'awesome_nested_set', ['~> 3.3', '>= 3.7.0'] s.add_dependency 'cancancan', ['>= 2.2', '< 4.0'] s.add_dependency 'carmen', '~> 1.1.0' + s.add_dependency 'db-query-matchers', '~> 0.14' s.add_dependency 'discard', '~> 1.0' s.add_dependency 'friendly_id', '~> 5.0' s.add_dependency 'image_processing', '~> 1.10' diff --git a/core/spec/lib/spree/manipulative_query_monitor_spec.rb b/core/spec/lib/spree/manipulative_query_monitor_spec.rb new file mode 100644 index 00000000000..33657cf0eed --- /dev/null +++ b/core/spec/lib/spree/manipulative_query_monitor_spec.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'spree/manipulative_query_monitor' + +RSpec.describe Spree::ManipulativeQueryMonitor do + describe ".call" do + it "logs when a create query is detected" do + allow(Rails.logger).to receive(:warn) + + described_class.call do + create :user + end + + expect(Rails.logger).to have_received(:warn).with(/Detected 1 manipulative queries.*INSERT.*/) + expect(Rails.logger).to have_received(:warn).with(/.*manipulative_query_monitor_spec.rb.*/) + end + + it "does not log when a select query is not detected" do + allow(Rails.logger).to receive(:warn) + + user = create :user + + described_class.call do + user.reload + end + + expect(Rails.logger).to_not have_received(:warn) + end + + it "logs when an update query is detected" do + allow(Rails.logger).to receive(:warn) + + user = create :user + + described_class.call do + user.update(email: "snowball@example.com") + end + + expect(Rails.logger).to have_received(:warn).with(/Detected 1 manipulative queries.*UPDATE.*/) + expect(Rails.logger).to have_received(:warn).with(/.*manipulative_query_monitor_spec.rb.*/) + end + + it "logs when a delete query is detected" do + allow(Rails.logger).to receive(:warn) + + user = create :user + + described_class.call do + user.delete + end + + expect(Rails.logger).to have_received(:warn).with(/Detected 1 manipulative queries.*DELETE FROM.*/) + expect(Rails.logger).to have_received(:warn).with(/.*manipulative_query_monitor_spec.rb.*/) + end + end +end diff --git a/core/spec/models/spree/in_memory_order_updater_spec.rb b/core/spec/models/spree/in_memory_order_updater_spec.rb new file mode 100644 index 00000000000..8ed4a80ddd4 --- /dev/null +++ b/core/spec/models/spree/in_memory_order_updater_spec.rb @@ -0,0 +1,646 @@ +# frozen_string_literal: true + +require 'rails_helper' + +module Spree + RSpec.describe InMemoryOrderUpdater, type: :model do + let!(:store) { create :store } + let(:order) { Spree::Order.create } + let(:updater) { described_class.new(order) } + + describe "#recalculate" do + subject { updater.recalculate(persist:) } + + let(:new_store) { create(:store) } + + context "when the persist flag is set to 'false'" do + let(:persist) { false } + + it "does not persist changes to order" do + order.store = new_store + + expect { + subject + }.not_to make_database_queries(manipulative: true) + + expect(order.store).to eq new_store + expect(order.reload.store).not_to eq new_store + end + + it "will log zero manipulative queries" do + allow(Rails.logger).to receive(:warn) + + subject + + expect(Rails.logger).not_to have_received(:warn) + end + + it "does not persist changes to the item count" do + order.line_items << build(:line_item) + + expect { + subject + }.not_to make_database_queries(manipulative: true) + + expect(order.item_count).to eq 1 + expect(order.reload.item_count).to eq 0 + end + + context 'when a shipment is attached to the order' do + let(:shipment) { create(:shipment) } + + before do + order.shipments << shipment + end + + it 'does not make manipulative database queries' do + expect { + subject + }.not_to make_database_queries(manipulative: true) + end + + context 'when the shipment has a selected shipping rate' do + let(:shipment) { create(:shipment, shipping_rates: [build(:shipping_rate, selected: true)]) } + + it 'does not make manipulative database queries' do + expect { + subject + }.not_to make_database_queries(manipulative: true) + end + end + end + end + + context "when the persist flag is set to 'true'" do + let(:persist) { true } + + it "persists any changes to order" do + order.store = new_store + + expect { + subject + }.to make_database_queries(manipulative: true) + + expect(order.reload.store).to eq new_store + end + + it "will not log manipulative queries" do + allow(Rails.logger).to receive(:warn) + order.store = new_store + + subject + + expect(Rails.logger).to_not have_received(:warn) + end + + context 'and manipulative query logging is disabled' do + around do |example| + Spree::InMemoryOrderUpdater.log_manipulative_queries = false + + example.run + + Spree::InMemoryOrderUpdater.log_manipulative_queries = true + end + + it 'will not log manipulative queries' do + allow(Rails.logger).to receive(:warn) + order.store = new_store + + subject + + expect(Rails.logger).not_to have_received(:warn) + end + end + + context "when the payment state changes" do + let(:order) { create(:completed_order_with_pending_payment) } + + it "logs a state change for the payment" do + expect { order.payments.first.update(state: "completed") } + .to enqueue_job(Spree::StateChangeTrackingJob) + .with(order, "balance_due", "paid", a_kind_of(Time), "payment") + .once + end + end + + context "when the shipment state changes" do + let(:order) { create(:order_ready_to_ship) } + + it "logs a state change for the shipment" do + expect { order.shipments.first.ship! } + .to enqueue_job(Spree::StateChangeTrackingJob) + .with(order, "ready", "shipped", a_kind_of(Time), "shipment") + .once + end + end + end + end + + context "order totals" do + before do + 2.times do + create(:line_item, order: order, price: 10) + end + end + + context 'with refund' do + it "updates payment totals" do + create(:payment_with_refund, order: order, amount: 33.25, refund_amount: 3) + updater.recalculate + expect(order.payment_total).to eq(30.25) + end + end + + it "update item total" do + expect { + updater.recalculate + }.to change { order.item_total }.to 20 + end + + it "respects changes to item totals" do + updater.recalculate + order.line_items.first.update(quantity: 2) + + expect { + updater.recalculate + }.to change { order.item_total }.by 10 + end + + it "update shipment total" do + create(:shipment, order: order, cost: 10) + expect { + updater.recalculate + }.to change { order.shipment_total }.to 10 + end + + context 'with a source-less line item adjustment' do + let(:line_item) { create(:line_item, order: order, price: 10) } + before do + create(:adjustment, source: nil, adjustable: line_item, order: order, amount: -5) + end + + it "updates the line item total" do + expect { updater.recalculate }.to change { line_item.reload.adjustment_total }.from(0).to(-5) + end + end + + it "update order adjustments" do + create(:adjustment, adjustable: order, order: order, source: nil, amount: 10) + + expect { + updater.recalculate + }.to change { + order.adjustment_total + }.from(0).to(10) + end + end + + describe 'promotion recalculation' do + it "calls the Promotion Adjustments Recalculator" do + adjuster = double(:call) + expect(Spree::Config.promotions.order_adjuster_class).to receive(:new).and_return(adjuster) + expect(adjuster).to receive(:call).with(persist: true) + updater.recalculate + end + end + + describe 'tax recalculation' do + let(:tax_category) { create(:tax_category) } + let(:ship_address) { create(:address, state: new_york) } + let(:new_york) { create(:state, state_code: "NY") } + let(:tax_zone) { create(:zone, states: [new_york]) } + + let!(:tax_rate) do + create( + :tax_rate, + name: "New York Sales Tax", + tax_categories: [tax_category], + zone: tax_zone, + included_in_price: false, + amount: 0.1 + ) + end + + let(:order) do + create( + :order_with_line_items, + line_items_attributes: [{ price: 10, variant: variant }], + ship_address: ship_address, + ) + end + let(:line_item) { order.line_items[0] } + + let(:variant) { create(:variant, tax_category:) } + + context 'when the item quantity has changed' do + before do + line_item.update!(quantity: 2) + end + + it 'updates the additional_tax_total' do + expect { + updater.recalculate + }.to change { + line_item.additional_tax_total + }.from(1).to(2) + end + end + + context 'when the address has changed to a different state' do + let(:new_shipping_address) { create(:address) } + + before do + order.ship_address = new_shipping_address + end + + it 'removes the old taxes' do + expect { + updater.recalculate + }.to change { + order.all_adjustments.tax.count + }.from(1).to(0) + + expect(order.additional_tax_total).to eq 0 + expect(order.adjustment_total).to eq 0 + end + end + + context "with an order-level tax adjustment" do + let(:colorado) { create(:state, state_code: "CO") } + let(:colorado_tax_zone) { create(:zone, states: [colorado]) } + let(:ship_address) { create(:address, state: colorado) } + + let!(:colorado_delivery_fee) do + create( + :tax_rate, + amount: 0.27, + calculator: Spree::Calculator::FlatFee.new, + level: "order", + name: "Colorado Delivery Fee", + tax_categories: [tax_category], + zone: colorado_tax_zone + ) + end + + before { updater.recalculate } + + it "updates the order-level tax adjustment" do + expect { + order.ship_address = create(:address) + updater.recalculate + }.to change { order.additional_tax_total }.from(0.27).to(0). + and change { order.adjustment_total }.from(0.27).to(0) + end + + it "deletes the order-level tax adjustments when it persists the order" do + expect { + order.ship_address = create(:address) + updater.recalculate + }.to change { order.all_adjustments.count }.from(1).to(0) + end + end + + context 'with a custom tax_calculator_class' do + let(:custom_calculator_class) { double } + let(:custom_calculator_instance) { double } + + before do + order # generate this first so we can expect it + stub_spree_preferences(tax_calculator_class: custom_calculator_class) + + allow(custom_calculator_class).to receive(:new).and_return(custom_calculator_instance) + allow(custom_calculator_instance).to receive(:calculate).and_return( + Spree::Tax::OrderTax.new( + order_id: order.id, + order_taxes: [ + Spree::Tax::ItemTax.new( + label: "Delivery Fee", + tax_rate:, + amount: 2.60, + included_in_price: false + ) + ], + line_item_taxes: [ + Spree::Tax::ItemTax.new( + item_id: line_item.id, + label: "Item Tax", + tax_rate:, + amount: 1.40, + included_in_price: false + ) + ], + shipment_taxes: [] + ) + ) + end + + it 'uses the configured class' do + updater.recalculate + + expect(custom_calculator_class).to have_received(:new).with(order).at_least(:once) + expect(custom_calculator_instance).to have_received(:calculate).at_least(:once) + end + + it 'updates the aggregate columns' do + expect { + updater.recalculate + }.to change { order.reload.additional_tax_total }.to(4.00) + .and change { order.reload.adjustment_total }.to(4.00) + end + end + end + + describe "price recalculation" do + let(:variant) { create(:variant, price: 98) } + let!(:order) { create(:order_with_line_items, line_items_attributes: [{ variant:, price: 98 }]) } + + subject { updater.recalculate } + + before do + variant.price = 100 + variant.save! + order.reload + end + + context "when recalculate_cart_prices is true" do + before do + stub_spree_preferences(recalculate_cart_prices: true) + end + + it "sets prices to the currently active price" do + expect { subject }.to change { order.line_items.first.price }.from(98).to(100) + end + end + + context "when recalculate_cart_prices is false" do + before do + stub_spree_preferences(recalculate_cart_prices: false) + end + + it "does not recalculate prices" do + expect { subject }.not_to change { order.line_items.first.price }.from(98) + end + end + end + + context "updating shipment state" do + let(:order) { create :order_with_line_items } + + context "when an inventory unit is backordered" do + before do + create :inventory_unit, order: order, state: "backordered" + end + + it "has a 'backorder' shipment state" do + updater.recalculate_shipment_state + expect(order.shipment_state).to eq('backorder') + end + end + + context "when there are no shipments" do + before do + order.shipments.destroy_all + end + + it "has no shipment state" do + updater.recalculate_shipment_state + expect(order.shipment_state).to be_nil + end + end + + context "when the order's shipments are 'shipped'" do + before do + inventory_unit = order.inventory_units.first + inventory_unit.update!(state: "shipped") + order.shipments.first.update!(state: "shipped") + end + + it "is shipped" do + updater.recalculate_shipment_state + expect(order.shipment_state).to eq("shipped") + end + end + + context "when the order's shipments are 'ready'" do + let(:order) { create :order_ready_to_ship, shipment_state: "pending" } + + it "is ready" do + updater.recalculate_shipment_state + expect(order.shipment_state).to eq("ready") + end + end + + context "when the order's shipments are 'pending'" do + it "is pending" do + create(:shipment, order: order, state: "pending") + updater.recalculate_shipment_state + expect(order.shipment_state).to eq("pending") + end + end + + context "when some of the order's shipments are shipped" do + before do + inventory_unit = create :inventory_unit, order: order, state: "shipped" + inventory_unit.shipment.update!(state: "shipped") + + create :inventory_unit, order: order, state: "on_hand" + end + + it "has a shipment state of 'partial'" do + updater.recalculate_shipment_state + expect(order.shipment_state).to eq('partial') + end + end + end + + context "updating payment state" do + let(:order) { build(:order) } + before { allow(order).to receive(:refund_total).and_return(0) } + + context 'no valid payments with non-zero order total' do + it "is failed" do + create(:payment, order: order, state: 'invalid') + order.total = 1 + order.payment_total = 0 + + updater.recalculate_payment_state + expect(order.payment_state).to eq('failed') + end + end + + context 'invalid payments are present but order total is zero' do + it 'is paid' do + order.payments << Spree::Payment.new(state: 'invalid') + order.total = 0 + order.payment_total = 0 + + expect { + updater.recalculate_payment_state + }.to change { order.payment_state }.to 'paid' + end + end + + context "payment total is greater than order total" do + it "is credit_owed" do + order.payment_total = 2 + order.total = 1 + + expect { + updater.recalculate_payment_state + }.to change { order.payment_state }.to 'credit_owed' + end + end + + context "order total is greater than payment total" do + it "is balance_due" do + order.payment_total = 1 + order.total = 2 + + expect { + updater.recalculate_payment_state + }.to change { order.payment_state }.to 'balance_due' + end + end + + context "order total equals payment total" do + it "is paid" do + order.payment_total = 30 + order.total = 30 + + expect { + updater.recalculate_payment_state + }.to change { order.payment_state }.to 'paid' + end + end + + context "order is canceled" do + before do + order.state = 'canceled' + end + + context "and is still unpaid" do + it "is void" do + order.payment_total = 0 + order.total = 30 + expect { + updater.recalculate_payment_state + }.to change { order.payment_state }.to 'void' + end + end + + context "and is paid" do + it "is credit_owed" do + order.payment_total = 30 + order.total = 30 + create(:payment, order: order, state: 'completed', amount: 30) + expect { + updater.recalculate_payment_state + }.to change { order.payment_state }.to 'credit_owed' + end + end + + context "and payment is refunded" do + it "is void" do + order.payment_total = 0 + order.total = 30 + expect { + updater.recalculate_payment_state + }.to change { order.payment_state }.to 'void' + end + end + end + end + + context "completed order" do + before { allow(order).to receive_messages completed?: true } + + it "updates payment state" do + expect(updater).to receive(:recalculate_payment_state) + updater.recalculate + end + + it "updates shipment state" do + expect(updater).to receive(:recalculate_shipment_state) + updater.recalculate + end + + context 'with a shipment' do + before { create(:shipment, order: order) } + let(:shipment){ order.shipments[0] } + + it "updates each shipment" do + expect(shipment).to receive(:recalculate_state) + updater.recalculate + end + + it "updates the shipment amount" do + expect(shipment).to receive(:assign_amounts) + expect(shipment).to receive(:persist_amounts) + updater.recalculate + end + end + end + + context "incompleted order" do + before { allow(order).to receive_messages completed?: false } + + it "doesnt update payment state" do + expect(updater).not_to receive(:recalculate_payment_state) + updater.recalculate + end + + it "doesnt update shipment state" do + expect(updater).not_to receive(:recalculate_shipment_state) + updater.recalculate + end + + it "doesnt update each shipment" do + shipment = stub_model(Spree::Shipment) + order.shipments = [shipment] + allow(order.shipments).to receive_messages(states: [], ready: [], pending: [], shipped: []) + allow(updater).to receive(:update_totals) # Otherwise this gets called and causes a scene + expect(updater).not_to receive(:recalculate_shipment_state) + updater.recalculate + end + end + + context "with item with no adjustment and incorrect totals" do + let!(:line_item) { create(:line_item, order: order, price: 10) } + + it "updates the totals" do + line_item.update!(adjustment_total: 100) + expect { + updater.recalculate + }.to change { line_item.reload.adjustment_total }.from(100).to(0) + end + end + + context "with 'order_recalculated' event subscription" do + let(:item) { spy('object') } + let(:bus) { Spree::Bus } + + let!(:subscription) do + bus.subscribe :order_recalculated do + item.do_something + end + end + + after { bus.unsubscribe subscription } + + it "fires the 'order_recalculated' event" do + updater.recalculate + + expect(item).to have_received(:do_something) + end + end + + context "with invalid associated objects" do + let(:order) { Spree::Order.create(ship_address: Spree::Address.new) } + subject { updater.recalculate } + + it "raises because of the invalid object" do + expect { subject }.to raise_error(ActiveRecord::RecordInvalid) + end + end + end +end diff --git a/core/spec/models/spree/shipment_spec.rb b/core/spec/models/spree/shipment_spec.rb index 485a73e4c21..aa9fec9c38e 100644 --- a/core/spec/models/spree/shipment_spec.rb +++ b/core/spec/models/spree/shipment_spec.rb @@ -588,25 +588,47 @@ end describe "#update_amounts" do - let(:shipment) { create(:shipment, cost: 3) } + subject { shipment.update_amounts } + + let(:shipment) { create(:shipment, cost: 1) } context 'when the selected shipping rate cost is different than the current shipment cost' do - before { shipment.selected_shipping_rate.update!(cost: 5) } + before { shipment.selected_shipping_rate.update!(cost: 999) } - it "updates the shipments cost" do + it "changes and persists the shipments cost" do expect { - shipment.update_amounts - }.to change { shipment.reload.cost }.to(5) + subject + }.to change { shipment.reload.cost }.to(999) end - it 'changes the updated_at column' do + it 'changes and persists the updated_at column' do expect { - shipment.update_amounts + subject }.to change { shipment.reload.updated_at } end end end + describe "#assign_amounts" do + subject { shipment.assign_amounts } + + let(:shipment) { create(:shipment, cost: 1) } + + before { shipment.selected_shipping_rate.update!(cost: 999) } + + it 'does not perform any database writes' do + expect { + subject + }.not_to make_database_queries(manipulative: true) + end + + it "changes but does not persist the shipments cost" do + subject + expect(shipment.cost).to eq 999 + expect(shipment.reload.cost).to eq 1 + end + end + context "changes shipping rate via general update" do let!(:ship_address) { create(:address) } let!(:tax_zone) { create(:global_zone) } # will include the above address @@ -967,6 +989,10 @@ it_behaves_like "customer and admin metadata fields: storage and validation", :shipment describe "state change tracking" do + # Ensure that we're only testing tracking jobs created by the change in + # state. + before { shipment } + it "enqueues a StateChangeTrackingJob when state changes" do expect { shipment.update!(state: 'shipped') @@ -974,7 +1000,7 @@ shipment, 'pending', 'shipped', - kind_of(Time), + be_within(1.second).of(Time.current), 'shipment' ) end @@ -985,20 +1011,6 @@ }.not_to have_enqueued_job(Spree::StateChangeTrackingJob) end - it "captures the transition timestamp accurately" do - before_time = Time.current - - shipment.update!(state: 'shipped') - - # Check that a job was enqueued with a timestamp close to when we made the change - expect(Spree::StateChangeTrackingJob).to have_been_enqueued.with do |shipment_id, prev_state, next_state, timestamp| - expect(shipment_id).to eq(shipment.id) - expect(prev_state).to eq('pending') - expect(next_state).to eq('shipped') - expect(timestamp).to be_within(1.second).of(before_time) - end - end - it "creates multiple state transitions" do clear_enqueued_jobs @@ -1012,7 +1024,7 @@ perform_enqueued_jobs do expect { shipment.update!(state: 'shipped') - }.to change(Spree::StateChange, :count).by(1) + }.to change(shipment.state_changes, :count).by(1) end state_change = Spree::StateChange.last diff --git a/legacy_promotions/app/models/spree/promotion/order_adjustments_recalculator.rb b/legacy_promotions/app/models/spree/promotion/order_adjustments_recalculator.rb index ca01e9c095a..847a4a0ae51 100644 --- a/legacy_promotions/app/models/spree/promotion/order_adjustments_recalculator.rb +++ b/legacy_promotions/app/models/spree/promotion/order_adjustments_recalculator.rb @@ -13,12 +13,12 @@ def initialize(order) @order = order end - def call + def call(persist: true) all_items = line_items + shipments all_items.each do |item| promotion_adjustments = item.adjustments.select(&:promotion?) - promotion_adjustments.each { |adjustment| recalculate(adjustment) } + promotion_adjustments.each { |adjustment| recalculate(adjustment, persist:) } Spree::Config.promotions.promotion_chooser_class.new(promotion_adjustments).update item.promo_total = promotion_adjustments.select(&:eligible?).sum(&:amount) @@ -28,7 +28,7 @@ def call # in #update_adjustment_total since they include the totals from the order's # line items and/or shipments. order_promotion_adjustments = order.adjustments.select(&:promotion?) - order_promotion_adjustments.each { |adjustment| recalculate(adjustment) } + order_promotion_adjustments.each { |adjustment| recalculate(adjustment, persist:) } Spree::Config.promotions.promotion_chooser_class.new(order_promotion_adjustments).update order.promo_total = all_items.sum(&:promo_total) + @@ -36,6 +36,8 @@ def call select(&:eligible?). select(&:promotion?). sum(&:amount) + + order.save! if persist order end @@ -52,7 +54,7 @@ def call # admin) or is closed, this is a noop. # # @return [BigDecimal] New amount of this adjustment - def recalculate(adjustment) + def recalculate(adjustment, persist:) if adjustment.finalized? return adjustment.amount end @@ -68,10 +70,7 @@ def recalculate(adjustment) adjustment.eligible = calculate_eligibility(adjustment) - # Persist only if changed - # This is only not a save! to avoid the extra queries to load the order - # (for validations) and to touch the adjustment. - adjustment.update_columns(eligible: adjustment.eligible, amount: adjustment.amount, updated_at: Time.current) if adjustment.changed? + adjustment.save!(validate: false) if persist end adjustment.amount end diff --git a/legacy_promotions/app/models/spree/promotion_chooser.rb b/legacy_promotions/app/models/spree/promotion_chooser.rb index 8ec0d089aba..ebe80bc56d8 100644 --- a/legacy_promotions/app/models/spree/promotion_chooser.rb +++ b/legacy_promotions/app/models/spree/promotion_chooser.rb @@ -14,7 +14,7 @@ def update if best_promotion_adjustment @adjustments.select(&:eligible?).each do |adjustment| next if adjustment == best_promotion_adjustment - adjustment.update_columns(eligible: false, updated_at: Time.current) + adjustment.eligible = false end best_promotion_adjustment.amount else diff --git a/legacy_promotions/app/patches/models/solidus_legacy_promotions/spree_in_memory_order_updater_patch.rb b/legacy_promotions/app/patches/models/solidus_legacy_promotions/spree_in_memory_order_updater_patch.rb new file mode 100644 index 00000000000..2eda08073a0 --- /dev/null +++ b/legacy_promotions/app/patches/models/solidus_legacy_promotions/spree_in_memory_order_updater_patch.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module SolidusLegacyPromotions + module SpreeInMemoryOrderUpdaterPatch + def update_adjustment_total(persist:) + update_adjustments(persist:) + + all_items = (line_items + shipments).reject(&:marked_for_destruction?) + valid_adjustments = adjustments.select(&:eligible?).reject(&:marked_for_destruction?) + order_tax_adjustments = valid_adjustments.select(&:tax?) + + order.adjustment_total = all_items.sum(&:adjustment_total) + valid_adjustments.sum(&:amount) + order.included_tax_total = all_items.sum(&:included_tax_total) + order_tax_adjustments.select(&:included?).sum(&:amount) + order.additional_tax_total = all_items.sum(&:additional_tax_total) + order_tax_adjustments.reject(&:included?).sum(&:amount) + + recalculate_order_total + end + + def assign_item_totals + [*line_items, *shipments].each do |item| + Spree::Config.item_total_class.new(item).recalculate! + + # The cancellation_total isn't persisted anywhere but is included in + # the adjustment_total. + # + # Core doesn't have "eligible" adjustments anymore, so we need to + # override the adjustment_total calculation to exclude them for legacy + # promotions. + item.adjustment_total = item.adjustments. + select(&:eligible?). + reject(&:included?). + reject(&:marked_for_destruction?). + sum(&:amount) + end + end + + Spree::InMemoryOrderUpdater.prepend self + end +end diff --git a/legacy_promotions/spec/models/spree/promotion/order_adjustments_recalculator_spec.rb b/legacy_promotions/spec/models/spree/promotion/order_adjustments_recalculator_spec.rb index 1f80f03e1f5..94d2f7921a2 100644 --- a/legacy_promotions/spec/models/spree/promotion/order_adjustments_recalculator_spec.rb +++ b/legacy_promotions/spec/models/spree/promotion/order_adjustments_recalculator_spec.rb @@ -3,7 +3,9 @@ require 'rails_helper' RSpec.describe Spree::Promotion::OrderAdjustmentsRecalculator do - subject { described_class.new(order).call } + subject { described_class.new(order).call(persist:) } + + let(:persist) { true } describe '#call ' do describe 'promotion recalculation' do @@ -44,6 +46,27 @@ order.promo_total }.from(-1).to(-2) end + + context "when persist is false" do + let(:persist) { false } + + it "does not persist changes to order" do + order.promotions << create(:promotion, :with_line_item_adjustment, adjustment_rate: 2) + Spree::PromotionHandler::Cart.new(order).activate + + expect(order.promo_total).to eq(-1) + + expect { + subject + }.not_to make_database_queries(manipulative: true) + + expect(order.promo_total).to eq(-2) + + expect { order.reload } + .to change { order.promo_total } + .from(-2).to(-1) + end + end end context 'promotion chooser customization' do diff --git a/legacy_promotions/spec/models/spree/promotion_integration_spec.rb b/legacy_promotions/spec/models/spree/promotion_integration_spec.rb index 72b48c1f7ec..404c61c448d 100644 --- a/legacy_promotions/spec/models/spree/promotion_integration_spec.rb +++ b/legacy_promotions/spec/models/spree/promotion_integration_spec.rb @@ -24,7 +24,7 @@ # Regression test for https://github.com/solidusio/solidus/pull/1591 context "with unsaved line_item changes" do - let(:calculator) { FactoryBot.create :flat_rate_calculator } + let(:calculator) { FactoryBot.create(:flat_rate_calculator) } let(:line_item) { order.line_items.first } before do @@ -80,36 +80,105 @@ ) end end + + context "with the in-memory order updater" do + subject { order.recalculate(persist: false) } + + before { + calculator.update!(preferred_amount: 5) + } + + around do |example| + default_order_recalculator = Spree::Config.order_recalculator_class.to_s + + Spree::Config.order_recalculator_class = 'Spree::InMemoryOrderUpdater' + + example.run + + Spree::Config.order_recalculator_class = default_order_recalculator + end + + it "updates the adjustment total but does not persist it" do + expect(order.adjustment_total).to eq(-10.0) + + expect { subject }. + to_not make_database_queries(manipulative: true) + + expect(order).to have_attributes( + total: 105, + adjustment_total: -5 + ) + + order.reload + + expect(order.adjustment_total).to eq(-10) + end + end end end describe "distributing amount across line items" do - let(:calculator) { Spree::Calculator::DistributedAmount.new } + subject { order.recalculate } + + let(:calculator) { Spree::Calculator::DistributedAmount.new(preferred_amount: 15) } let(:promotion) { create :promotion, name: '15 spread' } - let(:order) { + + before { + Spree::Promotion::Actions::CreateItemAdjustments.create!(calculator:, promotion:) + } + + let!(:order) { create :completed_order_with_promotion, promotion:, line_items_attributes: [{ price: 20 }, { price: 30 }, { price: 100 }] } - before do - calculator.preferred_amount = 15 - Spree::Promotion::Actions::CreateItemAdjustments.create!(calculator:, promotion:) - order.recalculate - end + it 'correctly distributes the entire discount', :aggregate_failures do + subject - it 'correctly distributes the entire discount' do expect(order.promo_total).to eq(-15) expect(order.line_items.map(&:adjustment_total)).to eq([-2, -3, -10]) end + context 'with the in memory order updater' do + subject { order.recalculate(persist: false) } + + around do |example| + default_order_recalculator = Spree::Config.order_recalculator_class.to_s + + Spree::Config.order_recalculator_class = 'Spree::InMemoryOrderUpdater' + + example.run + + Spree::Config.order_recalculator_class = default_order_recalculator + end + + it 'initializes the adjustments but does not persist them' do + expect { + subject + }.not_to make_database_queries(manipulative: true) + + expect(order.promo_total).to eq(-15) + expect(order.line_items.map(&:adjustment_total)).to eq([-2, -3, -10]) + + order.reload + + expect(order.promo_total).to eq(0) + expect(order.line_items.map(&:adjustment_total)).to eq([0, 0, 0]) + end + end + context 'with product promotion rule' do + subject { order.recalculate } + let(:first_product) { order.line_items.first.product } before do + calculator.preferred_amount = 15 + Spree::Promotion::Actions::CreateItemAdjustments.create!(calculator:, promotion:) rule = Spree::Promotion::Rules::Product.create!( promotion:, product_promotion_rules: [ @@ -118,10 +187,11 @@ ) promotion.rules << rule promotion.save! - order.recalculate end it 'still distributes the entire discount' do + subject + expect(order.promo_total).to eq(-15) expect(order.line_items.map(&:adjustment_total)).to eq([-15, 0, 0]) end @@ -162,23 +232,107 @@ order.complete }.to change{ promo_adjustment.reload.eligible }.to(false) end + end - it "adjusts the promo_total" do - expect{ - order.complete - }.to change(order, :promo_total).by(10) + describe "checking whether a promotion code is still eligible after one use" do + let(:promotion) do + FactoryBot.create( + :promotion, + :with_order_adjustment, + code: "discount", + per_code_usage_limit: 1, + weighted_order_adjustment_amount: 10 + ) end + let(:code) { promotion.codes.first } + let(:order) do + FactoryBot.create(:order_with_line_items, line_items_price: 40, shipment_cost: 0).tap do |order| + FactoryBot.create(:payment, amount: 30, order:) + promotion.activate(order:, promotion_code: code) + end + end + let(:promo_adjustment) { order.adjustments.promotion.first } - it "increases the total to remove the promo" do - expect{ - order.complete - }.to change(order, :total).from(30).to(40) + before do + order.next! until order.can_complete? + + FactoryBot.create(:order_with_line_items, line_items_price: 40, shipment_cost: 0).tap do |order| + FactoryBot.create(:payment, amount: 30, order:) + promotion.activate(order:, promotion_code: code) + order.next! until order.can_complete? + order.complete! + end end - it "resets the state of the order" do - expect{ - order.complete - }.to change{ order.reload.state }.from("confirm").to("address") + context 'with the in-memory order updater' do + subject { order.recalculate(persist:) } + + around do |example| + default_order_recalculator = Spree::Config.order_recalculator_class.to_s + + Spree::Config.order_recalculator_class = Spree::InMemoryOrderUpdater + + example.run + + Spree::Config.order_recalculator_class = default_order_recalculator + end + + context "when not persisting updates" do + let(:persist) { false } + + it "doesn't manipulate the database" do + expect { subject }.not_to make_database_queries(manipulative: true) + end + + it "changes but does not persist the promotion as ineligible" do + expect { subject } + .to change { order.adjustments.first.eligible } + .from(true) + .to(false) + end + + it "changes but does not persist the promo_total" do + expect { subject }.to change { order.promo_total }.from(-10).to(0) + end + + it "changes the total but does not persist the promo amount" do + expect { subject }.to change { order.total }.from(30).to(40) + end + end + + context "when persisting updates" do + let(:persist) { true } + + it "makes the promotion ineligible" do + expect { subject } + .to change { promo_adjustment.reload.eligible } + .from(true) + .to(false) + end + + it "adjusts the promo_total" do + expect { subject }.to change { order.reload.promo_total }.from(-10).to(0) + end + + it "increases the total to remove the promo" do + expect { subject }.to change { order.reload.total }.from(30).to(40) + end + + context "with an item adjustment" do + let(:promotion) do + FactoryBot.create( + :promotion_with_item_adjustment, + code: "discount", + per_code_usage_limit: 1, + adjustment_rate: 10 + ) + end + + it "adjusts the adjustment total" do + expect { subject }.to change { order.line_items.first.reload.adjustment_total }.from(-10).to(0) + end + end + end end end diff --git a/legacy_promotions/spec/rails_helper.rb b/legacy_promotions/spec/rails_helper.rb index 6b1e170f72e..d015f3daaa6 100644 --- a/legacy_promotions/spec/rails_helper.rb +++ b/legacy_promotions/spec/rails_helper.rb @@ -27,7 +27,6 @@ require 'rspec/rails' require 'rspec-activemodel-mocks' require 'database_cleaner' -require 'db-query-matchers' Dir["./spec/support/**/*.rb"].sort.each { |f| require f } diff --git a/promotions/app/models/solidus_promotions/order_adjuster.rb b/promotions/app/models/solidus_promotions/order_adjuster.rb index cd2cb29a358..9a1fae8b722 100644 --- a/promotions/app/models/solidus_promotions/order_adjuster.rb +++ b/promotions/app/models/solidus_promotions/order_adjuster.rb @@ -13,7 +13,7 @@ def initialize(order, dry_run_promotion: nil) ).call end - def call + def call(persist: true) # rubocop:disable Lint/UnusedMethodArgument return order unless SolidusPromotions::Promotion.order_activatable?(order) SetDiscountsToZero.call(order) diff --git a/promotions/app/patches/models/solidus_promotions/order_recalculator_patch.rb b/promotions/app/patches/models/solidus_promotions/order_recalculator_patch.rb index 84cb8945fb5..f8fe7cb30ad 100644 --- a/promotions/app/patches/models/solidus_promotions/order_recalculator_patch.rb +++ b/promotions/app/patches/models/solidus_promotions/order_recalculator_patch.rb @@ -12,5 +12,6 @@ def recalculate end Spree::OrderUpdater.prepend self + Spree::InMemoryOrderUpdater.prepend self end end diff --git a/promotions/spec/models/promotion/in_memory_integration_spec.rb b/promotions/spec/models/promotion/in_memory_integration_spec.rb new file mode 100644 index 00000000000..c80bb3fdf2a --- /dev/null +++ b/promotions/spec/models/promotion/in_memory_integration_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require "rails_helper" +require "solidus_promotions/promotion_map" +require "solidus_promotions/promotion_migrator" + +RSpec.describe "Promotion System" do + describe "with an in-memory order recalculator" do + around do |example| + prev_recalculator_class = Spree::Config.order_recalculator_class + Spree::Config.order_recalculator_class = Spree::InMemoryOrderUpdater + + example.run + + Spree::Config.order_recalculator_class = prev_recalculator_class + end + + it_behaves_like "a successfully integrated promotion system" + end +end diff --git a/promotions/spec/models/promotion/integration_spec.rb b/promotions/spec/models/promotion/integration_spec.rb index e0217a2a6bf..65da55cce70 100644 --- a/promotions/spec/models/promotion/integration_spec.rb +++ b/promotions/spec/models/promotion/integration_spec.rb @@ -5,304 +5,16 @@ require "solidus_promotions/promotion_migrator" RSpec.describe "Promotion System" do - context "A promotion that creates line item adjustments" do - let(:shirt) { create(:product, name: "Shirt") } - let(:pants) { create(:product, name: "Pants") } - let!(:promotion) { create(:solidus_promotion, name: "20% off Shirts", benefits: [benefit], apply_automatically: true) } - let(:order) { create(:order) } + describe "with the default order recalculator config" do + around do |example| + prev_recalculator_class = Spree::Config.order_recalculator_class + Spree::Config.order_recalculator_class = Spree::OrderUpdater - before do - benefit.conditions << condition - order.contents.add(shirt.master, 1) - order.contents.add(pants.master, 1) - end - - context "with an order-level condition" do - let(:condition) { SolidusPromotions::Conditions::OrderProduct.new(products: [shirt]) } - - context "with an line item level benefit" do - let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } - let(:benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: calculator) } - - it "creates one line item level adjustment" do - expect(order.adjustments).to be_empty - expect(order.total).to eq(31.98) - expect(order.item_total).to eq(39.98) - expect(order.item_total_before_tax).to eq(31.98) - expect(order.line_items.flat_map(&:adjustments).length).to eq(2) - end - end - - context "with an automation" do - let(:goodie) { create(:variant, price: 4) } - let(:benefit) { SolidusPromotions::Benefits::CreateDiscountedItem.new(preferred_variant_id: goodie.id, calculator: hundred_percent) } - let(:hundred_percent) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 100) } - let(:condition) { SolidusPromotions::Conditions::Product.new(products: [shirt]) } - - it "creates a new discounted line item" do - expect(order.adjustments).to be_empty - expect(order.line_items.count).to eq(3) - # 19.99 * 2 - expect(order.total).to eq(39.98) - # 19.99 * 2 + 4 * 1 - expect(order.item_total).to eq(43.98) - expect(order.item_total_before_tax).to eq(39.98) - expect(order.line_items.flat_map(&:adjustments).length).to eq(1) - end - - context "when a second base item is added" do - before do - order.contents.add(shirt.master) - end - - it "creates a new discounted line item" do - expect(order.adjustments).to be_empty - expect(order.line_items.count).to eq(3) - # 19.99 * 3 - expect(order.total).to eq(59.97) - expect(order.item_total).to eq(67.97) - expect(order.item_total_before_tax).to eq(59.97) - expect(order.line_items.flat_map(&:adjustments).length).to eq(1) - end - end - - context "when the goodie becomes unavailable" do - before do - order.contents.remove(shirt.master) - end - - it "removes the discounted line item" do - expect(order.adjustments).to be_empty - expect(order.line_items.length).to eq(1) - expect(order.promo_total).to eq(0) - expect(order.total).to eq(19.99) - expect(order.item_total).to eq(19.99) - expect(order.item_total_before_tax).to eq(19.99) - expect(order.line_items.flat_map(&:adjustments).length).to eq(0) - end - end - - context "with a line-item level promotion in the lane before it" do - let!(:other_promotion) { create(:solidus_promotion, :with_adjustable_benefit, lane: :pre, apply_automatically: true) } - - it "creates a new discounted line item" do - order.recalculate - expect(order.adjustments).to be_empty - expect(order.line_items.count).to eq(3) - expect(order.total).to eq(19.98) - expect(order.item_total).to eq(43.98) - expect(order.item_total_before_tax).to eq(19.98) - expect(order.line_items.flat_map(&:adjustments).length).to eq(3) - expect(order.line_items.detect { |line_item| line_item.managed_by_order_benefit == benefit }.adjustments.length).to eq(1) - expect(order.line_items.detect { |line_item| line_item.managed_by_order_benefit == benefit }.adjustments.first.amount).to eq(-4) - end - end - end - end - - context "with a line-item level condition" do - let(:condition) { SolidusPromotions::Conditions::LineItemProduct.new(products: [shirt]) } - - context "with an line item level benefit" do - let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } - let(:benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: calculator) } - - it "creates one line item level adjustment" do - expect(order.adjustments).to be_empty - expect(order.total).to eq(35.98) - expect(order.item_total).to eq(39.98) - expect(order.item_total_before_tax).to eq(35.98) - expect(order.line_items.flat_map(&:adjustments).length).to eq(1) - end - end - end - end - - context "with two promotions that should stack" do - let(:shirt) { create(:product, name: "Shirt", price: 30) } - let(:pants) { create(:product, name: "Pants", price: 40) } - let(:discounted_item_total_condition_amount) { 60 } - let(:discounted_item_total_condition) do - SolidusPromotions::Conditions::DiscountedItemTotal.new(preferred_amount: discounted_item_total_condition_amount) - end - let(:discounted_item_total_benefit) do - SolidusPromotions::Benefits::AdjustLineItem.new(calculator: discounted_item_total_calculator, conditions: [discounted_item_total_condition]) - end - let(:discounted_item_total_calculator) do - SolidusPromotions::Calculators::DistributedAmount.new(preferred_amount: 10) - end - let!(:distributed_amount_promo) do - create( - :solidus_promotion, - benefits: [discounted_item_total_benefit], - apply_automatically: true, - lane: :post - ) - end - - let(:shirts_condition) { SolidusPromotions::Conditions::LineItemProduct.new(products: [shirt]) } - let(:shirts_calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } - let(:shirts_benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: shirts_calculator, conditions: [shirts_condition]) } - let!(:shirts_promotion) do - create( - :solidus_promotion, - benefits: [shirts_benefit], - name: "20% off shirts", - apply_automatically: true - ) - end - let(:order) { create(:order) } - - before do - order.contents.add(shirt.master, 1) - order.contents.add(pants.master, 1) - end - - it "does all the right things" do - expect(order.adjustments).to be_empty - # shirt: 30 USD - 20% = 24 USD - # Remaining total: 64 USD - # 10 USD distributed off: 54 USD - expect(order.total).to eq(54.00) - expect(order.item_total).to eq(70.00) - expect(order.item_total_before_tax).to eq(54) - expect(order.line_items.flat_map(&:adjustments).length).to eq(3) - end + example.run - context "if the post lane promotion is ineligible" do - let(:discounted_item_total_condition_amount) { 68 } - - it "does all the right things" do - expect(order.adjustments).to be_empty - # shirt: 30 USD - 20% = 24 USD - # Remaining total: 64 USD - # The 10 off promotion does not apply because now the order total is below 68 - expect(order.total).to eq(64.00) - expect(order.item_total).to eq(70.00) - expect(order.item_total_before_tax).to eq(64) - expect(order.line_items.flat_map(&:adjustments).length).to eq(1) - end - end - end - - context "with a migrated spree_promotion that is attached to the current order" do - let(:shirt) { create(:variant) } - let(:spree_promotion) { create(:promotion, :with_adjustable_action, code: true) } - let(:order) { create(:order_with_line_items, line_items_attributes: [{ variant: shirt }]) } - - before do - Spree::Config.promotions = SolidusLegacyPromotions::Configuration.new - Spree::Config.order_contents_class = "Spree::OrderContents" - SolidusPromotions.config.sync_order_promotions = true - promotion_code = spree_promotion.codes.first - order.order_promotions << Spree::OrderPromotion.new( - promotion_code: promotion_code, - promotion: spree_promotion - ) - Spree::PromotionHandler::Cart.new(order).activate - order.recalculate - expect(order.line_items.first.adjustments.first.source).to eq(spree_promotion.promotion_actions.first) - promotion_map = SolidusPromotions::PROMOTION_MAP - SolidusPromotions::PromotionMigrator.new(promotion_map).call - expect(SolidusPromotions::Promotion.count).to eq(1) - - Spree::Config.promotions = SolidusPromotions::Configuration.new - Spree::Config.order_contents_class = "Spree::SimpleOrderContents" - end - - after do - SolidusPromotions.config.sync_order_promotions = false + Spree::Config.order_recalculator_class = prev_recalculator_class end - subject { order.recalculate } - - it "replaces existing adjustments with adjustments for the equivalent solidus promotion" do - expect { subject }.not_to change { order.all_adjustments.count } - end - - it "does not change the amount of any adjustments" do - expect { subject }.not_to change { order.reload.all_adjustments.sum(&:amount) } - end - end - - context "with a shipment-level condition" do - let!(:address) { create(:address) } - let(:shipping_zone) { create(:global_zone) } - let(:store) { create(:store) } - let!(:ups_ground) { create(:shipping_method, zones: [shipping_zone], cost: 23) } - let!(:dhl_saver) { create(:shipping_method, zones: [shipping_zone], cost: 37) } - let(:variant) { create(:variant, price: 13) } - let(:promotion) { create(:solidus_promotion, name: "20 percent off UPS Ground", apply_automatically: true) } - let(:condition) { SolidusPromotions::Conditions::ShippingMethod.new(preferred_shipping_method_ids: [ups_ground.id]) } - let(:order) { Spree::Order.create!(store: store) } - - before do - promotion.benefits << benefit - benefit.conditions << condition - - order.contents.add(variant, 1) - order.ship_address = address - order.bill_address = address - - order.create_proposed_shipments - - order.shipments.first.selected_shipping_rate_id = order.shipments.first.shipping_rates.detect do |r| - r.shipping_method == shipping_method - end.id - - order.recalculate - end - - context "with a line item level benefit" do - let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } - let(:benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: calculator) } - let(:shipping_method) { ups_ground } - - it "creates adjustments" do - expect(order.adjustments).to be_empty - expect(order.total).to eq(33.40) - expect(order.item_total).to eq(13) - expect(order.item_total_before_tax).to eq(10.40) - expect(order.promo_total).to eq(-2.60) - expect(order.line_items.flat_map(&:adjustments).length).to eq(1) - expect(order.shipments.flat_map(&:adjustments)).to be_empty - expect(order.shipments.flat_map(&:shipping_rates).flat_map(&:discounts)).to be_empty - end - end - - context "with a shipment level benefit" do - let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } - let(:benefit) { SolidusPromotions::Benefits::AdjustShipment.new(calculator: calculator) } - - context "when the order is eligible" do - let(:shipping_method) { ups_ground } - - it "creates adjustments" do - expect(order.adjustments).to be_empty - expect(order.total).to eq(31.40) - expect(order.item_total).to eq(13) - expect(order.item_total_before_tax).to eq(13) - expect(order.promo_total).to eq(-4.6) - expect(order.line_items.flat_map(&:adjustments)).to be_empty - expect(order.shipments.flat_map(&:adjustments)).not_to be_empty - expect(order.shipments.flat_map(&:shipping_rates).flat_map(&:discounts)).not_to be_empty - end - end - - context "when the order is not eligible" do - let(:shipping_method) { dhl_saver } - - it "creates no adjustments" do - expect(order.adjustments).to be_empty - expect(order.total).to eq(50) - expect(order.item_total).to eq(13) - expect(order.item_total_before_tax).to eq(13) - expect(order.promo_total).to eq(0) - expect(order.line_items.flat_map(&:adjustments)).to be_empty - expect(order.shipments.flat_map(&:adjustments)).to be_empty - expect(order.shipments.flat_map(&:shipping_rates).flat_map(&:discounts)).not_to be_empty - end - end - end + it_behaves_like "a successfully integrated promotion system" end end diff --git a/promotions/spec/rails_helper.rb b/promotions/spec/rails_helper.rb index 6cc5ff61fa4..c233338de47 100644 --- a/promotions/spec/rails_helper.rb +++ b/promotions/spec/rails_helper.rb @@ -5,7 +5,6 @@ require "spec_helper" require "solidus_legacy_promotions" -require 'db-query-matchers' # SOLIDUS DUMMY APP require "spree/testing_support/dummy_app" diff --git a/promotions/spec/support/shared_integration_examples.rb b/promotions/spec/support/shared_integration_examples.rb new file mode 100644 index 00000000000..b3398f2c21d --- /dev/null +++ b/promotions/spec/support/shared_integration_examples.rb @@ -0,0 +1,304 @@ +# frozen_string_literal: true + +RSpec.shared_examples "a successfully integrated promotion system" do + context "A promotion that creates line item adjustments" do + let(:shirt) { create(:product, name: "Shirt") } + let(:pants) { create(:product, name: "Pants") } + let!(:promotion) { create(:solidus_promotion, name: "20% off Shirts", benefits: [benefit], apply_automatically: true) } + let(:order) { create(:order) } + + before do + benefit.conditions << condition + order.contents.add(shirt.master, 1) + order.contents.add(pants.master, 1) + end + + context "with an order-level condition" do + let(:condition) { SolidusPromotions::Conditions::OrderProduct.new(products: [shirt]) } + + context "with an line item level benefit" do + let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } + let(:benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: calculator) } + + it "creates one line item level adjustment" do + expect(order.adjustments).to be_empty + expect(order.total).to eq(31.98) + expect(order.item_total).to eq(39.98) + expect(order.item_total_before_tax).to eq(31.98) + expect(order.line_items.flat_map(&:adjustments).length).to eq(2) + end + end + + context "with an automation" do + let(:goodie) { create(:variant, price: 4) } + let(:benefit) { SolidusPromotions::Benefits::CreateDiscountedItem.new(preferred_variant_id: goodie.id, calculator: hundred_percent) } + let(:hundred_percent) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 100) } + let(:condition) { SolidusPromotions::Conditions::Product.new(products: [shirt]) } + + it "creates a new discounted line item" do + expect(order.adjustments).to be_empty + expect(order.line_items.count).to eq(3) + # 19.99 * 2 + expect(order.total).to eq(39.98) + # 19.99 * 2 + 4 * 1 + expect(order.item_total).to eq(43.98) + expect(order.item_total_before_tax).to eq(39.98) + expect(order.line_items.flat_map(&:adjustments).length).to eq(1) + end + + context "when a second base item is added" do + before do + order.contents.add(shirt.master) + end + + it "creates a new discounted line item" do + expect(order.adjustments).to be_empty + expect(order.line_items.count).to eq(3) + # 19.99 * 3 + expect(order.total).to eq(59.97) + expect(order.item_total).to eq(67.97) + expect(order.item_total_before_tax).to eq(59.97) + expect(order.line_items.flat_map(&:adjustments).length).to eq(1) + end + end + + context "when the goodie becomes unavailable" do + before do + order.contents.remove(shirt.master) + end + + it "removes the discounted line item" do + expect(order.adjustments).to be_empty + expect(order.line_items.length).to eq(1) + expect(order.promo_total).to eq(0) + expect(order.total).to eq(19.99) + expect(order.item_total).to eq(19.99) + expect(order.item_total_before_tax).to eq(19.99) + expect(order.line_items.flat_map(&:adjustments).length).to eq(0) + end + end + + context "with a line-item level promotion in the lane before it" do + let!(:other_promotion) { create(:solidus_promotion, :with_adjustable_benefit, lane: :pre, apply_automatically: true) } + + it "creates a new discounted line item" do + order.recalculate + expect(order.adjustments).to be_empty + expect(order.line_items.count).to eq(3) + expect(order.total).to eq(19.98) + expect(order.item_total).to eq(43.98) + expect(order.item_total_before_tax).to eq(19.98) + expect(order.line_items.flat_map(&:adjustments).length).to eq(3) + expect(order.line_items.detect { |line_item| line_item.managed_by_order_benefit == benefit }.adjustments.length).to eq(1) + expect(order.line_items.detect { |line_item| line_item.managed_by_order_benefit == benefit }.adjustments.first.amount).to eq(-4) + end + end + end + end + + context "with a line-item level condition" do + let(:condition) { SolidusPromotions::Conditions::LineItemProduct.new(products: [shirt]) } + + context "with an line item level benefit" do + let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } + let(:benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: calculator) } + + it "creates one line item level adjustment" do + expect(order.adjustments).to be_empty + expect(order.total).to eq(35.98) + expect(order.item_total).to eq(39.98) + expect(order.item_total_before_tax).to eq(35.98) + expect(order.line_items.flat_map(&:adjustments).length).to eq(1) + end + end + end + end + + context "with two promotions that should stack" do + let(:shirt) { create(:product, name: "Shirt", price: 30) } + let(:pants) { create(:product, name: "Pants", price: 40) } + let(:discounted_item_total_condition_amount) { 60 } + let(:discounted_item_total_condition) do + SolidusPromotions::Conditions::DiscountedItemTotal.new(preferred_amount: discounted_item_total_condition_amount) + end + let(:discounted_item_total_benefit) do + SolidusPromotions::Benefits::AdjustLineItem.new(calculator: discounted_item_total_calculator, conditions: [discounted_item_total_condition]) + end + let(:discounted_item_total_calculator) do + SolidusPromotions::Calculators::DistributedAmount.new(preferred_amount: 10) + end + let!(:distributed_amount_promo) do + create( + :solidus_promotion, + benefits: [discounted_item_total_benefit], + apply_automatically: true, + lane: :post + ) + end + + let(:shirts_condition) { SolidusPromotions::Conditions::LineItemProduct.new(products: [shirt]) } + let(:shirts_calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } + let(:shirts_benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: shirts_calculator, conditions: [shirts_condition]) } + let!(:shirts_promotion) do + create( + :solidus_promotion, + benefits: [shirts_benefit], + name: "20% off shirts", + apply_automatically: true + ) + end + let(:order) { create(:order) } + + before do + order.contents.add(shirt.master, 1) + order.contents.add(pants.master, 1) + end + + it "does all the right things" do + expect(order.adjustments).to be_empty + # shirt: 30 USD - 20% = 24 USD + # Remaining total: 64 USD + # 10 USD distributed off: 54 USD + expect(order.total).to eq(54.00) + expect(order.item_total).to eq(70.00) + expect(order.item_total_before_tax).to eq(54) + expect(order.line_items.flat_map(&:adjustments).length).to eq(3) + end + + context "if the post lane promotion is ineligible" do + let(:discounted_item_total_condition_amount) { 68 } + + it "does all the right things" do + expect(order.adjustments).to be_empty + # shirt: 30 USD - 20% = 24 USD + # Remaining total: 64 USD + # The 10 off promotion does not apply because now the order total is below 68 + expect(order.total).to eq(64.00) + expect(order.item_total).to eq(70.00) + expect(order.item_total_before_tax).to eq(64) + expect(order.line_items.flat_map(&:adjustments).length).to eq(1) + end + end + end + + context "with a migrated spree_promotion that is attached to the current order" do + let(:shirt) { create(:variant) } + let(:spree_promotion) { create(:promotion, :with_adjustable_action, code: true) } + let(:order) { create(:order_with_line_items, line_items_attributes: [{ variant: shirt }]) } + + before do + Spree::Config.promotions = SolidusLegacyPromotions::Configuration.new + Spree::Config.order_contents_class = "Spree::OrderContents" + SolidusPromotions.config.sync_order_promotions = true + promotion_code = spree_promotion.codes.first + order.order_promotions << Spree::OrderPromotion.new( + promotion_code: promotion_code, + promotion: spree_promotion + ) + Spree::PromotionHandler::Cart.new(order).activate + order.recalculate + expect(order.line_items.first.adjustments.first.source).to eq(spree_promotion.promotion_actions.first) + promotion_map = SolidusPromotions::PROMOTION_MAP + SolidusPromotions::PromotionMigrator.new(promotion_map).call + expect(SolidusPromotions::Promotion.count).to eq(1) + + Spree::Config.promotions = SolidusPromotions::Configuration.new + Spree::Config.order_contents_class = "Spree::SimpleOrderContents" + end + + after do + SolidusPromotions.config.sync_order_promotions = false + end + + subject { order.recalculate } + + it "replaces existing adjustments with adjustments for the equivalent solidus promotion" do + expect { subject }.not_to change { order.all_adjustments.count } + end + + it "does not change the amount of any adjustments" do + expect { subject }.not_to change { order.reload.all_adjustments.sum(&:amount) } + end + end + + context "with a shipment-level condition" do + let!(:address) { create(:address) } + let(:shipping_zone) { create(:global_zone) } + let(:store) { create(:store) } + let!(:ups_ground) { create(:shipping_method, zones: [shipping_zone], cost: 23) } + let!(:dhl_saver) { create(:shipping_method, zones: [shipping_zone], cost: 37) } + let(:variant) { create(:variant, price: 13) } + let(:promotion) { create(:solidus_promotion, name: "20 percent off UPS Ground", apply_automatically: true) } + let(:condition) { SolidusPromotions::Conditions::ShippingMethod.new(preferred_shipping_method_ids: [ups_ground.id]) } + let(:order) { Spree::Order.create!(store: store) } + + before do + promotion.benefits << benefit + benefit.conditions << condition + + order.contents.add(variant, 1) + order.ship_address = address + order.bill_address = address + + order.create_proposed_shipments + + order.shipments.first.selected_shipping_rate_id = order.shipments.first.shipping_rates.detect do |r| + r.shipping_method == shipping_method + end.id + + order.recalculate + end + + context "with a line item level benefit" do + let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } + let(:benefit) { SolidusPromotions::Benefits::AdjustLineItem.new(calculator: calculator) } + let(:shipping_method) { ups_ground } + + it "creates adjustments" do + expect(order.adjustments).to be_empty + expect(order.total).to eq(33.40) + expect(order.item_total).to eq(13) + expect(order.item_total_before_tax).to eq(10.40) + expect(order.promo_total).to eq(-2.60) + expect(order.line_items.flat_map(&:adjustments).length).to eq(1) + expect(order.shipments.flat_map(&:adjustments)).to be_empty + expect(order.shipments.flat_map(&:shipping_rates).flat_map(&:discounts)).to be_empty + end + end + + context "with a shipment level benefit" do + let(:calculator) { SolidusPromotions::Calculators::Percent.new(preferred_percent: 20) } + let(:benefit) { SolidusPromotions::Benefits::AdjustShipment.new(calculator: calculator) } + + context "when the order is eligible" do + let(:shipping_method) { ups_ground } + + it "creates adjustments" do + expect(order.adjustments).to be_empty + expect(order.total).to eq(31.40) + expect(order.item_total).to eq(13) + expect(order.item_total_before_tax).to eq(13) + expect(order.promo_total).to eq(-4.6) + expect(order.line_items.flat_map(&:adjustments)).to be_empty + expect(order.shipments.flat_map(&:adjustments)).not_to be_empty + expect(order.shipments.flat_map(&:shipping_rates).flat_map(&:discounts)).not_to be_empty + end + end + + context "when the order is not eligible" do + let(:shipping_method) { dhl_saver } + + it "creates no adjustments" do + expect(order.adjustments).to be_empty + expect(order.total).to eq(50) + expect(order.item_total).to eq(13) + expect(order.item_total_before_tax).to eq(13) + expect(order.promo_total).to eq(0) + expect(order.line_items.flat_map(&:adjustments)).to be_empty + expect(order.shipments.flat_map(&:adjustments)).to be_empty + expect(order.shipments.flat_map(&:shipping_rates).flat_map(&:discounts)).not_to be_empty + end + end + end + end +end