Merge pull request #5076 from luisramos0/rails_f_helper

Remove Rails Foundation Helper and improve error display and logging in checkout controller

Gemfile

@@ -112,7 +112,6 @@ gem 'momentjs-rails'
 gem 'turbo-sprockets-rails3'
 
 gem "foundation-rails"
-gem 'foundation_rails_helper', github: 'willrjmarshall/foundation_rails_helper', branch: "rails3"
 
 gem 'jquery-migrate-rails'
 gem 'jquery-rails', '3.1.5'

Gemfile.lock

@@ -65,16 +65,6 @@ GIT
       rails-i18n
       spree_core (>= 1.1)
 
-GIT
-  remote: https://github.com/willrjmarshall/foundation_rails_helper.git
-  revision: 4d5d53fdc4b1fb71e66524d298c5c635de82cfbb
-  branch: rails3
-  specs:
-    foundation_rails_helper (0.4)
-      actionpack (>= 3.0)
-      activemodel (>= 3.0)
-      railties (>= 3.0)
-
 PATH
   remote: engines/catalog
   specs:
@@ -740,7 +730,6 @@ DEPENDENCIES
   foreigner
   foundation-icons-sass-rails
   foundation-rails
-  foundation_rails_helper!
   fuubar (~> 2.5.0)
   geocoder
   gmaps4rails

app/assets/javascripts/darkswarm/services/stripe_elements.js.coffee

@@ -15,6 +15,7 @@ Darkswarm.factory 'StripeElements', ($rootScope, Loading, RailsFlashLoader) ->
         if(response.error)
           Loading.clear()
           RailsFlashLoader.loadFlash({error: t("error") + ": #{response.error.message}"})
+          @triggerAngularDigest()
         else
           secrets.token = response.token.id
           secrets.cc_type = @mapCC(response.token.card.brand)
@@ -32,12 +33,17 @@ Darkswarm.factory 'StripeElements', ($rootScope, Loading, RailsFlashLoader) ->
         if(response.error)
           Loading.clear()
           RailsFlashLoader.loadFlash({error: t("error") + ": #{response.error.message}"})
+          @triggerAngularDigest()
         else
           secrets.token = response.paymentMethod.id
           secrets.cc_type = response.paymentMethod.card.brand
           secrets.card = response.paymentMethod.card
           submit()
 
+    triggerAngularDigest: ->
+      # $evalAsync is improved way of triggering a digest without calling $apply
+      $rootScope.$evalAsync()
+
     # Maps the brand returned by Stripe to that required by activemerchant
     mapCC: (ccType) ->
       switch ccType

app/assets/stylesheets/darkswarm/checkout.css.scss

@@ -109,9 +109,4 @@ checkout {
       }
     }
   }
-
-  .error {
-    color: #c82020;
-  }
 }
-

app/controllers/checkout_controller.rb

@@ -53,9 +53,8 @@ class CheckoutController < Spree::StoreController
   rescue Spree::Core::GatewayError => e
     rescue_from_spree_gateway_error(e)
   rescue StandardError => e
-    Bugsnag.notify(e)
     flash[:error] = I18n.t("checkout.failed")
-    update_failed
+    update_failed(e)
   end
 
   # Clears the cached order. Required for #current_order to return a new order
@@ -165,7 +164,7 @@ class CheckoutController < Spree::StoreController
       checkout_succeeded
       redirect_to(order_path(@order)) && return
     else
-      flash[:error] = order_workflow_error
+      flash[:error] = order_error
       checkout_failed
     end
   end
@@ -180,7 +179,6 @@ class CheckoutController < Spree::StoreController
 
       next if advance_order_state(@order)
 
-      flash[:error] = order_workflow_error
       return update_failed
     end
 
@@ -205,7 +203,7 @@ class CheckoutController < Spree::StoreController
     false
   end
 
-  def order_workflow_error
+  def order_error
     if @order.errors.present?
       @order.errors.full_messages.to_sentence
     else
@@ -218,7 +216,7 @@ class CheckoutController < Spree::StoreController
       checkout_succeeded
       update_succeeded_response
     else
-      update_failed
+      update_failed(RuntimeError.new("Order not complete after the checkout workflow"))
     end
   end
 
@@ -244,7 +242,10 @@ class CheckoutController < Spree::StoreController
     end
   end
 
-  def update_failed
+  def update_failed(error = RuntimeError.new(order_error))
+    Bugsnag.notify(error)
+
+    flash[:error] = order_error if flash.empty?
     checkout_failed
     update_failed_response
   end

app/helpers/application_helper.rb

