Merge pull request #485 from digineo/logical_refactoring

Small refactoring of logical expressions

app/models/avatar.rb

@@ -21,8 +21,7 @@ add an avatar based on auto detection (email address)
 
     # return if we run import mode
     return if Setting.get('import_mode')
-    return if !data[:url]
+    return if data[:url].blank?
-    return if data[:url].empty?
 
     Avatar.add(
       object: data[:object],
@@ -96,7 +95,7 @@ add avatar by url
 
     # check if avatar with url already exists
     avatar_already_exists = nil
-    if data[:source] && !data[:source].empty?
+    if data[:source].present?
       avatar_already_exists = Avatar.find_by(
         object_lookup_id: object_id,
         o_id: data[:o_id],
@@ -105,7 +104,7 @@ add avatar by url
     end
 
     # fetch image based on http url
-    if data[:url] && data[:url] =~ /^http/
+    if data[:url] =~ /^http/
 
       # check if source ist already updated within last 2 minutes
       if avatar_already_exists && avatar_already_exists.source_url == data[:url]

app/models/channel/driver/facebook.rb

@@ -82,8 +82,7 @@ class Channel::Driver::Facebook
     @sync['pages'].each { |page_to_sync_id, page_to_sync_params|
       page = get_page(page_to_sync_id)
       next if !page
-      next if !page_to_sync_params['group_id']
+      next if page_to_sync_params['group_id'].blank?
-      next if page_to_sync_params['group_id'].empty?
       page_client = Facebook.new(page['access_token'])
 
       posts = page_client.client.get_connection('me', 'feed', fields: 'id,from,to,message,created_time,permalink_url,comments{id,from,to,message,created_time}')

app/models/channel/driver/pop3.rb

@@ -51,7 +51,7 @@ returns
       ssl  = false
       port = 110
     end
-    if options.key?(:port) && !options[:port].empty?
+    if options.key?(:port) && options[:port].present?
       port = options[:port]
 
       # disable ssl for non ssl ports

app/models/channel/driver/smtp.rb

@@ -30,8 +30,8 @@ class Channel::Driver::Smtp
     if !options.key?(:port) || options[:port].empty?
       options[:port] = 25
     end
-    if !options.key?(:domain)
 
+    if !options.key?(:domain)
       # set fqdn, if local fqdn - use domain of sender
       fqdn = Setting.get('fqdn')
       if fqdn =~ /(localhost|\.local^|\.loc^)/i && (attr['from'] || attr[:from])
@@ -58,7 +58,7 @@ class Channel::Driver::Smtp
     }
 
     # add authentication only if needed
-    if options[:user] && !options[:user].empty?
+    if options[:user].present?
       smtp_params[:user_name] = options[:user]
       smtp_params[:password] = options[:password]
       smtp_params[:authentication] = options[:authentication]

app/models/channel/driver/twitter.rb

@@ -269,13 +269,10 @@ returns
   private
 
   def fetch_search
-    return if !@sync[:search]
+    return if @sync[:search].blank?
-    return if @sync[:search].empty?
     @sync[:search].each { |search|
-      next if !search[:term]
+      next if search[:term].blank?
-      next if search[:term].to_s.empty?
+      next if search[:group_id].blank?
-      next if !search[:group_id]
-      next if search[:group_id].to_s.empty?
       result_type = search[:type] || 'mixed'
       Rails.logger.debug " - searching for '#{search[:term]}'"
       older_import = 0
@@ -296,10 +293,8 @@ returns
   end
 
   def fetch_mentions
-    return if !@sync[:mentions]
+    return if @sync[:mentions].blank?
-    return if @sync[:mentions].empty?
+    return if @sync[:mentions][:group_id].blank?
-    return if !@sync[:mentions][:group_id]
-    return if @sync[:mentions][:group_id].to_s.empty?
     Rails.logger.debug ' - searching for mentions'
     older_import = 0
     older_import_max = 20
@@ -318,10 +313,8 @@ returns
   end
 
   def fetch_direct_messages
-    return if !@sync[:direct_messages]
+    return if @sync[:direct_messages].blank?
-    return if @sync[:direct_messages].empty?
+    return if @sync[:direct_messages][:group_id].blank?
-    return if !@sync[:direct_messages][:group_id]
-    return if @sync[:direct_messages][:group_id].to_s.empty?
     Rails.logger.debug ' - searching for direct_messages'
     older_import = 0
     older_import_max = 20

app/models/channel/email_parser.rb

@@ -74,9 +74,7 @@ class Channel::EmailParser
     mail = Mail.new(msg)
 
     # set all headers
-    mail.header.fields.each { |field|
+    mail.header.fields.select(&:name).each { |field|
-      next if !field.name
-
       # full line, encode, ready for storage
       data[field.name.to_s.downcase.to_sym] = Encode.conv('utf8', field.to_s)
 
@@ -318,12 +316,10 @@ class Channel::EmailParser
     end
 
     # for some broken sm mail clients (X-MimeOLE: Produced By Microsoft Exchange V6.5)
-    if !filename
+    filename ||= file.header[:content_location].to_s
-      filename = file.header[:content_location].to_s
-    end
 
     # generate file name
-    if !filename || filename.empty?
+    if filename.blank?
       attachment_count = 0
       (1..1000).each { |count|
         filename_exists = false

app/models/observer/ticket/article/communicate_facebook/background_job.rb

@@ -7,9 +7,7 @@ class Observer::Ticket::Article::CommunicateFacebook::BackgroundJob
     article = Ticket::Article.find(@article_id)
 
     # set retry count
-    if !article.preferences['delivery_retry']
+    article.preferences['delivery_retry'] ||= 0
-      article.preferences['delivery_retry'] = 0
-    end
     article.preferences['delivery_retry'] += 1
 
     ticket = Ticket.lookup(id: article.ticket_id)
@@ -24,7 +22,7 @@ class Observer::Ticket::Article::CommunicateFacebook::BackgroundJob
     end
 
     # fill in_reply_to
-    if !article.in_reply_to || article.in_reply_to.empty?
+    if article.in_reply_to.blank?
       article.in_reply_to = ticket.articles.first.message_id
     end
 

app/models/organization.rb

@@ -25,8 +25,7 @@ class Organization < ApplicationModel
   private
 
   def domain_cleanup
-    return if !domain
+    return if domain.blank?
-    return if domain.empty?
     domain.gsub!(/@/, '')
     domain.gsub!(/\s*/, '')
     domain.strip!

app/models/ticket/escalation.rb

@@ -163,7 +163,7 @@ returns
         }
       }
       config.hours = hours
-      if !hours || hours.empty?
+      if hours.blank?
         raise "No configured hours found in calendar #{calendar.inspect}"
       end
 

app/models/ticket/number/date.rb

@@ -73,7 +73,7 @@ module Ticket::Number::Date
   end
 
   def check(string)
-    return if !string || string.empty?
+    return if string.blank?
 
     # get config
     system_id           = Setting.get('system_id') || ''

app/models/ticket/number/increment.rb

@@ -70,7 +70,7 @@ module Ticket::Number::Increment
   end
 
   def check(string)
-    return if !string || string.empty?
+    return if string.blank?
 
     # get config
     system_id           = Setting.get('system_id') || ''

app/models/ticket/search_index.rb

@@ -21,7 +21,7 @@ returns
     # collect article data
     # add tags
     tags = Tag.tag_list(object: 'Ticket', o_id: id)
-    if tags && !tags.empty?
+    if tags.present?
       attributes[:tag] = tags
     end
 

app/models/user.rb

@@ -154,18 +154,7 @@ returns
 =end
 
   def role?(role_name)
-
+    roles.where(name: role_name).any?
-    result = false
-    roles.each { |role|
-      if role_name.class == Array
-        next if !role_name.include?(role.name)
-      elsif role.name != role_name
-        next
-      end
-      result = true
-      break
-    }
-    result
   end
 
 =begin
@@ -226,16 +215,13 @@ returns
   def self.authenticate(username, password)
 
     # do not authenticate with nothing
-    return if !username || username == ''
+    return if username.blank? || password.blank?
-    return if !password || password == ''
 
     # try to find user based on login
     user = User.find_by(login: username.downcase, active: true)
 
     # try second lookup with email
-    if !user
+    user ||= User.find_by(email: username.downcase, active: true)
-      user = User.find_by(email: username.downcase, active: true)
-    end
 
     # check failed logins
     max_login_failed = Setting.get('password_max_login_failed').to_i || 10
@@ -249,7 +235,7 @@ returns
     # set login failed +1
     if !user_auth && user
       sleep 1
-      user.login_failed = user.login_failed + 1
+      user.login_failed += 1
       user.save
     end
 
@@ -454,15 +440,13 @@ returns
 =end
 
   def self.password_reset_new_token(username)
-    return if !username || username == ''
+    return if username.blank?
 
     # try to find user based on login
     user = User.find_by(login: username.downcase, active: true)
 
     # try second lookup with email
-    if !user
+    user ||= User.find_by(email: username.downcase, active: true)
-      user = User.find_by(email: username.downcase, active: true)
-    end
 
     # check if email address exists
     return if !user
@@ -737,8 +721,7 @@ returns
   end
 
   def check_preferences_default
-    return if !@preferences_default
+    return if @preferences_default.blank?
-    return if @preferences_default.empty?
 
     preferences_tmp = @preferences_default.merge(preferences)
     self.preferences = preferences_tmp
@@ -871,8 +854,7 @@ returns
   end
 
   def avatar_for_email_check
-    return if !email
+    return if email.blank?
-    return if email.empty?
     return if email !~ /@/
     return if !changes['email'] && updated_at > Time.zone.now - 10.days
 
@@ -900,7 +882,7 @@ returns
   def check_password
 
     # set old password again if not given
-    if password == '' || !password
+    if password.blank?
 
       # get current record
       if id

app/models/user_device.rb

@@ -22,9 +22,7 @@ store new device for user if device not already known
   def self.add(user_agent, ip, user_id, fingerprint, type)
 
     # since gem browser 2 is not handling nil for user_agent, set it to ''
-    if user_agent.nil?
+    user_agent ||= ''
-      user_agent = ''
-    end
 
     # get location info
     location_details = Service::GeoIp.location(ip)
@@ -79,7 +77,7 @@ store new device for user if device not already known
       name = browser[:plattform]
     end
     if browser[:name] && browser[:name] != 'Other'
-      if name && !name.empty?
+      if name.present?
         name += ', '
       end
       name += browser[:name]