Merge pull request #13047 from mkllnk/spree-roles

Clarify that our only user role is "admin" and simplify code
This commit is contained in:
Filipe
2025-01-16 18:06:49 -06:00
committed by GitHub
27 changed files with 34 additions and 50 deletions

View File

@@ -144,7 +144,7 @@ module Admin
def product_scope
user = spree_current_user
scope = if user.has_spree_role?("admin") || user.enterprises.present?
scope = if user.admin? || user.enterprises.present?
Spree::Product
else
Spree::Product.active

View File

@@ -55,14 +55,14 @@ module Api
def scope
if @product
variants = if current_api_user.has_spree_role?("admin") || params[:show_deleted]
variants = if current_api_user.admin? || params[:show_deleted]
@product.variants.with_deleted
else
@product.variants
end
else
variants = Spree::Variant.where(nil)
if current_api_user.has_spree_role?("admin")
if current_api_user.admin?
unless params[:show_deleted]
variants = Spree::Variant.active
end

View File

@@ -11,8 +11,6 @@ module Spree
# http://spreecommerce.com/blog/2010/11/02/json-hijacking-vulnerability/
before_action :check_json_authenticity, only: :index
before_action :load_roles, only: [:edit, :new, :update, :create,
:generate_api_key, :clear_api_key]
def index
respond_with(@collection) do |format|
@@ -123,10 +121,6 @@ module Spree
sign_in(@user, event: :authentication, bypass: true)
end
def load_roles
@roles = Spree::Role.where(nil)
end
def new_email_unconfirmed?
params[:user][:email] != @user.email
end

View File

@@ -6,7 +6,7 @@ module SharedHelper
end
def admin_user?
spree_current_user&.has_spree_role? 'admin'
spree_current_user&.admin?
end
def current_shop_products_path

View File

@@ -218,7 +218,7 @@ class Enterprise < ApplicationRecord
}
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
joins(:enterprise_roles).where(enterprise_roles: { user_id: user.id })

View File

@@ -28,7 +28,7 @@ class EnterpriseFee < ApplicationRecord
scope :for_enterprises, lambda { |enterprises| where(enterprise_id: enterprises) }
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
where(enterprise_id: user.enterprises.select(&:id))

View File

@@ -38,7 +38,7 @@ class EnterpriseGroup < ApplicationRecord
scope :by_position, -> { order('position ASC') }
scope :on_front_page, -> { where(on_front_page: true) }
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
where(owner_id: user.id)

View File

@@ -75,7 +75,7 @@ class Exchange < ApplicationRecord
}
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
joins("LEFT JOIN enterprises senders ON senders.id = exchanges.sender_id").

View File

@@ -84,7 +84,7 @@ class OrderCycle < ApplicationRecord
}
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
where(coordinator_id: user.enterprises.to_a)
@@ -93,7 +93,7 @@ class OrderCycle < ApplicationRecord
# Return order cycles that user coordinates, sends to or receives from
scope :visible_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
with_exchanging_enterprises_outer.

View File

@@ -18,7 +18,7 @@ module Spree
user ||= Spree::User.new
if user.respond_to?(:has_spree_role?) && user.has_spree_role?('admin')
if user.try(:admin?)
can :manage, :all
else
can [:index, :read], Country

View File

@@ -55,7 +55,7 @@ module Spree
# -- Scopes
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
# Find line items that are from orders distributed by the user or supplied by the user

View File

@@ -125,7 +125,7 @@ module Spree
}
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
# Find orders that are distributed by the user or have products supplied by the user
@@ -140,7 +140,7 @@ module Spree
}
scope :distributed_by_user, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
where(spree_orders: { distributor_id: user.enterprises.select(&:id) })

View File

@@ -168,7 +168,7 @@ module Spree
scope :by_name, -> { order('spree_products.name') }
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
in_supplier(user.enterprises)

View File

@@ -4,5 +4,10 @@ module Spree
class Role < ApplicationRecord
has_and_belongs_to_many :users, join_table: 'spree_roles_users',
class_name: "Spree::User"
# The only role we have at the moment:
def self.admin
Spree::Role.find_or_create_by(name: 'admin')
end
end
end

View File

