Fixes #37431 - Fix Style/SoleNestedConditional cop

app/controllers/api/base_controller.rb

@@ -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

app/controllers/application_controller.rb

@@ -166,16 +166,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
 
@@ -195,13 +195,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

app/controllers/concerns/foreman/controller/users_mixin.rb

@@ -13,7 +13,8 @@ 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?)
     end

app/models/compute_resources/foreman/model/ovirt.rb

@@ -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

app/models/concerns/foreman/sti.rb

@@ -9,11 +9,9 @@ class << base
     module ClassMethods
       # 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

app/models/concerns/orchestration.rb

@@ -170,19 +170,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

app/models/concerns/orchestration/dhcp.rb

@@ -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

app/models/concerns/strip_leading_and_trailing_dot.rb

@@ -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

app/models/template.rb

@@ -299,16 +299,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

app/registries/menu/item.rb

@@ -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
 

app/services/ui_notifications/rss_notifications_checker.rb

@@ -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,

lib/net/dhcp/record.rb

@@ -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)