🧹 Remove disabling of Rubocop Naming/PredicateName (#2126)

This cop was originally disabled in order to get specs to run during CI
without code changes. Now that specs are running, the changes can be
safely made to appease rubocop rather than disabling it.

app/controllers/application_controller.rb

@@ -29,35 +29,25 @@ class ApplicationController < ActionController::Base
 
   protected
 
-  # rubocop:disable Naming/PredicateName
-  def is_hidden
-    # rubocop:enable Naming/PredicateName
+  def hidden?
     current_account.persisted? && !current_account.is_public?
   end
 
-  # rubocop:disable Naming/PredicateName
-  def is_api_or_pdf
-    # rubocop:enable Naming/PredicateName
+  def api_or_pdf?
     request.format.to_s.match('json') ||
       params[:print] ||
       request.path.include?('api') ||
       request.path.include?('pdf')
   end
 
-  # rubocop:disable Naming/PredicateName
-  def is_staging
-    # rubocop:enable Naming/PredicateName
+  def staging?
     ['staging'].include?(Rails.env)
   end
 
-  ##
-  # Extra authentication for palni-palci during development phase
   def authenticate_if_needed
     # Disable this extra authentication in test mode
     return true if Rails.env.test?
-    # rubocop:disable Naming/PredicateName
-    return unless (is_hidden || is_staging) && !is_api_or_pdf
-    # rubocop:enable Naming/PredicateName
+    return unless (hidden? || staging?) && !api_or_pdf?
     authenticate_or_request_with_http_basic do |username, password|
       username == "samvera" && password == "hyku"
     end

app/controllers/proprietor/users_controller.rb

@@ -94,7 +94,7 @@ def user_params
 
       params.require(:user).permit(:email,
                                    :password,
-                                   :is_superadmin,
+                                   :superadmin,
                                    :facebook_handle,
                                    :twitter_handle,
                                    :googleplus_handle,

app/models/hyrax/group.rb

@@ -110,7 +110,7 @@ def description_label
       label
     end
 
-    def has_site_role?(role_name) # rubocop:disable Naming/PredicateName
+    def site_role?(role_name)
       site_roles = roles.select { |role| role.resource_type == 'Site' }
 
       site_roles.map(&:name).include?(role_name.to_s)

app/models/user.rb

@@ -46,22 +46,18 @@ def to_s
     email
   end
 
-  # rubocop:disable Naming/PredicateName
-  def is_admin
-    # rubocop:enable Naming/PredicateName
+  def admin?
     has_role?(:admin) || has_role?(:admin, Site.instance)
   end
 
-  # rubocop:disable Naming/PredicateName
-  def is_superadmin
-    # rubocop:enable Naming/PredicateName
+  def superadmin?
     has_role? :superadmin
   end
 
   # This comes from a checkbox in the proprietor interface
   # Rails checkboxes are often nil or "0" so we handle that
   # case directly
-  def is_superadmin=(value)
+  def superadmin=(value)
     value = ActiveModel::Type::Boolean.new.cast(value)
     if value
       add_role :superadmin

app/services/group_aware_role_checker.rb

@@ -6,20 +6,20 @@ module GroupAwareRoleChecker
   # their role checker methods are automatically defined
   RolesService::DEFAULT_ROLES.each do |role_name|
     define_method(:"#{role_name}?") do
-      has_group_aware_role?(role_name)
+      group_aware_role?(role_name)
     end
   end
 
   private
 
   # Check for the presence of the passed role_name in the User's Roles and
   # the User's Hyrax::Group's Roles.
-  def has_group_aware_role?(role_name) # rubocop:disable Naming/PredicateName
+  def group_aware_role?(role_name)
     return false if current_user.new_record?
     return true if current_user.has_role?(role_name, Site.instance)
 
     current_user.hyrax_groups.each do |group|
-      return true if group.has_site_role?(role_name)
+      return true if group.site_role?(role_name)
     end
 
     false

app/services/hyrax/workflow/permission_grantor.rb

@@ -48,7 +48,7 @@ def grant_all_workflow_roles_to_creating_user_and_admins!
         workflow_agents = [Hyrax::Group.find_by!(name: ::Ability.admin_group_name)]
         # The default admin set does not have a creating user
         workflow_agents << creating_user if creating_user
-        workflow_agents |= Hyrax::Group.select { |g| g.has_site_role?(:admin) }.tap do |agent_list|
+        workflow_agents |= Hyrax::Group.select { |g| g.site_role?(:admin) }.tap do |agent_list|
           ::User.find_each do |u|
             agent_list << u if u.has_role?(:admin, Site.instance)
           end
@@ -59,7 +59,7 @@ def grant_all_workflow_roles_to_creating_user_and_admins!
 
       def grant_workflow_roles_to_editors!
         editor_sipity_roles = [Hyrax::RoleRegistry::APPROVING, Hyrax::RoleRegistry::DEPOSITING]
-        workflow_agents = Hyrax::Group.select { |g| g.has_site_role?(:work_editor) }.tap do |agent_list|
+        workflow_agents = Hyrax::Group.select { |g| g.site_role?(:work_editor) }.tap do |agent_list|
           ::User.find_each do |u|
             agent_list << u if u.has_role?(:work_editor, Site.instance)
           end
@@ -70,7 +70,7 @@ def grant_workflow_roles_to_editors!
 
       def grant_workflow_roles_to_depositors!
         depositor_sipity_role = [Hyrax::RoleRegistry::DEPOSITING]
-        workflow_agents = Hyrax::Group.select { |g| g.has_site_role?(:work_depositor) }.tap do |agent_list|
+        workflow_agents = Hyrax::Group.select { |g| g.site_role?(:work_depositor) }.tap do |agent_list|
           ::User.find_each do |u|
             agent_list << u if u.has_role?(:work_depositor, Site.instance)
           end

app/views/proprietor/users/_form.html.erb

@@ -1,7 +1,7 @@
 <div class="form-inputs">
   <%= f.input :display_name, label: 'Display Name' %>
   <%= f.input :email, required: true, hint: '' %>
-  <%= f.input :is_superadmin, as: :boolean %>
+  <%= f.input :superadmin?, as: :boolean %>
   <%= f.input :password, required: f.object.new_record? ? true : false %>
   <%= f.input :facebook_handle %>
   <%= f.input :twitter_handle %>

app/views/proprietor/users/index.html.erb

@@ -32,7 +32,7 @@
                 <td><%= user.department %></td>
                 <td><%= user.title %></td>
                 <td><%= user.affiliation %></td>
-                <td><%= user.is_superadmin %></td>
+                <td><%= user.superadmin? %></td>
                 <td class="col-md-2">
                   <%= link_to t('.manage'), proprietor_user_path(user) %>&nbsp;|&nbsp;
                   <%= link_to t('helpers.action.edit'), edit_proprietor_user_path(user) %>&nbsp;|&nbsp;

config/locales/en.yml

@@ -278,6 +278,7 @@ en:
         manage: Manage
         title: Title
         affiliation: Affiliation
+        superadmin: SuperAdmin
         actions: Actions
         header: Manage Users
         confirm_delete: Are you sure you wish to delete this user?

config/routes.rb

@@ -13,7 +13,7 @@
   mount Hyrax::IiifAv::Engine, at: '/'
   mount Riiif::Engine => 'images', as: :riiif if Hyrax.config.iiif_image_server?
 
-  authenticate :user, ->(u) { u.is_superadmin || u.is_admin } do
+  authenticate :user, ->(u) { u.superadmin? || u.admin? } do
     queue = ENV.fetch('HYRAX_ACTIVE_JOB_QUEUE', 'sidekiq')
     case queue
     when 'sidekiq'

spec/models/hyrax/group_spec.rb

@@ -254,7 +254,7 @@ module Hyrax
         end
       end
 
-      describe '#has_site_role?' do
+      describe '#site_role?' do
         subject(:group) { FactoryBot.build(:group) }
 
         before do
@@ -265,27 +265,27 @@ module Hyrax
           let(:role) { FactoryBot.build(:role, name: 'non-site role', resource_type: 'non-site type') }
 
           it 'returns false' do
-            expect(group.has_site_role?('non-site role')).to eq(false)
+            expect(group.site_role?('non-site role')).to eq(false)
           end
         end
 
         context 'when group has a site role that matches' do
           let(:role) { FactoryBot.build(:role, name: 'my_role', resource_type: 'Site') }
 
           it 'returns true' do
-            expect(group.has_site_role?('my_role')).to eq(true)
+            expect(group.site_role?('my_role')).to eq(true)
           end
 
           it 'handles being passed a symbol' do
-            expect(group.has_site_role?(:my_role)).to eq(true)
+            expect(group.site_role?(:my_role)).to eq(true)
           end
         end
 
         context 'when group has a site role that does not matches' do
           let(:role) { FactoryBot.build(:role, name: 'my_role', resource_type: 'Site') }
 
           it 'returns false' do
-            expect(group.has_site_role?('your_role')).to eq(false)
+            expect(group.site_role?('your_role')).to eq(false)
           end
         end
       end