Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update RuboCop support; use Bixby style guide #2655

Merged
merged 3 commits into from
Feb 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 9 additions & 49 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,38 +1,22 @@
require: rubocop-rspec
inherit_gem:
bixby: bixby_default.yml

inherit_from: .rubocop_fixme.yml

AllCops:
TargetRubyVersion: 2.1
DisplayCopNames: true
Include:
- '**/Rakefile'
- '**/config.ru'
Exclude:
- 'db/**/*'
- 'script/**/*'
- 'spec/test_app_templates/**/*'
- 'vendor/**/*'
- 'lib/hyrax/specs/**/*'

Bundler/DuplicatedGem: # This doesn't work with engine_cart
Enabled: false

Layout/IndentationConsistency:
EnforcedStyle: rails

Lint/ImplicitStringConcatenation:
Exclude:
- 'lib/generators/hyrax/**/*'

Lint/AmbiguousBlockAssociation:
Enabled: false

Metrics/LineLength:
Max: 200

Metrics/AbcSize:
Max: 26

Metrics/BlockLength:
ExcludedMethods: ['included']
Exclude:
Expand All @@ -47,12 +31,9 @@ Metrics/BlockLength:
- 'lib/tasks/*.rake'
- 'spec/**/*.rb'

Metrics/MethodLength:
Max: 13

Naming/FileName: # https://github.com/bbatsov/rubocop/issues/2973
Exclude:
- 'Gemfile'
# Naming/FileName: # https://github.com/bbatsov/rubocop/issues/2973
# Exclude:
# - 'Gemfile'

Style/AsciiComments:
Enabled: false
Expand All @@ -76,31 +57,16 @@ Style/NumericPredicate:
Style/SymbolArray:
Enabled: false

Style/WordArray:
Enabled: false

Style/RegexpLiteral:
Enabled: false

Style/StringLiterals:
Enabled: false

Style/ClassAndModuleChildren:
Enabled: false

Style/Documentation:
Enabled: false

Style/SingleLineBlockParams:
Enabled: false

Style/ZeroLengthPredicate:
Exclude:
- 'app/controllers/hyrax/file_sets_controller.rb'

Rails:
Enabled: true

Rails/ApplicationJob:
Enabled: false

Expand Down Expand Up @@ -141,12 +107,9 @@ RSpec/DescribeClass:
- 'spec/tasks/rake_spec.rb'
- 'spec/views/**/*'

RSpec/ContextWording:
Enabled: false

# By default RSpec/MessageSpies has the following:
# Prefer have_received for setting message expectations. Setup form as a spy using allow or instance_spy.
# The default assumes EnforcedStyle is 'have_received'. Most of our specs are 'receive'
# # By default RSpec/MessageSpies has the following:
# # Prefer have_received for setting message expectations. Setup form as a spy using allow or instance_spy.
# # The default assumes EnforcedStyle is 'have_received'. Most of our specs are 'receive'
RSpec/MessageSpies:
Enabled: false

Expand All @@ -163,9 +126,6 @@ RSpec/LetSetup:
RSpec/MessageExpectation:
Enabled: false

RSpec/MultipleExpectations:
Enabled: false

RSpec/NestedGroups:
Enabled: false

Expand Down
6 changes: 3 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ group :development, :test do
gem "simplecov", require: false
end

# rubocop:disable Bundler/DuplicatedGem
# BEGIN ENGINE_CART BLOCK
# engine_cart: 1.1.0
# engine_cart stanza: 0.10.0
Expand Down Expand Up @@ -44,7 +45,6 @@ else
end
end
# END ENGINE_CART BLOCK
# rubocop:enable Bundler/DuplicatedGem

