From dfa00a770a0f18b5b6a92e481133b270772cbe7b Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 19 Aug 2020 04:29:49 +0100 Subject: [PATCH 1/5] Bring user and ability related files from spree_core --- app/models/spree/role.rb | 5 ++++ config/initializers/user_class_extensions.rb | 25 ++++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 app/models/spree/role.rb create mode 100644 config/initializers/user_class_extensions.rb diff --git a/app/models/spree/role.rb b/app/models/spree/role.rb new file mode 100644 index 0000000000..2d2877d77f --- /dev/null +++ b/app/models/spree/role.rb @@ -0,0 +1,5 @@ +module Spree + class Role < ActiveRecord::Base + has_and_belongs_to_many :users, join_table: 'spree_roles_users', class_name: Spree.user_class.to_s + end +end diff --git a/config/initializers/user_class_extensions.rb b/config/initializers/user_class_extensions.rb new file mode 100644 index 0000000000..de8743a11d --- /dev/null +++ b/config/initializers/user_class_extensions.rb @@ -0,0 +1,25 @@ +Spree::Core::Engine.config.to_prepare do + if Spree.user_class + Spree.user_class.class_eval do + include Spree::Core::UserBanners + has_and_belongs_to_many :spree_roles, + :join_table => 'spree_roles_users', + :foreign_key => "user_id", + :class_name => "Spree::Role" + + has_many :spree_orders, :foreign_key => "user_id", :class_name => "Spree::Order" + + belongs_to :ship_address, :class_name => 'Spree::Address' + belongs_to :bill_address, :class_name => 'Spree::Address' + + # has_spree_role? simply needs to return true or false whether a user has a role or not. + def has_spree_role?(role_in_question) + spree_roles.where(:name => role_in_question.to_s).any? + end + + def last_incomplete_spree_order + spree_orders.incomplete.where(:created_by_id => self.id).order('created_at DESC').first + end + end + end +end From caf61e3a7e931e365e72173ac8fb0b68bbe3a071 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 19 Aug 2020 04:30:59 +0100 Subject: [PATCH 2/5] Run rubocop -a --- app/models/spree/role.rb | 2 ++ config/initializers/user_class_extensions.rb | 35 ++++++++++---------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/app/models/spree/role.rb b/app/models/spree/role.rb index 2d2877d77f..a2d8ea903e 100644 --- a/app/models/spree/role.rb +++ b/app/models/spree/role.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module Spree class Role < ActiveRecord::Base has_and_belongs_to_many :users, join_table: 'spree_roles_users', class_name: Spree.user_class.to_s diff --git a/config/initializers/user_class_extensions.rb b/config/initializers/user_class_extensions.rb index de8743a11d..8603f0d359 100644 --- a/config/initializers/user_class_extensions.rb +++ b/config/initializers/user_class_extensions.rb @@ -1,25 +1,26 @@ +# frozen_string_literal: true + Spree::Core::Engine.config.to_prepare do - if Spree.user_class - Spree.user_class.class_eval do - include Spree::Core::UserBanners - has_and_belongs_to_many :spree_roles, - :join_table => 'spree_roles_users', - :foreign_key => "user_id", - :class_name => "Spree::Role" + # has_spree_role? simply needs to return true or false whether a user has a role or not. + Spree.user_class&.class_eval do + include Spree::Core::UserBanners + has_and_belongs_to_many :spree_roles, + join_table: 'spree_roles_users', + foreign_key: "user_id", + class_name: "Spree::Role" - has_many :spree_orders, :foreign_key => "user_id", :class_name => "Spree::Order" + has_many :spree_orders, foreign_key: "user_id", class_name: "Spree::Order" - belongs_to :ship_address, :class_name => 'Spree::Address' - belongs_to :bill_address, :class_name => 'Spree::Address' + belongs_to :ship_address, class_name: 'Spree::Address' + belongs_to :bill_address, class_name: 'Spree::Address' - # has_spree_role? simply needs to return true or false whether a user has a role or not. - def has_spree_role?(role_in_question) - spree_roles.where(:name => role_in_question.to_s).any? - end + # has_spree_role? simply needs to return true or false whether a user has a role or not. + def has_spree_role?(role_in_question) + spree_roles.where(name: role_in_question.to_s).any? + end - def last_incomplete_spree_order - spree_orders.incomplete.where(:created_by_id => self.id).order('created_at DESC').first - end + def last_incomplete_spree_order + spree_orders.incomplete.where(created_by_id: id).order('created_at DESC').first end end end From 737fc699edfa6afe1d12292e1deb6079a45b2b21 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 19 Aug 2020 04:33:30 +0100 Subject: [PATCH 3/5] Fix rubocop issues --- app/models/spree/role.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/spree/role.rb b/app/models/spree/role.rb index a2d8ea903e..1c8f575835 100644 --- a/app/models/spree/role.rb +++ b/app/models/spree/role.rb @@ -2,6 +2,7 @@ module Spree class Role < ActiveRecord::Base - has_and_belongs_to_many :users, join_table: 'spree_roles_users', class_name: Spree.user_class.to_s + has_and_belongs_to_many :users, join_table: 'spree_roles_users', + class_name: Spree.user_class.to_s end end From ebf4175662bdcaf59fc404d3e5959d412f408684 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 19 Aug 2020 09:45:06 +0100 Subject: [PATCH 4/5] MErge user class extensions into the User class --- app/models/spree/user.rb | 17 +++++++++++++ config/initializers/user_class_extensions.rb | 26 -------------------- 2 files changed, 17 insertions(+), 26 deletions(-) delete mode 100644 config/initializers/user_class_extensions.rb diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index da276ec936..83f64748aa 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -7,6 +7,13 @@ module Spree belongs_to :ship_address, foreign_key: 'ship_address_id', class_name: 'Spree::Address' belongs_to :bill_address, foreign_key: 'bill_address_id', class_name: 'Spree::Address' + has_and_belongs_to_many :spree_roles, + join_table: 'spree_roles_users', + foreign_key: "user_id", + class_name: "Spree::Role" + + has_many :spree_orders, foreign_key: "user_id", class_name: "Spree::Order" + before_validation :set_login before_destroy :check_completed_orders @@ -41,10 +48,16 @@ module Spree User.admin.count > 0 end + # Whether a user has a role or not. + def has_spree_role?(role_in_question) + spree_roles.where(name: role_in_question.to_s).any? + end + def admin? has_spree_role?('admin') end + # handle_asynchronously will define send_reset_password_instructions_with_delay. # If handle_asynchronously is called twice, we get an infinite job loop. handle_asynchronously :send_reset_password_instructions unless method_defined? :send_reset_password_instructions_with_delay @@ -125,6 +138,10 @@ module Spree save! end + def last_incomplete_spree_order + spree_orders.incomplete.where(created_by_id: id).order('created_at DESC').first + end + protected def password_required? diff --git a/config/initializers/user_class_extensions.rb b/config/initializers/user_class_extensions.rb deleted file mode 100644 index 8603f0d359..0000000000 --- a/config/initializers/user_class_extensions.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -Spree::Core::Engine.config.to_prepare do - # has_spree_role? simply needs to return true or false whether a user has a role or not. - Spree.user_class&.class_eval do - include Spree::Core::UserBanners - has_and_belongs_to_many :spree_roles, - join_table: 'spree_roles_users', - foreign_key: "user_id", - class_name: "Spree::Role" - - has_many :spree_orders, foreign_key: "user_id", class_name: "Spree::Order" - - belongs_to :ship_address, class_name: 'Spree::Address' - belongs_to :bill_address, class_name: 'Spree::Address' - - # has_spree_role? simply needs to return true or false whether a user has a role or not. - def has_spree_role?(role_in_question) - spree_roles.where(name: role_in_question.to_s).any? - end - - def last_incomplete_spree_order - spree_orders.incomplete.where(created_by_id: id).order('created_at DESC').first - end - end -end From f28241cc5e290c84fb59aab59900aeedb86b7c16 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Wed, 19 Aug 2020 09:53:08 +0100 Subject: [PATCH 5/5] Merge duplicate Spree::User#superadmin? into existing Spree::admin? --- app/models/spree/user.rb | 10 +--------- config/routes/admin.rb | 2 +- spec/models/spree/user_spec.rb | 25 +++++-------------------- 3 files changed, 7 insertions(+), 30 deletions(-) diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 83f64748aa..059b26a2de 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -53,11 +53,11 @@ module Spree spree_roles.where(name: role_in_question.to_s).any? end + # Checks whether the specified user is a superadmin, with full control of the instance def admin? has_spree_role?('admin') end - # handle_asynchronously will define send_reset_password_instructions_with_delay. # If handle_asynchronously is called twice, we get an infinite job loop. handle_asynchronously :send_reset_password_instructions unless method_defined? :send_reset_password_instructions_with_delay @@ -120,14 +120,6 @@ module Spree end end - # Checks whether the specified user is a superadmin, with full control of the - # instance - # - # @return [Boolean] - def superadmin? - has_spree_role?('admin') - end - def generate_spree_api_key! self.spree_api_key = SecureRandom.hex(24) save! diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 931015e302..5b4a5b3479 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -1,7 +1,7 @@ Openfoodnetwork::Application.routes.draw do namespace :admin do - authenticated :spree_user, -> user { user.superadmin? } do + authenticated :spree_user, -> user { user.admin? } do mount DelayedJobWeb, at: '/delayed_job' end diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index fc89796e3c..abd34364c3 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -167,31 +167,16 @@ describe Spree.user_class do end end - describe '#superadmin?' do - let(:user) { create(:user) } - - context 'when the user has an admin spree role' do - before { user.spree_roles << Spree::Role.create(name: 'admin') } - - it 'returns true' do - expect(user.superadmin?).to eq(true) - end + describe '#admin?' do + it 'returns true when the user has an admin spree role' do + expect(create(:admin_user).admin?).to be_truthy end - context 'when the user does not have an admin spree role' do - it 'returns false' do - expect(user.superadmin?).to eq(false) - end + it 'returns false when the user does not have an admin spree role' do + expect(create(:user).admin?).to eq(false) end end - before(:all) { Spree::Role.create name: 'admin' } - - it '#admin?' do - expect(create(:admin_user).admin?).to be_truthy - expect(create(:user).admin?).to be_falsey - end - context '#destroy' do it 'can not delete if it has completed orders' do order = build(:order, completed_at: Time.zone.now)