From aaa12b13f7ec6f30e0e72dc2db98f8b8a58c1b5a Mon Sep 17 00:00:00 2001 From: Dennis Marchand Date: Sat, 20 Jun 2020 11:48:25 -0700 Subject: [PATCH 1/3] Use build over create where possible Building these instances instead of persisting to the database will speed up the spec suite and they do not need to be persisted. --- backend/spec/helpers/admin/stock_movements_helper_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/backend/spec/helpers/admin/stock_movements_helper_spec.rb b/backend/spec/helpers/admin/stock_movements_helper_spec.rb index 1ac2d8130ec..82a112beba6 100644 --- a/backend/spec/helpers/admin/stock_movements_helper_spec.rb +++ b/backend/spec/helpers/admin/stock_movements_helper_spec.rb @@ -11,7 +11,7 @@ subject { helper.pretty_originator(stock_movement) } context "originator has a number" do - let(:originator) { create(:order) } + let(:originator) { build(:order) } it "returns the originator's number" do expect(subject).to eq originator.number @@ -19,7 +19,7 @@ end context "originator doesn't have a number" do - let(:originator) { create(:user) } + let(:originator) { build(:user) } it "returns an empty string" do expect(subject).to eq "" From 02e809b2fd17f881b167d5f9b70a5b2a63315de6 Mon Sep 17 00:00:00 2001 From: Dennis Marchand Date: Sat, 20 Jun 2020 11:49:50 -0700 Subject: [PATCH 2/3] Clean up stock movements helper I think ideally the #pretty_originator method doesn't even take a stock movement but rather a originator since we never actually operate on the stock movement but I don't want to step that far so for now I will just save the originator as a variable so we can cut down on the amount of code someone working here needs to read/comprehend. --- .../app/helpers/spree/admin/stock_movements_helper.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/backend/app/helpers/spree/admin/stock_movements_helper.rb b/backend/app/helpers/spree/admin/stock_movements_helper.rb index 175f125cb6f..fe3b462f853 100644 --- a/backend/app/helpers/spree/admin/stock_movements_helper.rb +++ b/backend/app/helpers/spree/admin/stock_movements_helper.rb @@ -4,11 +4,13 @@ module Spree module Admin module StockMovementsHelper def pretty_originator(stock_movement) - if stock_movement.originator.respond_to?(:number) - if stock_movement.originator.respond_to?(:order) - link_to stock_movement.originator.number, [:edit, :admin, stock_movement.originator.order] + originator = stock_movement.originator + + if originator.respond_to?(:number) + if originator.respond_to?(:order) + link_to originator.number, [:edit, :admin, originator.order] else - stock_movement.originator.number + originator.number end else "" From b613149a6b0ccf2ffcd44387d20b5b65859ffbec Mon Sep 17 00:00:00 2001 From: Dennis Marchand Date: Sat, 20 Jun 2020 11:58:33 -0700 Subject: [PATCH 3/3] Display the users email associated with stock movements Previously on the stock movements page if a stock movement was originated by a user then we would not display any information about the originator. This commit alters that interface to display the users email address. --- .../helpers/spree/admin/stock_movements_helper.rb | 2 ++ .../helpers/admin/stock_movements_helper_spec.rb | 12 ++++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/backend/app/helpers/spree/admin/stock_movements_helper.rb b/backend/app/helpers/spree/admin/stock_movements_helper.rb index fe3b462f853..dc8db0b0ded 100644 --- a/backend/app/helpers/spree/admin/stock_movements_helper.rb +++ b/backend/app/helpers/spree/admin/stock_movements_helper.rb @@ -12,6 +12,8 @@ def pretty_originator(stock_movement) else originator.number end + elsif originator.respond_to?(:email) + originator.email else "" end diff --git a/backend/spec/helpers/admin/stock_movements_helper_spec.rb b/backend/spec/helpers/admin/stock_movements_helper_spec.rb index 82a112beba6..830d42444a1 100644 --- a/backend/spec/helpers/admin/stock_movements_helper_spec.rb +++ b/backend/spec/helpers/admin/stock_movements_helper_spec.rb @@ -18,8 +18,16 @@ end end - context "originator doesn't have a number" do - let(:originator) { build(:user) } + context "originator has an email" do + let(:originator) { build(:user, email: "stock_mover@example.com") } + + it "returns the originator's email" do + expect(subject).to eq "stock_mover@example.com" + end + end + + context "the stock movement doesn't have an originator" do + let(:originator) { nil } it "returns an empty string" do expect(subject).to eq ""