Skip to content

Commit

Permalink
Fixes #37431 - Fix Style/SoleNestedConditional cop
Browse files Browse the repository at this point in the history
  • Loading branch information
archanaserver authored and ehelms committed Oct 16, 2024
1 parent 936c410 commit b1cc7eb
Show file tree
Hide file tree
Showing 12 changed files with 46 additions and 69 deletions.
8 changes: 3 additions & 5 deletions app/controllers/api/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,9 @@ def add_info_headers
def setup_search_options
params[:search] ||= ""
params.each do |param, value|
if param =~ /(\w+)_id$/
if value.present?
query = " #{Regexp.last_match(1)} = #{value}"
params[:search] += query unless params[:search].include? query
end
if param =~ /(\w+)_id$/ && value.present?
query = " #{Regexp.last_match(1)} = #{value}"
params[:search] += query unless params[:search].include? query
end
end
end
Expand Down
32 changes: 15 additions & 17 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,16 @@ def controller_permission

def action_permission
case params[:action]
when 'new', 'create'
'create'
when 'edit', 'update'
'edit'
when 'destroy'
'destroy'
when 'index', 'show'
'view'
else
raise ::Foreman::Exception.new(N_("unknown permission for %s"), "#{params[:controller]}##{params[:action]}")
when 'new', 'create'
'create'
when 'edit', 'update'
'edit'
when 'destroy'
'destroy'
when 'index', 'show'
'view'
else
raise ::Foreman::Exception.new(N_("unknown permission for %s"), "#{params[:controller]}##{params[:action]}")
end
end

Expand All @@ -196,13 +196,11 @@ def setup_search_options
@original_search_parameter = params[:search]
params[:search] ||= ""
params.each do |param, value|
if param =~ /(\w+)_id$/
if value.present?
query = "#{Regexp.last_match(1)} = #{value}"
unless params[:search].include? query
params[:search] += ' and ' if params[:search].present?
params[:search] += query
end
if param =~ /(\w+)_id$/ && value.present?
query = "#{Regexp.last_match(1)} = #{value}"
unless params[:search].include? query
params[:search] += ' and ' if params[:search].present?
params[:search] += query
end
end
end
Expand Down
5 changes: 3 additions & 2 deletions app/controllers/concerns/foreman/controller/users_mixin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ def resource_scope(...)
protected

def clear_session_locale_on_update
if params[:user] && editing_self?
user_params = params[:user]
if user_params && editing_self? && user_params[:locale].blank?
# Remove locale from the session when set to "Browser Locale" and editing self
session.delete(:locale) if params[:user][:locale].try(:empty?)
session.delete(:locale)
end
end

Expand Down
4 changes: 1 addition & 3 deletions app/lib/net/dhcp/record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ def eql_for_conflicts?(other)
# If we're converting an 'ad-hoc' lease created by a host booting outside of Foreman's knowledge,
# then :hostname will be blank on the incoming lease - if the ip/mac still match, then this
# isn't a conflict. Only applicable on legacy proxy API without "type" attribute.
if legacy_dhcp_api?
to_compare << :hostname if other.attrs[:hostname].present? && attrs[:hostname].present?
end
to_compare << :hostname if legacy_dhcp_api? && other.attrs[:hostname].present? && attrs[:hostname].present?

logger.debug "Comparing #{attrs.values_at(*to_compare)} == #{other.attrs.values_at(*to_compare)}"
attrs.values_at(*to_compare) == other.attrs.values_at(*to_compare)
Expand Down
6 changes: 2 additions & 4 deletions app/models/compute_resources/foreman/model/ovirt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -619,10 +619,8 @@ def create_interfaces(vm, attrs, cluster_id)
raise Foreman::Exception.new("Interface network or vnic profile are missing.") if (interface[:network].nil? && interface[:vnic_profile].nil?)
interface[:network] = get_ovirt_id(cluster_networks, 'network', interface[:network]) if interface[:network].present?
interface[:vnic_profile] = get_ovirt_id(profiles, 'vnic profile', interface[:vnic_profile]) if interface[:vnic_profile].present?
if (interface[:network].present? && interface[:vnic_profile].present?)
unless (profiles.select { |profile| profile.network.id == interface[:network] }).present?
raise Foreman::Exception.new("Vnic Profile have a different network")
end
if interface[:network].present? && interface[:vnic_profile].present? && profiles.none? { |profile| profile.network.id == interface[:network] }
raise Foreman::Exception.new("Vnic Profile have a different network")
end
vm.add_interface(interface)
end
Expand Down
8 changes: 3 additions & 5 deletions app/models/concerns/foreman/sti.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ module STI
class_methods do
# ensures that the correct STI object is created when :type is passed.
def new(attributes = nil, &block)
if attributes.is_a?(Hash) && (type = attributes.with_indifferent_access.delete(:type)) && !type.empty?
if (klass = type.constantize) != self
raise "Invalid type #{type}" unless klass <= self
return klass.new(attributes, &block)
end
if attributes.is_a?(Hash) && (type = attributes.with_indifferent_access.delete(:type)).present? && (klass = type.constantize) != self
raise "Invalid type #{type}" unless klass <= self
return klass.new(attributes, &block)
end

