From cdcab7c7c6dd3d2338350e7bb8091790a80e1187 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Mon, 3 Feb 2025 15:57:45 +1100 Subject: [PATCH] Update insufficient stock logic We now check for insufficient stock when loading the checkout details or before updating the order. --- app/controllers/checkout_controller.rb | 5 ++ .../concerns/checkout_callbacks.rb | 1 - app/controllers/concerns/order_stock_check.rb | 10 ++-- spec/controllers/checkout_controller_spec.rb | 53 +++++++++++++++---- spec/system/consumer/checkout/guest_spec.rb | 28 ++++++++-- 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 15cc8ef2f7..52592c9138 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -26,6 +26,7 @@ class CheckoutController < BaseController if params[:step].blank? redirect_to_step_based_on_order else + handle_insufficient_stock if details_step? update_order_state check_step end @@ -36,6 +37,9 @@ class CheckoutController < BaseController end def update + return render cable_ready: cable_car.redirect_to(url: checkout_step_path(:details)) \ + unless sufficient_stock? + if confirm_order || update_order return if performed? @@ -152,6 +156,7 @@ class CheckoutController < BaseController # state. We need to do this when moving back to a previous checkout step, the update action takes # care of moving the order state forward. def update_order_state + # debugger return @order.back_to_payment if @order.confirmation? && payment_step? return unless @order.after_delivery_state? && details_step? diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb index 3b0fba0b1c..b6dbbfd6c1 100644 --- a/app/controllers/concerns/checkout_callbacks.rb +++ b/app/controllers/concerns/checkout_callbacks.rb @@ -25,7 +25,6 @@ module CheckoutCallbacks before_action :ensure_order_not_completed before_action :ensure_checkout_allowed - before_action :handle_insufficient_stock before_action :check_authorization end diff --git a/app/controllers/concerns/order_stock_check.rb b/app/controllers/concerns/order_stock_check.rb index db51cffbc5..aa926b4a1c 100644 --- a/app/controllers/concerns/order_stock_check.rb +++ b/app/controllers/concerns/order_stock_check.rb @@ -5,9 +5,12 @@ module OrderStockCheck extend ActiveSupport::Concern def valid_order_line_items? - @order.insufficient_stock_lines.empty? && - OrderCycles::DistributedVariantsService.new(@order.order_cycle, @order.distributor). - distributes_order_variants?(@order) + OrderCycles::DistributedVariantsService.new(@order.order_cycle, @order.distributor). + distributes_order_variants?(@order) + end + + def sufficient_stock? + Orders::CheckStockService.new(order: @order).sufficient_stock? end def handle_insufficient_stock @@ -17,6 +20,7 @@ module OrderStockCheck return if stock_service.sufficient_stock? @any_out_of_stock = true + @updated_variants = stock_service.update_line_items end def check_order_cycle_expiry diff --git a/spec/controllers/checkout_controller_spec.rb b/spec/controllers/checkout_controller_spec.rb index 71d3a3d91c..acef705a0c 100644 --- a/spec/controllers/checkout_controller_spec.rb +++ b/spec/controllers/checkout_controller_spec.rb @@ -144,6 +144,17 @@ RSpec.describe CheckoutController, type: :controller do expect(order.reload.state).to eq "payment" end + context "with insufficient stock" do + it "redirects to details page" do + allow(order).to receive_message_chain(:insufficient_stock_lines, + :empty?).and_return false + + put(:update, params:) + + expect_cable_ready_redirect(response) + end + end + describe "saving default addresses" do it "doesn't update default bill address on user" do expect { @@ -306,6 +317,17 @@ RSpec.describe CheckoutController, type: :controller do expect(response).to redirect_to checkout_step_path(:summary) end end + + context "with insufficient stock" do + it "redirects to details page" do + allow(order).to receive_message_chain(:insufficient_stock_lines, + :empty?).and_return false + + put(:update, params:) + + expect_cable_ready_redirect(response) + end + end end context "with no payment source" do @@ -468,6 +490,16 @@ RSpec.describe CheckoutController, type: :controller do end end + context "with insufficient stock" do + it "redirects to details page" do + allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return false + + put(:update, params:) + + expect_cable_ready_redirect(response) + end + end + context "when accepting T&Cs is required" do before do allow(TermsOfService).to receive(:platform_terms_required?) { true } @@ -582,17 +614,10 @@ RSpec.describe CheckoutController, type: :controller do ) end - shared_examples "handling stock issues" do |step| + shared_examples "handling not available items" do |step| context "#{step} step" do let(:step) { step.to_s } - it "redirects when some items are out of stock" do - allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return false - - get :edit - expect(response).to redirect_to cart_path - end - it "redirects when some items are not available" do allow(order).to receive_message_chain(:insufficient_stock_lines, :empty?).and_return true expect(order_cycle_distributed_variants).to receive( @@ -605,9 +630,9 @@ RSpec.describe CheckoutController, type: :controller do end end - it_behaves_like "handling stock issues", "details" - it_behaves_like "handling stock issues", "payment" - it_behaves_like "handling stock issues", "summary" + it_behaves_like "handling not available items", "details" + it_behaves_like "handling not available items", "payment" + it_behaves_like "handling not available items", "summary" end def mock_voucher_adjustment_service @@ -616,4 +641,10 @@ RSpec.describe CheckoutController, type: :controller do voucher_adjustment_service end + + def expect_cable_ready_redirect(response) + expect(response.parsed_body).to eq( + [{ "url" => "/checkout/details", "operation" => "redirectTo" }].to_json + ) + end end diff --git a/spec/system/consumer/checkout/guest_spec.rb b/spec/system/consumer/checkout/guest_spec.rb index 35b8b2fc8e..f90f205298 100644 --- a/spec/system/consumer/checkout/guest_spec.rb +++ b/spec/system/consumer/checkout/guest_spec.rb @@ -14,15 +14,24 @@ RSpec.describe "As a consumer, I want to checkout my order" do let(:product) { create(:taxed_product, supplier_id: supplier.id, price: 10, zone:, tax_rate_amount: 0.1) } + let(:product2) { + create(:taxed_product, supplier_id: supplier.id, price: 15, zone:, tax_rate_amount: 0.1) + } + let(:variant) { product.variants.first } + let(:variant2 ) { product2.variants.first } let!(:order_cycle) { create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], - coordinator: create(:distributor_enterprise), variants: [variant]) + coordinator: create(:distributor_enterprise), + variants: [variant, variant2]) } let(:order) { create(:order, order_cycle:, distributor:, bill_address_id: nil, ship_address_id: nil, state: "cart", - line_items: [create(:line_item, variant:)]) + line_items: [ + create(:line_item, variant:), + create(:line_item, variant: variant2, quantity: 3) + ]) } let(:fee_tax_rate) { create(:tax_rate, amount: 0.10, zone:, included_in_price: true) } @@ -277,11 +286,18 @@ RSpec.describe "As a consumer, I want to checkout my order" do shared_examples "when a line item is out of stock" do |session, step| context "as a #{session} user" do let(:user) { create(:user) } + before do + # Out of stock variant.on_demand = false variant.on_hand = 0 variant.save! + # Reduced stock + variant2.on_demand = false + variant2.on_hand = 1 + variant2.save! + if session == "logged" login_as(user) end @@ -292,7 +308,13 @@ RSpec.describe "As a consumer, I want to checkout my order" do expect(page).to have_selector 'closing', text: "Checkout now" within "#out-of-stock-items" do - expect(page).to have_content "Some items are out of stock" + expect(page).to have_content "Reduced stock available" + expect(page).to have_content( + "#{variant.name_to_display} - #{variant.unit_to_display} is now out of stock" + ) + expect(page).to have_content( + "#{variant2.name_to_display} - #{variant2.unit_to_display} now only has 1 remaining" + ) end end end