diff --git a/Gemfile.lock b/Gemfile.lock index d7f53aa31c..3e772a2836 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -383,9 +383,9 @@ GEM knapsack_pro (6.0.4) rake language_server-protocol (3.17.0.3) - launchy (2.5.0) - addressable (~> 2.7) - letter_opener (1.8.1) + launchy (2.5.2) + addressable (~> 2.8) + letter_opener (1.9.0) launchy (>= 2.2, < 3) link_header (0.0.8) listen (3.8.0) diff --git a/app/controllers/api/v0/shipments_controller.rb b/app/controllers/api/v0/shipments_controller.rb index 05376a2666..d742f2c29e 100644 --- a/app/controllers/api/v0/shipments_controller.rb +++ b/app/controllers/api/v0/shipments_controller.rb @@ -23,6 +23,8 @@ module Api OrderWorkflow.new(@order).advance_to_payment if @order.line_items.any? + @order.recreate_all_fees! + render json: @shipment, serializer: Api::ShipmentSerializer, status: :ok end diff --git a/app/views/admin/order_cycles/_advanced_settings.html.haml b/app/views/admin/order_cycles/_advanced_settings.html.haml index 455329b68c..6c04ce73b9 100644 --- a/app/views/admin/order_cycles/_advanced_settings.html.haml +++ b/app/views/admin/order_cycles/_advanced_settings.html.haml @@ -22,7 +22,7 @@ .omega.eight.columns = f.check_box :automatic_notifications - .row{ "data-controller": "remote-toggle", "data-remote-toggle-selector-value": "#advanced_settings" } + .row{ "data-controller": "toggle-control", "data-toggle-control-selector-value": "#advanced_settings" } .sixteen.columns.alpha.omega.text-center %input{ type: 'submit', value: t('.save_reload') } - %a{ href: "#", "data-action": "click->remote-toggle#toggle" }= t(:close) + %a{ href: "#", "data-action": "click->toggle-control#toggleAdvancedSettings" }= t(:close) diff --git a/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml b/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml index 2cc89117b0..8d5cd0d6ed 100644 --- a/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml +++ b/app/views/admin/order_cycles/_order_cycle_top_buttons.html.haml @@ -1,8 +1,8 @@ - content_for :page_actions do - %li{ "data-controller": "remote-toggle", "data-remote-toggle-selector-value": "#advanced_settings" } - %button#toggle_settings{ "data-action": "click->remote-toggle#toggle" } + %li{ "data-controller": "toggle-control", "data-toggle-control-selector-value": "#advanced_settings" } + %button#toggle_settings{ "data-action": "click->toggle-control#toggleAdvancedSettings" } = t('.advanced_settings') - %i.icon-chevron-down{ "data-remote-toggle-target": "chevron" } + %i.icon-chevron-down{ "data-toggle-control-target": "chevron" } #advanced_settings{ style: "display: none" } = render partial: "/admin/order_cycles/advanced_settings" diff --git a/app/webpacker/controllers/remote_toggle_controller.js b/app/webpacker/controllers/remote_toggle_controller.js deleted file mode 100644 index ad6944f837..0000000000 --- a/app/webpacker/controllers/remote_toggle_controller.js +++ /dev/null @@ -1,16 +0,0 @@ -import { Controller } from "stimulus"; - -export default class extends Controller { - static targets = ["chevron"]; - static values = { selector: String }; - - toggle(event) { - if (this.hasChevronTarget) { - this.chevronTarget.classList.toggle("icon-chevron-down"); - this.chevronTarget.classList.toggle("icon-chevron-up"); - } - - const element = document.querySelector(this.selectorValue); - element.style.display = element.style.display === "none" ? "block" : "none"; - } -} diff --git a/app/webpacker/controllers/toggle_control_controller.js b/app/webpacker/controllers/toggle_control_controller.js index d1e4a4692c..7ba2bdb85d 100644 --- a/app/webpacker/controllers/toggle_control_controller.js +++ b/app/webpacker/controllers/toggle_control_controller.js @@ -1,7 +1,8 @@ import { Controller } from "stimulus"; export default class extends Controller { - static targets = ["control", "content"]; + static targets = ["control", "content", "chevron"]; + static values = { selector: String }; disableIfPresent(event) { const input = event.currentTarget; @@ -32,7 +33,16 @@ export default class extends Controller { t.style.display = input.dataset.toggleShow === "true" ? "block" : "none"; }); } - //todo: can toggleDisplay with optional chevron-target replace RemoteToggleController? + + toggleAdvancedSettings(event) { + if (this.hasChevronTarget) { + this.chevronTarget.classList.toggle("icon-chevron-down"); + this.chevronTarget.classList.toggle("icon-chevron-up"); + } + + const element = document.querySelector(this.selectorValue); + element.style.display = element.style.display === "none" ? "block" : "none"; + } // private diff --git a/engines/order_management/app/services/order_management/order/updater.rb b/engines/order_management/app/services/order_management/order/updater.rb index 6c311dade7..97b77ace28 100644 --- a/engines/order_management/app/services/order_management/order/updater.rb +++ b/engines/order_management/app/services/order_management/order/updater.rb @@ -38,13 +38,6 @@ module OrderManagement update_pending_payment end - def update_pending_payment - return unless order.state.in? ["payment", "confirmation"] - return unless order.pending_payments.any? - - order.pending_payments.first.update_attribute :amount, order.total - end - # Updates the following Order total values: # # - payment_total - total value of all finalized Payments (excludes non-finalized Payments) @@ -239,6 +232,16 @@ module OrderManagement def requires_authorization? payments.requires_authorization.any? && payments.completed.empty? end + + def update_pending_payment + # We only want to update complete order pending payment when it's a cash payment. We assume + # that if the payment was a credit card it would alread have been processed, so we don't + # bother checking the payment type + return unless order.state.in? ["payment", "confirmation", "complete"] + return unless order.pending_payments.any? + + order.pending_payments.first.update_attribute :amount, order.total + end end end end diff --git a/engines/order_management/spec/services/order_management/order/updater_spec.rb b/engines/order_management/spec/services/order_management/order/updater_spec.rb index 921c4cc18a..0447e84cb8 100644 --- a/engines/order_management/spec/services/order_management/order/updater_spec.rb +++ b/engines/order_management/spec/services/order_management/order/updater_spec.rb @@ -8,7 +8,7 @@ module OrderManagement let(:order) { create(:order) } let(:updater) { OrderManagement::Order::Updater.new(order) } - context "updating order totals" do + describe "#update_totals" do before do 2.times { create(:line_item, order:, price: 10) } end @@ -19,11 +19,23 @@ module OrderManagement updater.update_totals expect(order.payment_total).to eq(10) end + end + + describe "#update_item_total" do + before do + 2.times { create(:line_item, order:, price: 10) } + end it "updates item total" do updater.update_item_total expect(order.item_total).to eq(20) end + end + + describe "#update_adjustment_total" do + before do + 2.times { create(:line_item, order:, price: 10) } + end it "updates adjustment totals" do allow(order).to receive_message_chain(:all_adjustments, :additional, :eligible, @@ -40,7 +52,7 @@ module OrderManagement end end - context "updating shipment state" do + describe "#update_shipment_state" do let(:shipment) { build(:shipment) } before do @@ -85,19 +97,111 @@ module OrderManagement order.state_changed('shipment') end - context "completed order" do + describe "#update" do + it "updates totals once" do + expect(updater).to receive(:update_totals).once + updater.update + end + + it "updates all adjustments" do + expect(updater).to receive(:update_all_adjustments) + updater.update + end + + context "completed order" do + before { allow(order).to receive(:completed?) { true } } + + it "updates payment state" do + expect(updater).to receive(:update_payment_state) + updater.update + end + + it "updates shipment state" do + expect(updater).to receive(:update_shipment_state) + updater.update + end + + context "whith pending payments" do + let(:order) { create(:completed_order_with_totals) } + + it "updates pending payments" do + payment = create(:payment, order:, amount: order.total) + + # update order so the order total will change + update_order_quantity(order) + order.payments.reload + + expect { updater.update }.to change { payment.reload.amount }.from(50).to(60) + 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(:update_payment_state) + updater.update + end + + it "doesnt update shipment state" do + expect(updater).not_to receive(:update_shipment_state) + updater.update + end + + it "doesnt update the order shipment" do + shipment = build(:shipment) + allow(order).to receive_messages shipments: [shipment] + + expect(shipment).not_to receive(:update!).with(order) + expect(updater).not_to receive(:update_shipments).with(order) + updater.update + end + + context "with pending payments" do + let!(:payment) { create(:payment, order:, amount: order.total) } + + context "with order in payment state" do + let(:order) { create(:order_with_totals, state: "payment") } + + it "updates pending payments" do + # update order so the order total will change + update_order_quantity(order) + order.payments.reload + + expect { updater.update }.to change { payment.reload.amount }.from(10).to(20) + end + end + + context "with order in confirmation state" do + let(:order) { create(:order_with_totals, state: "confirmation") } + + it "updates pending payments" do + # update order so the order total will change + update_order_quantity(order) + order.payments.reload + + expect { updater.update }.to change { payment.reload.amount }.from(10).to(20) + end + end + + context "with order in cart" do + let(:order) { create(:order_with_totals) } + + it "doesn't update pending payments" do + # update order so the order total will change + update_order_quantity(order) + + expect { updater.update }.not_to change { payment.reload.amount } + end + end + end + end + end + + describe "#update_shipments" do before { allow(order).to receive(:completed?) { true } } - it "updates payment state" do - expect(updater).to receive(:update_payment_state) - updater.update - end - - it "updates shipment state" do - expect(updater).to receive(:update_shipment_state) - updater.update - end - it "updates the order shipment" do shipment = build(:shipment) allow(order).to receive_messages shipments: [shipment] @@ -107,39 +211,6 @@ module OrderManagement 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(:update_payment_state) - updater.update - end - - it "doesnt update shipment state" do - expect(updater).not_to receive(:update_shipment_state) - updater.update - end - - it "doesnt update the order shipment" do - shipment = build(:shipment) - allow(order).to receive_messages shipments: [shipment] - - expect(shipment).not_to receive(:update!).with(order) - expect(updater).not_to receive(:update_shipments).with(order) - updater.update - end - end - - it "updates totals once" do - expect(updater).to receive(:update_totals).once - updater.update - end - - it "updates all adjustments" do - expect(updater).to receive(:update_all_adjustments) - updater.update - end - describe "#update_payment_state" do context "when the order has no valid payments" do it "is failed" do @@ -277,9 +348,28 @@ module OrderManagement end end end + + context "when unused payments records exist which require authorization, " \ + "but the order is fully paid" do + let!(:cash_payment) { + build(:payment, state: "completed", amount: order.new_outstanding_balance) + } + let!(:stripe_payment) { build(:payment, state: "requires_authorization") } + before do + order.payments << cash_payment + order.payments << stripe_payment + end + + it "cancels unused payments requiring authorization" do + expect(stripe_payment).to receive(:void_transaction!) + expect(cash_payment).to_not receive(:void_transaction!) + + order.updater.update_payment_state + end + end end - context '#shipping_address_from_distributor' do + describe '#shipping_address_from_distributor' do let(:distributor) { build(:distributor_enterprise) } let(:shipment) { create(:shipment_with, :shipping_method, shipping_method:) @@ -313,69 +403,52 @@ module OrderManagement end end - describe "updating order totals" do - describe "#update_totals_and_states" do - it "deals with legacy taxes" do - expect(updater).to receive(:handle_legacy_taxes) + describe "#update_totals_and_states" do + it "deals with legacy taxes" do + expect(updater).to receive(:handle_legacy_taxes) - updater.update_totals_and_states + updater.update_totals_and_states + end + end + + describe "#handle_legacy_taxes" do + context "when the order is incomplete" do + it "doesn't touch taxes" do + allow(order).to receive(:completed?) { false } + + expect(order).to_not receive(:create_tax_charge!) + updater.__send__(:handle_legacy_taxes) end end - describe "#handle_legacy_taxes" do - context "when the order is incomplete" do - it "doesn't touch taxes" do - allow(order).to receive(:completed?) { false } + context "when the order is complete" do + before { allow(order).to receive(:completed?) { true } } + + context "and the order has legacy taxes" do + let!(:legacy_tax_adjustment) { + create(:adjustment, order:, adjustable: order, included: false, + originator_type: "Spree::TaxRate") + } + + it "re-applies order taxes" do + expect(order).to receive(:create_tax_charge!) - expect(order).to_not receive(:create_tax_charge!) updater.__send__(:handle_legacy_taxes) end end - context "when the order is complete" do - before { allow(order).to receive(:completed?) { true } } + context "and the order has no legacy taxes" do + it "leaves taxes untouched" do + expect(order).to_not receive(:create_tax_charge!) - context "and the order has legacy taxes" do - let!(:legacy_tax_adjustment) { - create(:adjustment, order:, adjustable: order, included: false, - originator_type: "Spree::TaxRate") - } - - it "re-applies order taxes" do - expect(order).to receive(:create_tax_charge!) - - updater.__send__(:handle_legacy_taxes) - end - end - - context "and the order has no legacy taxes" do - it "leaves taxes untouched" do - expect(order).to_not receive(:create_tax_charge!) - - updater.__send__(:handle_legacy_taxes) - end + updater.__send__(:handle_legacy_taxes) end end end end - context "when unused payments records exist which require authorization, " \ - "but the order is fully paid" do - let!(:cash_payment) { - build(:payment, state: "completed", amount: order.new_outstanding_balance) - } - let!(:stripe_payment) { build(:payment, state: "requires_authorization") } - before do - order.payments << cash_payment - order.payments << stripe_payment - end - - it "cancels unused payments requiring authorization" do - expect(stripe_payment).to receive(:void_transaction!) - expect(cash_payment).to_not receive(:void_transaction!) - - order.updater.update_payment_state - end + def update_order_quantity(order) + order.line_items.first.update_attribute(:quantity, 2) end end end diff --git a/spec/controllers/api/v0/shipments_controller_spec.rb b/spec/controllers/api/v0/shipments_controller_spec.rb index 98584c9121..9ade418913 100644 --- a/spec/controllers/api/v0/shipments_controller_spec.rb +++ b/spec/controllers/api/v0/shipments_controller_spec.rb @@ -91,6 +91,18 @@ describe Api::V0::ShipmentsController, type: :controller do expect_error_response end + + it "applies any enterprise fees that are present" do + order_cycle = create(:simple_order_cycle, + coordinator: order.distributor, + coordinator_fees: [create(:enterprise_fee, amount: 20)], + distributors: [order.distributor], + variants: [variant]) + order.update!(order_cycle_id: order_cycle.id) + spree_post :create, params + + expect(order.line_item_adjustments.where(originator_type: "EnterpriseFee")).to be_present + end end it "can make a shipment ready" do diff --git a/spec/factories/tax_category_factory.rb b/spec/factories/tax_category_factory.rb index 0cb0f3af4a..5503091437 100644 --- a/spec/factories/tax_category_factory.rb +++ b/spec/factories/tax_category_factory.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :tax_category, class: Spree::TaxCategory do - name { "TaxCategory - #{rand(999_999)}" } + sequence(:name) { |n| "TaxCategory - #{n}" } description { generate(:random_string) } end end diff --git a/spec/factories/zone_factory.rb b/spec/factories/zone_factory.rb index 4d4ed09542..95c5ae8ddc 100644 --- a/spec/factories/zone_factory.rb +++ b/spec/factories/zone_factory.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :zone, class: Spree::Zone do - name { generate(:random_string) } + sequence(:name) { |n| "#{generate(:random_string)}#{n}" } description { generate(:random_string) } end diff --git a/spec/javascripts/stimulus/remote_toggle_controller_test.js b/spec/javascripts/stimulus/remote_toggle_controller_test.js deleted file mode 100644 index 507ce3827d..0000000000 --- a/spec/javascripts/stimulus/remote_toggle_controller_test.js +++ /dev/null @@ -1,59 +0,0 @@ -/** - * @jest-environment jsdom - */ - -import { Application } from "stimulus"; -import remote_toggle_controller from "../../../app/webpacker/controllers/remote_toggle_controller"; - -describe("RemoteToggleController", () => { - beforeAll(() => { - const application = Application.start(); - application.register("remote-toggle", remote_toggle_controller); - }); - - describe("#toggle", () => { - beforeEach(() => { - document.body.innerHTML = ` -