Maintenance: Updated rubocop(-* gems) to latest version (0.92.0).

.rubocop/default.yml

@@ -206,6 +206,13 @@ Rails/HasAndBelongsToMany:
 #  StyleGuide: 'https://github.com/bbatsov/rails-style-guide#has-many-through'
   Enabled: false
 
+Rails/MatchRoute:
+  Description: >-
+                  Don't use `match` to define any routes unless there is a need to map multiple request types
+                  among [:get, :post, :patch, :put, :delete] to a single action using the `:via` option.
+  StyleGuide: 'https://rails.rubystyle.guide/#no-match-routes'
+  Enabled: false
+
 Rails/SkipsModelValidations:
   Description: >-
                  Use methods that skips model validations with caution.
@@ -259,6 +266,13 @@ Lint/InterpolationCheck:
     - "test/unit/ticket_trigger_test.rb"
     - "test/unit/ticket_trigger_recursive_disabled_test.rb"
 
+Style/StringConcatenation:
+  Description: 'Checks for places where string concatenation can be replaced with string interpolation.'
+  StyleGuide: '#string-interpolation'
+  Enabled: true
+  Exclude:
+    - "config/routes/**/*"
+
 # RSpec tests
 Style/NumericPredicate:
   Description: >-

.rubocop/todo.rspec.yml

@@ -628,12 +628,16 @@ RSpec/NestedGroups:
     - 'spec/lib/signature_detection_spec.rb'
     - 'spec/lib/stats/ticket_waiting_time_spec.rb'
     - 'spec/lib/twitter_sync_spec.rb'
+    - 'spec/models/application_model/can_lookup_examples.rb'
     - 'spec/models/calendar_spec.rb'
     - 'spec/models/channel/driver/twitter_spec.rb'
     - 'spec/models/channel/email_parser_spec.rb'
     - 'spec/models/channel/filter/match/email_regex_spec.rb'
     - 'spec/models/channel/filter/out_of_office_check_spec.rb'
     - 'spec/models/concerns/has_collection_update_examples.rb'
+    - 'spec/models/concerns/has_groups_examples.rb'
+    - 'spec/models/concerns/has_history_examples.rb'
+    - 'spec/models/concerns/has_roles_examples.rb'
     - 'spec/models/concerns/has_ticket_create_screen_impact_examples.rb'
     - 'spec/models/cti/caller_id_spec.rb'
     - 'spec/models/cti/log_spec.rb'
@@ -737,3 +741,8 @@ RSpec/VerifiedDoubles:
     - 'spec/lib/sequencer/unit/import/zendesk/ticket/comment/source_based_spec.rb'
     - 'spec/models/observer/ticket/article/communicate_twitter/background_job_spec.rb'
     - 'spec/models/ticket/number_spec.rb'
+
+RSpec/MultipleMemoizedHelpers:
+  Description: Checks if example groups contain too many `let` and `subject` calls.
+  Enabled: false
+  StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers

.rubocop/todo.yml

@@ -44,10 +44,12 @@ Metrics/AbcSize:
     - 'app/controllers/integration/sipgate_controller.rb'
     - 'app/controllers/integration/smime_controller.rb'
     - 'app/controllers/knowledge_base/answers_controller.rb'
+    - 'app/controllers/knowledge_base/categories_controller.rb'
     - 'app/controllers/knowledge_base/manage_controller.rb'
     - 'app/controllers/knowledge_base/public/answers_controller.rb'
     - 'app/controllers/knowledge_base/public/categories_controller.rb'
     - 'app/controllers/knowledge_base/search_controller.rb'
+    - 'app/controllers/knowledge_bases_controller.rb'
     - 'app/controllers/links_controller.rb'
     - 'app/controllers/long_polling_controller.rb'
     - 'app/controllers/monitoring_controller.rb'
@@ -234,12 +236,14 @@ Metrics/AbcSize:
     - 'db/migrate/20170113000001_object_manager_attribute_create_middle.rb'
     - 'db/migrate/20170113000002_slack_group_config_issue_587.rb'
     - 'db/migrate/20170116000001_add_ticket_time_accounting_373.rb'
+    - 'db/migrate/20170116000002_fixed_typos_622.rb'
     - 'db/migrate/20170207081400_ticket_state_priority_defaults.rb'
     - 'db/migrate/20170403000001_fixed_admin_user_permission_920.rb'
     - 'db/migrate/20170419000001_ldap_support.rb'
     - 'db/migrate/20170419000002_overview_role_ids.rb'
     - 'db/migrate/20170516000001_trigger_recipient_update.rb'
     - 'db/migrate/20170608151442_enhanced_permissions.rb'
+    - 'db/migrate/20170713000002_ticket_zoom_setting2.rb'
     - 'db/migrate/20170905140038_cti_log_preferences_migration.rb'
     - 'db/migrate/20170910000002_out_of_office2.rb'
     - 'db/migrate/20171023000001_fixed_store_upgrade_ror_45.rb'
@@ -266,6 +270,7 @@ Metrics/AbcSize:
     - 'lib/email_helper/verify.rb'
     - 'lib/enrichment/clearbit/user.rb'
     - 'lib/excel_sheet.rb'
+    - 'lib/excel_sheet/ticket.rb'
     - 'lib/external_credential/facebook.rb'
     - 'lib/external_credential/google.rb'
     - 'lib/external_credential/twitter.rb'
@@ -687,6 +692,7 @@ Metrics/PerceivedComplexity:
     - 'app/controllers/application_controller/handles_errors.rb'
     - 'app/controllers/application_controller/logs_http_access.rb'
     - 'app/controllers/channels_email_controller.rb'
+    - 'app/controllers/channels_google_controller.rb'
     - 'app/controllers/concerns/creates_ticket_articles.rb'
     - 'app/controllers/first_steps_controller.rb'
     - 'app/controllers/form_controller.rb'
@@ -694,17 +700,21 @@ Metrics/PerceivedComplexity:
     - 'app/controllers/import_otrs_controller.rb'
     - 'app/controllers/integration/check_mk_controller.rb'
     - 'app/controllers/integration/smime_controller.rb'
+    - 'app/controllers/knowledge_base/search_controller.rb'
     - 'app/controllers/long_polling_controller.rb'
     - 'app/controllers/monitoring_controller.rb'
     - 'app/controllers/object_manager_attributes_controller.rb'
     - 'app/controllers/organizations_controller.rb'
     - 'app/controllers/reports_controller.rb'
     - 'app/controllers/search_controller.rb'
+    - 'app/controllers/sessions/collection_base.rb'
+    - 'app/controllers/sessions/collection_ticket.rb'
     - 'app/controllers/sessions_controller.rb'
     - 'app/controllers/ticket_articles_controller.rb'
     - 'app/controllers/tickets_controller.rb'
     - 'app/controllers/time_accountings_controller.rb'
     - 'app/controllers/users_controller.rb'
+    - 'app/jobs/collection_update_job.rb'
     - 'app/jobs/ticket_article_communicate_email_job.rb'
     - 'app/models/activity_stream/assets.rb'
     - 'app/models/application_model/can_assets.rb'
@@ -719,6 +729,7 @@ Metrics/PerceivedComplexity:
     - 'app/models/calendar.rb'
     - 'app/models/channel.rb'
     - 'app/models/channel/assets.rb'
+    - 'app/models/channel/driver/facebook.rb'
     - 'app/models/channel/driver/imap.rb'
     - 'app/models/channel/driver/pop3.rb'
     - 'app/models/channel/driver/sms/twilio.rb'
@@ -741,6 +752,7 @@ Metrics/PerceivedComplexity:
     - 'app/models/concerns/can_clone_attachments.rb'
     - 'app/models/concerns/can_csv_import.rb'
     - 'app/models/concerns/has_history.rb'
+    - 'app/models/concerns/has_rich_text.rb'
     - 'app/models/concerns/has_search_index_backend.rb'
     - 'app/models/concerns/has_search_sortable.rb'
     - 'app/models/cti/caller_id.rb'
@@ -751,7 +763,9 @@ Metrics/PerceivedComplexity:
     - 'app/models/external_sync.rb'
     - 'app/models/history.rb'
     - 'app/models/job.rb'
+    - 'app/models/job/assets.rb'
     - 'app/models/karma/activity_log.rb'
+    - 'app/models/knowledge_base.rb'
     - 'app/models/object_manager/attribute.rb'
     - 'app/models/observer/sla/ticket_rebuild_escalation.rb'
     - 'app/models/observer/ticket/article/communicate_email.rb'
@@ -777,6 +791,7 @@ Metrics/PerceivedComplexity:
     - 'app/models/package.rb'
     - 'app/models/package/migration.rb'
     - 'app/models/recent_view/assets.rb'
+    - 'app/models/role.rb'
     - 'app/models/role/assets.rb'
     - 'app/models/scheduler.rb'
     - 'app/models/setting.rb'
@@ -798,6 +813,7 @@ Metrics/PerceivedComplexity:
     - 'app/models/transaction/signature_detection.rb'
     - 'app/models/transaction/slack.rb'
     - 'app/models/translation.rb'
+    - 'app/models/trigger/assets.rb'
     - 'app/models/user.rb'
     - 'app/models/user/assets.rb'
     - 'app/models/user/search.rb'
@@ -805,9 +821,15 @@ Metrics/PerceivedComplexity:
     - 'app/models/user_device.rb'
     - 'app/policies/ticket/article_policy.rb'
     - 'db/migrate/20170207081400_ticket_state_priority_defaults.rb'
+    - 'db/migrate/20170403000001_fixed_admin_user_permission_920.rb'
     - 'db/migrate/20170516000001_trigger_recipient_update.rb'
+    - 'db/migrate/20171023000001_fixed_store_upgrade_ror_45.rb'
+    - 'db/migrate/20180502015927_issue_1219_zhtw_locale_typo.rb'
+    - 'db/migrate/20180502015927_issue_1219_zhtw_locale_typo.rb'
     - 'db/migrate/20180806000001_fixed_twitter_ticket_article_preferences7.rb'
     - 'db/migrate/20181017000001_cti_generic_api2.rb'
+    - 'db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb'
+    - 'db/migrate/20190724000001_rename_reserved_words.rb'
     - 'lib/auto_wizard.rb'
     - 'lib/core_ext/string.rb'
     - 'lib/email_helper/probe.rb'
@@ -821,6 +843,7 @@ Metrics/PerceivedComplexity:
     - 'lib/html_sanitizer.rb'
     - 'lib/import/exchange/item_attributes.rb'
     - 'lib/import/otrs/user.rb'
+    - 'lib/ldap/user.rb'
     - 'lib/models.rb'
     - 'lib/notification_factory/mailer.rb'
     - 'lib/notification_factory/renderer.rb'
@@ -834,12 +857,14 @@ Metrics/PerceivedComplexity:
     - 'lib/secure_mailing/smime/incoming.rb'
     - 'lib/service/geo_ip/zammad.rb'
     - 'lib/sessions.rb'
+    - 'lib/sessions/backend/activity_stream.rb'
     - 'lib/sessions/backend/ticket_overview_list.rb'
     - 'lib/sessions/client.rb'
     - 'lib/sessions/event/broadcast.rb'
     - 'lib/sessions/event/chat_session_close.rb'
     - 'lib/sessions/event/chat_session_update.rb'
     - 'lib/sessions/event/chat_status_customer.rb'