@@ -37,7 +37,7 @@ module Spree
after_save :touch_distributors
scope :managed_by, lambda { |user|
if user.has_spree_role?('admin')
if user.admin?
where(nil)
else
joins(:distributors).

View File

@@ -26,9 +26,7 @@ module Spree
after_create :associate_customers, :associate_orders
before_destroy :check_completed_orders
roles_table_name = Role.table_name
scope :admin, lambda { includes(:spree_roles).where("#{roles_table_name}.name" => "admin") }
scope :admin, lambda { includes(:spree_roles).where("spree_roles.name" => "admin") }
has_many :enterprise_roles, dependent: :destroy
has_many :enterprises, through: :enterprise_roles
@@ -60,14 +58,9 @@ 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
# Checks whether the specified user is a superadmin, with full control of the instance
def admin?
has_spree_role?('admin')
spree_roles.any? { |role| role.name == "admin" }
end
# Send devise-based user emails asyncronously via ActiveJob

View File

@@ -46,7 +46,7 @@ class ProductScopeQuery
end
def product_scope
if @user.has_spree_role?("admin") || @user.enterprises.present?
if @user.admin? || @user.enterprises.present?
scope = Spree::Product
if @params[:show_deleted]
scope = scope.with_deleted

View File

@@ -7,7 +7,7 @@
.field
= label_tag nil, t(".roles")
%ul
- @roles.each do |role|
- [Spree::Role.admin].each do |role|
%li
= check_box_tag "user[spree_role_ids][]", role.id, @user.spree_roles.include?(role), id: "user_spree_role_#{role.name}"
= label_tag role.name

View File

@@ -63,8 +63,7 @@ def create_admin_user
ValidEmail2::Address.define_method(:valid_mx?) { true }
if admin.save
role = Spree::Role.find_or_create_by(name: 'admin')
admin.spree_roles << role
admin.spree_roles << Spree::Role.admin
say "New admin user persisted!"
else
say "There was some problems with persisting new admin user:"

View File

@@ -5,10 +5,6 @@ require 'yaml'
# We need mail_configuration to create a user account, because it sends a confirmation email.
MailConfiguration.apply!
puts "[db:seed] Seeding Roles"
Spree::Role.where(:name => "admin").first_or_create
Spree::Role.where(:name => "user").first_or_create
puts "[db:seed] Seeding Countries"
unless Spree::Country.find_by(iso: ENV['DEFAULT_COUNTRY_CODE'])
require File.join(File.dirname(__FILE__), 'default', 'countries')

View File

@@ -148,7 +148,6 @@ module Admin
order_cycles: 6,
proxy_orders: 1,
schedules: 1,
spree_roles: 9,
spree_variants: 8,
tags: 1
},

View File

@@ -30,7 +30,7 @@ RSpec.describe Api::V0::ProductsController, type: :controller do
before do
allow(current_api_user)
.to receive(:has_spree_role?).with("admin").and_return(false)
.to receive(:admin?).and_return(false)
end
it "gets a single product" do
@@ -96,7 +96,7 @@ RSpec.describe Api::V0::ProductsController, type: :controller do
context "as an administrator" do
before do
allow(current_api_user)
.to receive(:has_spree_role?).with("admin").and_return(true)
.to receive(:admin?).and_return(true)
end
it "can create a new product" do
@@ -153,7 +153,7 @@ RSpec.describe Api::V0::ProductsController, type: :controller do
context 'as a normal user' do
before do
allow(current_api_user)
.to receive(:has_spree_role?).with("admin").and_return(false)
.to receive(:admin?).and_return(false)
end
it 'denies access' do
@@ -204,7 +204,7 @@ RSpec.describe Api::V0::ProductsController, type: :controller do
context 'as an administrator' do
before do
allow(current_api_user)
.to receive(:has_spree_role?).with("admin").and_return(true)
.to receive(:admin?).and_return(true)
end
it 'responds with a successful response' do

View File

@@ -21,7 +21,7 @@ RSpec.describe Spree::Admin::MailMethodsController do
id: nil,
owned_groups: nil)
allow(user).to receive_messages(enterprises: [create(:enterprise)],
has_spree_role?: true,
admin?: true,
locale: nil)
allow(controller).to receive_messages(spree_current_user: user)

View File

@@ -33,7 +33,7 @@ RSpec.describe Spree::Admin::UsersController do
end
it 'should deny access to users without an admin role' do
allow(user).to receive_messages has_spree_role?: false
allow(user).to receive_messages admin?: false
spree_post :index
expect(response).to redirect_to('/unauthorized')
end

View File

@@ -140,8 +140,6 @@ RSpec.describe Reporting::Reports::OrdersAndDistributors::Base do
subject # build context first
expect { subject.table_rows }.to query_database [
"Spree::Role Exists?",
"Spree::Role Exists?",
"SQL",
"Spree::LineItem Load",
"Spree::PaymentMethod Load",

View File

@@ -21,7 +21,7 @@ RSpec.describe Spree::Ability do
let(:resource) { Object.new }
context 'with admin user' do
before(:each) { allow(user).to receive(:has_spree_role?).and_return(true) }
before(:each) { allow(user).to receive(:admin?).and_return(true) }
it_should_behave_like 'access granted'
it_should_behave_like 'index allowed'
end

View File

@@ -199,7 +199,7 @@ module Permissions
let(:permissions) { Permissions::Order.new(user, search_params) }
before do
allow(user).to receive(:has_spree_role?) { "admin" }
allow(user).to receive(:admin?) { "admin" }
end
it "only returns line items from completed, " \