unless File.exist?(file)
eval_gemfile File.expand_path('spec/test_app_templates/Gemfile.extra', File.dirname(__FILE__))
end
eval_gemfile File.expand_path('spec/test_app_templates/Gemfile.extra', File.dirname(__FILE__)) unless File.exist?(file)
1 change: 0 additions & 1 deletion app/actors/hyrax/actors/file_set_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,6 @@ def assign_visibility?(file_set_params = {})
# Must clear the fileset from the thumbnail_id, representative_id and rendering_ids fields on the work
# and force it to be re-solrized.
# Although ActiveFedora clears the children nodes it leaves those fields in Solr populated.
# rubocop:disable Metrics/AbcSize
# rubocop:disable Metrics/CyclomaticComplexity
def unlink_from_work
work = file_set.parent
Expand Down
4 changes: 1 addition & 3 deletions app/actors/hyrax/actors/interpret_visibility_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,7 @@ def validate(env, intention, attributes)

def apply_visibility(env, intention)
result = apply_lease(env, intention) && apply_embargo(env, intention)
if env.attributes[:visibility]
env.curation_concern.visibility = env.attributes[:visibility]
end
env.curation_concern.visibility = env.attributes[:visibility] if env.attributes[:visibility]
result
end

Expand Down
4 changes: 4 additions & 0 deletions app/actors/hyrax/actors/model_actor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ module Hyrax
module Actors
# This is a proxy for the model specific actor
class ModelActor < AbstractActor
# See: https://github.com/bbatsov/rubocop/issues/5393
# rubocop:disable Rails/Delegate

# @param [Hyrax::Actors::Environment] env
# @return [Boolean] true if update was successful
def update(env)
Expand All @@ -19,6 +22,7 @@ def create(env)
def destroy(env)
model_actor(env).destroy(env)
end
# rubocop:enable Rails/Delegate

private

Expand Down
8 changes: 3 additions & 5 deletions app/controllers/concerns/hyrax/works_controller_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,10 @@ def presenter
end

def parent_presenter
return @parent_presenter unless params[:parent_id]

@parent_presenter ||=
begin
if params[:parent_id]
@parent_presenter ||= show_presenter.new(search_result_document(id: params[:parent_id]), current_ability, request)
end
end
show_presenter.new(search_result_document(id: params[:parent_id]), current_ability, request)
end

# Include 'hyrax/base' in the search path for views, while prefering
Expand Down
8 changes: 2 additions & 6 deletions app/controllers/hyrax/api/items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,9 @@ def show
def my_load_and_authorize_resource
@work = Hyrax::WorkRelation.new.find(params[:id])

unless user.can? :edit, @work
return render plain: "#{user} lacks access to #{@work}", status: :unauthorized
end
return render plain: "#{user} lacks access to #{@work}", status: :unauthorized unless user.can? :edit, @work

if @work.arkivo_checksum.nil?
return render plain: "Forbidden: #{@work} not deposited via Arkivo", status: :forbidden
end
return render plain: "Forbidden: #{@work} not deposited via Arkivo", status: :forbidden if @work.arkivo_checksum.nil?
rescue ActiveFedora::ObjectNotFoundError
return render plain: "id '#{params[:id]}' not found", status: :not_found
end
Expand Down
4 changes: 1 addition & 3 deletions app/controllers/hyrax/transfers_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ def index
# any existing edit permissions on the work.
def accept
@proxy_deposit_request.transfer!(params[:reset])
if params[:sticky]
current_user.can_receive_deposits_from << @proxy_deposit_request.sending_user
end
current_user.can_receive_deposits_from << @proxy_deposit_request.sending_user if params[:sticky]
redirect_to hyrax.transfers_path, notice: "Transfer complete"
end

