Merge pull request #10428 from jibees/9145-Splitcheckout,-add-error-message-when-no-shipping-methods-are-available-to-the-customer

SplitCheckout: improve error messages in flash banner to improve user experience

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

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"

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
 

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