From ef607da2c16511b19cc230e672bdf9e9366682cd Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 17 Mar 2023 10:40:53 +1100 Subject: [PATCH 1/2] Revert "Fix: Customers can checkout with non-matching shipping and product categories" --- .../concerns/checkout_callbacks.rb | 22 +++---------------- app/controllers/split_checkout_controller.rb | 2 +- app/views/split_checkout/_details.html.haml | 4 ++-- 3 files changed, 6 insertions(+), 22 deletions(-) diff --git a/app/controllers/concerns/checkout_callbacks.rb b/app/controllers/concerns/checkout_callbacks.rb index 5bb44eb9f3..4b6608b947 100644 --- a/app/controllers/concerns/checkout_callbacks.rb +++ b/app/controllers/concerns/checkout_callbacks.rb @@ -15,9 +15,7 @@ module CheckoutCallbacks prepend_before_action :require_distributor_chosen before_action :load_order, :associate_user, :load_saved_addresses, :load_saved_credit_cards - before_action :allowed_shipping_methods, if: -> { - params[:step] == "details" - } + before_action :load_shipping_methods, if: -> { params[:step] == "details" } before_action :ensure_order_not_completed before_action :ensure_checkout_allowed @@ -48,22 +46,8 @@ module CheckoutCallbacks @selected_card = nil end - def allowed_shipping_methods - @allowed_shipping_methods ||= sorted_available_shipping_methods.filter( - &method(:supports_all_products_shipping_categories?) - ) - end - - def sorted_available_shipping_methods - available_shipping_methods.sort { |a, b| a.name.casecmp(b.name) } - end - - def supports_all_products_shipping_categories?(shipping_method) - (products_shipping_categories - shipping_method.shipping_categories.pluck(:id)).empty? - end - - def products_shipping_categories - @products_shipping_categories ||= @order.products.pluck(:shipping_category_id).uniq + def load_shipping_methods + @shipping_methods = available_shipping_methods.sort { |a, b| a.name.casecmp(b.name) } end def redirect_to_shop? diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index cbb7c4e41c..3d8a708fc9 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -24,7 +24,7 @@ class SplitCheckoutController < ::BaseController check_step if params[:step] recalculate_tax if params[:step] == "summary" - flash_error_when_no_shipping_method_available if allowed_shipping_methods.none? + flash_error_when_no_shipping_method_available if available_shipping_methods.none? end def update diff --git a/app/views/split_checkout/_details.html.haml b/app/views/split_checkout/_details.html.haml index 8b20be09fb..e81c8c08a7 100644 --- a/app/views/split_checkout/_details.html.haml +++ b/app/views/split_checkout/_details.html.haml @@ -76,8 +76,8 @@ - display_ship_address = false - ship_method_description = nil - - selected_shipping_method ||= @allowed_shipping_methods[0].id if @allowed_shipping_methods.length == 1 - - @allowed_shipping_methods.each do |shipping_method| + - selected_shipping_method ||= @shipping_methods[0].id if @shipping_methods.length == 1 + - @shipping_methods.each do |shipping_method| - ship_method_is_selected = shipping_method.id == selected_shipping_method.to_i %div.checkout-input.checkout-input-radio = fields_for shipping_method do |shipping_method_form| From 788457618fdb628f608b73a441a42e8b36195407 Mon Sep 17 00:00:00 2001 From: David Cook Date: Fri, 17 Mar 2023 13:03:41 +1100 Subject: [PATCH 2/2] Check ship address required based on all available methods This check was implemented based on 'allowed' shipping methods, but we need to revert that logic. So for now, we can check all 'available' shipping methods. This could potentially result in the same query being run twice, because load_shipping_methods also loads it. I opted to keep things simple and not try to optimise here. --- app/controllers/split_checkout_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 3d8a708fc9..e33bd0038b 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -168,7 +168,7 @@ class SplitCheckoutController < ::BaseController end def shipping_method_ship_address_not_required? - selected_shipping_method = allowed_shipping_methods&.select do |sm| + selected_shipping_method = available_shipping_methods&.select do |sm| sm.id.to_s == params[:shipping_method_id] end