-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Merged
jarednorman
merged 18 commits into
solidusio:main
from
SuperGoodSoft:in-memory-order-updater
Mar 24, 2026
Merged
In-memory order updater #5872
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
836761e
Move require for DB query matchers to initializer
forkata 8b06435
Copy existing `OrderUpdater` implementation
benjaminwil 26b6f86
Use `Shipment#recalculate_state`
adammathys 9574313
Add `persist` flag to `#recalculate`
benjaminwil 6c4307f
Prevent shipment updates from making DB writes
Noah-Silvera 53b55b0
Reorder private methods
AlistairNorman 1be7bd9
Pass persist flag to legacy promotion recalculator
adammathys 3f4a089
Pass persist to promotion.order_adjuster_class
sofiabesenski4 a72b27a
Support conditional persist in promotion chooser
AlistairNorman b7b73c4
Test in-memory order updater in legacy promotions
sofiabesenski4 a6525b4
Add in-memory updater patch for legacy promotion system
Noah-Silvera b9f711a
Create a manipulative query handler
senemsoy 7d7ff2d
Monitor manipulative queries in InMemoryOrderUpdater
sofiabesenski4 3ab0490
Test in-memory order updater with new promotions
adammathys 433a558
Remove persist flag from update_shipment_amounts
ebab38b
Rework item total updating to not use persist flag
f615ea1
Only log state changes when persisting new state
975cb36
Add price sync logic to in-memory updater
forkata File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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_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 | ||
|
mamhoff marked this conversation as resolved.
|
||
|
|
||
| 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 | ||
|
mamhoff marked this conversation as resolved.
|
||
| 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 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: there is a cleaner way to access the call stack via |
||
|
|
||
| Rails.logger.warn(message) | ||
| end | ||
| end | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.