super
Expand Down
20 changes: 8 additions & 12 deletions app/models/concerns/orchestration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,19 +167,15 @@ def process(queue_name)

rollback
ensure
unless q.nil?
if processed > 0
logger.info("Processed #{processed} tasks from queue '#{q.name}', completed #{q.completed.count}/#{q.all.count}") unless q.empty?
# rubocop:disable Rails/FindEach
q.all.each do |task|
msg = "Task '#{task.name}' *#{task.status}*"
if task.status?(:completed) || task.status?(:pending)
logger.debug msg
else
logger.error msg
end
if !q.nil? && (processed > 0)
logger.info("Processed #{processed} tasks from queue '#{q.name}', completed #{q.completed.count}/#{q.all.count}") unless q.empty?
q.all.each do |task| # rubocop:disable Rails/FindEach
msg = "Task '#{task.name}' *#{task.status}*"
if task.status?(:completed) || task.status?(:pending)
logger.debug msg
else
logger.error msg
end
# rubocop:enable Rails/FindEach
end
end
end
Expand Down
8 changes: 3 additions & 5 deletions app/models/concerns/orchestration/dhcp.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,11 +173,9 @@ def dhcp_update_required?
(!old.build? && build? && !all_dhcp_records_valid?))
# Handle jumpstart
# TODO, abstract this way once interfaces are fully used
if is_a?(Host::Base) && jumpstart?
if !old.build? || (old.medium != medium || old.arch != arch) ||
(os && old.os && (old.os.name != os.name || old.os != os))
return true
end
if is_a?(Host::Base) && jumpstart? && (!old.build? || (old.medium != medium || old.arch != arch) ||
(os && old.os && (old.os.name != os.name || old.os != os)))
return true
end
false
end
Expand Down
4 changes: 2 additions & 2 deletions app/models/concerns/strip_leading_and_trailing_dot.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ module StripLeadingAndTrailingDot
def strip_dots
changes.each do |column, values|
# return string if RuntimeError: can't modify frozen String
if values.last.is_a?(String) && dot_strip_attrs.include?(column)
send("#{column}=", values.last.gsub(/(^\.|\.$)/, '')) if respond_to?("#{column}=")
if values.last.is_a?(String) && dot_strip_attrs.include?(column) && respond_to?("#{column}=")
send("#{column}=", values.last.gsub(/(^\.|\.$)/, ''))
end
end
end
Expand Down
12 changes: 4 additions & 8 deletions app/models/template.rb
Original file line number Diff line number Diff line change
Expand Up @@ -300,16 +300,12 @@ def template_changes
actual_changes = changes

# Locked & Default are Special
if actual_changes.include?('locked') && !modify_locked
if User.current.nil? || !User.current.can?("lock_#{self.class.to_s.underscore.pluralize}", self)
errors.add(:base, _("You are not authorized to lock templates."))
end
if actual_changes.include?('locked') && !modify_locked && (User.current.nil? || !User.current.can?("lock_#{self.class.to_s.underscore.pluralize}", self))
errors.add(:base, _("You are not authorized to lock templates."))
end

if actual_changes.include?('default') && !modify_default
if User.current.nil? || !(User.current.can?(:create_organizations) || User.current.can?(:create_locations))
errors.add(:base, _("You are not authorized to make a template default."))
end
if actual_changes.include?('default') && !modify_default && (User.current.nil? || !(User.current.can?(:create_organizations) || User.current.can?(:create_locations)))
errors.add(:base, _("You are not authorized to make a template default."))
end

# API request can be changing the locked content (not allowed_changes) but the locked attribute at the same
Expand Down
4 changes: 1 addition & 3 deletions app/registries/menu/item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ def initialize(name, options)
end

def to_hash
if @condition.present?
return unless @condition.call
end
return if @condition.present? && !@condition.call
{type: :item, exact: @exact, html_options: @html_options, name: @caption || @name, url: url} if authorized?
end

Expand Down
4 changes: 1 addition & 3 deletions app/services/ui_notifications/rss_notifications_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ def deliver!
feed.items[0, @latest_posts].each do |feed_item|
item = Item.new(feed_item)
blueprint = rss_notification_blueprint
if notification_already_exists?(item)
next unless @force_repost
end
next if notification_already_exists?(item) && !@force_repost
Notification.create(
:initiator => User.anonymous_admin,
:audience => @audience,
Expand Down

0 comments on commit b1cc7eb

Please sign in to comment.