From f4c3fbf8bc5000bd4de074877215d4ae8a8e8590 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 26 May 2017 17:16:07 +1000 Subject: [PATCH] Refactoring credit cards interface, and backend logic --- .../credit_cards_controller.js.coffee | 18 ++---- .../darkswarm/services/credit_card.js.coffee | 30 +++++---- .../darkswarm/services/credit_cards.js.coffee | 6 ++ .../darkswarm/services/navigation.js.coffee | 3 + .../stylesheets/darkswarm/account.css.scss | 15 ++++- .../spree/credit_cards_controller.rb | 2 +- app/helpers/injection_helper.rb | 6 +- app/models/spree/credit_card_decorator.rb | 2 - app/serializers/api/credit_card_serializer.rb | 26 +++++--- app/serializers/api/payment_serializer.rb | 2 +- app/views/spree/users/_cards.html.haml | 63 ++++--------------- .../spree/users/_new_card_form.html.haml | 35 +++++++++++ app/views/spree/users/_saved_cards.html.haml | 13 ++++ config/locales/en.yml | 3 +- .../spree/credit_cards_controller_spec.rb | 16 +++-- spec/features/consumer/account/cards_spec.rb | 31 +++++++++ .../consumer/shopping/checkout_spec.rb | 4 +- .../credit_card_serializer_spec.rb | 2 +- 18 files changed, 169 insertions(+), 108 deletions(-) create mode 100644 app/assets/javascripts/darkswarm/services/credit_cards.js.coffee create mode 100644 app/views/spree/users/_new_card_form.html.haml create mode 100644 app/views/spree/users/_saved_cards.html.haml create mode 100644 spec/features/consumer/account/cards_spec.rb diff --git a/app/assets/javascripts/darkswarm/controllers/credit_cards_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/credit_cards_controller.js.coffee index d1d1833610..1ddfc45d16 100644 --- a/app/assets/javascripts/darkswarm/controllers/credit_cards_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/credit_cards_controller.js.coffee @@ -1,18 +1,12 @@ -Darkswarm.controller "CreditCardsCtrl", ($scope, $timeout, CreditCard, savedCreditCards, StripeJS, Dates, Loading) -> +Darkswarm.controller "CreditCardsCtrl", ($scope, $timeout, CreditCard, CreditCards, StripeJS, Dates) -> angular.extend(this, new FieldsetMixin($scope)) - $scope.savedCreditCards = savedCreditCards + $scope.savedCreditCards = CreditCards.saved $scope.CreditCard = CreditCard + $scope.secrets = CreditCard.secrets + $scope.showForm = CreditCard.show + $scope.storeCard = CreditCard.requestToken + $scope.allow_name_change = true $scope.disable_fields = false - $scope.months = Dates.months $scope.years = Dates.years - - $scope.secrets = CreditCard.secrets - $scope.add_card_visible = false - - $scope.storeCard = => - CreditCard.requestToken($scope.secrets) - - $scope.toggle = -> - $scope.add_card_visible = !($scope.add_card_visible) diff --git a/app/assets/javascripts/darkswarm/services/credit_card.js.coffee b/app/assets/javascripts/darkswarm/services/credit_card.js.coffee index 00c3d8212e..4f0488ad17 100644 --- a/app/assets/javascripts/darkswarm/services/credit_card.js.coffee +++ b/app/assets/javascripts/darkswarm/services/credit_card.js.coffee @@ -1,20 +1,21 @@ -Darkswarm.factory 'CreditCard', ($injector, $rootScope, StripeJS, Navigation, $http, RailsFlashLoader, Loading)-> +Darkswarm.factory 'CreditCard', ($injector, $rootScope, CreditCards, StripeJS, Navigation, $http, RailsFlashLoader, Loading)-> new class CreditCard + visible: false errors: {} secrets: {} - requestToken: (secrets) -> - secrets.name = @full_name(secrets) - StripeJS.requestToken(secrets, @submit, t("saving_credit_card")) + requestToken: => + @setFullName() + StripeJS.requestToken(@secrets, @submit, t("saving_credit_card")) submit: => params = @process_params() $http.put('/credit_cards/new_from_token', params ) - .success (data, status) -> - $rootScope.$apply -> - Loading.clear() - Navigation.go '/account' - .error (response, status) -> + .success (data, status) => + Loading.clear() + @reset() + CreditCards.add(data) + .error (response, status) => if response.path Navigation.go response.path else @@ -22,8 +23,8 @@ Darkswarm.factory 'CreditCard', ($injector, $rootScope, StripeJS, Navigation, $h @errors = response.errors RailsFlashLoader.loadFlash(response.flash) - full_name: (secrets) -> - secrets.first_name + " " + secrets.last_name + setFullName: -> + @secrets.name = "#{@secrets.first_name} #{@secrets.last_name}" process_params: -> {"exp_month": @secrets.card.exp_month, @@ -31,3 +32,10 @@ Darkswarm.factory 'CreditCard', ($injector, $rootScope, StripeJS, Navigation, $h "last4": @secrets.card.last4, "token": @secrets.token, "cc_type": @secrets.card.brand} + + show: => @visible = true + + reset: => + @visible = false + delete @secrets[k] for k, v of @secrets + delete @errors[k] for k, v of @errors diff --git a/app/assets/javascripts/darkswarm/services/credit_cards.js.coffee b/app/assets/javascripts/darkswarm/services/credit_cards.js.coffee new file mode 100644 index 0000000000..c588ef681c --- /dev/null +++ b/app/assets/javascripts/darkswarm/services/credit_cards.js.coffee @@ -0,0 +1,6 @@ +Darkswarm.factory 'CreditCards', (savedCreditCards)-> + new class CreditCard + saved: savedCreditCards + + add: (card) -> + @saved.push card diff --git a/app/assets/javascripts/darkswarm/services/navigation.js.coffee b/app/assets/javascripts/darkswarm/services/navigation.js.coffee index e2b04f3ad1..f511e29e0c 100644 --- a/app/assets/javascripts/darkswarm/services/navigation.js.coffee +++ b/app/assets/javascripts/darkswarm/services/navigation.js.coffee @@ -21,3 +21,6 @@ Darkswarm.factory 'Navigation', ($location, $window) -> $window.location.href = path else $window.location.pathname = path + + reload: -> + $window.location.reload() diff --git a/app/assets/stylesheets/darkswarm/account.css.scss b/app/assets/stylesheets/darkswarm/account.css.scss index 3b5050f66c..78f8c0dfd9 100644 --- a/app/assets/stylesheets/darkswarm/account.css.scss +++ b/app/assets/stylesheets/darkswarm/account.css.scss @@ -5,8 +5,21 @@ color: #4a4a4a; } -.card { +.credit_cards { + .saved_cards { + table { + width: 100%; + } + } + .new_card { + opacity: 0; + -webkit-transition: opacity 0.4s linear; + transition: opacity 0.4s linear; + &.visible { + opacity: 1; + } + } } .orders { diff --git a/app/controllers/spree/credit_cards_controller.rb b/app/controllers/spree/credit_cards_controller.rb index 037455cc89..ea9f1890f5 100644 --- a/app/controllers/spree/credit_cards_controller.rb +++ b/app/controllers/spree/credit_cards_controller.rb @@ -22,7 +22,7 @@ module Spree @credit_card.last_digits = credit_card_params[:last_digits] @credit_card.user_id = @user.id if @credit_card.save - render json: @credit_card, status: :ok + render json: @credit_card, serializer: ::Api::CreditCardSerializer, status: :ok else render json: "error saving credit card", status: 500 end diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 89609c7b04..447bc5241f 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -70,11 +70,7 @@ module InjectionHelper end def inject_saved_credit_cards - if spree_current_user - data = spree_current_user.credit_cards - else - data = nil - end + data = spree_current_user.try(:credit_cards) inject_json_ams "savedCreditCards", data, Api::CreditCardSerializer end diff --git a/app/models/spree/credit_card_decorator.rb b/app/models/spree/credit_card_decorator.rb index 32c14a1285..0744bf5da5 100644 --- a/app/models/spree/credit_card_decorator.rb +++ b/app/models/spree/credit_card_decorator.rb @@ -10,6 +10,4 @@ Spree::CreditCard.class_eval do def has_payment_profile? gateway_customer_profile_id.present? || gateway_payment_profile_id.present? end - - end diff --git a/app/serializers/api/credit_card_serializer.rb b/app/serializers/api/credit_card_serializer.rb index 44791b404a..34b3bd124f 100644 --- a/app/serializers/api/credit_card_serializer.rb +++ b/app/serializers/api/credit_card_serializer.rb @@ -1,16 +1,22 @@ class Api::CreditCardSerializer < ActiveModel::Serializer - attributes :id, :formatted, :delete_link + attributes :id, :brand, :number, :expiry, :formatted, :delete_link + + def brand + object.cc_type.capitalize + end + + def number + 'x-' + object.last_digits + end + + def expiry + m = object.month.to_i + m = m < 10 ? "0#{m}" : m.to_s + "#{m}/#{object.year}" + end def formatted - elements = [] - elements << object.cc_type.capitalize if object.cc_type - if object.last_digits - 3.times { elements << I18n.t(:card_masked_digit) * 4 } - elements << object.last_digits - end - elements << I18n.t(:card_expiry_abbreviation) - elements << object.month.to_s + "/" + object.year.to_s if object.month # TODO: I18n - elements.join(" ") + "#{brand} #{number} #{I18n.t(:card_expiry_abbreviation)}:#{expiry}" end def delete_link diff --git a/app/serializers/api/payment_serializer.rb b/app/serializers/api/payment_serializer.rb index 8465998db6..f610fcdb58 100644 --- a/app/serializers/api/payment_serializer.rb +++ b/app/serializers/api/payment_serializer.rb @@ -2,7 +2,7 @@ module Api class PaymentSerializer < ActiveModel::Serializer attributes :amount, :updated_at, :payment_method, :state def payment_method - object.payment_method.name + object.payment_method.try(:name) end def amount diff --git a/app/views/spree/users/_cards.html.haml b/app/views/spree/users/_cards.html.haml index a887f0e8df..108e45cc37 100644 --- a/app/views/spree/users/_cards.html.haml +++ b/app/views/spree/users/_cards.html.haml @@ -1,56 +1,15 @@ %script{ type: "text/ng-template", id: "account/cards.html" } - %h3= t(:my_credit_cards) .credit_cards{"ng-controller" => "CreditCardsCtrl"} - %h4 - = t(:saved_cards) .row - .card_list.small-12.columns{"ng-repeat" => "card in savedCreditCards"} - %p.card - %span{"ng-bind" => "card.formatted"} - %a{"rel" => "nofollow", "data-method" => "delete", "ng-href" => "{{card.delete_link}}" } - Delete - .row - %h4{"ng-click" => "toggle()"} - .columns.small-10.medium-10 - = t(:add_new_credit_card) - .columns.small-2.medium-2.text-right - %span.margin-top - %i{"ng-class" => "{'ofn-i_005-caret-down' : !add_card_visible, 'ofn-i_006-caret-up' : add_card_visible}"} - .row - .new_card.small-12.columns{"ng-if" => 'add_card_visible', "ng-class" => "{'closed' : !add_card_visible, 'open' : add_card_visible}"} - %form{novalidate: true, "ng-submit" => "storeCard()"} - .row - .small-6.columns - %label - = t :first_name - -# Changing name not permitted by default (in checkout) - can be enabled by setting an allow_name_change variable in $scope - %input{type: :text, "ng-model" => "secrets.first_name","ng-disabled" => "!allow_name_change", "ng-value" => "order.bill_address.firstname"} + .small-12.medium-6.columns + %span{ ng: { hide: 'savedCreditCards.length > 0' } } +   + .saved_cards{ ng: { show: 'savedCreditCards.length > 0' } } + %h3= t(:saved_cards) + = render 'saved_cards' + %button.button.primary{ ng: { click: 'showForm()', hide: 'CreditCard.visible' } } + = t(:add_a_card) - .small-6.columns - %label - = t :last_name - %input{type: :text, "ng-model" => "secrets.last_name", "ng-disabled" => "!allow_name_change", "ng-value" => "order.bill_address.lastname"} - - .small-6.columns - %label - = t(:card_number) - %input{type: :text, "ng-model" => "secrets.card_number", "ng-required" => "!secrets.selected_card", maxlength: 19, autocomplete: "off", "ng-disabled" => "!!secrets.selected_card"} - .small-6.columns - %label - = t(:card_securitycode) - %input{type: :text, "ng-model" => "secrets.card_verification_value", "ng-required" => "!secrets.selected_card", autocomplete: "off", "ng-disabled" => "!!secrets.selected_card"} - - .row - .small-12.columns - %label{for: "secrets.card_month"} - = t :card_expiry_date, "ng-disabled" => "!!secrets.selected_card" - - .row - .small-6.columns - %select{"ng-model" => "secrets.card_month", "ng-options" => "currMonth.value as currMonth.key for currMonth in months", name: "secrets.card_month", "ng-required" => "!secrets.selected_card", "ng-disabled" => "!!secrets.selected_card"} - .small-6.columns - %select{"ng-model" => "secrets.card_year", "ng-options" => "year for year in years", name: "secrets.card_year", "ng-required" => "!secrets.selected_card", "ng-disabled" => "!!secrets.selected_card"} - - %p - %button.button.primary{type: :submit} - = t :add_card + .small-12.medium-6.columns.new_card{ ng: { class: '{visible: CreditCard.visible}' } } + %h3= t(:add_a_new_card) + = render 'new_card_form' diff --git a/app/views/spree/users/_new_card_form.html.haml b/app/views/spree/users/_new_card_form.html.haml new file mode 100644 index 0000000000..03dc9ff1c5 --- /dev/null +++ b/app/views/spree/users/_new_card_form.html.haml @@ -0,0 +1,35 @@ +%form{ novalidate: true, "ng-submit" => "storeCard()" } + .row + .small-6.columns + %label + = t(:first_name) + -# Changing name not permitted by default (in checkout) - can be enabled by setting an allow_name_change variable in $scope + %input#first_name{type: :text, "ng-model" => "secrets.first_name","ng-disabled" => "!allow_name_change", "ng-value" => "order.bill_address.firstname"} + + .small-6.columns + %label + = t(:last_name) + %input#last_name{type: :text, "ng-model" => "secrets.last_name", "ng-disabled" => "!allow_name_change", "ng-value" => "order.bill_address.lastname"} + .row + .small-6.columns + %label + = t(:card_number) + %input#card_number{type: :text, "ng-model" => "secrets.card_number", "ng-required" => "!secrets.selected_card", maxlength: 19, autocomplete: "off", "ng-disabled" => "!!secrets.selected_card"} + .small-6.columns + %label + = t(:card_securitycode) + %input#security_code{type: :text, "ng-model" => "secrets.card_verification_value", "ng-required" => "!secrets.selected_card", autocomplete: "off", "ng-disabled" => "!!secrets.selected_card"} + + .row + .small-12.columns + %label{for: "secrets.card_month"} + = t(:card_expiry_date) + .row + .small-6.columns + %select#card_month{"ng-model" => "secrets.card_month", "ng-options" => "currMonth.value as currMonth.key for currMonth in months", name: "secrets.card_month", "ng-required" => "!secrets.selected_card", "ng-disabled" => "!!secrets.selected_card"} + .small-6.columns + %select#card_year{"ng-model" => "secrets.card_year", "ng-options" => "year for year in years", name: "secrets.card_year", "ng-required" => "!secrets.selected_card", "ng-disabled" => "!!secrets.selected_card"} + + %p + %button.button.primary{type: :submit} + = t(:add_card) diff --git a/app/views/spree/users/_saved_cards.html.haml b/app/views/spree/users/_saved_cards.html.haml new file mode 100644 index 0000000000..8df81eeeca --- /dev/null +++ b/app/views/spree/users/_saved_cards.html.haml @@ -0,0 +1,13 @@ +%table + %tr + %th= t(:card_type) + %th= t(:card_number) + %th= t(:card_expiry_date) + %th= t(:delete?) + %tr.card{ id: "card{{ card.id }}", ng: { repeat: "card in savedCreditCards" } } + %td.brand{ ng: { bind: '::card.brand' } } + %td.number{ ng: { bind: '::card.number' } } + %td.expiry{ ng: { bind: '::card.expiry' } } + %td.actions + %a{"rel" => "nofollow", "data-method" => "delete", "ng-href" => "{{card.delete_link}}" } + Delete diff --git a/config/locales/en.yml b/config/locales/en.yml index 4fe7a745e3..e8d5a5f616 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -168,7 +168,8 @@ en: my_credit_cards: My credit cards add_new_credit_card: Add new credit card saved_cards: Saved cards - add_card: Add card + add_a_card: Add a Card + add_card: Add Card saving_credit_card: Saving credit card... diff --git a/spec/controllers/spree/credit_cards_controller_spec.rb b/spec/controllers/spree/credit_cards_controller_spec.rb index dc543f061b..e37987b61f 100644 --- a/spec/controllers/spree/credit_cards_controller_spec.rb +++ b/spec/controllers/spree/credit_cards_controller_spec.rb @@ -4,17 +4,15 @@ require 'support/request/authentication_workflow' describe Spree::CreditCardsController do include AuthenticationWorkflow let(:user) { create_enterprise_user } + let(:token) { "tok_234bd2c22" } it "Creates a credit card from token + params" do controller.stub(:spree_current_user) { user } - controller.stub(:create_customer) { - sc = Stripe::Customer.new - sc.default_source = "card_1AEEbN2eZvKYlo2CMk6QwrN7" - sc.email = nil - sc.stub(:id) {"cus_AZNMJzuACN3Sgt"} - sc } - token = "tok_234bd2c22" + stub_request(:post, "https://api.stripe.com/v1/customers") + .with(:body => { email: user.email, source: token }) + .to_return(status: 200, body: JSON.generate({ id: "cus_AZNMJ", default_source: "card_1AEEb" })) + expect{ post :new_from_token, { "exp_month" => 12, "exp_year" => 2020, @@ -23,8 +21,8 @@ describe Spree::CreditCardsController do "cc_type" => "visa" } }.to change(Spree::CreditCard, :count).by(1) - Spree::CreditCard.last.gateway_payment_profile_id.should eq "card_1AEEbN2eZvKYlo2CMk6QwrN7" - Spree::CreditCard.last.gateway_customer_profile_id.should eq "cus_AZNMJzuACN3Sgt" + Spree::CreditCard.last.gateway_payment_profile_id.should eq "card_1AEEb" + Spree::CreditCard.last.gateway_customer_profile_id.should eq "cus_AZNMJ" Spree::CreditCard.last.user_id.should eq user.id Spree::CreditCard.last.last_digits.should eq "4242" end diff --git a/spec/features/consumer/account/cards_spec.rb b/spec/features/consumer/account/cards_spec.rb new file mode 100644 index 0000000000..0dbf2108dc --- /dev/null +++ b/spec/features/consumer/account/cards_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +feature "Credit Cards", js: true do + include AuthenticationWorkflow + describe "as a logged in user" do + let(:user) { create(:user) } + let!(:card) { create(:credit_card, user_id: user.id) } + + before do + quick_login_as user + end + + it "lists saved cards, shows interface for adding new cards" do + visit "/account" + + click_link 'My Credit Cards' + + expect(page).to have_content I18n.t(:saved_cards) + + within(".card#card#{card.id}") do + expect(page).to have_content card.cc_type.capitalize + expect(page).to have_content card.last_digits + end + + click_button I18n.t(:add_a_card) + expect(page).to have_field 'first_name' + expect(page).to have_field 'card_number' + expect(page).to have_field 'card_month' + end + end +end diff --git a/spec/features/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index fb128714f7..ae2d0e0f38 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -171,12 +171,12 @@ feature "As a consumer I want to check out my cart", js: true, retry: 3 do end it "disables the input fields when a saved card is selected" do - select "Visa XXXX XXXX XXXX 1111 Exp 01/2025", from: "selected_card" + select "Visa x-1111 Exp:01/2025", from: "selected_card" page.should have_css "#secrets\\.card_number[disabled]" end it "allows use of a saved card" do - select "Visa XXXX XXXX XXXX 1111 Exp 01/2025", from: "selected_card" + select "Visa x-1111 Exp:01/2025", from: "selected_card" place_order page.should have_content "Your order has been processed successfully" end diff --git a/spec/serializers/credit_card_serializer_spec.rb b/spec/serializers/credit_card_serializer_spec.rb index 146055de05..100067b2e5 100644 --- a/spec/serializers/credit_card_serializer_spec.rb +++ b/spec/serializers/credit_card_serializer_spec.rb @@ -10,7 +10,7 @@ describe Api::CreditCardSerializer do end it "formats an identifying string with the card number masked" do - expect(serializer.formatted).to eq "Visa XXXX XXXX XXXX 1111 Exp 12/2013" + expect(serializer.formatted).to eq "Visa x-1111 Exp:12/2013" end end