+    - 'lib/signature_detection.rb'
     - 'lib/stats.rb'
     - 'lib/stats/ticket_channel_distribution.rb'
     - 'lib/stats/ticket_in_process.rb'
@@ -851,7 +876,10 @@ Metrics/PerceivedComplexity:
     - 'lib/telegram.rb'
     - 'lib/twitter_sync.rb'
     - 'lib/user_agent.rb'
+    - 'test/browser/admin_object_manager_test.rb'
+    - 'test/browser/keyboard_shortcuts_test.rb'
     - 'test/browser_test_helper.rb'
+    - 'test/integration/slack_test.rb'
 
 Rails/AssertNot:
   Exclude:
@@ -873,3 +901,43 @@ Rails/HasManyOrHasOneDependent:
     - 'app/models/signature.rb'
     - 'app/models/ticket/state_type.rb'
     - 'app/models/user.rb'
+
+Style/OptionalBooleanParameter:
+  Exclude:
+    - 'app/models/application_model/can_activity_stream_log.rb'
+    - 'app/models/application_model/can_cleanup_param.rb'
+    - 'app/models/avatar.rb'
+    - 'app/models/channel.rb'
+    - 'app/models/channel/driver/facebook.rb'
+    - 'app/models/channel/driver/sendmail.rb'
+    - 'app/models/channel/driver/sms/massenversand.rb'
+    - 'app/models/channel/driver/sms/twilio.rb'
+    - 'app/models/channel/driver/smtp.rb'
+    - 'app/models/channel/driver/telegram.rb'
+    - 'app/models/channel/driver/twitter.rb'
+    - 'app/models/channel/email_build.rb'
+    - 'app/models/channel/email_parser.rb'
+    - 'app/models/chat/session.rb'
+    - 'app/models/concerns/has_history.rb'
+    - 'app/models/job.rb'
+    - 'app/models/karma/activity_log.rb'
+    - 'app/models/object_manager/attribute.rb'
+    - 'app/models/package.rb'
+    - 'app/models/scheduler.rb'
+    - 'app/models/setting.rb'
+    - 'app/models/store/file.rb'
+    - 'app/models/text_module.rb'
+    - 'app/models/ticket/escalation.rb'
+    - 'app/models/translation.rb'
+    - 'app/models/user.rb'
+    - 'lib/app_version.rb'
+    - 'lib/core_ext/string.rb'
+    - 'lib/external_credential/facebook.rb'
+    - 'lib/external_credential/google.rb'
+    - 'lib/external_credential/twitter.rb'
+    - 'lib/html_sanitizer.rb'
+    - 'lib/models.rb'
+    - 'lib/sessions/backend/ticket_overview_list.rb'
+    - 'lib/sessions/node.rb'
+    - 'spec/support/system_init_done.rb'
+    - 'test/browser_test_helper.rb'

Gemfile.lock

@@ -150,7 +150,7 @@ GEM
       json
     composite_primary_keys (11.2.0)
       activerecord (~> 5.2.1)
-    concurrent-ruby (1.1.6)
+    concurrent-ruby (1.1.7)
     coveralls (0.8.23)
       json (>= 1.8, < 3)
       simplecov (~> 0.16.1)
@@ -248,7 +248,7 @@ GEM
     http-form_data (2.1.1)
     http_parser.rb (0.6.0)
     httpclient (2.8.3)
-    i18n (1.8.3)
+    i18n (1.8.5)
       concurrent-ruby (~> 1.0)
     icalendar (2.5.3)
       ice_cube (~> 0.16)
@@ -291,7 +291,7 @@ GEM
     mini_portile2 (2.4.0)
     mini_racer (0.2.9)
       libv8 (>= 6.9.411)
-    minitest (5.14.1)
+    minitest (5.14.2)
     msgpack (1.2.4)
     multi_json (1.14.1)
     multi_xml (0.6.0)
@@ -358,7 +358,7 @@ GEM
       omniauth-oauth2 (>= 1.4.0)
     openssl (2.1.2)
     parallel (1.19.2)
-    parser (2.7.1.4)
+    parser (2.7.1.5)
       ast (~> 2.4.1)
     pg (0.21.0)
     pluginator (1.5.0)
@@ -427,7 +427,7 @@ GEM
     rb-inotify (0.10.0)
       ffi (~> 1.0)
     rchardet (1.8.0)
-    regexp_parser (1.7.1)
+    regexp_parser (1.8.0)
     rest-client (2.0.2)
       http-cookie (>= 1.0.2, < 2.0)
       mime-types (>= 1.16, < 4.0)
@@ -452,25 +452,26 @@ GEM
       rspec-support (~> 3.9.0)
     rspec-support (3.9.3)
     rszr (0.5.2)
-    rubocop (0.88.0)
+    rubocop (0.92.0)
       parallel (~> 1.10)
-      parser (>= 2.7.1.1)
+      parser (>= 2.7.1.5)
       rainbow (>= 2.2.2, < 4.0)
       regexp_parser (>= 1.7)
       rexml
-      rubocop-ast (>= 0.1.0, < 1.0)
+      rubocop-ast (>= 0.5.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 1.4.0, < 2.0)
-    rubocop-ast (0.1.0)
+    rubocop-ast (0.5.0)
-      parser (>= 2.7.0.1)
+      parser (>= 2.7.1.5)
-    rubocop-performance (1.7.0)
+    rubocop-performance (1.8.1)
-      rubocop (>= 0.82.0)
+      rubocop (>= 0.87.0)
-    rubocop-rails (2.6.0)
+      rubocop-ast (>= 0.4.0)
+    rubocop-rails (2.8.1)
       activesupport (>= 4.2.0)
       rack (>= 1.1)
-      rubocop (>= 0.82.0)
-    rubocop-rspec (1.42.0)
       rubocop (>= 0.87.0)
+    rubocop-rspec (1.43.2)
+      rubocop (~> 0.87)
     ruby-progressbar (1.10.1)
     ruby-saml (1.10.2)
       nokogiri (>= 1.5.10)

app/controllers/application_controller/handles_errors.rb

@@ -62,7 +62,7 @@ module ApplicationController::HandlesErrors
         @message = errors[:error_human] || errors[:error] || param[:message]
         @traceback = !Rails.env.production?
         file = File.open(Rails.root.join('public', "#{status_code}.html"), 'r')
-        render inline: file.read, status: status
+        render inline: file.read, status: status # rubocop:disable Rails/RenderInline
       end
     end
   end

app/controllers/application_controller/has_user.rb

@@ -84,7 +84,7 @@ module ApplicationController::HasUser
 
     # check if remote ip need to be updated
     if session[:user_id]
-      if !session[:remote_ip] || session[:remote_ip] != request.remote_ip
+      if !session[:remote_ip] || session[:remote_ip] != request.remote_ip # rubocop:disable Style/SoleNestedConditional
         session[:remote_ip] = request.remote_ip
         session[:geo]       = Service::GeoIp.location(request.remote_ip)
       end

app/controllers/application_controller/logs_http_access.rb

@@ -33,7 +33,7 @@ module ApplicationController::LogsHttpAccess
     end
     body = request.body.read
     if body
-      request_data[:content] += "\n" + body
+      request_data[:content] += "\n#{body}"
     end
     request_data[:content] = request_data[:content].slice(0, 8000)
 
@@ -50,7 +50,7 @@ module ApplicationController::LogsHttpAccess
     end
     body = response.body
     if body
-      response_data[:content] += "\n" + body
+      response_data[:content] += "\n#{body}"
     end
     response_data[:content] = response_data[:content].slice(0, 8000)
     record = {

app/controllers/channels_email_controller.rb

@@ -61,9 +61,7 @@ class ChannelsEmailController < ApplicationController
     )
 
     # verify if user+host already exists
-    if result[:result] == 'ok'
+    return if result[:result] == 'ok' && account_duplicate?(result)
-      return if account_duplicate?(result)
-    end
 
     render json: result
   end

app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb

@@ -10,9 +10,10 @@ module ChecksUserAttributesByCurrentUserPermission
     return true if current_user.permissions?('admin.user')
 
     # regular agents are not allowed to set Groups and Roles
+    suffixes = %w[_ids s]
     %w[Role Group].each do |model|
 
-      %w[_ids s].each do |suffix|
+      suffixes.each do |suffix|
         attribute = "#{model.downcase}#{suffix}"
         values    = params[attribute]
 

app/controllers/concerns/creates_ticket_articles.rb

@@ -76,17 +76,19 @@ module CreatesTicketArticles
 
     # add attachments as param
     if params[:attachments].present?
+      required_keys    = %w[mime-type filename data]
+      preferences_keys = %w[charset mime-type]
       params[:attachments].each_with_index do |attachment, index|
 
         # validation
-        %w[mime-type filename data].each do |key|
+        required_keys.each do |key|
           next if attachment[key]
 
           raise Exceptions::UnprocessableEntity, "Attachment needs '#{key}' param for attachment with index '#{index}'"
         end
 
         preferences = {}
-        %w[charset mime-type].each do |key|
+        preferences_keys.each do |key|
           next if !attachment[key]
 
           store_key = key.tr('-', '_').camelize.gsub(/(.+)([A-Z])/, '\1_\2').tr('_', '-')

app/controllers/form_controller.rb

@@ -61,7 +61,7 @@ class FormController < ApplicationController
         Rails.logger.info "Can't verify email #{params[:email]}: #{message}"
 
         # ignore 450, graylistings
-        errors['email'] = message if !message.include?('450')
+        errors['email'] = message if message.exclude?('450')
       end
     end
 

app/controllers/getting_started_controller.rb

@@ -36,9 +36,7 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password}
     return if auto_wizard_enabled_response
 
     # if master user already exists, we need to be authenticated
