From 5979f25c2a324cb87a9783eff4e5d4aaa1d409da Mon Sep 17 00:00:00 2001 From: Harmony Bouvier Date: Fri, 3 Jan 2025 14:25:28 -0800 Subject: [PATCH 1/4] Set tax adjustment amount without persistance To support the in-memory order updater, we want adjustments to be modified without writing to the database immediately. We added `autosave: true` in the association on line_items ensures that the adjustment amount will be saved with the line_item, or by extension the order when it is eventually persisted by the order updater. The tests for the OrderTaxation have been updated to use `size` instead of `count` to ensure an in-memory reported count/size. Co-authored-by: Adam Mueller Co-authored-by: Andrew Stewart Co-authored-by: Jared Norman Co-authored-by: Noah Silvera Co-authored-by: benjamin wil Co-authored-by: Alistair Norman --- core/app/models/spree/line_item.rb | 2 +- core/app/models/spree/order_taxation.rb | 3 ++- core/spec/models/spree/order_taxation_spec.rb | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core/app/models/spree/line_item.rb b/core/app/models/spree/line_item.rb index 17d6c05c8c8..33578794eb9 100644 --- a/core/app/models/spree/line_item.rb +++ b/core/app/models/spree/line_item.rb @@ -18,7 +18,7 @@ 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 diff --git a/core/app/models/spree/order_taxation.rb b/core/app/models/spree/order_taxation.rb index 0ecd01f8ad0..04fd01f438c 100644 --- a/core/app/models/spree/order_taxation.rb +++ b/core/app/models/spree/order_taxation.rb @@ -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 diff --git a/core/spec/models/spree/order_taxation_spec.rb b/core/spec/models/spree/order_taxation_spec.rb index 9a4725b7b47..a792d68125c 100644 --- a/core/spec/models/spree/order_taxation_spec.rb +++ b/core/spec/models/spree/order_taxation_spec.rb @@ -68,7 +68,7 @@ it "creates a new tax adjustment", aggregate_failures: true do apply - expect(line_item.adjustments.count).to eq 1 + expect(line_item.adjustments.size).to eq 1 tax_adjustment = line_item.adjustments.first expect(tax_adjustment.label).to eq "Tax!" @@ -136,7 +136,7 @@ expect { taxation.apply(new_taxes) }.to change { - line_item.adjustments.count + line_item.adjustments.size }.from(1).to(0) end end @@ -174,7 +174,7 @@ end it "creates a new tax adjustment", aggregate_failures: true do - expect(order.adjustments.count).to eq 1 + expect(order.adjustments.size).to eq 1 tax_adjustment = order.adjustments.first expect(tax_adjustment.label).to eq "Order Tax!" From c2cd0616dc578012296f429a044f1513764f1f63 Mon Sep 17 00:00:00 2001 From: Noah Silvera Date: Fri, 10 Jan 2025 17:20:12 -0500 Subject: [PATCH 2/4] Mark adjustments for removal in `OrderTaxation` Instead of actually destroying adjustments in the OrderTaxation class, we can just mark them for removal. This will allow them to be removed when the order is persisted, instead of being removed during the recalculate process. This is an improvement in service of recalculating order changes without writing to the database. We also need to ensure `Order#adjustments` association has autosave enabled so that the marked for destruction adjustments are removed when we persist the order. We're also improving our OrderUpdater tests to better verify our changes maintain the existing behaviour. Co-authored-by: Adam Mueller Co-authored-by: Alistair Norman Co-authored-by: Chris Todorov Co-authored-by: Harmony Bouvier Co-authored-by: Kendra Riga Co-authored-by: Senem Soy Co-authored-by: Sofia Besenski Co-authored-by: benjamin wil --- core/app/models/spree/order.rb | 2 +- core/app/models/spree/order_taxation.rb | 2 +- core/app/models/spree/order_updater.rb | 7 +- core/spec/models/spree/order_taxation_spec.rb | 11 ++- core/spec/models/spree/order_updater_spec.rb | 79 +++++++++++++++++-- 5 files changed, 84 insertions(+), 17 deletions(-) diff --git a/core/app/models/spree/order.rb b/core/app/models/spree/order.rb index 6f5e10b2a02..e0a76ef5b2c 100644 --- a/core/app/models/spree/order.rb +++ b/core/app/models/spree/order.rb @@ -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, diff --git a/core/app/models/spree/order_taxation.rb b/core/app/models/spree/order_taxation.rb index 04fd01f438c..58fa57f6e2a 100644 --- a/core/app/models/spree/order_taxation.rb +++ b/core/app/models/spree/order_taxation.rb @@ -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. diff --git a/core/app/models/spree/order_updater.rb b/core/app/models/spree/order_updater.rb index a11dd1da281..300096bd05c 100644 --- a/core/app/models/spree/order_updater.rb +++ b/core/app/models/spree/order_updater.rb @@ -157,9 +157,12 @@ def update_adjustment_total recalculate_adjustments all_items = line_items + shipments - order_tax_adjustments = adjustments.select(&:tax?) + # 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) + adjustments.sum(&:amount) + 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) diff --git a/core/spec/models/spree/order_taxation_spec.rb b/core/spec/models/spree/order_taxation_spec.rb index a792d68125c..9fc2b9fc649 100644 --- a/core/spec/models/spree/order_taxation_spec.rb +++ b/core/spec/models/spree/order_taxation_spec.rb @@ -132,12 +132,11 @@ ) end - it "removes the tax adjustment" do - expect { - taxation.apply(new_taxes) - }.to change { - line_item.adjustments.size - }.from(1).to(0) + it "marks the tax adjustment for destruction" do + order.save! + taxation.apply(new_taxes) + + expect(line_item.adjustments.first).to be_marked_for_destruction end end diff --git a/core/spec/models/spree/order_updater_spec.rb b/core/spec/models/spree/order_updater_spec.rb index 509bd90ebc3..0b680e117f4 100644 --- a/core/spec/models/spree/order_updater_spec.rb +++ b/core/spec/models/spree/order_updater_spec.rb @@ -69,28 +69,39 @@ module Spree end describe 'tax recalculation' do - let!(:ship_address) { create(:address) } - let!(:tax_zone) { create(:global_zone) } # will include the above address - let!(:tax_rate) { create(:tax_rate, zone: tax_zone, tax_categories: [tax_category]) } + 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: }], - ship_address:, + 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:) } - let(:tax_category) { create(:tax_category) } context 'when the item quantity has changed' do before do line_item.update!(quantity: 2) end - it 'updates the promotion amount' do + it 'updates the additional_tax_total' do expect { order.recalculate }.to change { @@ -99,6 +110,60 @@ module Spree 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 { + order.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 { order.recalculate } + + it "updates the order-level tax adjustment" do + expect { + order.ship_address = create(:address) + order.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) + order.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 } From e72d799bb07217e74af1003cae295837debd8415 Mon Sep 17 00:00:00 2001 From: tvdeyen Date: Tue, 9 Sep 2025 08:06:03 +0000 Subject: [PATCH 3/4] Prepare release for Solidus v4.6.0 This code has been automatically generated by our 'Prepare release' GitHub action. The actual release is not part of the automation, and it still needs to be manually done by a maintainer. --- CHANGELOG.md | 89 +++++++++++++++++++ .../install/app_templates/frontend/starter.rb | 2 +- core/lib/spree/core/version.rb | 2 +- docker-compose.yml | 2 +- 4 files changed, 92 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 96981c17cf8..c7035d84c67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,92 @@ +## Solidus v4.6.0 (2025-09-09) + + + +## Solidus + +* Fix typos by @jackhac in https://github.com/solidusio/solidus/pull/6207 + +## Solidus Core + +* Fix typos by @jackhac in https://github.com/solidusio/solidus/pull/6207 +* Display the store's currency in the Admin Order Index Component by @magpieuk in https://github.com/solidusio/solidus/pull/5929 +* Respect Spree.user_class' table name in metadata migration by @tvdeyen in https://github.com/solidusio/solidus/pull/6157 +* Better Spree::UserAddress scope deprecation warnings by @tvdeyen in https://github.com/solidusio/solidus/pull/6163 +* Add new order events by @benjaminwil in https://github.com/solidusio/solidus/pull/6170 +* [Docs] Fix Meta Data Restriction Comment to reflect default setting by @fthobe in https://github.com/solidusio/solidus/pull/6171 +* Separate order mailer subscriber from reimbursement mailer subscriber by @benjaminwil in https://github.com/solidusio/solidus/pull/6156 +* Fixed migrations so you can rollback them all by @aiperon in https://github.com/solidusio/solidus/pull/6188 +* Inherit from ActiveRecord::Migration version for all supported Rails by @harmonymjb in https://github.com/solidusio/solidus/pull/6192 +* Move OrderMailerSubscriber#send_confirmation_email by @benjaminwil in https://github.com/solidusio/solidus/pull/6199 +* Add reverse charge status to stores by @fthobe in https://github.com/solidusio/solidus/pull/6136 +* Fix flaky test errors using chrome 134 by @tvdeyen in https://github.com/solidusio/solidus/pull/6203 +* Move carton shipped emails to subscriber by @benjaminwil in https://github.com/solidusio/solidus/pull/6219 +* Change migration version to 7.0 by @AlistairNorman in https://github.com/solidusio/solidus/pull/6220 +* Add subscribers for inventory cancellation and order cancellation emails by @benjaminwil in https://github.com/solidusio/solidus/pull/6205 +* Make linters happy by @tvdeyen in https://github.com/solidusio/solidus/pull/6223 +* Disallow migrations with the wrong versions by @benjaminwil in https://github.com/solidusio/solidus/pull/6221 +* Add reverse charge fields to address by @fthobe in https://github.com/solidusio/solidus/pull/6168 +* Dummy app generator: Only configure app/assets/javascripts if present by @mamhoff in https://github.com/solidusio/solidus/pull/6227 +* Use Firefox for system specs by @mamhoff in https://github.com/solidusio/solidus/pull/6230 +* Configurable Solidus event subscribers by @benjaminwil in https://github.com/solidusio/solidus/pull/6234 +* Move Taxon -> Promotion Rule association to legacy promotions by @mamhoff in https://github.com/solidusio/solidus/pull/6243 +* Require spree/config in spree/core by @mamhoff in https://github.com/solidusio/solidus/pull/6248 +* Addressbook: Add foreign key, dependent/inverse_of options by @mamhoff in https://github.com/solidusio/solidus/pull/6265 +* Replace `puts` in tasks and generators with Rails.logger or Logger.new by @mamhoff in https://github.com/solidusio/solidus/pull/6244 + +## Solidus Admin + +* Fix typos by @jackhac in https://github.com/solidusio/solidus/pull/6207 +* Display the store's currency in the Admin Order Index Component by @magpieuk in https://github.com/solidusio/solidus/pull/5929 +* Fix flaky test errors using chrome 134 by @tvdeyen in https://github.com/solidusio/solidus/pull/6203 +* Make linters happy by @tvdeyen in https://github.com/solidusio/solidus/pull/6223 +* Add reverse charge fields to address by @fthobe in https://github.com/solidusio/solidus/pull/6168 +* Use Firefox for system specs by @mamhoff in https://github.com/solidusio/solidus/pull/6230 +* Fix install_lookbook step by @chaimann in https://github.com/solidusio/solidus/pull/6154 +* Fix ui/forms/input component for tag: :textarea by @chaimann in https://github.com/solidusio/solidus/pull/6174 +* [Admin] Fix Unclosed form_tag in table component by @swamp09 in https://github.com/solidusio/solidus/pull/6172 +* Fix flaky specs by @mamhoff in https://github.com/solidusio/solidus/pull/6197 +* [Backend] Fix issue refunding uncompleted payments by @jtapia in https://github.com/solidusio/solidus/pull/6094 +* [Admin][UI] New select component by @chaimann in https://github.com/solidusio/solidus/pull/6190 +* Refactor `ui/forms/address` component by @chaimann in https://github.com/solidusio/solidus/pull/6191 +* Use semantic links to edit option types by @forkata in https://github.com/solidusio/solidus/pull/6201 +* Admin select component performance by @chaimann in https://github.com/solidusio/solidus/pull/6213 +* Refactor address form component (properly this time) by @chaimann in https://github.com/solidusio/solidus/pull/6225 +* [Admin][UI] Alert component by @chaimann in https://github.com/solidusio/solidus/pull/6226 +* [Admin] fix table sorting by @chaimann in https://github.com/solidusio/solidus/pull/6238 +* Update importmap-rails to v2 by @tvdeyen in https://github.com/solidusio/solidus/pull/6202 + +## Solidus Backend + +* Add reverse charge status to stores by @fthobe in https://github.com/solidusio/solidus/pull/6136 +* Fix flaky test errors using chrome 134 by @tvdeyen in https://github.com/solidusio/solidus/pull/6203 +* Add reverse charge fields to address by @fthobe in https://github.com/solidusio/solidus/pull/6168 +* Fix flaky specs by @mamhoff in https://github.com/solidusio/solidus/pull/6197 +* [Backend] Fix issue refunding uncompleted payments by @jtapia in https://github.com/solidusio/solidus/pull/6094 +* Add 500ms delay before AJAX in Select2 by @mamhoff in https://github.com/solidusio/solidus/pull/6235 + +## Solidus API + +* Add reverse charge status to stores by @fthobe in https://github.com/solidusio/solidus/pull/6136 +* Add reverse charge fields to address by @fthobe in https://github.com/solidusio/solidus/pull/6168 +* Refactor load_user_roles into current_user_roles helper by @mamhoff in https://github.com/solidusio/solidus/pull/6245 +* Refactor "current_api_user" into instacached helper by @mamhoff in https://github.com/solidusio/solidus/pull/6246 + +## Solidus Promotions + +* Fix typos by @jackhac in https://github.com/solidusio/solidus/pull/6207 +* Fix flaky test errors using chrome 134 by @tvdeyen in https://github.com/solidusio/solidus/pull/6203 +* Replace `puts` in tasks and generators with Rails.logger or Logger.new by @mamhoff in https://github.com/solidusio/solidus/pull/6244 +* Fix flaky specs by @mamhoff in https://github.com/solidusio/solidus/pull/6197 +* Update importmap-rails to v2 by @tvdeyen in https://github.com/solidusio/solidus/pull/6202 +* [Promotions] Set Flickwerk patches in initializer by @tvdeyen in https://github.com/solidusio/solidus/pull/6161 +* Fix Rubocop offense by @mamhoff in https://github.com/solidusio/solidus/pull/6196 +* Use `human_attribute_name` for promo calculator labels by @mamhoff in https://github.com/solidusio/solidus/pull/6195 +* Promotions: Add a PercentWithCap calculator by @mamhoff in https://github.com/solidusio/solidus/pull/6200 + +**Full Changelog**: https://github.com/solidusio/solidus/compare/v4.5.0...v4.6.0 + + ## Solidus v4.5.0 (2025-02-19) diff --git a/core/lib/generators/solidus/install/app_templates/frontend/starter.rb b/core/lib/generators/solidus/install/app_templates/frontend/starter.rb index 7a6c7ee377d..7876d2ea284 100644 --- a/core/lib/generators/solidus/install/app_templates/frontend/starter.rb +++ b/core/lib/generators/solidus/install/app_templates/frontend/starter.rb @@ -1 +1 @@ -apply 'https://github.com/solidusio/solidus_starter_frontend/raw/main/template.rb' +apply 'https://github.com/solidusio/solidus_starter_frontend/raw/v4.6/template.rb' diff --git a/core/lib/spree/core/version.rb b/core/lib/spree/core/version.rb index e623f5decdf..a4d26d8fab4 100644 --- a/core/lib/spree/core/version.rb +++ b/core/lib/spree/core/version.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Spree - VERSION = "4.6.0.dev" + VERSION = "4.6.0" def self.solidus_version = VERSION diff --git a/docker-compose.yml b/docker-compose.yml index e3d42120cd7..908a2810806 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -28,7 +28,7 @@ services: NODE_VERSION: 20 MYSQL_VERSION: "8.0" BUNDLER_VERSION: 2 - image: solidus-4.6.0.dev + image: solidus-4.6.0 command: bash -c "(bundle check || bundle) && bash -c 'echo Container initialized, see README.md for further steps.' && tail -f /dev/null" environment: CAPYBARA_DRIVER: selenium_chrome_headless_docker_friendly From cc85f98f32c4f473b7693a87b93689d6665a92d7 Mon Sep 17 00:00:00 2001 From: tvdeyen Date: Tue, 9 Sep 2025 08:14:12 +0000 Subject: [PATCH 4/4] Post-release chores after having released Solidus v4.6.0 This code has been automatically generated by our 'Prepare post-release' GitHub action. Make sure that: - [ ] The new release has been published, along with its tag. See https://github.com/solidusio/solidus/releases/tag/v4.6.0 - [ ] The corresponding patch branch exists. See https://github.com/solidusio/solidus/tree/v4.6 - [ ] The corresponding backport-v4.6 label exists. See https://github.com/solidusio/solidus/labels --- .../solidus/install/app_templates/frontend/starter.rb | 2 +- core/lib/spree/core/version.rb | 4 ++-- docker-compose.yml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/core/lib/generators/solidus/install/app_templates/frontend/starter.rb b/core/lib/generators/solidus/install/app_templates/frontend/starter.rb index 7876d2ea284..7a6c7ee377d 100644 --- a/core/lib/generators/solidus/install/app_templates/frontend/starter.rb +++ b/core/lib/generators/solidus/install/app_templates/frontend/starter.rb @@ -1 +1 @@ -apply 'https://github.com/solidusio/solidus_starter_frontend/raw/v4.6/template.rb' +apply 'https://github.com/solidusio/solidus_starter_frontend/raw/main/template.rb' diff --git a/core/lib/spree/core/version.rb b/core/lib/spree/core/version.rb index a4d26d8fab4..2f26a1a4552 100644 --- a/core/lib/spree/core/version.rb +++ b/core/lib/spree/core/version.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true module Spree - VERSION = "4.6.0" + VERSION = "4.7.0.dev" def self.solidus_version = VERSION def self.minimum_required_rails_version = "7.0" - def self.previous_solidus_minor_version = "4.5" + def self.previous_solidus_minor_version = "4.6" def self.solidus_gem_version = Gem::Version.new(solidus_version) end diff --git a/docker-compose.yml b/docker-compose.yml index 908a2810806..3e5b7009f97 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -28,7 +28,7 @@ services: NODE_VERSION: 20 MYSQL_VERSION: "8.0" BUNDLER_VERSION: 2 - image: solidus-4.6.0 + image: solidus-4.7.0.dev command: bash -c "(bundle check || bundle) && bash -c 'echo Container initialized, see README.md for further steps.' && tail -f /dev/null" environment: CAPYBARA_DRIVER: selenium_chrome_headless_docker_friendly