Expand Down
4 changes: 1 addition & 3 deletions app/controllers/hyrax/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ def show
def search(query)
clause = query.blank? ? nil : "%" + query.downcase + "%"
base = ::User.where(*base_query)
if clause.present?
base = base.where("#{Hydra.config.user_key_field} like lower(?) OR display_name like lower(?)", clause, clause)
end
base = base.where("#{Hydra.config.user_key_field} like lower(?) OR display_name like lower(?)", clause, clause) if clause.present?
base.registered
.where("#{Hydra.config.user_key_field} not in (?)",
[::User.batch_user_key, ::User.audit_user_key])
Expand Down
5 changes: 1 addition & 4 deletions app/forms/hyrax/forms/permission_template_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ def initialize(model)
# @return [Hash{Symbol => String, Boolean}] { :content_tab (for confirmation message),
# :updated (true/false),
# :error_code (for flash error lookup) }
# rubocop:disable Metrics/MethodLength
def update(attributes)
@attributes = attributes
return_info = { content_tab: tab_to_update }
Expand Down Expand Up @@ -219,9 +218,7 @@ def managers_updated?
def grants_as_collection
return [] unless attributes[:access_grants_attributes]
attributes_collection = attributes[:access_grants_attributes]
if attributes_collection.respond_to?(:permitted?)
attributes_collection = attributes_collection.to_h
end
attributes_collection = attributes_collection.to_h if attributes_collection.respond_to?(:permitted?)
if attributes_collection.is_a? Hash
attributes_collection = attributes_collection
.sort_by { |i, _| i.to_i }
Expand Down
4 changes: 1 addition & 3 deletions app/forms/hyrax/forms/work_form.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,7 @@ def collections_for_select
# are going into a mediated deposit workflow.
def self.sanitize_params(form_params)
admin_set_id = form_params[:admin_set_id]
if admin_set_id && workflow_for(admin_set_id: admin_set_id).allows_access_grant?
return super
end
return super if admin_set_id && workflow_for(admin_set_id: admin_set_id).allows_access_grant?
params_without_permissions = permitted_params.reject { |arg| arg.respond_to?(:key?) && arg.key?(:permissions_attributes) }
form_params.permit(*params_without_permissions)
end
Expand Down
4 changes: 1 addition & 3 deletions app/helpers/hyrax/citations_behaviors/common_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@ def persistent_url(work)
end

def clean_end_punctuation(text)
if text && ([".", ",", ":", ";", "/"].include? text[-1, 1])
return text[0, text.length - 1]
end
return text[0, text.length - 1] if text && ([".", ",", ":", ";", "/"].include? text[-1, 1])
text
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,13 @@ module Formatters
class ChicagoFormatter < BaseFormatter
include Hyrax::CitationsBehaviors::PublicationBehavior
include Hyrax::CitationsBehaviors::TitleBehavior

# rubocop:disable Metrics/MethodLength
def format(work)
text = ""

# setup formatted author list
authors_list = all_authors(work)
text << format_authors(authors_list)
if text.present?
text = "<span class=\"citation-author\">#{text}</span>"
end
text = "<span class=\"citation-author\">#{text}</span>" if text.present?
# Get Pub Date
pub_date = setup_pub_date(work)
text << " #{pub_date}." unless pub_date.nil?
Expand Down
1 change: 0 additions & 1 deletion app/helpers/hyrax/file_set_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def media_display(presenter, locals = {})
locals.merge(file_set: presenter)
end

# rubocop:disable Metrics/MethodLength
def media_display_partial(file_set)
'hyrax/file_sets/media_display/' +
if file_set.image?
Expand Down
1 change: 0 additions & 1 deletion app/helpers/hyrax/hyrax_helper_behavior.rb
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ def count_classes_for(unread_count)
end
end

# rubocop:disable Metrics/MethodLength
def search_action_for_dashboard
case params[:controller]
when "hyrax/my/collections"
Expand Down
1 change: 0 additions & 1 deletion app/helpers/hyrax/trophy_helper.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
module Hyrax
module TrophyHelper
# rubocop:disable Metrics/MethodLength
def display_trophy_link(user, id, args = {}, &_block)
return unless user
trophy = user.trophies.where(work_id: id).exists?
Expand Down
4 changes: 1 addition & 3 deletions app/inputs/multi_value_select_input.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,7 @@ def build_field_options(value)
def build_field(value, index)
render_options = select_options
html_options = build_field_options(value)
if options[:item_helper]
(render_options, html_options) = options[:item_helper].call(value, index, render_options, html_options)
end
(render_options, html_options) = options[:item_helper].call(value, index, render_options, html_options) if options[:item_helper]
template.select_tag(attribute_name, template.options_for_select(render_options, value), html_options)
end
end
6 changes: 1 addition & 5 deletions app/models/admin_set.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ class AdminSet < ActiveFedora::Base
property :creator, predicate: ::RDF::Vocab::DC11.creator do |index|
index.as :symbol
end