-    if setup_done
+    return if setup_done && !authentication_check
-      return if !authentication_check
-    end
 
     # return result
     render json: {

app/controllers/long_polling_controller.rb

@@ -104,7 +104,7 @@ class LongPollingController < ApplicationController
     return if !params[:client_id]
 
     sessions = Sessions.sessions
-    return if !sessions.include?(params[:client_id].to_s)
+    return if sessions.exclude?(params[:client_id].to_s)
 
     params[:client_id].to_s
   end

app/controllers/monitoring_controller.rb

@@ -35,12 +35,13 @@ curl http://localhost/api/v1/monitoring/health_check?token=XXX
 
     # channel check
     last_run_tolerance = Time.zone.now - 1.hour
+    options_keys = %w[host user uid]
     Channel.where(active: true).each do |channel|
 
       # inbound channel
       if channel.status_in == 'error'
         message = "Channel: #{channel.area} in "
-        %w[host user uid].each do |key|
+        options_keys.each do |key|
           next if channel.options[key].blank?
 
           message += "key:#{channel.options[key]};"
@@ -56,7 +57,7 @@ curl http://localhost/api/v1/monitoring/health_check?token=XXX
       next if channel.status_out != 'error'
 
       message = "Channel: #{channel.area} out "
-      %w[host user uid].each do |key|
+      options_keys.each do |key|
         next if channel.options[key].blank?
 
         message += "key:#{channel.options[key]};"
@@ -149,7 +150,7 @@ curl http://localhost/api/v1/monitoring/health_check?token=XXX
     end
 
     # stuck import jobs
-    import_backends.each do |backend|
+    import_backends.each do |backend| # rubocop:disable Style/CombinableLoops
 
       job = ImportJob.where(
         name:        backend,
@@ -218,7 +219,7 @@ curl http://localhost/api/v1/monitoring/status?token=XXX
 
   def status
     last_login = nil
-    last_login_user = User.where('last_login IS NOT NULL').order(last_login: :desc).limit(1).first
+    last_login_user = User.where.not(last_login: nil).order(last_login: :desc).limit(1).first
     if last_login_user
       last_login = last_login_user.last_login
     end

app/controllers/object_manager_attributes_controller.rb

@@ -76,23 +76,23 @@ class ObjectManagerAttributesController < ApplicationController
     @permitted_params ||= begin
       permitted = params.permit!.to_h
 
-      if permitted[:data_type].match?(/^(boolean)$/)
+      if permitted[:data_type].match?(/^(boolean)$/) && permitted[:data_option][:options]
-        if permitted[:data_option][:options]
+        # rubocop:disable Lint/BooleanSymbol
-          # rubocop:disable Lint/BooleanSymbol
+        if permitted[:data_option][:options][:false]
-          if permitted[:data_option][:options][:false]
+          permitted[:data_option][:options][false] = permitted[:data_option][:options].delete(:false)
-            permitted[:data_option][:options][false] = permitted[:data_option][:options].delete(:false)
-          end
-          if permitted[:data_option][:options][:true]
-            permitted[:data_option][:options][true] = permitted[:data_option][:options].delete(:true)
-          end
-          case permitted[:data_option][:default]
-          when 'true'
-            permitted[:data_option][:default] = true
-          when 'false'
-            permitted[:data_option][:default] = false
-          end
-          # rubocop:enable Lint/BooleanSymbol
         end
+
+        if permitted[:data_option][:options][:true]
+          permitted[:data_option][:options][true] = permitted[:data_option][:options].delete(:true)
+        end
+
+        case permitted[:data_option][:default]
+        when 'true'
+          permitted[:data_option][:default] = true
+        when 'false'
+          permitted[:data_option][:default] = false
+        end
+        # rubocop:enable Lint/BooleanSymbol
       end
 
       if permitted[:data_option]

app/controllers/search_controller.rb

@@ -80,7 +80,7 @@ class SearchController < ApplicationController
       objects_without_direct_search_index.each do |object|
         object_result = search_generic_backend(object.constantize, assets, generic_search_params)
         if object_result.present?
-          result = result.concat(object_result)
+          result.concat(object_result)
         end
       end
 
@@ -102,7 +102,7 @@ class SearchController < ApplicationController
       objects_in_order.each do |object|
         object_result = search_generic_backend(object, assets, generic_search_params)
         if object_result.present?
-          result = result.concat(object_result)
+          result.concat(object_result)
         end
       end
     end

app/controllers/sessions_controller.rb

@@ -19,12 +19,12 @@ class SessionsController < ApplicationController
     raise Exceptions::NotAuthorized, 'SSO authentication disabled!' if !Setting.get('auth_sso')
 
     user = begin
-              login = request.env['REMOTE_USER'] ||
+      login = request.env['REMOTE_USER'] ||
-                      request.env['HTTP_REMOTE_USER'] ||
+              request.env['HTTP_REMOTE_USER'] ||
-                      request.headers['X-Forwarded-User']
+              request.headers['X-Forwarded-User']
 
-              User.lookup(login: login&.downcase)
+      User.lookup(login: login&.downcase)
-            end
+    end
 
     raise Exceptions::NotAuthorized, 'Missing SSO ENV REMOTE_USER or X-Forwarded-User header' if login.blank?
     raise Exceptions::NotAuthorized, "No such user '#{login}' found!" if user.blank?

app/controllers/tickets_controller.rb

@@ -313,10 +313,10 @@ class TicketsController < ApplicationController
     ticket_lists = Ticket
                    .where(
                      customer_id: ticket.customer_id,
-                     state_id:    Ticket::State.by_category(:open).pluck(:id),
+                     state_id:    Ticket::State.by_category(:open).pluck(:id), # rubocop:disable Rails/PluckInWhere
                    )
                    .where(access_condition)
-                   .where('id != ?', [ ticket.id ])
+                   .where.not(id: [ ticket.id ])
                    .order(created_at: :desc)
                    .limit(6)
 
@@ -329,7 +329,7 @@ class TicketsController < ApplicationController
                        state_id: Ticket::State.by_category(:merged).pluck(:id),
                      )
                      .where(access_condition)
-                     .where('id != ?', [ ticket.id ])
+                     .where.not(id: [ ticket.id ])
                      .order(created_at: :desc)
                      .limit(6)
     end

app/controllers/users_controller.rb

@@ -794,7 +794,7 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content
         content:   file_resize[:content],
         mime_type: file_resize[:mime_type],
       },
-      source:    'upload ' + Time.zone.now.to_s,
+      source:    "upload #{Time.zone.now}",
       deletable: true,
     )
 

app/jobs/concerns/has_active_job_lock.rb

@@ -81,14 +81,12 @@ module HasActiveJobLock
 
   private
 
-  def in_active_job_lock_transaction
+  def in_active_job_lock_transaction(&block)
     # re-use active DB transaction if present
     return yield if ActiveRecord::Base.connection.open_transactions.nonzero?
 
     # start own serializable DB transaction to prevent race conditions on DB level
-    ActiveJobLock.transaction(isolation: :serializable) do
+    ActiveJobLock.transaction(isolation: :serializable, &block)
-      yield
-    end
   rescue ActiveRecord::SerializationFailure => e
     # PostgeSQL prevents locking on records that are already locked
     # for UPDATE in Serializable Isolation Level transactions,

app/jobs/ticket_user_ticket_counter_job.rb

@@ -32,7 +32,7 @@ class TicketUserTicketCounterJob < ApplicationJob
 
     needs_update = false
     ticket_count.each_key do |ticket_state_category|
-      preferences_key = ('tickets_' + ticket_state_category.to_s).to_sym
+      preferences_key = :"tickets_#{ticket_state_category}"
       next if customer[:preferences][preferences_key] == ticket_count[ticket_state_category]
 
       needs_update = true

app/models/application_model/can_associations.rb

@@ -35,7 +35,7 @@ returns
       assoc_name = assoc.name
       next if association_attributes_ignored.include?(assoc_name)
 
-      real_ids = assoc_name[0, assoc_name.length - 1] + '_ids'
+      real_ids = "#{assoc_name[0, assoc_name.length - 1]}_ids"
       real_ids = real_ids.to_sym
       next if !params.key?(real_ids)
 
@@ -64,10 +64,10 @@ returns
       assoc_name = assoc.name
       next if association_attributes_ignored.include?(assoc_name)
 
-      real_ids = assoc_name[0, assoc_name.length - 1] + '_ids'
+      real_ids = "#{assoc_name[0, assoc_name.length - 1]}_ids"
       next if !respond_to?(real_ids)
 
-      real_values = assoc_name[0, assoc_name.length - 1] + 's'
+      real_values = "#{assoc_name[0, assoc_name.length - 1]}s"
       real_values = real_values.to_sym
       next if !respond_to?(real_values)
       next if !params[real_values]

app/models/application_model/checks_attribute_values_and_length.rb

@@ -30,10 +30,8 @@ module ApplicationModel::ChecksAttributeValuesAndLength
       next if value.blank?
 
       # strip null byte chars (postgresql will complain about it)
-      if column.type == :text
+      if column.type == :text && Rails.application.config.db_null_byte == false
-        if Rails.application.config.db_null_byte == false
+        self[name].delete!("\u0000")
-          self[name].delete!("\u0000")
-        end
       end
 
       # for varchar check length and replace null bytes

app/models/application_model/checks_user_columns_fillup.rb

@@ -27,16 +27,14 @@ returns
 =end
 
   def fill_up_user_create
-    if self.class.column_names.include? 'updated_by_id'
+    if self.class.column_names.include?('updated_by_id') && UserInfo.current_user_id
-      if UserInfo.current_user_id
+      if updated_by_id && updated_by_id != UserInfo.current_user_id
-        if updated_by_id && updated_by_id != UserInfo.current_user_id
+        logger.info "NOTICE create - self.updated_by_id is different: #{updated_by_id}/#{UserInfo.current_user_id}"
-          logger.info "NOTICE create - self.updated_by_id is different: #{updated_by_id}/#{UserInfo.current_user_id}"
-        end
-        self.updated_by_id = UserInfo.current_user_id
       end
+      self.updated_by_id = UserInfo.current_user_id
     end
 
-    return true if !self.class.column_names.include? 'created_by_id'
+    return true if self.class.column_names.exclude?('created_by_id')
 
     return true if !UserInfo.current_user_id
 
@@ -62,7 +60,7 @@ returns
 =end
 
   def fill_up_user_update
-    return true if !self.class.column_names.include? 'updated_by_id'
+    return true if self.class.column_names.exclude?('updated_by_id')
     return true if !UserInfo.current_user_id
 
     self.updated_by_id = UserInfo.current_user_id

app/models/authorization.rb

@@ -62,10 +62,8 @@ class Authorization < ApplicationModel
 
   def self.create_from_hash(hash, user = nil)
 
-    if !user && Setting.get('auth_third_party_auto_link_at_inital_login')
+    if !user && Setting.get('auth_third_party_auto_link_at_inital_login') && hash['info'] && hash['info']['email'].present?
-      if hash['info'] && hash['info']['email'].present?
+      user = User.find_by(email: hash['info']['email'].downcase)
-        user = User.find_by(email: hash['info']['email'].downcase)
-      end
     end
 
     if !user

app/models/avatar.rb

@@ -127,9 +127,7 @@ add avatar by url
         url = data[:url].to_s
 
         # check if source was updated within last 2 minutes
-        if avatar_already_exists&.source_url == url
+        return if avatar_already_exists&.source_url == url && avatar_already_exists.updated_at > 2.minutes.ago
-          return if avatar_already_exists.updated_at > 2.minutes.ago
-        end
 
         # twitter workaround to get bigger avatar images
         # see also https://dev.twitter.com/overview/general/user-profile-images-and-banners
@@ -172,9 +170,7 @@ add avatar by url
         url = data[:url].to_s
 
         # check if source ist already updated within last 3 minutes
-        if avatar_already_exists&.source_url == url
+        return if avatar_already_exists&.source_url == url && avatar_already_exists.updated_at > 2.minutes.ago
-          return if avatar_already_exists.updated_at > 2.minutes.ago
-        end
 
         # fetch image
         image = Service::Image.user(url)

app/models/calendar.rb

@@ -8,8 +8,8 @@ class Calendar < ApplicationModel
   store :public_holidays
 
   before_create  :validate_public_holidays, :validate_hours, :fetch_ical
-  before_update  :validate_public_holidays, :validate_hours, :fetch_ical
   after_create   :sync_default, :min_one_check
+  before_update  :validate_public_holidays, :validate_hours, :fetch_ical
   after_update   :sync_default, :min_one_check
   after_destroy  :min_one_check
 

app/models/channel.rb

@@ -8,12 +8,11 @@ class Channel < ApplicationModel
   store :options
   store :preferences
 
+  after_initialize :refresh_xoaut2!
   after_create   :email_address_check
   after_update   :email_address_check
   after_destroy  :email_address_check
 