@@ -1,6 +1,4 @@
 module ApplicationHelper
-  include FoundationRailsHelper::FlashHelper
-
   def feature?(feature)
     OpenFoodNetwork::FeatureToggle.enabled? feature
   end

app/views/checkout/_form.html.haml

@@ -3,7 +3,7 @@
   = inject_available_payment_methods
   = inject_saved_credit_cards
 
-= f_form_for current_order,
+= form_for current_order,
   html: {name: "checkout",
     id: "checkout_form",
     novalidate: true,

app/views/checkout/_shipping.html.haml

@@ -52,7 +52,8 @@
 
       .row
         .small-12.columns
-          = f.text_area :special_instructions, label: t(:checkout_instructions), size: "60x4", "ng-model" => "order.special_instructions"
+          %label{ for: 'order_special_instructions'}= t(:checkout_instructions)
+          = f.text_area :special_instructions, size: "60x4", "ng-model" => "order.special_instructions"
 
       .row
         .small-12.columns.text-right

app/views/user_passwords/edit.html.haml

@@ -1,4 +1,4 @@
-= f_form_for @spree_user, :as => :spree_user, :url => spree.spree_user_password_path, :method => :put do |f|
+= form_for @spree_user, :as => :spree_user, :url => spree.spree_user_password_path, :method => :put do |f|
   = render :partial => 'spree/shared/error_messages', :locals => { :target => @spree_user }
   %fieldset
     .row
@@ -6,9 +6,11 @@
         %legend= t(:change_my_password)
     .row
       .small-12.medium-6.large-4.columns.medium-centered.large-centered
+        %label{ for: 'spree_user_password'}= t(:password)
         = f.password_field :password
     .row
       .small-12.medium-6.large-4.columns.medium-centered.large-centered
+        %label{ for: 'spree_user_password_confirmation'}= t(:password_confirmation)
         = f.password_field :password_confirmation
     = f.hidden_field :reset_password_token
     .row

config/locales/en.yml

@@ -40,6 +40,8 @@ en:
         shipping_category_id: "Shipping Category"
         variant_unit: "Variant Unit"
         variant_unit_name: "Variant Unit Name"
+      spree/credit_card:
+        base: "Credit Card"
       order_cycle:
         orders_close_at: Close date
     errors:
@@ -50,6 +52,10 @@ en:
               taken: "There's already an account for this email. Please login or reset your password."
         spree/order:
           no_card: There are no authorised credit cards available to charge
+        spree/credit_card:
+          attributes:
+            base:
+              card_expired: "has expired"
         order_cycle:
           attributes:
             orders_close_at:

spec/controllers/checkout_controller_spec.rb

@@ -197,13 +197,13 @@ describe CheckoutController, type: :controller do
       allow(controller).to receive(:current_order).and_return(order)
     end
 
-    it "returns errors" do
+    it "returns errors and flash if order.update_attributes fails" do
       spree_post :update, format: :json, order: {}
       expect(response.status).to eq(400)
-      expect(response.body).to eq({ errors: assigns[:order].errors, flash: {} }.to_json)
+      expect(response.body).to eq({ errors: assigns[:order].errors, flash: { error: order.errors.full_messages.to_sentence } }.to_json)
     end
 
-    it "returns flash" do
+    it "returns errors and flash if order.next fails" do
       allow(order).to receive(:update_attributes).and_return true
       allow(order).to receive(:next).and_return false
       spree_post :update, format: :json, order: {}

spec/javascripts/unit/darkswarm/services/stripe_elements_spec.js.coffee

@@ -46,11 +46,10 @@ describe 'StripeElements Service', ->
       it "doesn't submit the form, shows an error message instead", inject (Loading, RailsFlashLoader) ->
         spyOn(Loading, "clear")
         spyOn(RailsFlashLoader, "loadFlash")
-        StripeElements.requestToken(secrets, submit)
-        $rootScope.$digest() # required for #then to by called
-        expect(submit).not.toHaveBeenCalled()
-        expect(Loading.clear).toHaveBeenCalled()
-        expect(RailsFlashLoader.loadFlash).toHaveBeenCalledWith({error: "Error: There was a problem"})
+        StripeElements.requestToken(secrets, submit).then (data) ->
+          expect(submit).not.toHaveBeenCalled()
+          expect(Loading.clear).toHaveBeenCalled()
+          expect(RailsFlashLoader.loadFlash).toHaveBeenCalledWith({error: "Error: There was a problem"})
 
   describe 'mapCC', ->
     it "maps the brand returned by Stripe to that required by activemerchant", ->