diff --git a/app/controllers/spree/admin/users_controller.rb b/app/controllers/spree/admin/users_controller.rb index 4591f1f5d5..00fd64e8a9 100644 --- a/app/controllers/spree/admin/users_controller.rb +++ b/app/controllers/spree/admin/users_controller.rb @@ -20,17 +20,9 @@ module Spree end def create - if params[:user] - roles = params[:user].delete("spree_role_ids") - end - @user = Spree::User.new(user_params) if @user.save - if roles - @user.spree_roles = roles.compact_blank.collect{ |r| Spree::Role.find(r) } - end - flash[:success] = Spree.t(:created_successfully) redirect_to edit_admin_user_path(@user) else @@ -39,15 +31,7 @@ module Spree end def update - if params[:user] - roles = params[:user].delete("spree_role_ids") - end - if @user.update(user_params) - if roles - @user.spree_roles = roles.compact_blank.collect{ |r| Spree::Role.find(r) } - end - flash[:success] = update_message redirect_to edit_admin_user_path(@user) else @@ -131,7 +115,7 @@ module Spree def user_params ::PermittedAttributes::User.new(params).call( - %i[enterprise_limit show_api_key_view] + %i[admin enterprise_limit show_api_key_view] ) end end diff --git a/app/models/spree/role.rb b/app/models/spree/role.rb deleted file mode 100644 index 9e34c4d8d2..0000000000 --- a/app/models/spree/role.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -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 diff --git a/app/models/spree/user.rb b/app/models/spree/user.rb index 371ab636f7..72f2a3c80a 100644 --- a/app/models/spree/user.rb +++ b/app/models/spree/user.rb @@ -18,15 +18,11 @@ module Spree belongs_to :ship_address, class_name: 'Spree::Address' belongs_to :bill_address, class_name: 'Spree::Address' - has_and_belongs_to_many :spree_roles, - join_table: 'spree_roles_users', - class_name: "Spree::Role" - before_validation :set_login after_create :associate_customers, :associate_orders before_destroy :check_completed_orders - scope :admin, lambda { includes(:spree_roles).where("spree_roles.name" => "admin") } + scope :admin, -> { where(admin: true) } has_many :enterprise_roles, dependent: :destroy has_many :enterprises, through: :enterprise_roles @@ -58,11 +54,6 @@ module Spree User.admin.count > 0 end - # Checks whether the specified user is a superadmin, with full control of the instance - def admin? - spree_roles.any? { |role| role.name == "admin" } - end - # Send devise-based user emails asyncronously via ActiveJob # See: https://github.com/heartcombo/devise/tree/v3.5.10#activejob-integration def send_devise_notification(notification, *args) diff --git a/app/views/spree/admin/users/_form.html.haml b/app/views/spree/admin/users/_form.html.haml index 4aac09f35f..2064cb4f74 100644 --- a/app/views/spree/admin/users/_form.html.haml +++ b/app/views/spree/admin/users/_form.html.haml @@ -5,13 +5,8 @@ = f.email_field :email, class: "fullwidth" = error_message_on :user, :email .field - = label_tag nil, t(".roles") - %ul - - [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 - = hidden_field_tag "user[spree_role_ids][]", "" + = f.label :admin, t(".admin") + = f.check_box :admin = f.field_container :locale do = f.label :locale, t(".locale") = f.select :locale, locale_options, class: "fullwidth" diff --git a/config/locales/en.yml b/config/locales/en.yml index 48ba87aee5..07a974d49a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4660,7 +4660,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using form: disabled: "Disabled?" email: "Email" - roles: "Roles" + admin: "Super admin?" enterprise_limit: "Enterprise Limit" confirm_password: "Confirm Password" password: "Password" diff --git a/db/default/users.rb b/db/default/users.rb index 23ba8f549b..85fdd0379c 100644 --- a/db/default/users.rb +++ b/db/default/users.rb @@ -60,7 +60,6 @@ def create_admin_user ValidEmail2::Address.define_method(:valid_mx?) { true } if admin.save - admin.spree_roles << Spree::Role.admin say "New admin user persisted!" else say "There was some problems with persisting new admin user:" @@ -81,6 +80,7 @@ def read_user_attributes end { + admin: true, password:, password_confirmation: password, email:, diff --git a/db/migrate/20250107013958_add_admin_to_spree_users.rb b/db/migrate/20250107013958_add_admin_to_spree_users.rb new file mode 100644 index 0000000000..057a43425f --- /dev/null +++ b/db/migrate/20250107013958_add_admin_to_spree_users.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +# We'll replace our only role "admin" with a simple flag. +class AddAdminToSpreeUsers < ActiveRecord::Migration[7.0] + def change + add_column :spree_users, :admin, :boolean, default: false, null: false + end +end diff --git a/db/migrate/20250107014617_copy_admin_attribute_to_users.rb b/db/migrate/20250107014617_copy_admin_attribute_to_users.rb new file mode 100644 index 0000000000..869c20d11f --- /dev/null +++ b/db/migrate/20250107014617_copy_admin_attribute_to_users.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CopyAdminAttributeToUsers < ActiveRecord::Migration[7.0] + def up + execute <<~SQL.squish + UPDATE spree_users SET admin = true WHERE id IN ( + SELECT user_id FROM spree_roles_users WHERE role_id IN ( + SELECT id FROM spree_roles WHERE name = 'admin' + ) + ) + SQL + end +end diff --git a/db/schema.rb b/db/schema.rb index d97d554f99..de0ff812d2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -945,6 +945,7 @@ ActiveRecord::Schema[7.0].define(version: 2025_01_13_055412) do t.string "provider" t.string "uid" t.datetime "terms_of_service_accepted_at" + t.boolean "admin", default: false, null: false t.index ["confirmation_token"], name: "index_spree_users_on_confirmation_token", unique: true t.index ["email"], name: "email_idx_unique", unique: true t.index ["persistence_token"], name: "index_users_on_persistence_token" diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb index b91bf12377..35cefae5d1 100644 --- a/spec/controllers/admin/order_cycles_controller_spec.rb +++ b/spec/controllers/admin/order_cycles_controller_spec.rb @@ -431,11 +431,7 @@ module Admin describe "notifying producers" do let(:user) { create(:user) } - let(:admin_user) do - user = create(:user) - user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin') - user - end + let(:admin_user) { create(:admin_user) } let(:order_cycle) { create(:simple_order_cycle) } before do diff --git a/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/spec/controllers/spree/admin/payment_methods_controller_spec.rb index e6d3288603..7124e9fecd 100644 --- a/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -235,8 +235,6 @@ module Spree let(:user) do new_user = create(:user, email: 'enterprise@hub.com', password: 'blahblah', password_confirmation: 'blahblah', ) - # for some reason unbeknown to me, this new user gets admin permissions by default. - new_user.spree_roles = [] new_user.enterprise_roles.build(enterprise:).save new_user.save new_user diff --git a/spec/controllers/spree/admin/users_controller_spec.rb b/spec/controllers/spree/admin/users_controller_spec.rb index 97f0155f93..f040604cb6 100644 --- a/spec/controllers/spree/admin/users_controller_spec.rb +++ b/spec/controllers/spree/admin/users_controller_spec.rb @@ -3,37 +3,39 @@ require 'spec_helper' RSpec.describe Spree::Admin::UsersController do - context '#authorize_admin' do + describe '#authorize_admin' do let(:user) { create(:user) } - let(:test_user) { create(:user) } before do allow(controller).to receive_messages spree_current_user: user - allow(Spree::User).to receive(:find).with(test_user.id.to_s).and_return(test_user) - user.spree_roles.clear end - it 'should grant access to users with an admin role' do - user.spree_roles << Spree::Role.find_or_create_by(name: 'admin') - spree_post :index - expect(response).to render_template :index - end + context "as a super admin" do + let(:user) { create(:admin_user) } + let(:test_user) { create(:user) } - it "allows admins to update a user's show api key view" do - user.spree_roles << Spree::Role.find_or_create_by(name: 'admin') - spree_put :update, id: test_user.id, user: { show_api_key_view: true } - expect(response).to redirect_to spree.edit_admin_user_path(test_user) - end + before do + allow(Spree::User).to receive(:find).with(test_user.id.to_s).and_return(test_user) + end - it "re-renders the edit form if error" do - user.spree_roles << Spree::Role.find_or_create_by(name: 'admin') - spree_put :update, id: test_user.id, user: { password: "blah", password_confirmation: "" } + it 'should grant access to users with an admin role' do + spree_post :index + expect(response).to render_template :index + end - expect(response).to render_template :edit + it "allows admins to update a user's show api key view" do + spree_put :update, id: test_user.id, user: { show_api_key_view: true } + expect(response).to redirect_to spree.edit_admin_user_path(test_user) + end + + it "re-renders the edit form if error" do + spree_put :update, id: test_user.id, user: { password: "blah", password_confirmation: "" } + + expect(response).to render_template :edit + end end it 'should deny access to users without an admin role' do - allow(user).to receive_messages admin?: false spree_post :index expect(response).to redirect_to('/unauthorized') end diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 5646f839ab..03d612bac4 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -35,7 +35,7 @@ FactoryBot.define do end factory :admin_user do - spree_roles { [Spree::Role.find_or_create_by!(name: 'admin')] } + admin { true } end factory :oidc_user do diff --git a/spec/lib/reports/customers_report_spec.rb b/spec/lib/reports/customers_report_spec.rb index 19f7283a78..9705260873 100644 --- a/spec/lib/reports/customers_report_spec.rb +++ b/spec/lib/reports/customers_report_spec.rb @@ -7,11 +7,7 @@ module Reporting module Customers RSpec.describe Base do context "as a site admin" do - let(:user) do - user = create(:user) - user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin') - user - end + let(:user) { create(:admin_user) } subject { Base.new user, {} } describe "addresses report" do @@ -198,12 +194,7 @@ module Reporting end context "as an enterprise user" do - let(:user) do - user = create(:user) - user.spree_roles = [] - user.save! - user - end + let(:user) { create(:user) } subject { Base.new user, {} } diff --git a/spec/lib/reports/order_cycle_management_report_spec.rb b/spec/lib/reports/order_cycle_management_report_spec.rb index 17136d5942..a5883b8d27 100644 --- a/spec/lib/reports/order_cycle_management_report_spec.rb +++ b/spec/lib/reports/order_cycle_management_report_spec.rb @@ -10,11 +10,7 @@ module Reporting subject { Base.new(user, params) } let(:params) { {} } - let(:user) do - user = create(:user) - user.spree_roles << Spree::Role.find_or_create_by!(name: "admin") - user - end + let(:user) { create(:admin_user) } describe "fetching orders" do it 'calls the OutstandingBalanceQuery query object' do diff --git a/spec/lib/reports/products_and_inventory_report_spec.rb b/spec/lib/reports/products_and_inventory_report_spec.rb index 4962220a94..3eaca20d01 100644 --- a/spec/lib/reports/products_and_inventory_report_spec.rb +++ b/spec/lib/reports/products_and_inventory_report_spec.rb @@ -7,11 +7,7 @@ module Reporting module ProductsAndInventory RSpec.describe Base do context "As a site admin" do - let(:user) do - user = create(:user) - user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin') - user - end + let(:user) { create(:admin_user) } subject do Base.new user, {} end @@ -72,7 +68,6 @@ module Reporting let(:enterprise_user) do user = create(:user) user.enterprise_roles.create(enterprise: supplier) - user.spree_roles = [] user.save! user end @@ -259,11 +254,7 @@ module Reporting end RSpec.describe AllProducts do - let(:user) do - user = create(:user) - user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin') - user - end + let(:user) { create(:admin_user) } let(:report) do AllProducts.new user, { fields_to_hide: [] } end diff --git a/spec/models/enterprise_group_spec.rb b/spec/models/enterprise_group_spec.rb index 7e7c5fd95c..6ce1aa856f 100644 --- a/spec/models/enterprise_group_spec.rb +++ b/spec/models/enterprise_group_spec.rb @@ -77,7 +77,6 @@ RSpec.describe EnterpriseGroup do it "finds a user's enterprise groups" do user = create(:user) - user.spree_roles = [] eg1 = create(:enterprise_group, owner: user) eg2 = create(:enterprise_group) diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index de519e52e8..1d05f99bd6 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -651,7 +651,6 @@ RSpec.describe Enterprise do describe "managed_by" do it "shows only enterprises for given user" do user = create(:user) - user.spree_roles = [] e1 = create(:enterprise) e2 = create(:enterprise) e1.enterprise_roles.build(user:).save diff --git a/spec/models/exchange_spec.rb b/spec/models/exchange_spec.rb index b790d47356..2377e3618e 100644 --- a/spec/models/exchange_spec.rb +++ b/spec/models/exchange_spec.rb @@ -107,11 +107,7 @@ RSpec.describe Exchange do let(:oc) { create(:simple_order_cycle, coordinator:) } describe "finding exchanges managed by a particular user" do - let(:user) do - user = create(:user) - user.spree_roles = [] - user - end + let(:user) { create(:user) } before { Exchange.destroy_all } diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 873f6730ad..395be0a985 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -62,8 +62,7 @@ RSpec.describe OrderCycle do it "finds order cycles accessible by a user" do e1 = create(:enterprise, is_primary_producer: true, sells: "any") e2 = create(:enterprise, is_primary_producer: true, sells: "any") - user = create(:user, enterprises: [e2], spree_roles: []) - user.spree_roles = [] + user = create(:user, enterprises: [e2]) oc_coordinated = create(:simple_order_cycle, coordinator: e2) oc_sent = create(:simple_order_cycle, suppliers: [e2]) diff --git a/spec/models/spree/ability_spec.rb b/spec/models/spree/ability_spec.rb index be228abfca..1339a39328 100644 --- a/spec/models/spree/ability_spec.rb +++ b/spec/models/spree/ability_spec.rb @@ -9,14 +9,6 @@ RSpec.describe Spree::Ability do let(:subject) { Spree::Ability.new(user) } let(:token) { nil } - before do - user.spree_roles.clear - end - - after(:each) { - user.spree_roles = [] - } - context 'for general resource' do let(:resource) { Object.new } @@ -42,8 +34,10 @@ RSpec.describe Spree::Ability do let(:fakedispatch_ability) { Spree::Ability.new(fakedispatch_user) } context 'with admin user' do + let(:user) { create(:admin_user) } + it 'should be able to admin' do - user.spree_roles << Spree::Role.find_or_create_by(name: 'admin') + user.update!(admin: true) expect(subject).to be_able_to :admin, resource expect(subject).to be_able_to :index, resource_order expect(subject).to be_able_to :show, resource_product @@ -303,7 +297,6 @@ RSpec.describe Spree::Ability do # create supplier_enterprise1 user without full admin access let(:user) do user = create(:user) - user.spree_roles = [] s1.enterprise_roles.build(user:).save user end @@ -487,7 +480,6 @@ RSpec.describe Spree::Ability do context "when is a distributor enterprise user" do let(:user) do user = create(:user) - user.spree_roles = [] d1.enterprise_roles.build(user:).save user end @@ -699,7 +691,6 @@ RSpec.describe Spree::Ability do context 'Order Cycle co-ordinator, distributor enterprise manager' do let(:user) do user = create(:user) - user.spree_roles = [] d1.enterprise_roles.build(user:).save user end @@ -738,7 +729,6 @@ RSpec.describe Spree::Ability do context 'enterprise manager' do let(:user) do user = create(:user) - user.spree_roles = [] s1.enterprise_roles.build(user:).save user end @@ -797,7 +787,7 @@ RSpec.describe Spree::Ability do let(:manage_actions) { [:admin, :index, :read, :update, :bulk_update, :bulk_reset] } describe "when admin" do - before { user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin') } + let(:user) { create(:admin_user) } it "should have permission" do is_expected.to have_ability(manage_actions, for: variant_override) diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 9f9937f897..59884d05e8 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -508,7 +508,6 @@ module Spree it "shows only products for given user" do user = create(:user) - user.spree_roles = [] e1.enterprise_roles.build(user:).save product = Product.managed_by user diff --git a/spec/models/spree/user_spec.rb b/spec/models/spree/user_spec.rb index a2a0ab3897..ea9531f707 100644 --- a/spec/models/spree/user_spec.rb +++ b/spec/models/spree/user_spec.rb @@ -209,11 +209,11 @@ RSpec.describe Spree::User do end describe '#admin?' do - it 'returns true when the user has an admin spree role' do + it 'returns true when the user has an admin role' do expect(create(:admin_user).admin?).to be_truthy end - it 'returns false when the user does not have an admin spree role' do + it 'returns false when the user does not have an admin role' do expect(create(:user).admin?).to eq(false) end end diff --git a/spec/support/controller_helper.rb b/spec/support/controller_helper.rb index c7cc66b6e2..f2f9b61bed 100644 --- a/spec/support/controller_helper.rb +++ b/spec/support/controller_helper.rb @@ -3,11 +3,7 @@ module OpenFoodNetwork module ControllerHelper def controller_login_as_admin - @admin_user ||= begin - user = create(:user) - user.spree_roles << Spree::Role.find_or_create_by!(name: 'admin') - user - end + @admin_user ||= create(:admin_user) allow(controller).to receive_messages(spree_current_user: @admin_user) end @@ -15,7 +11,6 @@ module OpenFoodNetwork def controller_login_as_enterprise_user(enterprises) @enterprise_user ||= begin user = create(:user) - user.spree_roles = [] enterprises.each do |enterprise| enterprise.enterprise_roles.create!(user:) end diff --git a/spec/system/admin/configuration/shipping_categories_spec.rb b/spec/system/admin/configuration/shipping_categories_spec.rb index 7e884d0aeb..ea2fc73142 100644 --- a/spec/system/admin/configuration/shipping_categories_spec.rb +++ b/spec/system/admin/configuration/shipping_categories_spec.rb @@ -5,7 +5,6 @@ require 'system_helper' RSpec.describe "Shipping Categories" do include AuthenticationHelper include WebHelper - let(:admin_role) { Spree::Role.find_or_create_by!(name: 'admin') } let(:admin_user) { create(:user) } context 'user visits shipping categories page' do diff --git a/spec/system/admin/multilingual_spec.rb b/spec/system/admin/multilingual_spec.rb index e8de421d75..0f9f0900bd 100644 --- a/spec/system/admin/multilingual_spec.rb +++ b/spec/system/admin/multilingual_spec.rb @@ -5,11 +5,9 @@ require 'system_helper' RSpec.describe 'Multilingual' do include AuthenticationHelper include WebHelper - let(:admin_role) { Spree::Role.find_or_create_by!(name: 'admin') } - let(:admin_user) { create(:user) } + let(:admin_user) { create(:admin_user) } before do - admin_user.spree_roles << admin_role login_as admin_user visit spree.admin_dashboard_path end