-  after_initialize :refresh_xoaut2!
-
   # rubocop:disable Style/ClassVars
   @@channel_stream = {}
   @@channel_stream_started_till_at = {}

app/models/channel/driver/imap.rb

@@ -229,9 +229,7 @@ example
     message_ids.each do |message_id|
       count += 1
 
-      if (count % active_check_interval).zero?
+      break if (count % active_check_interval).zero? && channel_has_changed?(channel)
-        break if channel_has_changed?(channel)
-      end
       break if max_process_count_has_reached?(channel, count, count_max)
 
       Rails.logger.info " - message #{count}/#{count_all}"
@@ -240,7 +238,7 @@ example
       timeout(FETCH_METADATA_TIMEOUT) do
         message_meta = @imap.fetch(message_id, ['RFC822.SIZE', 'FLAGS', 'INTERNALDATE', 'RFC822.HEADER'])[0]
       rescue Net::IMAP::ResponseParseError => e
-        raise if !e.message.include?('unknown token')
+        raise if e.message.exclude?('unknown token')
 
         result = 'error'
         notice += <<~NOTICE
@@ -444,9 +442,7 @@ returns
 
     # verify if message is already imported via same channel, if not, import it again
     ticket = article.ticket
-    if ticket&.preferences && ticket.preferences[:channel_id].present? && channel.present?
+    return false if ticket&.preferences && ticket.preferences[:channel_id].present? && channel.present? && ticket.preferences[:channel_id] != channel[:id]
-      return false if ticket.preferences[:channel_id] != channel[:id]
-    end
 
     timeout(1.minute) do
       @imap.store(message_id, '+FLAGS', [:Seen])
@@ -468,7 +464,7 @@ returns
 =end
 
   def deleted?(message_meta, count, count_all)
-    return false if !message_meta.attr['FLAGS'].include?(:Deleted)
+    return false if message_meta.attr['FLAGS'].exclude?(:Deleted)
 
     Rails.logger.info "  - ignore message #{count}/#{count_all} - because message has already delete flag"
     true
@@ -539,10 +535,8 @@ returns
     true
   end
 
-  def timeout(seconds)
+  def timeout(seconds, &block)
-    Timeout.timeout(seconds) do
+    Timeout.timeout(seconds, &block)
-      yield
-    end
   end
 
 end

app/models/channel/driver/mail_stdin.rb

@@ -22,7 +22,7 @@ e. g. if you want to process this mail by using a certain inbound channel
 
 =end
 
-  def initialize(params = {})
+  def initialize(params = {}) # rubocop:disable Lint/MissingSuper
     Rails.logger.info 'read main from STDIN'
 
     msg = ARGF.read

app/models/channel/driver/pop3.rb

@@ -142,27 +142,23 @@ returns
     mails.first(2000).each do |m|
       count += 1
 
-      if (count % active_check_interval).zero?
+      break if (count % active_check_interval).zero? && channel_has_changed?(channel)
-        break if channel_has_changed?(channel)
-      end
 
       Rails.logger.info " - message #{count}/#{count_all}"
       mail = m.pop
       next if !mail
 
       # ignore verify messages
-      if mail.match?(/(X-Zammad-Ignore: true|X-Zammad-Verify: true)/)
+      if mail.match?(/(X-Zammad-Ignore: true|X-Zammad-Verify: true)/) && mail =~ /X-Zammad-Verify-Time:\s(.+?)\s/
-        if mail =~ /X-Zammad-Verify-Time:\s(.+?)\s/
+        begin
-          begin
+          verify_time = Time.zone.parse($1)
-            verify_time = Time.zone.parse($1)
+          if verify_time > Time.zone.now - 30.minutes
-            if verify_time > Time.zone.now - 30.minutes
+            info = "  - ignore message #{count}/#{count_all} - because it's a verify message"
-              info = "  - ignore message #{count}/#{count_all} - because it's a verify message"
+            Rails.logger.info info
-              Rails.logger.info info
+            next
-              next
-            end
-          rescue => e
-            Rails.logger.error e
           end
+        rescue => e
+          Rails.logger.error e
         end
       end
 

app/models/channel/driver/sms/massenversand.rb

@@ -53,6 +53,6 @@ class Channel::Driver::Sms::Massenversand
       sender:    options[:sender]
     }
 
-    options[:gateway] + '?' + URI.encode_www_form(params)
+    "#{options[:gateway]}?#{URI.encode_www_form(params)}"
   end
 end

app/models/channel/driver/sms/twilio.rb

@@ -41,10 +41,8 @@ class Channel::Driver::Sms::Twilio
     user = User.where(mobile: attr[:From]).order(:updated_at).first
     if !user
       _from_comment, preferences = Cti::CallerId.get_comment_preferences(attr[:From], 'from')
-      if preferences && preferences['from'] && preferences['from'][0]
+      if preferences && preferences['from'] && preferences['from'][0] && preferences['from'][0]['level'] == 'known' && preferences['from'][0]['object'] == 'User'
-        if preferences['from'][0]['level'] == 'known' && preferences['from'][0]['object'] == 'User'
+        user = User.find_by(id: preferences['from'][0]['o_id'])
-          user = User.find_by(id: preferences['from'][0]['o_id'])
-        end
       end
     end
     if !user

app/models/channel/driver/smtp.rb

@@ -30,10 +30,8 @@ class Channel::Driver::Smtp
     if !options.key?(:port) || options[:port].blank?
       options[:port] = 25
     end
-    if !options.key?(:ssl)
+    if !options.key?(:ssl) && options[:port].to_i == 465
-      if options[:port].to_i == 465
+      options[:ssl] = true
-        options[:ssl] = true
-      end
     end
     if !options.key?(:domain)
       # set fqdn, if local fqdn - use domain of sender

app/models/channel/driver/twitter.rb

@@ -178,7 +178,7 @@ returns
 
         # ignore older messages
         if @sync[:import_older_tweets] != true
-          if (@channel.created_at - 15.days) > tweet.created_at.dup.utc || older_import >= older_import_max
+          if (@channel.created_at - 15.days) > tweet.created_at.dup.utc || older_import >= older_import_max # rubocop:disable Style/SoleNestedConditional
             older_import += 1
             Rails.logger.debug { "tweet to old: #{tweet.id}/#{tweet.created_at}" }
             next

app/models/channel/email_parser.rb

@@ -123,14 +123,14 @@ returns
 
     message = "Can't process email, you will find it for bug reporting under #{filename}, please create an issue at https://github.com/zammad/zammad/issues"
 
-    p 'ERROR: ' + message # rubocop:disable Rails/Output
+    p "ERROR: #{message}" # rubocop:disable Rails/Output
-    p 'ERROR: ' + e.inspect # rubocop:disable Rails/Output
+    p "ERROR: #{e.inspect}" # rubocop:disable Rails/Output
     Rails.logger.error message
     Rails.logger.error e
 
     return false if exception == false
 
