diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index f8ccbdff587..e4d9ac8f68c 100644 --- a/app/controllers/api/base_controller.rb +++ b/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 diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 45b147ee849..772a12ca54e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -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 @@ -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 diff --git a/app/controllers/concerns/foreman/controller/users_mixin.rb b/app/controllers/concerns/foreman/controller/users_mixin.rb index 0045acad048..c205a745822 100644 --- a/app/controllers/concerns/foreman/controller/users_mixin.rb +++ b/app/controllers/concerns/foreman/controller/users_mixin.rb @@ -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 diff --git a/app/lib/net/dhcp/record.rb b/app/lib/net/dhcp/record.rb index 431adcf982b..6c613a369c2 100644 --- a/app/lib/net/dhcp/record.rb +++ b/app/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) diff --git a/app/models/compute_resources/foreman/model/ovirt.rb b/app/models/compute_resources/foreman/model/ovirt.rb index c6fb3db9f59..c344b75157d 100644 --- a/app/models/compute_resources/foreman/model/ovirt.rb +++ b/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 diff --git a/app/models/concerns/foreman/sti.rb b/app/models/concerns/foreman/sti.rb index 94560866813..1ef85bf61ad 100644 --- a/app/models/concerns/foreman/sti.rb +++ b/app/models/concerns/foreman/sti.rb @@ -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 diff --git a/app/models/concerns/orchestration.rb b/app/models/concerns/orchestration.rb index f7fb14630b1..e4028934ec9 100644 --- a/app/models/concerns/orchestration.rb +++ b/app/models/concerns/orchestration.rb @@ -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 diff --git a/app/models/concerns/orchestration/dhcp.rb b/app/models/concerns/orchestration/dhcp.rb index a7899c1089c..579cd36ce0a 100644 --- a/app/models/concerns/orchestration/dhcp.rb +++ b/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 diff --git a/app/models/concerns/strip_leading_and_trailing_dot.rb b/app/models/concerns/strip_leading_and_trailing_dot.rb index fa1c8492182..4202b17e1c1 100644 --- a/app/models/concerns/strip_leading_and_trailing_dot.rb +++ b/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 diff --git a/app/models/template.rb b/app/models/template.rb index 67b61c51e3d..f64a1542120 100644 --- a/app/models/template.rb +++ b/app/models/template.rb @@ -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 diff --git a/app/registries/menu/item.rb b/app/registries/menu/item.rb index adfc7756e2a..7067ce73d7a 100644 --- a/app/registries/menu/item.rb +++ b/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 diff --git a/app/services/ui_notifications/rss_notifications_checker.rb b/app/services/ui_notifications/rss_notifications_checker.rb index a804525adc6..a6ab85c966f 100644 --- a/app/services/ui_notifications/rss_notifications_checker.rb +++ b/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,