Skip to content

In-memory order updater #5872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 35 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
c581e45
Set tax adjustment amount without persistance
Jan 3, 2025
7179b08
Mark adjustments for removal in OrderTaxation
Noah-Silvera Jan 10, 2025
a78730b
Ignore order adjustments marked for destruction
adammathys Jan 31, 2025
a551c31
Add `db-query-matchers` gem
benjaminwil Oct 11, 2024
7859fc5
Copy existing `OrderUpdater` implementation
benjaminwil Oct 11, 2024
9849123
Add `persist` flag to `#recalculate`
benjaminwil Oct 11, 2024
e54235f
Add describe block to Shipment#update_amounts test
Noah-Silvera Oct 25, 2024
c9207bb
Conditionally persist Shipment#update_amounts changes
Noah-Silvera Oct 25, 2024
f44288b
Preventing InMemoryOrderUpdater#update_shipment_amounts from making d…
Noah-Silvera Oct 25, 2024
4e3c5e4
Rename method that recalculates shipment state
forkata Nov 8, 2024
1f792df
Rename method that recalculates payment state
forkata Nov 8, 2024
7a9346e
Rename update_ private methods
AlistairNorman Nov 22, 2024
a97e8ab
Rename recalculate_adjustments
AlistairNorman Nov 22, 2024
0eae41e
Remove describe block for private method
AlistairNorman Nov 22, 2024
d4ac26a
Reorder private methods
AlistairNorman Nov 22, 2024
a37a827
Test that changes to item totals are respected
adammathys Jan 30, 2025
3fdfcdf
Pass persist flag to legacy promotion recalculator
adammathys Jan 31, 2025
46ef153
Pass persist to promotion.order_adjuster_class
sofiabesenski4 Dec 6, 2024
0f95e81
Support conditional persist in promotion chooser
AlistairNorman Feb 7, 2025
cb78eb5
Rename order adjuster subject
Noah-Silvera Feb 14, 2025
5595bec
Add missing Promotions::OrderAdjuster spec
Noah-Silvera Feb 14, 2025
7a96b45
Set required line item attributes earlier
senemsoy Feb 28, 2025
ce0c64c
Don't persist line item on promotion application
Noah-Silvera Feb 14, 2025
ff98e48
Mark adjustments for destruction instead of destroying
forkata Mar 14, 2025
5e8713b
Set nil tax category in line item product spec
nvandoorn Mar 28, 2025
9279f93
Set nil tax category in product spec
nvandoorn Mar 28, 2025
dc4400c
Test InMemoryOrder updater in legacy promotions
sofiabesenski4 Mar 28, 2025
3a5118d
Don't use mock_model in model test
kjriga Apr 3, 2025
a867d41
Test InMemoryOrder updater in legacy promotions
sofiabesenski4 Mar 28, 2025
8e9c86d
Add high-level integration test for legacy promotion system
Noah-Silvera May 15, 2025
306eb45
Add FIXME for coupon spec failure
kjriga Apr 3, 2025
03d6f37
Use new configuration setting for item total class
forkata May 22, 2025
2c3a54a
Allow the order updater to handle non-persisted line items
forkata May 22, 2025
7d622cb
Create a manipulative query handler
senemsoy May 22, 2025
e7f9d88
WIP use manipulative query monitor
senemsoy May 22, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ gem 'pg', '~> 1.0', require: false if dbs.match?(/all|postgres/)
gem 'fast_sqlite', require: false if dbs.match?(/all|sqlite/)
gem 'sqlite3', '>= 2.1', require: false if dbs.match?(/all|sqlite/)

gem 'db-query-matchers'
gem 'database_cleaner', '~> 2.0', require: false
gem 'rspec-activemodel-mocks', '~> 1.1', require: false
gem 'rspec-rails', '~> 6.0.3', require: false
Expand Down
240 changes: 240 additions & 0 deletions core/app/models/spree/in_memory_order_updater.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
# frozen_string_literal: true

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
@monitor = self.log_manipulative_queries ? Spree::ManipulativeQueryMonitor : Proc.new
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)
order.transaction do
recalculate_item_count
update_shipment_amounts(persist:)
update_totals(persist:)
if order.completed?
recalculate_payment_state
update_shipments
recalculate_shipment_state
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 +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
log_state_change('shipment') do
order.shipment_state = determine_shipment_state
end

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
log_state_change('payment') do
order.payment_state = determine_payment_state
end

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
update_item_totals(persist:)
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 update_shipment_amounts(persist:)
shipments.each { _1.update_amounts(persist:) }
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 update_item_totals(persist:)
[*line_items, *shipments].each do |item|
Spree::Config.item_total_class.new(item).recalculate!