-    raise e.inspect + "\n" + e.backtrace.join("\n")
+    raise %(#{e.inspect}\n#{e.backtrace.join("\n")})
   end
 
   def _process(channel, msg)
@@ -365,13 +365,13 @@ returns
 
     from = from.gsub('<>', '').strip
     mail_address = begin
-                     Mail::AddressList.new(from).addresses
+      Mail::AddressList.new(from).addresses
-                                      .select { |a| a.address.present? }
+                       .select { |a| a.address.present? }
-                                      .partition { |a| a.address.match?(EMAIL_REGEX) }
+                       .partition { |a| a.address.match?(EMAIL_REGEX) }
-                                      .flatten.first
+                       .flatten.first
-                   rescue Mail::Field::ParseError => e
+    rescue Mail::Field::ParseError => e
-                     STDOUT.puts e
+      $stdout.puts e
-                   end
+    end
 
     if mail_address&.address.present?
       data[:from_email]        = mail_address.address
@@ -569,10 +569,10 @@ process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
 
   def body_text(message, **options)
     body_text = begin
-                  message.body.to_s
+      message.body.to_s
-                rescue Mail::UnknownEncodingType # see test/data/mail/mail043.box / issue #348
+    rescue Mail::UnknownEncodingType # see test/data/mail/mail043.box / issue #348
-                  message.body.raw_source
+      message.body.raw_source
-                end
+    end
 
     body_text = body_text.utf8_encode(from: message.charset, fallback: :read_as_sanitized_binary)
     body_text = Mail::Utilities.to_lf(body_text)
@@ -703,10 +703,8 @@ process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
     filename ||= file.header[:content_location].to_s.force_encoding('utf-8')
 
     # generate file name based on content-id
-    if filename.blank? && headers_store['Content-ID'].present?
+    if filename.blank? && headers_store['Content-ID'].present? && headers_store['Content-ID'] =~ /(.+?)@.+?/i
-      if headers_store['Content-ID'] =~ /(.+?)@.+?/i
+      filename = $1
-        filename = $1
-      end
     end
 
     file_body = String.new(file.body.to_s)
@@ -845,7 +843,7 @@ process unprocessable_mails (tmp/unprocessable_mail/*.eml) again
     begin
       reply_mail = compose_postmaster_reply(msg)
     rescue NotificationFactory::FileNotFoundError => e
-      Rails.logger.error 'No valid postmaster email_oversized template found. Skipping postmaster reply. ' + e.inspect
+      Rails.logger.error "No valid postmaster email_oversized template found. Skipping postmaster reply. #{e.inspect}"
       return
     end
 

app/models/channel/filter/bounce_delivery_permanent_failed.rb

@@ -9,6 +9,7 @@ module Channel::Filter::BounceDeliveryPermanentFailed
     return if !mail[:attachments]
 
     # remember, do not send notifications to certain recipients again if failed permanent
+    lines = %w[to cc]
     mail[:attachments].each do |attachment|
       next if !attachment[:preferences]
       next if attachment[:preferences]['Mime-Type'] != 'message/rfc822'
@@ -29,7 +30,7 @@ module Channel::Filter::BounceDeliveryPermanentFailed
       # get recipient of origin article, if only one - mark this user to not sent notifications anymore
       recipients = []
       if article.sender.name == 'System' || article.sender.name == 'Agent'
-        %w[to cc].each do |line|
+        lines.each do |line|
           next if article[line].blank?
 
           recipients = []

app/models/channel/filter/identify_sender.rb

@@ -24,30 +24,27 @@ module Channel::Filter::IdentifySender
     end
 
     # get correct customer
-    if !customer_user && Setting.get('postmaster_sender_is_agent_search_for_customer') == true
+    if !customer_user && Setting.get('postmaster_sender_is_agent_search_for_customer') == true && mail[ :'x-zammad-ticket-create-article-sender' ] == 'Agent'
-      if mail[ :'x-zammad-ticket-create-article-sender' ] == 'Agent'
+      # get first recipient and set customer
+      begin
+        to = :'raw-to'
+        if mail[to]&.addrs
+          items = mail[to].addrs
+          items.each do |item|
 
-        # get first recipient and set customer
+            # skip if recipient is system email
-        begin
+            next if EmailAddress.exists?(email: item.address.downcase)
-          to = :'raw-to'
-          if mail[to]&.addrs
-            items = mail[to].addrs
-            items.each do |item|
 
-              # skip if recipient is system email
+            customer_user = user_create(
-              next if EmailAddress.exists?(email: item.address.downcase)
+              login:     item.address,
-
+              firstname: item.display_name,
-              customer_user = user_create(
+              email:     item.address,
-                login:     item.address,
+            )
-                firstname: item.display_name,
+            break
-                email:     item.address,
-              )
-              break
-            end
           end
-        rescue => e
-          Rails.logger.error "SenderIsSystemAddress: ##{e.inspect}"
         end
+      rescue => e
+        Rails.logger.error "SenderIsSystemAddress: ##{e.inspect}"
       end
     end
 
@@ -194,7 +191,7 @@ module Channel::Filter::IdentifySender
   end
 
   def self.sanitize_email(string)
-    string += '@local' if !string.include?('@')
+    string += '@local' if string.exclude?('@')
 
     string.downcase
           .strip

app/models/channel/filter/monitoring_base.rb

@@ -47,26 +47,20 @@ class Channel::Filter::MonitoringBase
     return if result['host'].blank?
 
     # icinga - get state by body - new templates
-    if result['state'].blank?
+    if result['state'].blank? && mail[:body] =~ /.+?\sis\s(.+?)!/
-      if mail[:body] =~ /.+?\sis\s(.+?)!/
+      result['state'] = $1
-        result['state'] = $1
-      end
     end
 
     # icinga - get state by subject - new templates "state:" is not in body anymore
     # Subject: [PROBLEM] Ping IPv4 on host1234.dc.example.com is WARNING!
     # Subject: [PROBLEM] Host host1234.dc.example.com is DOWN!
-    if result['state'].blank?
+    if result['state'].blank? && mail[:subject] =~ /(on|Host)\s.+?\sis\s(.+?)!/
-      if mail[:subject] =~ /(on|Host)\s.+?\sis\s(.+?)!/
+      result['state'] = $2
-        result['state'] = $2
-      end
     end
 
     # monit - get missing attributes from body
-    if result['service'].blank?
+    if result['service'].blank? && mail[:body] =~ /\sService\s(.+?)\s/
-      if mail[:body] =~ /\sService\s(.+?)\s/
+      result['service'] = $1
-        result['service'] = $1
-      end
     end
 
     # possible event types https://mmonit.com/monit/documentation/#Setting-an-event-filter

app/models/channel/filter/sender_is_system_address.rb

@@ -26,7 +26,7 @@ module Channel::Filter::SenderIsSystemAddress
         return true
       end
     rescue => e
-      Rails.logger.error 'SenderIsSystemAddress: ' + e.inspect
+      Rails.logger.error "SenderIsSystemAddress: #{e.inspect}"
     end
 
     # check if sender is agent
@@ -53,7 +53,7 @@ module Channel::Filter::SenderIsSystemAddress
       end
       return true
     rescue => e
-      Rails.logger.error 'SenderIsSystemAddress: ' + e.inspect
+      Rails.logger.error "SenderIsSystemAddress: #{e.inspect}"
     end
 
     true

app/models/concerns/can_be_published.rb

@@ -150,7 +150,6 @@ module CanBePublished
     KnowledgeBase::Answer
       .published
       .joins(category: :knowledge_base)
-      .where(knowledge_bases: { active: true })
+      .exists?(knowledge_bases: { active: true })
-      .exists?
   end
 end

app/models/concerns/can_clone_attachments.rb

@@ -34,12 +34,10 @@ returns
       next if new_attachment.preferences['content-alternative'] == true
 
       # only_attached_attachments mode is used by apply attached attachments to forwared article
-      if options[:only_attached_attachments] == true
+      if options[:only_attached_attachments] == true && is_html_content == true
-        if is_html_content == true
 
-          content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
+        content_id = new_attachment.preferences['Content-ID'] || new_attachment.preferences['content_id']
-          next if content_id.present? && body.present? && body.match?(/#{Regexp.quote(content_id)}/i)
+        next if content_id.present? && body.present? && body.match?(/#{Regexp.quote(content_id)}/i)
-        end
       end
 
       # only_inline_attachments mode is used when quoting HTML mail with #{article.body_as_html}

app/models/concerns/can_csv_import.rb

@@ -60,8 +60,11 @@ returns
       end
 
       header, *rows = ::CSV.parse(data[:string], data[:parse_params])
-      header&.each { |column| column.try(:strip!) }
+
-      header&.each { |column| column.try(:downcase!) }
+      header&.each do |column|
+        column.try(:strip!)
+        column.try(:downcase!)
+      end
 
       begin
         raise "Delete is not possible for #{self}." if delete && !csv_delete_possible
@@ -237,7 +240,7 @@ returns
       if records.count < 20
         record_ids = records.pluck(:id).concat(csv_object_ids_ignored)
         local_records = where.not(id: record_ids).limit(20 - records.count)
-        records = records.concat(local_records)
+        records.concat(local_records)
       end
       records_attributes_with_association_names = []
       records.each do |record|

app/models/concerns/has_groups.rb

@@ -41,7 +41,7 @@ module HasGroups
         query      = select("#{ActiveRecord::Base.connection.quote_table_name('groups')}.*, #{ActiveRecord::Base.connection.quote_table_name(table_name)}.*")
         return query if access.blank?
 
-        access.push('full') if !access.include?('full')
+        access.push('full') if access.exclude?('full')
 
         query.where("#{table_name}.access" => access)
       end
@@ -366,7 +366,7 @@ module HasGroups
 
     def ensure_group_access_list_parameter(access)
       access = [access] if access.is_a?(String)
-      access.push('full') if !access.include?('full')
+      access.push('full') if access.exclude?('full')
       access
     end
   end

app/models/concerns/has_rich_text.rb

@@ -50,10 +50,11 @@ Checks if file is used inline
     parsed = Loofah.scrub_fragment(raw, scrubber).to_s
     parsed = HtmlSanitizer.strict(parsed)
 
+    line_breaks = ["\n", "\r", "\r\n"]
     scrubber_cleaner = Loofah::Scrubber.new(direction: :bottom_up) do |node|
       case node.name
       when 'span'
-        node.children.reject { |t| ["\n", "\r", "\r\n"].include?(t.text) }.each { |child| node.before child }
+        node.children.reject { |t| line_breaks.include?(t.text) }.each { |child| node.before child }
 
         node.remove
       when 'div'

app/models/concerns/has_roles.rb

@@ -107,7 +107,7 @@ module HasRoles
 
     def ensure_group_access_list_parameter(access)
       access = [access] if access.is_a?(String)
-      access.push('full') if !access.include?('full')
+      access.push('full') if access.exclude?('full')
       access
     end
   end

app/models/cti/caller_id.rb

@@ -122,7 +122,7 @@ returns
         local_caller_ids = Cti::CallerId.extract_numbers(value)
         next if local_caller_ids.blank?
 
-        caller_ids = caller_ids.concat(local_caller_ids)
+        caller_ids.concat(local_caller_ids)
       end
 
       # search for caller ids to keep

app/models/cti/driver/base.rb

@@ -137,7 +137,7 @@ class Cti::Driver::Base
     # open user profile if user has a ticket in the last 30 days
     if customer_id
       last_activity = Setting.get('cti_customer_last_activity')
-      if Ticket.where(customer_id: customer_id).where('updated_at > ?', last_activity.seconds.ago).exists?
+      if Ticket.where(customer_id: customer_id).exists?(['updated_at > ?', last_activity.seconds.ago])
         PushMessages.send_to(user.id, {
                                event: 'remote_task',
                                data:  {

app/models/cti/driver/sipgate_io.rb

@@ -76,11 +76,12 @@ class Cti::Driver::SipgateIo < Cti::Driver::Base
     end
 
     list = {}
+    items = %w[firstname lastname email]
     result['items'].each do |entry|
       next if entry['id'].blank?
 
       name = ''
-      %w[firstname lastname email].each do |item|
+      items.each do |item|
         next if entry[item].blank?
 
         name += ' ' if name.present?

app/models/cti/log.rb

@@ -304,7 +304,7 @@ returns
       assets = list.map(&:preferences)
                    .map { |p| p.slice(:from, :to) }
                    .map(&:values).flatten
-                   .map { |caller_id| caller_id[:user_id] }.compact
+                   .pluck(:user_id).compact
                    .map { |user_id| User.lookup(id: user_id) }.compact
                    .each.with_object({}) { |user, a| user.assets(a) }
 
@@ -522,13 +522,13 @@ returns queues of user
       queues = []
       config[:notify_map]&.each do |row|
         next if row[:user_ids].blank?
-        next if !row[:user_ids].include?(user.id.to_s) && !row[:user_ids].include?(user.id)
+        next if row[:user_ids].exclude?(user.id.to_s) && row[:user_ids].exclude?(user.id)
 
         queues.push row[:queue]
       end
       if user.phone.present?
         caller_ids = Cti::CallerId.extract_numbers(user.phone)
-        queues = queues.concat(caller_ids)
+        queues.concat(caller_ids)
       end
       queues
     end

app/models/email_address.rb

@@ -11,8 +11,8 @@ class EmailAddress < ApplicationModel
 
   before_validation :check_email
   before_create   :check_if_channel_exists_set_inactive
-  before_update   :check_if_channel_exists_set_inactive
   after_create    :update_email_address_id
+  before_update   :check_if_channel_exists_set_inactive
   after_update    :update_email_address_id
   before_destroy  :delete_group_reference
 

app/models/external_sync.rb

@@ -5,7 +5,7 @@ class ExternalSync < ApplicationModel
 
   class << self
 
-    def changed?(object:, previous_changes: {}, current_changes:)
+    def changed?(object:, current_changes:, previous_changes: {})
       changed = false
       previous_changes ||= {}
       current_changes.each do |attribute, value|
@@ -23,7 +23,7 @@ class ExternalSync < ApplicationModel
       changed
     end
 
-    def map(mapping: {}, source:)
+    def map(source:, mapping: {})
 
       information_source = if source.is_a?(Hash)
                              source.deep_symbolize_keys
@@ -63,6 +63,7 @@ class ExternalSync < ApplicationModel
       information_source = structure.clone
       result             = nil
       information_path   = remote_key.split('.')
+      storable_classes   = %w[String Integer Float Bool Array]
       information_path.each do |segment|
 
         segment_sym = segment.to_sym
@@ -71,15 +72,14 @@ class ExternalSync < ApplicationModel
           value = information_source[segment_sym]
         elsif information_source.respond_to?(segment_sym)
           # prevent accessing non-attributes (e.g. destroy)
-          if information_source.respond_to?(:attributes)
+          break if information_source.respond_to?(:attributes) && !information_source.attributes.key?(segment)
-            break if !information_source.attributes.key?(segment)
+
-          end
           value = information_source.send(segment_sym)
         end
         break if !value
 
         storable = value.class.ancestors.any? do |ancestor|
-          %w[String Integer Float Bool Array].include?(ancestor.to_s)
+          storable_classes.include?(ancestor.to_s)
         end
 
         if storable

app/models/knowledge_base.rb

@@ -148,13 +148,14 @@ class KnowledgeBase < ApplicationModel
     end
   end
 
+  before_validation :patch_custom_address
   after_create :set_defaults
 
   def validate_custom_address
     return if custom_address.nil?
 
     # not domain, but no leading slash
-    if !custom_address.include?('.') && custom_address[0] != '/'
+    if custom_address.exclude?('.') && custom_address[0] != '/'
       errors.add(:custom_address, 'must begin with a slash ("/").')
     end
 
@@ -177,8 +178,6 @@ class KnowledgeBase < ApplicationModel
     self.custom_address = nil if custom_address == ''
   end
 
-  before_validation :patch_custom_address
-
   def multi_lingual_support?
     Setting.get 'kb_multi_lingual_support'
   end
@@ -188,6 +187,6 @@ class KnowledgeBase < ApplicationModel
     CanBePublished.update_active_publicly!
   end
 
-  after_save    :set_kb_active_setting
   after_destroy :set_kb_active_setting
+  after_save    :set_kb_active_setting
 end

app/models/knowledge_base/answer.rb

@@ -119,7 +119,7 @@ class KnowledgeBase::Answer < ApplicationModel
       {
         id:          attachment.id,
         url:         url,
-        preview_url: url + '?preview=1',
+        preview_url: "#{url}?preview=1",
         filename:    attachment.filename,
         size:        attachment.size,
         preferences: attachment.preferences

app/models/knowledge_base/answer/translation/content.rb

@@ -51,6 +51,7 @@ class KnowledgeBase::Answer::Translation::Content < ApplicationModel
     translation&.touch # rubocop:disable Rails/SkipsModelValidations
   end
 
+  before_save :sanitize_body
   after_save  :touch_translation
   after_touch :touch_translation
 
@@ -58,5 +59,4 @@ class KnowledgeBase::Answer::Translation::Content < ApplicationModel
     self.body = HtmlSanitizer.dynamic_image_size(body)
   end
 
-  before_save :sanitize_body
 end

app/models/object_manager/element/backend.rb

@@ -54,12 +54,13 @@ class ObjectManager::Element::Backend
   end
 
   def screen_permission_options(permission_options)
+    booleans = [true, false]
     permission_options.each_with_object({}) do |(permission, options), result|
 
       next if !authorized?(permission)
 
       options.each do |key, value|
-        next if [true, false].include?(result[key]) && !value
+        next if booleans.include?(result[key]) && !value
 
         result[key] = value
       end

app/models/observer/ticket/article_changes.rb

@@ -48,7 +48,7 @@ class Observer::Ticket::ArticleChanges < ActiveRecord::Observer
   def article_count_update(record)
     current_count = record.ticket.article_count
     sender = Ticket::Article::Sender.lookup(name: 'System')
-    count = Ticket::Article.where(ticket_id: record.ticket_id).where('sender_id NOT IN (?)', sender.id).count
+    count = Ticket::Article.where(ticket_id: record.ticket_id).where.not(sender_id: sender.id).count
     return false if current_count == count
 
     record.ticket.article_count = count

app/models/observer/ticket/last_owner_update.rb

@@ -41,7 +41,7 @@ class Observer::Ticket::LastOwnerUpdate < ActiveRecord::Observer
     # check if state is not new/open
     if record.changes_to_save['state_id'].present?
       state_ids = Ticket::State.by_category(:work_on).pluck(:id)
-      if !state_ids.include?(record.changes_to_save['state_id'][1])
+      if state_ids.exclude?(record.changes_to_save['state_id'][1])
         record.last_owner_update_at = nil
         return true
       end

app/models/observer/ticket/ref_object_touch.rb

@@ -22,10 +22,8 @@ class Observer::Ticket::RefObjectTouch < ActiveRecord::Observer
 
     # touch old customer if changed
     cutomer_id_changed = record.saved_changes['customer_id']
-    if cutomer_id_changed && cutomer_id_changed[0] != cutomer_id_changed[1]
+    if cutomer_id_changed && cutomer_id_changed[0] != cutomer_id_changed[1] && cutomer_id_changed[0]
-      if cutomer_id_changed[0]
+      User.find(cutomer_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations
-        User.find(cutomer_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations
-      end
     end
 
     # touch new/current customer
@@ -33,10 +31,8 @@ class Observer::Ticket::RefObjectTouch < ActiveRecord::Observer
 
     # touch old organization if changed
     organization_id_changed = record.saved_changes['organization_id']
-    if organization_id_changed && organization_id_changed[0] != organization_id_changed[1]
+    if organization_id_changed && organization_id_changed[0] != organization_id_changed[1] && organization_id_changed[0]
-      if organization_id_changed[0]
+      Organization.find(organization_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations
-        Organization.find(organization_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations
-      end
     end
 
     # touch new/current organization

app/models/package.rb

@@ -91,7 +91,7 @@ note: will not take down package migrations, use Package.unlink instead
         logger.info "unlink: #{entry}"
         File.delete(entry)
       end
-      backup_file = entry + '.link_backup'
+      backup_file = "#{entry}.link_backup"
       if File.exist?(backup_file)
         logger.info "Restore backup file of #{backup_file} -> #{entry}."
         File.rename(backup_file, entry)
@@ -102,7 +102,7 @@ note: will not take down package migrations, use Package.unlink instead
   # check if zpm is a package source repo
   def self._package_base_dir?(package_base_dir)
     package = false
-    Dir.glob(package_base_dir + '/*.szpm') do |entry|
+    Dir.glob("#{package_base_dir}/*.szpm") do |entry|
       package = entry.sub(%r{^.*/(.+?)\.szpm$}, '\1')
     end
     if package == false
