From 445fd0f9e5ae96146d2731a037ad4074f390b241 Mon Sep 17 00:00:00 2001 From: Joshua Wehner Date: Wed, 10 Aug 2022 16:43:30 +0200 Subject: [PATCH] Try renaming moderator -> super_moderator (#18261) * Try renaming moderator -> super_moderator * Still finding 'moderator' words * Fixes for failing specs * Update test with new role name * Update app/services/moderator/manage_activity_and_roles.rb Co-authored-by: Suzanne Aitchison Co-authored-by: Suzanne Aitchison --- app/controllers/reactions_controller.rb | 2 +- app/helpers/admin/users_helper.rb | 2 +- app/models/role.rb | 2 +- app/models/tag_adjustment.rb | 2 +- app/models/user.rb | 2 +- app/policies/application_policy.rb | 2 +- app/policies/article_policy.rb | 2 +- app/policies/authorizer.rb | 6 +-- app/policies/comment_policy.rb | 1 + app/policies/response_template_policy.rb | 2 +- app/policies/user_policy.rb | 2 +- .../moderator/manage_activity_and_roles.rb | 4 +- ...730_rename_moderator_to_super_moderator.rb | 7 +++ spec/factories/users.rb | 4 +- spec/helpers/admin/users_helper_spec.rb | 2 +- ...ename_moderator_to_super_moderator_spec.rb | 46 +++++++++++++++++++ spec/models/role_spec.rb | 2 +- spec/policies/article_policy_spec.rb | 2 +- spec/policies/authorizer_spec.rb | 8 ++-- spec/policies/user_policy_spec.rb | 2 +- spec/requests/api/v1/articles_spec.rb | 2 +- .../manage_activity_and_roles_spec.rb | 4 +- spec/support/seeds/seeds_e2e.rb | 2 +- 23 files changed, 82 insertions(+), 28 deletions(-) create mode 100644 lib/data_update_scripts/20220802100730_rename_moderator_to_super_moderator.rb create mode 100644 spec/lib/data_update_scripts/rename_moderator_to_super_moderator_spec.rb diff --git a/app/controllers/reactions_controller.rb b/app/controllers/reactions_controller.rb index 48191a825..eaf53fe63 100644 --- a/app/controllers/reactions_controller.rb +++ b/app/controllers/reactions_controller.rb @@ -125,7 +125,7 @@ class ReactionsController < ApplicationController reactable_type: params[:reactable_type], category: category } - if (current_user&.any_admin? || current_user&.moderator?) && + if (current_user&.any_admin? || current_user&.super_moderator?) && Reaction::NEGATIVE_PRIVILEGED_CATEGORIES.include?(category) create_params[:status] = "confirmed" end diff --git a/app/helpers/admin/users_helper.rb b/app/helpers/admin/users_helper.rb index 7de7e4892..d640fc08c 100644 --- a/app/helpers/admin/users_helper.rb +++ b/app/helpers/admin/users_helper.rb @@ -5,7 +5,7 @@ module Admin if logged_in_user.super_admin? special_roles = Constants::Role::SPECIAL_ROLES if FeatureFlag.enabled?(:moderator_role) - special_roles = special_roles.dup << "Moderator" + special_roles = special_roles.dup << "Super Moderator" end options["Roles"] = special_roles end diff --git a/app/models/role.rb b/app/models/role.rb index cd97568bb..75aed3465 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -5,7 +5,7 @@ class Role < ApplicationRecord comment_suspended creator mod_relations_admin - moderator + super_moderator podcast_admin restricted_liquid_tag single_resource_admin diff --git a/app/models/tag_adjustment.rb b/app/models/tag_adjustment.rb index 79662be27..8c26aa62d 100644 --- a/app/models/tag_adjustment.rb +++ b/app/models/tag_adjustment.rb @@ -19,7 +19,7 @@ class TagAdjustment < ApplicationRecord end def elevated_user? - user.any_admin? || user.moderator? + user.any_admin? || user.super_moderator? end def has_privilege_to_adjust? diff --git a/app/models/user.rb b/app/models/user.rb index 24cbbb7cd..3c74edcb5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -402,7 +402,7 @@ class User < ApplicationRecord :comment_suspended?, :creator?, :has_trusted_role?, - :moderator?, + :super_moderator?, :podcast_admin_for?, :restricted_liquid_tag_for?, :single_resource_admin_for?, diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index af865780a..5eb1dc112 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -199,7 +199,7 @@ class ApplicationPolicy delegate :support_admin?, to: :user - delegate :moderator?, :super_admin?, :any_admin?, :suspended?, to: :user, prefix: true + delegate :super_moderator?, :super_admin?, :any_admin?, :suspended?, to: :user, prefix: true alias minimal_admin? user_any_admin? deprecate minimal_admin?: "Deprecating #{self}#minimal_admin?, use #{self}#user_any_admin?" diff --git a/app/policies/article_policy.rb b/app/policies/article_policy.rb index 2811109f8..41c17f5bb 100644 --- a/app/policies/article_policy.rb +++ b/app/policies/article_policy.rb @@ -150,7 +150,7 @@ class ArticlePolicy < ApplicationPolicy end def elevated_user? - user_any_admin? || user_moderator? + user_any_admin? || user_super_moderator? end # this method performs the same checks that determine: diff --git a/app/policies/authorizer.rb b/app/policies/authorizer.rb index 9d987974c..d0f909503 100644 --- a/app/policies/authorizer.rb +++ b/app/policies/authorizer.rb @@ -76,7 +76,7 @@ module Authorizer end def accesses_mod_response_templates? - has_trusted_role? || any_admin? || moderator? || tag_moderator? + has_trusted_role? || any_admin? || super_moderator? || tag_moderator? end # When you need to know if we trust the user, but don't want to @@ -96,8 +96,8 @@ module Authorizer has_role?(:trusted) end - def moderator? - has_role?(:moderator) + def super_moderator? + has_role?(:super_moderator) end def podcast_admin_for?(podcast) diff --git a/app/policies/comment_policy.rb b/app/policies/comment_policy.rb index d9b350e1a..b7b483260 100644 --- a/app/policies/comment_policy.rb +++ b/app/policies/comment_policy.rb @@ -32,6 +32,7 @@ class CommentPolicy < ApplicationPolicy end def moderator_create? + # NOTE: Here, when we say "moderator", we mean "tag_moderator" user_moderator? || user_any_admin? end diff --git a/app/policies/response_template_policy.rb b/app/policies/response_template_policy.rb index d10711ed7..91d9436ed 100644 --- a/app/policies/response_template_policy.rb +++ b/app/policies/response_template_policy.rb @@ -62,7 +62,7 @@ class ResponseTemplatePolicy < ApplicationPolicy end def user_moderator? - user_any_admin? || user.moderator_for_tags&.present? + user_any_admin? || user.super_moderator? || user.moderator_for_tags&.present? end def mod_comment? diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb index 2160e8a4f..e013fc3d9 100644 --- a/app/policies/user_policy.rb +++ b/app/policies/user_policy.rb @@ -90,7 +90,7 @@ class UserPolicy < ApplicationPolicy end def elevated_user? - user_any_admin? || user_moderator? + user_any_admin? || user_super_moderator? end alias toggle_suspension_status? elevated_user? diff --git a/app/services/moderator/manage_activity_and_roles.rb b/app/services/moderator/manage_activity_and_roles.rb index 6f7650370..2375ebfaf 100644 --- a/app/services/moderator/manage_activity_and_roles.rb +++ b/app/services/moderator/manage_activity_and_roles.rb @@ -68,8 +68,8 @@ module Moderator when "Suspended" || "Spammer" user.add_role(:suspended) remove_privileges - when "Moderator" - assign_elevated_role_to_user(user, :moderator) + when "Super Moderator" + assign_elevated_role_to_user(user, :super_moderator) TagModerators::AddTrustedRole.call(user) when "Good standing" regular_member diff --git a/lib/data_update_scripts/20220802100730_rename_moderator_to_super_moderator.rb b/lib/data_update_scripts/20220802100730_rename_moderator_to_super_moderator.rb new file mode 100644 index 000000000..ea710131f --- /dev/null +++ b/lib/data_update_scripts/20220802100730_rename_moderator_to_super_moderator.rb @@ -0,0 +1,7 @@ +module DataUpdateScripts + class RenameModeratorToSuperModerator + def run + Role.where(name: "moderator").update_all(name: "super_moderator") + end + end +end diff --git a/spec/factories/users.rb b/spec/factories/users.rb index ee2c1560b..f4165dcd1 100644 --- a/spec/factories/users.rb +++ b/spec/factories/users.rb @@ -71,8 +71,8 @@ FactoryBot.define do after(:build) { |user| user.add_role(:admin) } end - trait :moderator do - after(:build) { |user| user.add_role(:moderator) } + trait :super_moderator do + after(:build) { |user| user.add_role(:super_moderator) } end trait :single_resource_admin do diff --git a/spec/helpers/admin/users_helper_spec.rb b/spec/helpers/admin/users_helper_spec.rb index 63db5dfc0..2ebb857b8 100644 --- a/spec/helpers/admin/users_helper_spec.rb +++ b/spec/helpers/admin/users_helper_spec.rb @@ -24,7 +24,7 @@ describe Admin::UsersHelper do roles = helper.role_options(user) expect(roles).to have_key("Roles") - expect(roles["Roles"]).to include "Moderator" + expect(roles["Roles"]).to include "Super Moderator" end end diff --git a/spec/lib/data_update_scripts/rename_moderator_to_super_moderator_spec.rb b/spec/lib/data_update_scripts/rename_moderator_to_super_moderator_spec.rb new file mode 100644 index 000000000..9231c677a --- /dev/null +++ b/spec/lib/data_update_scripts/rename_moderator_to_super_moderator_spec.rb @@ -0,0 +1,46 @@ +require "rails_helper" +require Rails.root.join( + "lib/data_update_scripts/20220802100730_rename_moderator_to_super_moderator.rb", +) + +describe DataUpdateScripts::RenameModeratorToSuperModerator do + before do + create :user + create :user, :tag_moderator + create :user, :super_admin + end + + context "when there are no moderators" do + it "does nothing" do + expect(described_class.new.run).to eq(0) + end + end + + context "when there are users with the moderator role" do + let!(:moderator) do + # moderator is no longer a valid name, so to stage a user with the old role + # we can't use the convenience methods as we need to bypass validation + role = Role.new name: "moderator" + role.save validate: false + + create(:user) do |user| + user.roles << role + end + end + + it "updates those records" do + expect(moderator.roles.pluck(:name)).to contain_exactly("moderator") + expect(described_class.new.run).to eq(1) + expect(moderator.reload).to be_super_moderator + end + end + + context "when rename has already run" do + let!(:super_moderator) { create :user, :super_moderator } + + it "does nothing" do + expect(described_class.new.run).to eq(0) + expect(super_moderator.reload).to be_super_moderator + end + end +end diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb index cd0349bee..62ceed765 100644 --- a/spec/models/role_spec.rb +++ b/spec/models/role_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Role, type: :model do expected_roles = %w[ admin codeland_admin comment_suspended mod_relations_admin podcast_admin restricted_liquid_tag single_resource_admin super_admin support_admin suspended tag_moderator tech_admin - trusted warned workshop_pass creator moderator + trusted warned workshop_pass creator super_moderator ] expect(described_class::ROLES).to match_array(expected_roles) end diff --git a/spec/policies/article_policy_spec.rb b/spec/policies/article_policy_spec.rb index df648b316..5c5113994 100644 --- a/spec/policies/article_policy_spec.rb +++ b/spec/policies/article_policy_spec.rb @@ -15,7 +15,7 @@ RSpec.describe ArticlePolicy do let(:trusted) { create(:user, :trusted) } let(:other_users) { create(:user) } let(:author) { create(:user) } - let(:moderator) { create(:user, :moderator) } + let(:moderator) { create(:user, :super_moderator) } let(:tag_mod) { create(:user, :tag_moderator) } let(:tagmod_tag) { tag_mod.roles.find_by(name: "tag_moderator").resource } let(:random_tag) { create(:tag, name: "randomtag") } diff --git a/spec/policies/authorizer_spec.rb b/spec/policies/authorizer_spec.rb index 6f35b3bcf..e9c6c8c1a 100644 --- a/spec/policies/authorizer_spec.rb +++ b/spec/policies/authorizer_spec.rb @@ -6,7 +6,7 @@ RSpec.describe Authorizer, type: :policy do let(:authorizer_mod_role) { described_class.for(user: mod_user) } let(:user) { create(:user) } - let(:mod_user) { create(:user, :moderator) } + let(:mod_user) { create(:user, :super_moderator) } describe "#any_admin?" do it "queries the user's roles" do @@ -16,10 +16,10 @@ RSpec.describe Authorizer, type: :policy do end end - describe "#moderator?" do + describe "#super_moderator?" do it "queries the user's roles" do - expect(authorizer.moderator?).to be_falsey - expect(authorizer_mod_role.moderator?).to be_truthy + expect(authorizer.super_moderator?).to be_falsey + expect(authorizer_mod_role.super_moderator?).to be_truthy end end diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb index 3074d8e65..02562158a 100644 --- a/spec/policies/user_policy_spec.rb +++ b/spec/policies/user_policy_spec.rb @@ -52,7 +52,7 @@ RSpec.describe UserPolicy, type: :policy do end context "when the user is a moderator" do - let(:user) { build(:user, :moderator) } + let(:user) { build(:user, :super_moderator) } it { is_expected.to permit_actions(%i[moderation_routes]) } end diff --git a/spec/requests/api/v1/articles_spec.rb b/spec/requests/api/v1/articles_spec.rb index e1f25892f..e9f2c6160 100644 --- a/spec/requests/api/v1/articles_spec.rb +++ b/spec/requests/api/v1/articles_spec.rb @@ -1212,7 +1212,7 @@ RSpec.describe "Api::V1::Articles", type: :request do end context "when authorized as moderator" do - before { user.add_role(:moderator) } + before { user.add_role(:super_moderator) } it "unpublishes an article" do expect(published_article.published).to be true diff --git a/spec/services/moderator/manage_activity_and_roles_spec.rb b/spec/services/moderator/manage_activity_and_roles_spec.rb index 86093e848..2f3e06997 100644 --- a/spec/services/moderator/manage_activity_and_roles_spec.rb +++ b/spec/services/moderator/manage_activity_and_roles_spec.rb @@ -120,12 +120,12 @@ RSpec.describe Moderator::ManageActivityAndRoles, type: :service do end.to raise_error(StandardError) end - it "updates user to moderator" do + it "updates user to super moderator" do expect do described_class.handle_user_roles( admin: admin, user: user, - user_params: { note_for_current_role: "Upgrading to moderator", user_status: "Moderator" }, + user_params: { note_for_current_role: "Upgrading to super_moderator", user_status: "Super Moderator" }, ) end.to raise_error(StandardError) end diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index 64b3e8434..2024f7b11 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -200,7 +200,7 @@ seeder.create_if_doesnt_exist(User, "email", "moderator-user@forem.local") do user.profile.update(website_url: Faker::Internet.url) - user.add_role(:moderator) + user.add_role(:super_moderator) user.add_role(:trusted) end