Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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: 0 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 0 additions & 8 deletions admin/spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
262 changes: 262 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,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_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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be extracted into a PR


# 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could also be extracted into PR


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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should also be deprecated because even though they are private we all have evidence that this are overwritten in subclasses. can be extracted into a PR

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!
end
end

def persist_item_totals
[*line_items, *shipments].each do |item|
next unless item.changed?

item.save! unless item.persisted?

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

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
shipments.each(&:persist_amounts)
persist_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
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
2 changes: 1 addition & 1 deletion core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
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
23 changes: 15 additions & 8 deletions core/app/models/spree/shipment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 changed?
update_columns(
cost:,
updated_at: Time.current
)
end
end

Expand Down
9 changes: 9 additions & 0 deletions core/config/initializers/db_query_matchers.rb
Original file line number Diff line number Diff line change
@@ -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
19 changes: 19 additions & 0 deletions core/lib/spree/manipulative_query_monitor.rb
Original file line number Diff line number Diff line change
@@ -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")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: there is a cleaner way to access the call stack via db-query-matchers configuration.


Rails.logger.warn(message)
end
end
end
end
1 change: 1 addition & 0 deletions core/solidus_core.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Loading
Loading