@@ -130,18 +130,18 @@ execute migration down + unlink files
     Package::Migration.migrate(package, 'reverse')
 
     # link files
-    Dir.glob(package_base_dir + '/**/*') do |entry|
+    Dir.glob("#{package_base_dir}/**/*") do |entry|
       entry = entry.sub('//', '/')
       file = entry
       file = file.sub(/#{package_base_dir}/, '')
-      dest = @@root + '/' + file
+      dest = "#{@@root}/#{file}"
 
       if File.symlink?(dest.to_s)
         logger.info "Unlink file: #{dest}"
         File.delete(dest.to_s)
       end
 
-      backup_file = dest.to_s + '.link_backup'
+      backup_file = "#{dest}.link_backup"
       if File.exist?(backup_file)
         logger.info "Restore backup file of #{backup_file} -> #{dest}."
         File.rename(backup_file, dest.to_s)
@@ -163,7 +163,7 @@ link files + execute migration up
     package = _package_base_dir?(package_base_dir)
 
     # link files
-    Dir.glob(package_base_dir + '/**/*') do |entry|
+    Dir.glob("#{package_base_dir}/**/*") do |entry|
       entry = entry.sub('//', '/')
       file = entry
       file = file.sub(/#{package_base_dir}/, '')
@@ -176,17 +176,15 @@ link files + execute migration up
       end
 
       # get new file destination
-      dest = @@root + '/' + file
+      dest = "#{@@root}/#{file}"
 
-      if File.directory?(entry.to_s)
+      if File.directory?(entry.to_s) && !File.exist?(dest.to_s)
-        if !File.exist?(dest.to_s)
+        logger.info "Create dir: #{dest}"
-          logger.info "Create dir: #{dest}"
+        FileUtils.mkdir_p(dest.to_s)
-          FileUtils.mkdir_p(dest.to_s)
-        end
       end
 
       if File.file?(entry.to_s) && (File.file?(dest.to_s) && !File.symlink?(dest.to_s))
-        backup_file = dest.to_s + '.link_backup'
+        backup_file = "#{dest}.link_backup"
         if File.exist?(backup_file)
           raise "Can't link #{entry} -> #{dest}, destination and .link_backup already exists!"
         end
@@ -410,11 +408,11 @@ execute all pending package migrations at once
   def self._read_file(file, fullpath = false)
     location = case fullpath
                when false
-                 @@root + '/' + file
+                 "#{@@root}/#{file}"
                when true
                  file
                else
-                 fullpath + '/' + file
+                 "#{fullpath}/#{file}"
                end
 
     begin
@@ -436,7 +434,7 @@ execute all pending package migrations at once
         logger.debug { "NOTICE: file '#{location}' already exists, skip install" }
         return true
       end
-      backup_location = location + '.save'
+      backup_location = "#{location}.save"
       logger.info "NOTICE: backup old file '#{location}' to #{backup_location}"
       File.rename(location, backup_location)
     end
@@ -478,7 +476,7 @@ execute all pending package migrations at once
     end
 
     # rename existing file
-    backup_location = location + '.save'
+    backup_location = "#{location}.save"
     if File.exist?(backup_location)
       logger.info "NOTICE: restore old file '#{backup_location}' to #{location}"
       File.rename(backup_location, location)

app/models/recent_view.rb

@@ -58,7 +58,7 @@ class RecentView < ApplicationModel
 
       viewable_ticket_ids = Ticket.where('id IN (?) AND state_id in (?)',
                                          recent_views.map(&:o_id),
-                                         Ticket::State.by_category(:viewable_agent_new).pluck(:id))
+                                         Ticket::State.by_category(:viewable_agent_new).pluck(:id)) # rubocop:disable Rails/PluckInWhere
                                   .pluck(:id)
 
       recent_views = recent_views.select { |rv| viewable_ticket_ids.include?(rv.o_id) }

app/models/role.rb

@@ -42,7 +42,7 @@ grant permission to role
     raise "Invalid permission #{key}" if !permission
     return true if permission_ids.include?(permission.id)
 
-    self.permission_ids = permission_ids.push permission.id
+    self.permission_ids = permission_ids.push permission.id # rubocop:disable Style/RedundantSelfAssignment
     true
   end
 
@@ -57,7 +57,7 @@ revoke permission of role
   def permission_revoke(key)
     permission = Permission.lookup(name: key)
     raise "Invalid permission #{key}" if !permission
-    return true if !permission_ids.include?(permission.id)
+    return true if permission_ids.exclude?(permission.id)
 
     self.permission_ids = self.permission_ids -= [permission.id]
     true

app/models/scheduler.rb

@@ -347,7 +347,7 @@ class Scheduler < ApplicationModel
       loop do
         success, failure = Delayed::Worker.new.work_off
         if failure.nonzero?
-          raise "#{failure} failed background jobs: #{Delayed::Job.where('last_error IS NOT NULL').inspect}"
+          raise "#{failure} failed background jobs: #{Delayed::Job.where.not(last_error: nil).inspect}"
         end
         break if success.zero?
       end

app/models/setting.rb

@@ -6,8 +6,8 @@ class Setting < ApplicationModel
   store         :state_initial
   store         :preferences
   before_create :state_check, :set_initial, :check_broadcast
-  before_update :state_check, :check_broadcast
   after_create  :reset_change_id, :reset_cache
+  before_update :state_check, :check_broadcast
   after_update  :reset_change_id, :reset_cache
 
   attr_accessor :state

app/models/store.rb

@@ -13,8 +13,8 @@ class Store < ApplicationModel
 
   store :preferences
 
-  after_create :generate_previews
   before_create :oversized_preferences_check
+  after_create :generate_previews
   before_update :oversized_preferences_check
 
 =begin

app/models/store/provider/file.rb

@@ -85,6 +85,8 @@ class Store::Provider::File
     length2 = 5
     length3 = 7
     last_position = 0
+
+    # rubocop:disable Style/CombinableLoops
     (0..1).each do |_count|
       end_position = last_position + length1
       parts.push sha[last_position, length1]
@@ -100,8 +102,9 @@ class Store::Provider::File
       parts.push sha[last_position, length3]
       last_position = end_position
     end
+    # rubocop:enable Style/CombinableLoops
 
-    path     = parts[ 0..6 ].join('/') + '/'
+    path     = "#{parts[ 0..6 ].join('/')}/"
     file     = sha[last_position, sha.length]
     location = "#{base}/#{path}"
 

app/models/ticket.rb

@@ -412,11 +412,7 @@ returns
     state_type = Ticket::StateType.lookup(id: state.state_type_id)
 
     # always to set unseen for ticket owner and users which did not the update
-    if state_type.name != 'merged'
+    return false if state_type.name != 'merged' && user_id_check && user_id_check == owner_id && user_id_check != updated_by_id
-      if user_id_check
-        return false if user_id_check == owner_id && user_id_check != updated_by_id
-      end
-    end
 
     # set all to seen if pending action state is a closed or merged state
     if state_type.name == 'pending action' && state.next_state_id
@@ -901,10 +897,10 @@ perform changes on ticket
     logger.debug { "Perform #{perform_origin} #{perform.inspect} on Ticket.find(#{id})" }
 
     article = begin
-                Ticket::Article.find_by(id: item.try(:dig, :article_id))
+      Ticket::Article.find_by(id: item.try(:dig, :article_id))
-              rescue ArgumentError
+    rescue ArgumentError
-                nil
+      nil
-              end
+    end
 
     # if the configuration contains the deletion of the ticket then
     # we skip all other ticket changes because they does not matter

app/models/ticket/article.rb

@@ -19,9 +19,9 @@ class Ticket::Article < ApplicationModel
   belongs_to :updated_by, class_name: 'User', optional: true
   belongs_to :origin_by,  class_name: 'User', optional: true
 
+  before_save   :touch_ticket_if_needed
   before_create :check_subject, :check_body, :check_message_id_md5
   before_update :check_subject, :check_body, :check_message_id_md5
-  before_save   :touch_ticket_if_needed
   after_destroy :store_delete
 
   store :preferences

app/models/ticket/escalation.rb

@@ -136,15 +136,15 @@ returns
       close_at_changed = false
     end
 
-    if !force && preferences[:escalation_calculation]
+    if !force &&
-      if first_response_at_changed == false &&
+       preferences[:escalation_calculation] &&
-         last_update_at_changed == false &&
+       first_response_at_changed == false &&
-         close_at_changed == false &&
+       last_update_at_changed == false &&
-         sla_changed == false &&
+       close_at_changed == false &&
-         calendar_changed == false &&
+       sla_changed == false &&
-         escalation_calculation['escalation_disabled'] == escalation_disabled
+       calendar_changed == false &&
-        return false
+       escalation_calculation['escalation_disabled'] == escalation_disabled
-      end
+      return false
     end
 
     # reset escalation attributes

app/models/ticket/screen_options.rb

@@ -58,7 +58,7 @@ returns
       state_type = params[:ticket].state.state_type
     end
     state_types = ['open', 'closed', 'pending action', 'pending reminder']
-    if state_type && !state_types.include?(state_type.name)
+    if state_type && state_types.exclude?(state_type.name)
       state_ids.push params[:ticket].state_id
     end
     state_types.each do |type|

app/models/ticket/subject.rb

@@ -71,7 +71,7 @@ returns
 
     # resize subject based on config
     if subject.length > ticket_subject_size.to_i
-      subject = subject[ 0, ticket_subject_size.to_i ] + '[...]'
+      subject = "#{subject[ 0, ticket_subject_size.to_i ]}[...]"
     end
 
     subject.strip!

app/models/token.rb

@@ -75,14 +75,10 @@ returns
     user = token.user
 
     # persistent token not valid if user is inactive
-    if !data[:inactive_user]
+    return if !data[:inactive_user] && token.persistent && user.active == false
-      return if token.persistent && user.active == false
-    end
 
     # add permission check
-    if data[:permission]
+    return if data[:permission] && !token.permissions?(data[:permission])
-      return if !token.permissions?(data[:permission])
-    end
 
     # return token user
     user

app/models/transaction/notification.rb

@@ -116,7 +116,7 @@ class Transaction::Notification
           history_type_id:   History.type_lookup('notification').id,
           history_object_id: History.object_lookup('Ticket').id,
           o_id:              ticket.id
-        ).where('created_at > ?', Time.zone.now.beginning_of_day).where('value_to LIKE ?', "%#{identifier}(#{@item[:type]}:%").exists?
+        ).where('created_at > ?', Time.zone.now.beginning_of_day).exists?(['value_to LIKE ?', "%#{identifier}(#{@item[:type]}:%"])
 
         next if already_notified
       end

