diff --git a/app/controllers/split_checkout_controller.rb b/app/controllers/split_checkout_controller.rb index 6ded33fb28..752cec3010 100644 --- a/app/controllers/split_checkout_controller.rb +++ b/app/controllers/split_checkout_controller.rb @@ -49,7 +49,7 @@ class SplitCheckoutController < ::BaseController def render_error flash.now[:error] ||= I18n.t( 'split_checkout.errors.saving_failed', - messages: @order.errors.full_messages.to_sentence + messages: order_error_messages ) render status: :unprocessable_entity, operations: cable_car. @@ -57,6 +57,50 @@ class SplitCheckoutController < ::BaseController replace("#flashes", partial("shared/flashes", locals: { flashes: flash })) end + def order_error_messages + # Remove ship_address.* errors if no shipping method is not selected + remove_ship_address_errors if no_ship_address_needed? + + # Reorder errors to make sure the most important ones are shown first + # and finally, return the error messages to sentence + reorder_errors.map(&:full_message).to_sentence + end + + def no_ship_address_needed? + @order.errors[:shipping_method].present? || params[:ship_address_same_as_billing] == "1" + end + + def remove_ship_address_errors + @order.errors.delete("ship_address.firstname") + @order.errors.delete("ship_address.address1") + @order.errors.delete("ship_address.city") + @order.errors.delete("ship_address.phone") + @order.errors.delete("ship_address.lastname") + @order.errors.delete("ship_address.zipcode") + end + + def reorder_errors + @order.errors.sort_by do |e| + case e.attribute + when /email/i then 0 + when /phone/i then 1 + when /bill_address/i then 2 + bill_address_error_order(e) + else 20 + end + end + end + + def bill_address_error_order(error) + case error.attribute + when /firstname/i then 0 + when /lastname/i then 1 + when /address1/i then 2 + when /city/i then 3 + when /zipcode/i then 4 + else 5 + end + end + def flash_error_when_no_shipping_method_available flash[:error] = I18n.t('split_checkout.errors.no_shipping_methods_available') end diff --git a/config/locales/en.yml b/config/locales/en.yml index 019042608e..49b7506aef 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -27,14 +27,22 @@ en: spree/shipping_method: Shipping Method attributes: spree/order/ship_address: - address1: "Shipping address line 1" + address1: "Shipping address (Street + House number)" address2: "Shipping address line 2" - city: "Shipping address suburb 1" + city: "Shipping address city" country: "Shipping address country" phone: "Phone number" firstname: "First name" lastname: "Last name" zipcode: "Shipping address postcode" + spree/order/bill_address: + address1: "Billing address (Street + House number)" + zipcode: "Billing address postcode" + city: "Billing address city" + country: "Billing address country" + firstname: "Billing address first name" + lastname: "Billing address last name" + phone: Customer phone spree/user: password: "Password" password_confirmation: "Password confirmation" diff --git a/spec/models/spree/order_spec.rb b/spec/models/spree/order_spec.rb index 400c02fe86..8becdc5ce9 100644 --- a/spec/models/spree/order_spec.rb +++ b/spec/models/spree/order_spec.rb @@ -12,7 +12,13 @@ describe Spree::Order do it "provides friendly error messages" do order.ship_address = Spree::Address.new order.save - expect(order.errors.full_messages).to include "Shipping address line 1 can't be blank" + expect(order.errors.full_messages).to include "Shipping address (Street + House number) can't be blank" + end + + it "provides friendly error messages for bill address" do + order.bill_address = Spree::Address.new + order.save + expect(order.errors.full_messages).to include "Billing address (Street + House number) can't be blank" end end diff --git a/spec/system/consumer/split_checkout_spec.rb b/spec/system/consumer/split_checkout_spec.rb index 80eec57fcb..b293aff3df 100644 --- a/spec/system/consumer/split_checkout_spec.rb +++ b/spec/system/consumer/split_checkout_spec.rb @@ -187,6 +187,35 @@ describe "As a consumer, I want to checkout my order" do click_on "Checkout as guest" end + context "should show proper list of errors" do + before do + click_button "Next - Payment method" + expect(page).to have_content "Saving failed, please update the highlighted fields." + end + + it "should not display any shipping errors messages when shipping method is not selected" do + expect(page).not_to have_content "Shipping address line 1 can't be blank" + expect(page).not_to have_content "Shipping address suburb 1 can't be blank" + expect(page).not_to have_content "Shipping address postcode can't be blank" + end + + it "should not display bill address phone number error message" do + expect(page).not_to have_content "Bill address phone can't be blank" + expect(page).to have_content "Customer phone can't be blank" + end + + context "with no email filled in" do + before do + fill_in "Email", with: "" + click_button "Next - Payment method" + end + + it "should display error message in the right order" do + expect(page).to have_content "Customer E-Mail can't be blank, Customer E-Mail is invalid, Customer phone can't be blank, Billing address first name can't be blank, Billing address last name can't be blank, Billing address (Street + House number) can't be blank, Billing address city can't be blank, Billing address postcode can't be blank, and Shipping method Select a shipping method" + end + end + end + it "should allow visit '/checkout/details'" do expect(page).to have_current_path("/checkout/details") end @@ -306,8 +335,6 @@ describe "As a consumer, I want to checkout my order" do click_button "Next - Payment method" expect(page).to have_content "Saving failed, please update the highlighted fields." - expect(page).to have_content "Shipping address line 1 can't be blank" - expect(page).to have_content "Shipping address same as billing address?" expect(page).to have_content "Save as default shipping address" expect(page).to have_checked_field "Shipping address same as billing address?" end @@ -563,7 +590,7 @@ describe "As a consumer, I want to checkout my order" do end within ".flash[type='error']" do expect(page).to have_content("Saving failed, please update the highlighted fields") - expect(page).to have_content("can't be blank", count: 13) + expect(page).to have_content("can't be blank", count: 7) expect(page).to have_content("is invalid", count: 1) end end