# rubocop:disable Rails/HasManyOrHasOneDependent
has_many :members,
predicate: Hyrax.config.admin_set_predicate,
class_name: 'ActiveFedora::Base'
Expand All @@ -58,9 +56,7 @@ def default_set?

# Creates the default AdminSet and an associated PermissionTemplate with workflow
def self.find_or_create_default_admin_set_id
unless exists?(DEFAULT_ID)
Hyrax::AdminSetCreateService.create_default_admin_set(admin_set_id: DEFAULT_ID, title: DEFAULT_TITLE)
end
Hyrax::AdminSetCreateService.create_default_admin_set(admin_set_id: DEFAULT_ID, title: DEFAULT_TITLE) unless exists?(DEFAULT_ID)
DEFAULT_ID
end

Expand Down
4 changes: 1 addition & 3 deletions app/models/concerns/hyrax/ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,7 @@ def proxy_deposit_abilities
end
end

if Flipflop.proxy_deposit? && registered_user?
can :create, ProxyDepositRequest
end
can :create, ProxyDepositRequest if Flipflop.proxy_deposit? && registered_user?

can :accept, ProxyDepositRequest, receiving_user_id: current_user.id, status: 'pending'
can :reject, ProxyDepositRequest, receiving_user_id: current_user.id, status: 'pending'
Expand Down
6 changes: 1 addition & 5 deletions app/models/concerns/hyrax/suppressible.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ module Suppressible
# Override this method if you have some criteria by which records should not display in the search results.
def suppressed?
return false if state.nil?
# This is a clumsy check only needed under RDF < 2.0 where there
# is a bug with `AT::Resource#==`
if RDF::VERSION.to_s < '2.0'
return state.rdf_subject == Vocab::FedoraResourceStatus.inactive
end

state == Vocab::FedoraResourceStatus.inactive
end

Expand Down
4 changes: 1 addition & 3 deletions app/models/featured_work_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ class FeaturedWorkList

# @param [ActionController::Parameters] a collection of nested perameters
def featured_works_attributes=(attributes_collection)
if attributes_collection.respond_to?(:permitted?)
attributes_collection = attributes_collection.to_h
end
attributes_collection = attributes_collection.to_h if attributes_collection.respond_to?(:permitted?)
attributes_collection = attributes_collection.sort_by { |i, _| i.to_i }.map { |_, attributes| attributes } if attributes_collection.is_a? Hash
attributes_collection.each do |attributes|
raise "Missing id" if attributes['id'].blank?
Expand Down
4 changes: 1 addition & 3 deletions app/presenters/hyrax/work_show_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,7 @@ def manifest_helper
end

def featured?
if @featured.nil?
@featured = FeaturedWork.where(work_id: solr_document.id).exists?
end
@featured = FeaturedWork.where(work_id: solr_document.id).exists? if @featured.nil?
@featured
end

Expand Down
4 changes: 1 addition & 3 deletions app/services/hyrax/admin_set_create_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ def access_grants_attributes
{ agent_type: 'group', agent_id: admin_group_name, access: Hyrax::PermissionTemplateAccess::MANAGE }
].tap do |attribute_list|
# Grant manage access to the creating_user if it exists. Should exist for all but default Admin Set
if creating_user
attribute_list << { agent_type: 'user', agent_id: creating_user.user_key, access: Hyrax::PermissionTemplateAccess::MANAGE }
end
attribute_list << { agent_type: 'user', agent_id: creating_user.user_key, access: Hyrax::PermissionTemplateAccess::MANAGE } if creating_user
end
end

Expand Down
Loading