From ee351d91a9e0fb11cc53b6dbffdc9ef11bfcaef4 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 18 Feb 2021 13:50:58 -0800 Subject: [PATCH 01/13] show authorization links in separate table --- app/controllers/spree/users_controller.rb | 1 + app/queries/payments_requiring_action.rb | 14 +++++++++++ app/views/spree/users/_fat.html.haml | 2 -- app/views/spree/users/_transactions.html.haml | 24 +++++++++++++++++++ config/locales/en.yml | 1 + 5 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 app/queries/payments_requiring_action.rb diff --git a/app/controllers/spree/users_controller.rb b/app/controllers/spree/users_controller.rb index c9ee2e15fe..93350cd187 100644 --- a/app/controllers/spree/users_controller.rb +++ b/app/controllers/spree/users_controller.rb @@ -13,6 +13,7 @@ module Spree before_action :enable_embedded_shopfront def show + @payments_requiring_action = PaymentsRequiringAction.new(spree_current_user).query @orders = orders_collection customers = spree_current_user.customers diff --git a/app/queries/payments_requiring_action.rb b/app/queries/payments_requiring_action.rb new file mode 100644 index 0000000000..ce5bf5721d --- /dev/null +++ b/app/queries/payments_requiring_action.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class PaymentsRequiringAction + def initialize(user) + @user = user + end + + attr_reader :user + + def query + Spree::Payment.joins(order: [:user]).where.not(cvv_response_message: nil). + where("spree_users.id = ?", user.id) + end +end diff --git a/app/views/spree/users/_fat.html.haml b/app/views/spree/users/_fat.html.haml index 56439d0e19..dfbcfade38 100644 --- a/app/views/spree/users/_fat.html.haml +++ b/app/views/spree/users/_fat.html.haml @@ -16,8 +16,6 @@ %td.order3.show-for-large-up %i{"ng-class" => "{'ofn-i_012-warning': payment.state == 'invalid' || payment.state == 'void' || payment.state == 'failed'}"} %span{"ng-bind" => "::'spree.payment_states.' + payment.state | t | capitalize"} - %span{"ng-if" => "payment.cvv_response_message.length > 0" } - %a{"ng-href" => "{{payment.cvv_response_message}}", "ng-bind" => "::'spree.payment_states.authorise' | t | capitalize" } %td.order4.show-for-large-up %td.order5.text-right{"ng-class" => "{'credit' : payment.amount > 0, 'debit' : payment.amount < 0, 'paid' : payment.amount == 0}","ng-bind" => "::payment.amount | localizeCurrency"} %td.order6.show-for-large-up diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index 6c8bea5353..7343eef31c 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -1,4 +1,28 @@ %script{ type: "text/ng-template", id: "account/transactions.html" } + - if @payments_requiring_action.present? + .active_table + %h3.requiring-authorization= t(".authorization_required") + .small-12.columns + %table + %tr + %th.order1= t :transaction + %th.order2= t :transaction_date + %th.order3.show-for-large-up= t :payment_state + %th.order5.text-right= t :value + %tbody + - @payments_requiring_action.each do |payment| + %tr + %td.order1 + = link_to payment.order.number, main_app.order_path(payment.order) + %td.order2 + = payment.updated_at.strftime("%Y-%m-%d") + %td.order3 + %a{ href: "#{payment.cvv_response_message}" } + %button.bright + Authorize + %td.order5 + = payment.display_amount + .active_table.orders{"ng-controller" => "OrdersCtrl", "ng-cloak" => true} %h3.my-orders= t(".transaction_history") %distributor.active_table_node.row.animate-repeat{"ng-if" => "Orders.shops.length > 0", "ng-repeat" => "shop in Orders.shops", diff --git a/config/locales/en.yml b/config/locales/en.yml index 113c4f672a..675e219eea 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3732,6 +3732,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using past_orders: Past Orders transactions: transaction_history: Transaction History + authorisation_required: Authorisation Required open_orders: order: Order shop: Shop From 0b245ad7b19dd3cd02e882e4f5c637f18e39f7e8 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 22 Feb 2021 09:18:06 -0800 Subject: [PATCH 02/13] make user attr private --- app/queries/payments_requiring_action.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/queries/payments_requiring_action.rb b/app/queries/payments_requiring_action.rb index ce5bf5721d..e350a501a1 100644 --- a/app/queries/payments_requiring_action.rb +++ b/app/queries/payments_requiring_action.rb @@ -5,10 +5,12 @@ class PaymentsRequiringAction @user = user end - attr_reader :user - def query Spree::Payment.joins(order: [:user]).where.not(cvv_response_message: nil). where("spree_users.id = ?", user.id) end + + private + + attr_reader :user end From 83bc9d2a125247a7a6a739a67db4bce587d8d28e Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 22 Feb 2021 12:20:53 -0800 Subject: [PATCH 03/13] add unit spec for Payments query --- .../queries/payments_requiring_action_spec.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 spec/queries/payments_requiring_action_spec.rb diff --git a/spec/queries/payments_requiring_action_spec.rb b/spec/queries/payments_requiring_action_spec.rb new file mode 100644 index 0000000000..e138febeec --- /dev/null +++ b/spec/queries/payments_requiring_action_spec.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PaymentsRequiringAction do + let(:user) { create(:user) } + let(:order) { create(:order, user: user) } + let(:payments_requiring_action) { described_class.new(user) } + + describe '#query' do + context "payment has a cvv_response_message" do + let(:payment) do + create(:payment, order: order, cvv_response_message: "https://stripe.com/redirect") + end + + it "finds the payment" do + expect(payments_requiring_action.query.all).to include(payment) + end + end + + context "payment has no cvv_response_message" do + let(:payment) do + create(:payment, order: order, cvv_response_message: nil) + end + + it "does not find the payment" do + expect(payments_requiring_action.query.all).to_not include(payment) + end + end + end +end From e5eb8f97f9097fafd1c82076c86ef67632db1bc0 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 22 Feb 2021 13:02:30 -0800 Subject: [PATCH 04/13] add basic feature spec for authorisation table --- app/views/spree/users/_transactions.html.haml | 2 +- .../consumer/account/payments_spec.rb | 42 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 spec/features/consumer/account/payments_spec.rb diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index 7343eef31c..2acce38612 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -1,7 +1,7 @@ %script{ type: "text/ng-template", id: "account/transactions.html" } - if @payments_requiring_action.present? .active_table - %h3.requiring-authorization= t(".authorization_required") + %h3.requiring-authorization= t(".authorisation_required") .small-12.columns %table %tr diff --git a/spec/features/consumer/account/payments_spec.rb b/spec/features/consumer/account/payments_spec.rb new file mode 100644 index 0000000000..9f6ddf33c9 --- /dev/null +++ b/spec/features/consumer/account/payments_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +feature "Payments requiring action", js: true do + include AuthenticationHelper + + describe "as a logged in user" do + let(:user) { create(:user) } + let(:order) { create(:order, user: user) } + + before do + login_as user + end + + context "there is a payment requiring authorization" do + let!(:payment) do + create(:payment, order: order, cvv_response_message: "https://stripe.com/redirect") + end + + it "shows a table of payments requiring authorization" do + visit "/account" + + find("a", :text => %r{#{I18n.t('spree.users.show.tabs.transactions')}}i).click + expect(page).to have_content I18n.t("spree.users.transactions.authorisation_required") + end + end + + context "there are no payments requiring authorization" do + let!(:payment) do + create(:payment, order: order, cvv_response_message: nil) + end + + it "does not show the table of payments requiring authorization" do + visit "/account" + + find("a", :text => %r{#{I18n.t('spree.users.show.tabs.transactions')}}i).click + expect(page).to_not have_content I18n.t("spree.users.transactions.authorisation_required") + end + end + end +end From 7bb49b51fd868844578411ab0c40b2ea83c84386 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 22 Feb 2021 13:06:59 -0800 Subject: [PATCH 05/13] use scope on payment model --- app/models/spree/payment.rb | 1 + app/queries/payments_requiring_action.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/spree/payment.rb b/app/models/spree/payment.rb index f2ac9ee02e..0896defdc3 100644 --- a/app/models/spree/payment.rb +++ b/app/models/spree/payment.rb @@ -49,6 +49,7 @@ module Spree scope :pending, -> { with_state('pending') } scope :failed, -> { with_state('failed') } scope :valid, -> { where('state NOT IN (?)', %w(failed invalid)) } + scope :authorization_action_required, -> { where.not(cvv_response_message: nil) } # order state machine (see http://github.com/pluginaweek/state_machine/tree/master for details) state_machine initial: :checkout do diff --git a/app/queries/payments_requiring_action.rb b/app/queries/payments_requiring_action.rb index e350a501a1..f9b8c16f69 100644 --- a/app/queries/payments_requiring_action.rb +++ b/app/queries/payments_requiring_action.rb @@ -6,8 +6,8 @@ class PaymentsRequiringAction end def query - Spree::Payment.joins(order: [:user]).where.not(cvv_response_message: nil). - where("spree_users.id = ?", user.id) + Spree::Payment.joins(order: [:user]).where("spree_users.id = ?", user.id). + authorization_action_required end private From c23fb27031fcbb13b798858e790892ae62a58b38 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 23 Feb 2021 11:01:24 -0800 Subject: [PATCH 06/13] remove unnecessary join --- app/queries/payments_requiring_action.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/queries/payments_requiring_action.rb b/app/queries/payments_requiring_action.rb index f9b8c16f69..f8cc1bc2bc 100644 --- a/app/queries/payments_requiring_action.rb +++ b/app/queries/payments_requiring_action.rb @@ -6,7 +6,7 @@ class PaymentsRequiringAction end def query - Spree::Payment.joins(order: [:user]).where("spree_users.id = ?", user.id). + Spree::Payment.joins(:order).where("spree_orders.user_id = ?", user.id). authorization_action_required end From 7bbfb6b8db1bea677a174f69d5d4d2cc55fa30e2 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 23 Feb 2021 11:16:35 -0800 Subject: [PATCH 07/13] use rpsec subject in payments spec --- spec/queries/payments_requiring_action_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/queries/payments_requiring_action_spec.rb b/spec/queries/payments_requiring_action_spec.rb index e138febeec..a3af3af3cd 100644 --- a/spec/queries/payments_requiring_action_spec.rb +++ b/spec/queries/payments_requiring_action_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe PaymentsRequiringAction do let(:user) { create(:user) } let(:order) { create(:order, user: user) } - let(:payments_requiring_action) { described_class.new(user) } + subject(:payments_requiring_action) { described_class.new(user) } describe '#query' do context "payment has a cvv_response_message" do From 8696882549be49ba59157cc3c138bc7ad64fa50c Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 22 Feb 2021 10:38:46 +0100 Subject: [PATCH 08/13] Refactor payment auth button to fit into table row This looks a bit more cohesive with the rest of the row cells while still standing out from the rest. --- app/assets/stylesheets/darkswarm/ui.scss | 7 +++++++ app/views/spree/users/_transactions.html.haml | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/darkswarm/ui.scss b/app/assets/stylesheets/darkswarm/ui.scss index a9a43b0b5f..c13c51f024 100644 --- a/app/assets/stylesheets/darkswarm/ui.scss +++ b/app/assets/stylesheets/darkswarm/ui.scss @@ -61,6 +61,13 @@ @include border-radius(0.5em); outline: none; + + &.x-small { + padding: 0.5rem 0.75rem; + font-size: $text-xs; + font-weight: 600; + margin: 0; + } } .button.primary, button.primary { diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index 2acce38612..0648973eec 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -18,7 +18,7 @@ = payment.updated_at.strftime("%Y-%m-%d") %td.order3 %a{ href: "#{payment.cvv_response_message}" } - %button.bright + %button.x-small Authorize %td.order5 = payment.display_amount From cbb919f28b84fea91b4280dec73c16be86cf27b5 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 22 Feb 2021 10:42:20 +0100 Subject: [PATCH 09/13] Left-align table with table below --- app/views/spree/users/_transactions.html.haml | 41 +++++++++---------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index 0648973eec..a40b011abb 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -1,27 +1,26 @@ %script{ type: "text/ng-template", id: "account/transactions.html" } - if @payments_requiring_action.present? .active_table - %h3.requiring-authorization= t(".authorisation_required") - .small-12.columns - %table - %tr - %th.order1= t :transaction - %th.order2= t :transaction_date - %th.order3.show-for-large-up= t :payment_state - %th.order5.text-right= t :value - %tbody - - @payments_requiring_action.each do |payment| - %tr - %td.order1 - = link_to payment.order.number, main_app.order_path(payment.order) - %td.order2 - = payment.updated_at.strftime("%Y-%m-%d") - %td.order3 - %a{ href: "#{payment.cvv_response_message}" } - %button.x-small - Authorize - %td.order5 - = payment.display_amount + %h3.requiring-authorization= t(".authorization_required") + %table + %tr + %th.order1= t :transaction + %th.order2= t :transaction_date + %th.order3.show-for-large-up= t :payment_state + %th.order5.text-right= t :value + %tbody + - @payments_requiring_action.each do |payment| + %tr + %td.order1 + = link_to payment.order.number, main_app.order_path(payment.order) + %td.order2 + = payment.updated_at.strftime("%Y-%m-%d") + %td.order3 + %a{ href: "#{payment.cvv_response_message}" } + %button.x-small + Authorize + %td.order5 + = payment.display_amount .active_table.orders{"ng-controller" => "OrdersCtrl", "ng-cloak" => true} %h3.my-orders= t(".transaction_history") From ad147ed8f59c854e4739c103c67f0790ca678fcc Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 22 Feb 2021 10:53:21 +0100 Subject: [PATCH 10/13] Make payment auth table as wide as table below This makes this page look a bit more consistent. Note I also had to fix the price in the value column. To do this I pulled out the width property from `.orders` which defines too many things. This way we can make the auth table full-width while not being tied to all the other properties which are not needed in this table. Then, `.orders`'s nested `.order1, .order2` etc. column class become useless. --- app/assets/stylesheets/darkswarm/account.scss | 5 ++++- app/views/spree/users/_transactions.html.haml | 18 +++++++++--------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/app/assets/stylesheets/darkswarm/account.scss b/app/assets/stylesheets/darkswarm/account.scss index bb6701f15d..64e81497b1 100644 --- a/app/assets/stylesheets/darkswarm/account.scss +++ b/app/assets/stylesheets/darkswarm/account.scss @@ -99,7 +99,6 @@ .transaction-group {} table { - width: 100%; border-radius: $radius-medium $radius-medium 0 0; tr:nth-of-type(even) { @@ -143,3 +142,7 @@ width: 10%; } } + +table.full { + width: 100%; +} diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index a40b011abb..6173cf12d2 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -2,24 +2,24 @@ - if @payments_requiring_action.present? .active_table %h3.requiring-authorization= t(".authorization_required") - %table + %table.full %tr - %th.order1= t :transaction - %th.order2= t :transaction_date - %th.order3.show-for-large-up= t :payment_state - %th.order5.text-right= t :value + %th= t :transaction + %th= t :transaction_date + %th.show-for-large-up= t :payment_state + %th.text-right= t :value %tbody - @payments_requiring_action.each do |payment| %tr - %td.order1 + %td = link_to payment.order.number, main_app.order_path(payment.order) - %td.order2 + %td = payment.updated_at.strftime("%Y-%m-%d") - %td.order3 + %td %a{ href: "#{payment.cvv_response_message}" } %button.x-small Authorize - %td.order5 + %td.text-right = payment.display_amount .active_table.orders{"ng-controller" => "OrdersCtrl", "ng-cloak" => true} From 2146ed277f79d3cbfd209ef759dec107eaba5640 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 22 Feb 2021 11:43:46 +0100 Subject: [PATCH 11/13] Fix table header spacing when status is hidden --- app/assets/stylesheets/darkswarm/account.scss | 16 ++++++++++++++++ app/assets/stylesheets/darkswarm/tables.scss | 6 ++++-- app/views/spree/users/_transactions.html.haml | 4 ++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/darkswarm/account.scss b/app/assets/stylesheets/darkswarm/account.scss index 64e81497b1..772a87cb2f 100644 --- a/app/assets/stylesheets/darkswarm/account.scss +++ b/app/assets/stylesheets/darkswarm/account.scss @@ -146,3 +146,19 @@ table.full { width: 100%; } + +// Note this relies on the using `.show-for-large-up` as well. +.requiring-authorization tr { + position: relative; + + th:last-child { + position: absolute; + // The following calculation is the equivalent of: + // + // $table-cell-padding + 2 * browser's default border-spacing + 2 * table border + // + // Unfortunately we can't use Scss's interpolation + // https://sass-lang.com/documentation/interpolation. We're using a too old version perhaps? + right: calc(12px + 2*2px + 2*1px); + } +} diff --git a/app/assets/stylesheets/darkswarm/tables.scss b/app/assets/stylesheets/darkswarm/tables.scss index b2632d52f1..35c61d26b0 100644 --- a/app/assets/stylesheets/darkswarm/tables.scss +++ b/app/assets/stylesheets/darkswarm/tables.scss @@ -1,9 +1,11 @@ +$table-cell-padding: 12px; + table { thead tr, tbody tr { th, td { box-sizing: border-box; - padding-left: 12px; - padding-right: 12px; + padding-left: $table-cell-padding; + padding-right: $table-cell-padding; overflow: hidden; } } diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index 6173cf12d2..b6e6b8c36c 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -1,7 +1,7 @@ %script{ type: "text/ng-template", id: "account/transactions.html" } - if @payments_requiring_action.present? - .active_table - %h3.requiring-authorization= t(".authorization_required") + .active_table.requiring-authorization + %h3= t(".authorization_required") %table.full %tr %th= t :transaction From c3179b4304970c2dad69e686ebb9f6cc04048875 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 22 Feb 2021 12:06:19 +0100 Subject: [PATCH 12/13] Restore table's top rounded corners --- app/assets/stylesheets/darkswarm/account.scss | 10 ++++++++-- app/views/spree/users/_transactions.html.haml | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/app/assets/stylesheets/darkswarm/account.scss b/app/assets/stylesheets/darkswarm/account.scss index 772a87cb2f..d73e4b7367 100644 --- a/app/assets/stylesheets/darkswarm/account.scss +++ b/app/assets/stylesheets/darkswarm/account.scss @@ -143,8 +143,14 @@ } } -table.full { - width: 100%; +table { + &.full { + width: 100%; + } + + &.top-rounded { + border-radius: $radius-medium $radius-medium 0 0; + } } // Note this relies on the using `.show-for-large-up` as well. diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index b6e6b8c36c..75c479aada 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -2,7 +2,7 @@ - if @payments_requiring_action.present? .active_table.requiring-authorization %h3= t(".authorization_required") - %table.full + %table.full.top-rounded %tr %th= t :transaction %th= t :transaction_date From 855e38cd7319b8e5f837c2f3e85a679b376128f4 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 24 Feb 2021 08:21:16 -0800 Subject: [PATCH 13/13] add I18n --- app/views/spree/users/_transactions.html.haml | 4 ++-- config/locales/en.yml | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/views/spree/users/_transactions.html.haml b/app/views/spree/users/_transactions.html.haml index 75c479aada..e36d6c9382 100644 --- a/app/views/spree/users/_transactions.html.haml +++ b/app/views/spree/users/_transactions.html.haml @@ -1,7 +1,7 @@ %script{ type: "text/ng-template", id: "account/transactions.html" } - if @payments_requiring_action.present? .active_table.requiring-authorization - %h3= t(".authorization_required") + %h3= t(".authorisation_required") %table.full.top-rounded %tr %th= t :transaction @@ -18,7 +18,7 @@ %td %a{ href: "#{payment.cvv_response_message}" } %button.x-small - Authorize + = t(".authorise") %td.text-right = payment.display_amount diff --git a/config/locales/en.yml b/config/locales/en.yml index 675e219eea..d47778e18d 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -3733,6 +3733,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using transactions: transaction_history: Transaction History authorisation_required: Authorisation Required + authorise: Authorize open_orders: order: Order shop: Shop