app/models/translation.rb

@@ -492,17 +492,17 @@ Get source file at https://i18n.zammad.com/api/v1/translations_empty_translation
   end
 
   def cache_clear
-    Cache.delete('TranslationMapOnlyContent::' + locale.downcase)
+    Cache.delete("TranslationMapOnlyContent::#{locale.downcase}")
     true
   end
 
   def self.cache_set(locale, data)
-    Cache.write('TranslationMapOnlyContent::' + locale.downcase, data)
+    Cache.write("TranslationMapOnlyContent::#{locale.downcase}", data)
   end
   private_class_method :cache_set
 
   def self.cache_get(locale)
-    Cache.get('TranslationMapOnlyContent::' + locale.downcase)
+    Cache.get("TranslationMapOnlyContent::#{locale.downcase}")
   end
   private_class_method :cache_get
 end

app/models/user.rb

@@ -44,11 +44,11 @@ class User < ApplicationModel
   before_validation :check_name, :check_email, :check_login, :ensure_uniq_email, :ensure_password, :ensure_roles, :ensure_identifier
   before_validation :check_mail_delivery_failed, on: :update
   before_create     :check_preferences_default, :validate_preferences, :validate_ooo, :domain_based_assignment, :set_locale
-  before_update     :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute
   after_create      :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
+  before_update     :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute
   after_update      :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
-  after_commit      :update_caller_id
   before_destroy    :destroy_longer_required_objects, :destroy_move_dependency_ownership
+  after_commit      :update_caller_id
 
   store :preferences
 
@@ -782,13 +782,12 @@ returns
   end
 
   def check_preferences_default
-    if @preferences_default.blank?
+    if @preferences_default.blank? && id
-      if id
+      roles.each do |role|
-        roles.each do |role|
+        check_notifications(role, false)
-          check_notifications(role, false)
-        end
       end
     end
+
     return if @preferences_default.blank?
 
     preferences_tmp = @preferences_default.merge(preferences)
@@ -930,10 +929,8 @@ try to find correct name
     end
 
     # if email has changed, login is old email, change also login
-    if changes && changes['email']
+    if changes && changes['email'] && changes['email'][0] == login
-      if changes['email'][0] == login
+      self.login = email
-        self.login = email
-      end
     end
 
     # generate auto login
@@ -1089,7 +1086,7 @@ raise 'Minimum one user need to have admin permissions'
       # if user already has a ticket.agent role
       hint = false
       role_ids.each do |locale_role_id|
-        next if !ticket_agent_role_ids.include?(locale_role_id)
+        next if ticket_agent_role_ids.exclude?(locale_role_id)
 
         hint = true
         break
@@ -1176,6 +1173,7 @@ raise 'Minimum one user need to have admin permissions'
   def destroy_move_dependency_ownership
     result = Models.references(self.class.to_s, id)
 
+    user_columns = %w[created_by_id updated_by_id origin_by_id owner_id archived_by_id published_by_id internal_by_id]
     result.each do |class_name, references|
       next if class_name.blank?
       next if references.blank?
@@ -1184,7 +1182,7 @@ raise 'Minimum one user need to have admin permissions'
       references.each do |column, reference_found|
         next if !reference_found
 
-        if %w[created_by_id updated_by_id origin_by_id owner_id archived_by_id published_by_id internal_by_id].include?(column)
+        if user_columns.include?(column)
           ref_class.where(column => id).find_in_batches(batch_size: 1000) do |batch_list|
             batch_list.each do |record|
               record.update!(column => 1)

app/models/user/assets.rb

@@ -92,7 +92,7 @@ returns
 
       # add organization
       if self.organization_id
-        if !data[:Organization] || !data[:Organization][self.organization_id]
+        if !data[:Organization] || !data[:Organization][self.organization_id] # rubocop:disable Style/SoleNestedConditional
           organization = Organization.lookup(id: self.organization_id)
           if organization
             data = organization.assets(data)

app/models/user_device.rb

@@ -110,9 +110,7 @@ store new device for user if device not already known
       fingerprint: fingerprint,
     )
 
