Skip to content

Commit

Permalink
🧹 Handle Rubocop Naming/PredicateName errors
Browse files Browse the repository at this point in the history
This check was originally disabled in order to get specs passing with
minimal code changes. Now that specs are running, the corrections can
safely be made to appease rubocop.
  • Loading branch information
laritakr committed Jan 4, 2024
1 parent 0405409 commit 6fed62b
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 32 deletions.
18 changes: 4 additions & 14 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/models/hyrax/group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 2 additions & 6 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,11 @@ 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

Expand Down
6 changes: 3 additions & 3 deletions app/services/group_aware_role_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions app/services/hyrax/workflow/permission_grantor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions spec/models/hyrax/group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ module Hyrax
end
end

describe '#has_site_role?' do
describe '#site_role?' do
subject(:group) { FactoryBot.build(:group) }

before do
Expand All @@ -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
Expand Down

0 comments on commit 6fed62b

Please sign in to comment.