next unless persist && item.changed?

item.update_columns(
promo_total: item.promo_total,
included_tax_total: item.included_tax_total,
additional_tax_total: item.additional_tax_total,
adjustment_total: item.adjustment_total,
updated_at: Time.current,
)
end
end

# give each of the shipments a chance to update themselves
def update_shipments
shipments.each(&:update_state)
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 persist_totals
order.save!
end

def log_state_change(name)
state = "#{name}_state"
old_state = order.public_send(state)
yield
new_state = order.public_send(state)
if old_state != new_state
order.state_changes.new(
previous_state: old_state,
next_state: new_state,
user_id: order.user_id,
name:
)
end
end
end
end
5 changes: 3 additions & 2 deletions core/app/models/spree/line_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ class LineItem < Spree::Base

has_one :product, through: :variant

has_many :adjustments, as: :adjustable, inverse_of: :adjustable, dependent: :destroy
has_many :adjustments, as: :adjustable, inverse_of: :adjustable, dependent: :destroy, autosave: true
has_many :inventory_units, inverse_of: :line_item

before_validation :normalize_quantity
before_validation :set_required_attributes
after_initialize :set_required_attributes

validates :variant, presence: true
validates :quantity, numericality: {
Expand Down Expand Up @@ -159,6 +159,7 @@ def normalize_quantity
# Sets tax category, price-related attributes from
# its variant if they are nil and a variant is present.
def set_required_attributes
return if persisted?
return unless variant
self.tax_category ||= variant.tax_category
set_pricing_attributes
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/null_promotion_adjuster.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def initialize(order)
@order = order
end

def call
def call(persist: true) # rubocop:disable Lint/UnusedMethodArgument
@order
end
end
Expand Down
10 changes: 5 additions & 5 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def states
has_many :cartons, -> { distinct }, through: :inventory_units

# Adjustments and promotions
has_many :adjustments, -> { order(:created_at) }, as: :adjustable, inverse_of: :adjustable, dependent: :destroy
has_many :adjustments, -> { order(:created_at) }, as: :adjustable, inverse_of: :adjustable, dependent: :destroy, autosave: true
has_many :line_item_adjustments, through: :line_items, source: :adjustments
has_many :shipment_adjustments, through: :shipments, source: :adjustments
has_many :all_adjustments,
Expand Down Expand Up @@ -418,7 +418,7 @@ def valid_credit_cards

def fulfill!
shipments.each { |shipment| shipment.update_state if shipment.persisted? }
recalculator.update_shipment_state
recalculator.recalculate_shipment_state
save!
end

Expand Down Expand Up @@ -758,13 +758,13 @@ def finalize
all_adjustments.each(&:finalize!)

# update payment and shipment(s) states, and save
recalculator.update_payment_state
recalculator.recalculate_payment_state
shipments.each do |shipment|
shipment.update_state
shipment.finalize!
end

recalculator.update_shipment_state
recalculator.recalculate_shipment_state
save!

touch :completed_at
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions core/app/models/spree/order_taxation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def update_adjustments(item, taxed_items)

# Remove any tax adjustments tied to rates which no longer match.
unmatched_adjustments = tax_adjustments - active_adjustments
item.adjustments.destroy(unmatched_adjustments)
unmatched_adjustments.each(&:mark_for_destruction)
end

# Update or create a new tax adjustment on an item.
Expand All @@ -77,7 +77,8 @@ def update_adjustment(item, tax_item)
label: tax_item.label,
included: tax_item.included_in_price
)
tax_adjustment.update!(amount: tax_item.amount)

tax_adjustment.amount = tax_item.amount
tax_adjustment
end
end
Expand Down
Loading
Loading