From b80274f49d9844ffca0b56024d9993f26e571343 Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Tue, 2 May 2023 14:12:31 +1000 Subject: [PATCH] Per review comment, Use named value on voucher submit button to distinguish between submission types The voucher apply button is inside form that has another a submit button, it leads to a weird situation where either one will submit the whole payments page form. Adding a named parameter on the voucher apply button means we can distinguish between the two by checking for the presence of params[:apply_voucher]. --- app/controllers/split_checkout_controller.rb | 19 +++++++++++++------ .../_voucher_section.cable_ready.haml | 2 +- .../split_checkout_controller_spec.rb | 2 ++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 3c3cf45a59..4a4d1f0aca 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -30,11 +30,7 @@ class SplitCheckoutController < ::BaseController end def update - if add_voucher - return render_voucher_section_or_redirect - elsif @order.errors.present? - return render_error - end + return process_voucher if params[:apply_voucher].present? if confirm_order || update_order return if performed? @@ -206,8 +202,19 @@ class SplitCheckoutController < ::BaseController selected_shipping_method.first.require_ship_address == false end + def process_voucher + if add_voucher + render_voucher_section_or_redirect + elsif @order.errors.present? + render_error + end + end + def add_voucher - return unless payment_step? && params.dig(:order, :voucher_code).present? + if params.dig(:order, :voucher_code).blank? + @order.errors.add(:voucher, I18n.t('split_checkout.errors.voucher_not_found')) + return false + end # Fetch Voucher voucher = Voucher.find_by(code: params[:order][:voucher_code], enterprise: @order.distributor) diff --git a/app/views/split_checkout/_voucher_section.cable_ready.haml b/app/views/split_checkout/_voucher_section.cable_ready.haml index 7708716eda..786eff7288 100644 --- a/app/views/split_checkout/_voucher_section.cable_ready.haml +++ b/app/views/split_checkout/_voucher_section.cable_ready.haml @@ -16,4 +16,4 @@ = t("split_checkout.step2.voucher.warning_forfeit_remaining_amount") - else = text_field_tag "[order][voucher_code]", params.dig(:order, :voucher_code), data: { action: "input->toggle-button-disabled#inputIsChanged", }, placeholder: t("split_checkout.step2.voucher.placeholder") , class: "voucher" - = submit_tag t("split_checkout.step2.voucher.apply"), disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } + = submit_tag t("split_checkout.step2.voucher.apply"), name: "apply_voucher", disabled: true, class: "button cancel voucher", "data-disable-with": false, data: { "toggle-button-disabled-target": "button" } diff --git a/spec/controllers/split_checkout_controller_spec.rb b/spec/controllers/split_checkout_controller_spec.rb index a08ca12576..db3571d81b 100644 --- a/spec/controllers/split_checkout_controller_spec.rb +++ b/spec/controllers/split_checkout_controller_spec.rb @@ -242,6 +242,7 @@ describe SplitCheckoutController, type: :controller do describe "adding a voucher" do let(:checkout_params) do { + apply_voucher: "true", order: { voucher_code: voucher.code } @@ -261,6 +262,7 @@ describe SplitCheckoutController, type: :controller do context "when voucher doesn't exist" do let(:checkout_params) do { + apply_voucher: "true", order: { voucher_code: "non_voucher" }