-    if user_device
+    return action(user_device.id, user_agent, ip, user_id, type) if user_device
-      return action(user_device.id, user_agent, ip, user_id, type) if user_device
-    end
 
     # create new device
     user_device = create!(

app/policies/controllers/application_controller_policy.rb

@@ -2,6 +2,8 @@ class Controllers::ApplicationControllerPolicy < ApplicationPolicy
   class_attribute(:action_permissions_map, default: {})
 
   def self.inherited(subclass)
+    super
+
     subclass.action_permissions_map = action_permissions_map.deep_dup
   end
 

app/policies/ticket/article_policy.rb

@@ -22,7 +22,7 @@ class Ticket::ArticlePolicy < ApplicationPolicy
     # which were created by themselves within the last x minutes
 
     if !user.permissions?('ticket.agent')
-      return not_authorized('agent permission required') if !user.permissions?('ticket.agent')
+      return not_authorized('agent permission required')
     end
 
     if record.created_by_id != user.id

config/environments/production.rb

@@ -87,7 +87,7 @@ Rails.application.configure do
   # config.logger = ActiveSupport::TaggedLogging.new(Syslog::Logger.new 'app-name')
 
   if ENV['RAILS_LOG_TO_STDOUT'].present?
-    logger           = ActiveSupport::Logger.new(STDOUT)
+    logger           = ActiveSupport::Logger.new($stdout)
     logger.formatter = config.log_formatter
     config.logger    = ActiveSupport::TaggedLogging.new(logger)
   end

db/migrate/20170905140038_cti_log_preferences_migration.rb

@@ -35,6 +35,7 @@ class CtiLogPreferencesMigration < ActiveRecord::Migration[5.0]
   def change
 
     # correct all entries
+    directions = %w[from to]
     Cti::Log.all.pluck(:id).each do |item_id|
       item = Cti::Log.find(item_id)
       next if !item.preferences
@@ -42,7 +43,7 @@ class CtiLogPreferencesMigration < ActiveRecord::Migration[5.0]
 
       # check from and to keys which hold the instances
       preferences = {}
-      %w[from to].each do |direction|
+      directions.each do |direction|
         next if item.preferences[direction].blank?
 
         # loop over all instances and covert them

db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb

@@ -9,11 +9,12 @@ class Issue2541FixNotificationEmailWithoutBody < ActiveRecord::Migration[5.1]
     UserInfo.current_user_id = 1
 
     # update jobs and triggers
+    actions = %w[notification.email notification.sms]
     [::Job, ::Trigger].each do |model|
       model.all.each do |record|
         next if record.perform.blank?
 
-        %w[notification.email notification.sms].each do |action|
+        actions.each do |action|
           next if record.perform[action].blank?
           next if record.perform[action]['body'].present?
 

lib/auto_wizard.rb

@@ -66,20 +66,16 @@ returns
     UserInfo.current_user_id = admin_user.id
 
     # set default calendar
-    if auto_wizard_hash['CalendarSetup']
+    if auto_wizard_hash['CalendarSetup'] && auto_wizard_hash['CalendarSetup']['Ip']
-      if auto_wizard_hash['CalendarSetup']['Ip']
+      Calendar.init_setup(auto_wizard_hash['CalendarSetup']['Ip'])
-        Calendar.init_setup(auto_wizard_hash['CalendarSetup']['Ip'])
-      end
     end
 
     # load text modules
-    if auto_wizard_hash['TextModuleLocale']
+    if auto_wizard_hash['TextModuleLocale'] && auto_wizard_hash['TextModuleLocale']['Locale']
-      if auto_wizard_hash['TextModuleLocale']['Locale']
+      begin
-        begin
+        TextModule.load(auto_wizard_hash['TextModuleLocale']['Locale'])
-          TextModule.load(auto_wizard_hash['TextModuleLocale']['Locale'])
+      rescue => e
-        rescue => e
+        Rails.logger.error "Unable to load text modules #{auto_wizard_hash['TextModuleLocale']['Locale']}: #{e.message}"
-          Rails.logger.error "Unable to load text modules #{auto_wizard_hash['TextModuleLocale']['Locale']}: #{e.message}"
-        end
       end
     end
 

lib/calendar_subscriptions.rb

@@ -11,7 +11,7 @@ class CalendarSubscriptions
 
       next if calendar_subscription.name !~ /\Adefaults_calendar_subscriptions_(.*)\z/
 
-      object_name                 = $1
+      object_name                 = $1 # rubocop:disable Lint/OutOfRangeRegexpRef
       @preferences[ object_name ] = calendar_subscription.state_current[:value]
     end
 

lib/core_ext/activesupport/lib/active_support/tagged_logging/formatter.rb

@@ -21,7 +21,7 @@ module ActiveSupport
   module TaggedLogging
     module Formatter
       # This method is invoked when a log event occurs.
-      def call(severity, timestamp, progname, msg)
+      def call(severity, timestamp, progname, msg) # rubocop:disable Lint/UselessMethodDefinition
         # super(severity, timestamp, progname, "#{tags_text}#{msg}")
         super(severity, timestamp, progname, msg)
       end

lib/core_ext/open-uri.rb

@@ -1,7 +1,8 @@
 # rubocop:disable Naming/FileName
 if Kernel.respond_to?(:open_uri_original_open)
   module Kernel
-    private
+
+    module_function
 
     # see: https://github.com/ruby/ruby/pull/1675
     def open(name, *rest, &block)
@@ -15,7 +16,6 @@ if Kernel.respond_to?(:open_uri_original_open)
         open_uri_original_open(name, *rest, &block)
       end
     end
-    module_function :open # rubocop:disable Style/AccessModifierDeclarations
   end
 end
 # rubocop:enable Naming/FileName

lib/core_ext/string.rb

@@ -32,7 +32,7 @@ class String
     quote = split("\n")
     body_quote = ''
     quote.each do |line|
-      body_quote = body_quote + '> ' + line + "\n"
+      body_quote = "#{body_quote}> #{line}\n"
     end
     body_quote
   end
@@ -195,7 +195,7 @@ class String
 
     # blockquote handling
     string.gsub!(%r{<blockquote(| [^>]*)>(.+?)</blockquote>}m) do
-      "\n" + $2.html2text(true).gsub(/^(.*)$/, '&gt; \1') + "\n"
+      "\n#{$2.html2text(true).gsub(/^(.*)$/, '&gt; \1')}\n" # rubocop:disable Lint/OutOfRangeRegexpRef
     end
 
     # pre/code handling 2/2
@@ -294,7 +294,7 @@ class String
 
     # add extracted links
     if link_list != ''
-      string += "\n\n\n" + link_list
+      string += "\n\n\n#{link_list}"
     end
 
     # remove double multiple empty lines

lib/email_helper.rb

@@ -236,7 +236,7 @@ returns
           },
         },
       ]
-      inbounds = inbounds.concat(inbound)
+      inbounds.concat(inbound)
     end
     inbounds
   end
@@ -473,7 +473,7 @@ returns
           },
         },
       ]
-      outbounds = outbounds.concat(outbound)
+      outbounds.concat(outbound)
     end
     outbounds
   end

lib/email_helper/probe.rb

@@ -67,7 +67,7 @@ returns on fail
 
       # get mx records, try to find provider based on mx records
       mx_records = EmailHelper.mx_records(domain)
-      domains = domains.concat(mx_records)
+      domains.concat(mx_records)
       provider_map.each_value do |settings|
         domains.each do |domain_to_check|
 

lib/email_helper/verify.rb

@@ -56,7 +56,7 @@ or
     def self.email(params)
 
       # send verify email
-      subject = params[:subject].presence || '#' + rand(99_999_999_999).to_s
+      subject = params[:subject].presence || "##{rand(99_999_999_999)}"
       result = EmailHelper::Probe.outbound(params[:outbound], params[:sender], subject)
       if result[:result] != 'ok'
         result[:source] = 'outbound'

lib/excel_sheet.rb

@@ -1,6 +1,6 @@
 class ExcelSheet
 
-  def initialize(title:, header:, records:, timezone: nil, locale:)
+  def initialize(title:, header:, records:, locale:, timezone: nil)
     @title           = title
     @header          = header
     @records         = records

lib/excel_sheet/ticket.rb

@@ -45,7 +45,7 @@ class ExcelSheet::Ticket < ExcelSheet
       { display: 'Time Units Total', name: 'time_unit', width: 10, data_type: 'float' },
     ]
 
-    header = header.concat(@additional_attributes_header) if @additional_attributes_header
+    header.concat(@additional_attributes_header) if @additional_attributes_header
 
     # ObjectManager attributes
     objects = ObjectManager::Attribute.where(active:           true,

lib/external_credential/twitter.rb

@@ -160,7 +160,7 @@ class ExternalCredential::Twitter
     rescue
       begin
         webhooks = client.webhooks
-        raise "Dev Environment Label invalid. Please use an existing one #{webhooks[:environments].map { |e| e[:environment_name] }}, or create a new one."
+        raise "Dev Environment Label invalid. Please use an existing one #{webhooks[:environments].pluck(:environment_name)}, or create a new one."
       rescue Twitter::Error => e
         raise "#{e.message} Are you sure you created a development environment on developer.twitter.com?"
       end

lib/facebook.rb

@@ -350,8 +350,9 @@ result
     # no changes in post is from page user it self
     if post['from'] && post['from']['id'].to_s == page['id'].to_s
       if !ticket
-        return Ticket::State.find_by(name: 'closed') if !ticket
+        return Ticket::State.find_by(name: 'closed')
       end
+
       return ticket.state
     end
 

lib/html_sanitizer.rb

@@ -67,10 +67,8 @@ satinize html string based on whiltelist
         end
 
         # check if href is different to text
-        if node.name == 'a' && !url_same?(node['href'], node.text)
+        if node.name == 'a' && !url_same?(node['href'], node.text) && node['title'].blank?
-          if node['title'].blank?
+          node['title'] = node['href']
-            node['title'] = node['href']
-          end
         end
       end
 
@@ -92,7 +90,7 @@ satinize html string based on whiltelist
         end
 
         # replace tags, keep subtree
-        if !tags_whitelist.include?(node.name)
+        if tags_whitelist.exclude?(node.name)
           node.replace node.children.to_s
           Loofah::Scrubber::STOP
         end
@@ -111,7 +109,7 @@ satinize html string based on whiltelist
           classes = node['class'].gsub(/\t|\n|\r/, '').split(' ')
           class_new = ''
           classes.each do |local_class|
-            next if !classes_whitelist.include?(local_class.to_s.strip)
+            next if classes_whitelist.exclude?(local_class.to_s.strip)
 
             if class_new != ''
               class_new += ' '
@@ -151,8 +149,8 @@ satinize html string based on whiltelist
             next if !prop[0]
 
             key = prop[0].strip
-            next if !css_properties_whitelist.include?(node.name)
+            next if css_properties_whitelist.exclude?(node.name)
-            next if !css_properties_whitelist[node.name].include?(key)
+            next if css_properties_whitelist[node.name].exclude?(key)
             next if css_values_blacklist[node.name]&.include?(local_pear.gsub(/[[:space:]]/, '').strip)
 
             style += "#{local_pear};"
@@ -246,7 +244,7 @@ cleanup html string:
     #return string
     tags_backlist = %w[span center]
     scrubber = Loofah::Scrubber.new do |node|
-      next if !tags_backlist.include?(node.name)
+      next if tags_backlist.exclude?(node.name)
 
       hit = false
       local_node = nil
@@ -327,7 +325,7 @@ cleanup html string:
 
       # remove not needed new lines
       if node.class == Nokogiri::XML::Text
-        if !node.parent || (node.parent.name != 'pre' && node.parent.name != 'code')
+        if !node.parent || (node.parent.name != 'pre' && node.parent.name != 'code') # rubocop:disable Style/SoleNestedConditional
           content = node.content
           if content
             if content != ' ' && content != "\n"

lib/import/exchange/item_attributes.rb

@@ -22,10 +22,11 @@ module Import
       private
 
       def booleanize_values(properties)
+        booleans = %w[true false]
         properties.each do |key, value|
           case value
           when String
-            next if !%w[true false].include?(value)
+            next if booleans.exclude?(value)
 
             properties[key] = value == 'true'
           when Hash
@@ -80,11 +81,12 @@ module Import
       end
 
       def flatten(properties, prefix: nil)
+        keys = %i[text id]
         properties.each_with_object({}) do |(key, value), result|
 
           result_key = key
           if prefix
-            result_key = if %i[text id].include?(key) && ( !result[result_key] || result[result_key] == value )
+            result_key = if keys.include?(key) && ( !result[result_key] || result[result_key] == value )
                            prefix
                          else
                            :"#{prefix}.#{key}"

lib/import/integration_base.rb

@@ -11,6 +11,8 @@ module Import
   class IntegrationBase < Import::Base
 
     def self.inherited(subclass)
+      super
+
       subclass.extend(Forwardable)
 
       # delegate instance methods to the generic class implementations

lib/import/otrs.rb

@@ -108,7 +108,7 @@ module Import
           ActiveRecord::Base.connection.close
         end
       end
-      (1..thread_count).each do |thread|
+      (1..thread_count).each do |thread| # rubocop:disable Style/CombinableLoops
         threads[thread].join
       end
     end

lib/import/otrs/history_factory.rb

@@ -25,7 +25,7 @@ module Import
       end
 
       def check_supported(history)
-        return if !supported_types.include?(history['HistoryType'])
+        return if supported_types.exclude?(history['HistoryType'])
 
         history['HistoryType']
       end

lib/import/otrs/requester.rb

@@ -106,8 +106,8 @@ module Import
         params[:Action] = 'ZammadMigrator'
         params[:Key]    = Setting.get('import_otrs_endpoint_key')
 
-        log 'POST: ' + url
+        log "POST: #{url}"
-        log 'PARAMS: ' + params.inspect
+        log "PARAMS: #{params.inspect}"
 
         response = UserAgent.post(
           url,