diff --git a/.rubocop/default.yml b/.rubocop/default.yml index 8473df998..cff8ea296 100644 --- a/.rubocop/default.yml +++ b/.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: >- diff --git a/.rubocop/todo.rspec.yml b/.rubocop/todo.rspec.yml index 9868a442c..237eb4ca5 100644 --- a/.rubocop/todo.rspec.yml +++ b/.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 diff --git a/.rubocop/todo.yml b/.rubocop/todo.yml index cd6ee5d78..59659c0fb 100644 --- a/.rubocop/todo.yml +++ b/.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' diff --git a/Gemfile.lock b/Gemfile.lock index 5f111e046..ac5ddf625 100644 --- a/Gemfile.lock +++ b/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) - parser (>= 2.7.0.1) - rubocop-performance (1.7.0) - rubocop (>= 0.82.0) - rubocop-rails (2.6.0) + rubocop-ast (0.5.0) + parser (>= 2.7.1.5) + rubocop-performance (1.8.1) + rubocop (>= 0.87.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) diff --git a/app/controllers/application_controller/handles_errors.rb b/app/controllers/application_controller/handles_errors.rb index 0f19e2dfd..65e213194 100644 --- a/app/controllers/application_controller/handles_errors.rb +++ b/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 diff --git a/app/controllers/application_controller/has_user.rb b/app/controllers/application_controller/has_user.rb index b4d0429ec..d7d1c7643 100644 --- a/app/controllers/application_controller/has_user.rb +++ b/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 diff --git a/app/controllers/application_controller/logs_http_access.rb b/app/controllers/application_controller/logs_http_access.rb index 855aad73b..e627ece7b 100644 --- a/app/controllers/application_controller/logs_http_access.rb +++ b/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 = { diff --git a/app/controllers/channels_email_controller.rb b/app/controllers/channels_email_controller.rb index 708151895..d963e881a 100644 --- a/app/controllers/channels_email_controller.rb +++ b/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 account_duplicate?(result) - end + return if result[:result] == 'ok' && account_duplicate?(result) render json: result end diff --git a/app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb b/app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb index eae06c9cd..90c39455a 100644 --- a/app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb +++ b/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] diff --git a/app/controllers/concerns/creates_ticket_articles.rb b/app/controllers/concerns/creates_ticket_articles.rb index 3d367af57..65570ea5d 100644 --- a/app/controllers/concerns/creates_ticket_articles.rb +++ b/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('_', '-') diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index ee2d73f19..260fdbc02 100644 --- a/app/controllers/form_controller.rb +++ b/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 diff --git a/app/controllers/getting_started_controller.rb b/app/controllers/getting_started_controller.rb index 023ba3aa0..3b59518e8 100644 --- a/app/controllers/getting_started_controller.rb +++ b/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 !authentication_check - end + return if setup_done && !authentication_check # return result render json: { diff --git a/app/controllers/long_polling_controller.rb b/app/controllers/long_polling_controller.rb index 35cfa4236..218d28ae4 100644 --- a/app/controllers/long_polling_controller.rb +++ b/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 diff --git a/app/controllers/monitoring_controller.rb b/app/controllers/monitoring_controller.rb index 153590b00..90e16311c 100644 --- a/app/controllers/monitoring_controller.rb +++ b/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 diff --git a/app/controllers/object_manager_attributes_controller.rb b/app/controllers/object_manager_attributes_controller.rb index 4299e9b1a..6f0c999e0 100644 --- a/app/controllers/object_manager_attributes_controller.rb +++ b/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_option][:options] - # rubocop:disable Lint/BooleanSymbol - if permitted[:data_option][:options][: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 + if permitted[:data_type].match?(/^(boolean)$/) && permitted[:data_option][:options] + # rubocop:disable Lint/BooleanSymbol + if permitted[:data_option][:options][: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] diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index bad76eb12..7b0fc5054 100644 --- a/app/controllers/search_controller.rb +++ b/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 diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 4cca6b9b7..0fa181d7a 100644 --- a/app/controllers/sessions_controller.rb +++ b/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'] || - request.env['HTTP_REMOTE_USER'] || - request.headers['X-Forwarded-User'] + login = request.env['REMOTE_USER'] || + request.env['HTTP_REMOTE_USER'] || + request.headers['X-Forwarded-User'] - User.lookup(login: login&.downcase) - end + User.lookup(login: login&.downcase) + 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? diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 1f51e5b36..a90f4baa5 100644 --- a/app/controllers/tickets_controller.rb +++ b/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 diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index b06478fe3..fdb8eab86 100644 --- a/app/controllers/users_controller.rb +++ b/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, ) diff --git a/app/jobs/concerns/has_active_job_lock.rb b/app/jobs/concerns/has_active_job_lock.rb index 563e7a1ef..0fba6c228 100644 --- a/app/jobs/concerns/has_active_job_lock.rb +++ b/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 - yield - end + ActiveJobLock.transaction(isolation: :serializable, &block) rescue ActiveRecord::SerializationFailure => e # PostgeSQL prevents locking on records that are already locked # for UPDATE in Serializable Isolation Level transactions, diff --git a/app/jobs/ticket_user_ticket_counter_job.rb b/app/jobs/ticket_user_ticket_counter_job.rb index f6957315e..cf8fc28ed 100644 --- a/app/jobs/ticket_user_ticket_counter_job.rb +++ b/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 diff --git a/app/models/application_model/can_associations.rb b/app/models/application_model/can_associations.rb index 4aaa724fb..9604b7d16 100644 --- a/app/models/application_model/can_associations.rb +++ b/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] diff --git a/app/models/application_model/checks_attribute_values_and_length.rb b/app/models/application_model/checks_attribute_values_and_length.rb index 2f37dff5d..770d2c443 100644 --- a/app/models/application_model/checks_attribute_values_and_length.rb +++ b/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 Rails.application.config.db_null_byte == false - self[name].delete!("\u0000") - end + if column.type == :text && Rails.application.config.db_null_byte == false + self[name].delete!("\u0000") end # for varchar check length and replace null bytes diff --git a/app/models/application_model/checks_user_columns_fillup.rb b/app/models/application_model/checks_user_columns_fillup.rb index 3563e8f7d..bfe558524 100644 --- a/app/models/application_model/checks_user_columns_fillup.rb +++ b/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 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}" - end - self.updated_by_id = UserInfo.current_user_id + if self.class.column_names.include?('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}" 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 diff --git a/app/models/authorization.rb b/app/models/authorization.rb index 9fdc3f339..781f15b14 100644 --- a/app/models/authorization.rb +++ b/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 hash['info'] && hash['info']['email'].present? - user = User.find_by(email: hash['info']['email'].downcase) - end + if !user && Setting.get('auth_third_party_auto_link_at_inital_login') && hash['info'] && hash['info']['email'].present? + user = User.find_by(email: hash['info']['email'].downcase) end if !user diff --git a/app/models/avatar.rb b/app/models/avatar.rb index fbd827e81..4f58bce3a 100644 --- a/app/models/avatar.rb +++ b/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.updated_at > 2.minutes.ago - end + return if avatar_already_exists&.source_url == url && avatar_already_exists.updated_at > 2.minutes.ago # 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.updated_at > 2.minutes.ago - end + return if avatar_already_exists&.source_url == url && avatar_already_exists.updated_at > 2.minutes.ago # fetch image image = Service::Image.user(url) diff --git a/app/models/calendar.rb b/app/models/calendar.rb index 5b866cd94..e16dfc134 100644 --- a/app/models/calendar.rb +++ b/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 diff --git a/app/models/channel.rb b/app/models/channel.rb index 7f91cfeca..e65576c74 100644 --- a/app/models/channel.rb +++ b/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 = {} diff --git a/app/models/channel/driver/imap.rb b/app/models/channel/driver/imap.rb index b9a3886f7..c834e404f 100644 --- a/app/models/channel/driver/imap.rb +++ b/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 channel_has_changed?(channel) - end + break if (count % active_check_interval).zero? && channel_has_changed?(channel) 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[:channel_id] != channel[:id] - end + return false if ticket&.preferences && ticket.preferences[:channel_id].present? && channel.present? && ticket.preferences[:channel_id] != channel[:id] 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) - Timeout.timeout(seconds) do - yield - end + def timeout(seconds, &block) + Timeout.timeout(seconds, &block) end end diff --git a/app/models/channel/driver/mail_stdin.rb b/app/models/channel/driver/mail_stdin.rb index d8aa0b849..6bb2f0ec4 100644 --- a/app/models/channel/driver/mail_stdin.rb +++ b/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 diff --git a/app/models/channel/driver/pop3.rb b/app/models/channel/driver/pop3.rb index b808cf0f2..c73231585 100644 --- a/app/models/channel/driver/pop3.rb +++ b/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 channel_has_changed?(channel) - end + break if (count % active_check_interval).zero? && channel_has_changed?(channel) 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 =~ /X-Zammad-Verify-Time:\s(.+?)\s/ - begin - verify_time = Time.zone.parse($1) - if verify_time > Time.zone.now - 30.minutes - info = " - ignore message #{count}/#{count_all} - because it's a verify message" - Rails.logger.info info - next - end - rescue => e - Rails.logger.error e + if mail.match?(/(X-Zammad-Ignore: true|X-Zammad-Verify: true)/) && mail =~ /X-Zammad-Verify-Time:\s(.+?)\s/ + begin + verify_time = Time.zone.parse($1) + if verify_time > Time.zone.now - 30.minutes + info = " - ignore message #{count}/#{count_all} - because it's a verify message" + Rails.logger.info info + next end + rescue => e + Rails.logger.error e end end diff --git a/app/models/channel/driver/sms/massenversand.rb b/app/models/channel/driver/sms/massenversand.rb index f1cbaacaa..c74a9e6e0 100644 --- a/app/models/channel/driver/sms/massenversand.rb +++ b/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 diff --git a/app/models/channel/driver/sms/twilio.rb b/app/models/channel/driver/sms/twilio.rb index 1ba4a7a80..941c037a0 100644 --- a/app/models/channel/driver/sms/twilio.rb +++ b/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['from'][0]['level'] == 'known' && preferences['from'][0]['object'] == 'User' - user = User.find_by(id: preferences['from'][0]['o_id']) - end + if preferences && preferences['from'] && preferences['from'][0] && preferences['from'][0]['level'] == 'known' && preferences['from'][0]['object'] == 'User' + user = User.find_by(id: preferences['from'][0]['o_id']) end end if !user diff --git a/app/models/channel/driver/smtp.rb b/app/models/channel/driver/smtp.rb index 05376feca..a1f32fe39 100644 --- a/app/models/channel/driver/smtp.rb +++ b/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[:port].to_i == 465 - options[:ssl] = true - end + if !options.key?(:ssl) && options[:port].to_i == 465 + options[:ssl] = true end if !options.key?(:domain) # set fqdn, if local fqdn - use domain of sender diff --git a/app/models/channel/driver/twitter.rb b/app/models/channel/driver/twitter.rb index 2c602ebe9..69c06b48d 100644 --- a/app/models/channel/driver/twitter.rb +++ b/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 diff --git a/app/models/channel/email_parser.rb b/app/models/channel/email_parser.rb index c466723b2..2cc8b07af 100644 --- a/app/models/channel/email_parser.rb +++ b/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: ' + e.inspect # rubocop:disable Rails/Output + p "ERROR: #{message}" # 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 - .select { |a| a.address.present? } - .partition { |a| a.address.match?(EMAIL_REGEX) } - .flatten.first - rescue Mail::Field::ParseError => e - STDOUT.puts e - end + Mail::AddressList.new(from).addresses + .select { |a| a.address.present? } + .partition { |a| a.address.match?(EMAIL_REGEX) } + .flatten.first + rescue Mail::Field::ParseError => e + $stdout.puts e + 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 - rescue Mail::UnknownEncodingType # see test/data/mail/mail043.box / issue #348 - message.body.raw_source - end + message.body.to_s + rescue Mail::UnknownEncodingType # see test/data/mail/mail043.box / issue #348 + message.body.raw_source + 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 headers_store['Content-ID'] =~ /(.+?)@.+?/i - filename = $1 - end + if filename.blank? && headers_store['Content-ID'].present? && headers_store['Content-ID'] =~ /(.+?)@.+?/i + filename = $1 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 diff --git a/app/models/channel/filter/bounce_delivery_permanent_failed.rb b/app/models/channel/filter/bounce_delivery_permanent_failed.rb index 59e7b5895..1c3c66dde 100644 --- a/app/models/channel/filter/bounce_delivery_permanent_failed.rb +++ b/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 = [] diff --git a/app/models/channel/filter/identify_sender.rb b/app/models/channel/filter/identify_sender.rb index 8a0345f00..1cdbcb855 100644 --- a/app/models/channel/filter/identify_sender.rb +++ b/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 mail[ :'x-zammad-ticket-create-article-sender' ] == 'Agent' + if !customer_user && Setting.get('postmaster_sender_is_agent_search_for_customer') == true && 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 - begin - to = :'raw-to' - if mail[to]&.addrs - items = mail[to].addrs - items.each do |item| + # skip if recipient is system email + next if EmailAddress.exists?(email: item.address.downcase) - # skip if recipient is system email - next if EmailAddress.exists?(email: item.address.downcase) - - customer_user = user_create( - login: item.address, - firstname: item.display_name, - email: item.address, - ) - break - end + customer_user = user_create( + login: item.address, + firstname: item.display_name, + email: item.address, + ) + break 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 diff --git a/app/models/channel/filter/monitoring_base.rb b/app/models/channel/filter/monitoring_base.rb index ec36b68da..3c20460cf 100644 --- a/app/models/channel/filter/monitoring_base.rb +++ b/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 mail[:body] =~ /.+?\sis\s(.+?)!/ - result['state'] = $1 - end + if result['state'].blank? && mail[:body] =~ /.+?\sis\s(.+?)!/ + result['state'] = $1 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 mail[:subject] =~ /(on|Host)\s.+?\sis\s(.+?)!/ - result['state'] = $2 - end + if result['state'].blank? && mail[:subject] =~ /(on|Host)\s.+?\sis\s(.+?)!/ + result['state'] = $2 end # monit - get missing attributes from body - if result['service'].blank? - if mail[:body] =~ /\sService\s(.+?)\s/ - result['service'] = $1 - end + if result['service'].blank? && mail[:body] =~ /\sService\s(.+?)\s/ + result['service'] = $1 end # possible event types https://mmonit.com/monit/documentation/#Setting-an-event-filter diff --git a/app/models/channel/filter/sender_is_system_address.rb b/app/models/channel/filter/sender_is_system_address.rb index 75fe818fa..ef2592532 100644 --- a/app/models/channel/filter/sender_is_system_address.rb +++ b/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 diff --git a/app/models/concerns/can_be_published.rb b/app/models/concerns/can_be_published.rb index 017009ff0..2c02bc4b2 100644 --- a/app/models/concerns/can_be_published.rb +++ b/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? + .exists?(knowledge_bases: { active: true }) end end diff --git a/app/models/concerns/can_clone_attachments.rb b/app/models/concerns/can_clone_attachments.rb index c77ca39c0..347ea22d4 100644 --- a/app/models/concerns/can_clone_attachments.rb +++ b/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 is_html_content == true + if options[:only_attached_attachments] == true && is_html_content == true - 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) - end + 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) end # only_inline_attachments mode is used when quoting HTML mail with #{article.body_as_html} diff --git a/app/models/concerns/can_csv_import.rb b/app/models/concerns/can_csv_import.rb index be81c8793..32e669c6f 100644 --- a/app/models/concerns/can_csv_import.rb +++ b/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| diff --git a/app/models/concerns/has_groups.rb b/app/models/concerns/has_groups.rb index 69659fd08..a2b9d68a7 100644 --- a/app/models/concerns/has_groups.rb +++ b/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 diff --git a/app/models/concerns/has_rich_text.rb b/app/models/concerns/has_rich_text.rb index 39a9e2154..7015239c5 100644 --- a/app/models/concerns/has_rich_text.rb +++ b/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' diff --git a/app/models/concerns/has_roles.rb b/app/models/concerns/has_roles.rb index 112748d7b..3acba8924 100644 --- a/app/models/concerns/has_roles.rb +++ b/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 diff --git a/app/models/cti/caller_id.rb b/app/models/cti/caller_id.rb index dfd7b65cb..c11dac400 100644 --- a/app/models/cti/caller_id.rb +++ b/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 diff --git a/app/models/cti/driver/base.rb b/app/models/cti/driver/base.rb index 8613a266a..f378463a3 100644 --- a/app/models/cti/driver/base.rb +++ b/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: { diff --git a/app/models/cti/driver/sipgate_io.rb b/app/models/cti/driver/sipgate_io.rb index bf95b3ad2..c4019e238 100644 --- a/app/models/cti/driver/sipgate_io.rb +++ b/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? diff --git a/app/models/cti/log.rb b/app/models/cti/log.rb index fe3ca2e2d..c7e4caca2 100644 --- a/app/models/cti/log.rb +++ b/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 diff --git a/app/models/email_address.rb b/app/models/email_address.rb index 060268c5f..5d86c579a 100644 --- a/app/models/email_address.rb +++ b/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 diff --git a/app/models/external_sync.rb b/app/models/external_sync.rb index c3ac2c7e9..faf951787 100644 --- a/app/models/external_sync.rb +++ b/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.attributes.key?(segment) - end + break if information_source.respond_to?(:attributes) && !information_source.attributes.key?(segment) + 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 diff --git a/app/models/knowledge_base.rb b/app/models/knowledge_base.rb index 4c61ac86d..b6d235a3e 100644 --- a/app/models/knowledge_base.rb +++ b/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 diff --git a/app/models/knowledge_base/answer.rb b/app/models/knowledge_base/answer.rb index 61b253232..a97569ddb 100644 --- a/app/models/knowledge_base/answer.rb +++ b/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 diff --git a/app/models/knowledge_base/answer/translation/content.rb b/app/models/knowledge_base/answer/translation/content.rb index 82d57f97f..b2d925e97 100644 --- a/app/models/knowledge_base/answer/translation/content.rb +++ b/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 diff --git a/app/models/object_manager/element/backend.rb b/app/models/object_manager/element/backend.rb index d84cbbdf1..322012fe6 100644 --- a/app/models/object_manager/element/backend.rb +++ b/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 diff --git a/app/models/observer/ticket/article_changes.rb b/app/models/observer/ticket/article_changes.rb index 01c8c80e0..127009946 100644 --- a/app/models/observer/ticket/article_changes.rb +++ b/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 diff --git a/app/models/observer/ticket/last_owner_update.rb b/app/models/observer/ticket/last_owner_update.rb index 867adc0b5..4e113e77a 100644 --- a/app/models/observer/ticket/last_owner_update.rb +++ b/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 diff --git a/app/models/observer/ticket/ref_object_touch.rb b/app/models/observer/ticket/ref_object_touch.rb index ec415216f..f59730db9 100644 --- a/app/models/observer/ticket/ref_object_touch.rb +++ b/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[0] - User.find(cutomer_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations - end + if cutomer_id_changed && cutomer_id_changed[0] != cutomer_id_changed[1] && cutomer_id_changed[0] + User.find(cutomer_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations 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[0] - Organization.find(organization_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations - end + if organization_id_changed && organization_id_changed[0] != organization_id_changed[1] && organization_id_changed[0] + Organization.find(organization_id_changed[0]).touch # rubocop:disable Rails/SkipsModelValidations end # touch new/current organization diff --git a/app/models/package.rb b/app/models/package.rb index 835f5ac83..98e333f22 100644 --- a/app/models/package.rb +++ b/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.exist?(dest.to_s) - logger.info "Create dir: #{dest}" - FileUtils.mkdir_p(dest.to_s) - end + if File.directory?(entry.to_s) && !File.exist?(dest.to_s) + logger.info "Create dir: #{dest}" + FileUtils.mkdir_p(dest.to_s) 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) diff --git a/app/models/recent_view.rb b/app/models/recent_view.rb index af190aece..52391aca4 100644 --- a/app/models/recent_view.rb +++ b/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) } diff --git a/app/models/role.rb b/app/models/role.rb index 015acdeac..97a18ef22 100644 --- a/app/models/role.rb +++ b/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 diff --git a/app/models/scheduler.rb b/app/models/scheduler.rb index 222d27654..ba14ddda2 100644 --- a/app/models/scheduler.rb +++ b/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 diff --git a/app/models/setting.rb b/app/models/setting.rb index ca18f9a07..a3a2658f5 100644 --- a/app/models/setting.rb +++ b/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 diff --git a/app/models/store.rb b/app/models/store.rb index fc2d0a4e7..4667215e1 100644 --- a/app/models/store.rb +++ b/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 diff --git a/app/models/store/provider/file.rb b/app/models/store/provider/file.rb index 11e2c10c8..cc3a4fd5d 100644 --- a/app/models/store/provider/file.rb +++ b/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}" diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 5e8273b8c..faed6e6e3 100644 --- a/app/models/ticket.rb +++ b/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' - if user_id_check - return false if user_id_check == owner_id && user_id_check != updated_by_id - end - end + return false if state_type.name != 'merged' && user_id_check && user_id_check == owner_id && user_id_check != updated_by_id # 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)) - rescue ArgumentError - nil - end + Ticket::Article.find_by(id: item.try(:dig, :article_id)) + rescue ArgumentError + nil + end # if the configuration contains the deletion of the ticket then # we skip all other ticket changes because they does not matter diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 9576a24f7..e0b24c27c 100644 --- a/app/models/ticket/article.rb +++ b/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 diff --git a/app/models/ticket/escalation.rb b/app/models/ticket/escalation.rb index 454b4f2f3..c52ce7031 100644 --- a/app/models/ticket/escalation.rb +++ b/app/models/ticket/escalation.rb @@ -136,15 +136,15 @@ returns close_at_changed = false end - if !force && preferences[:escalation_calculation] - if first_response_at_changed == false && - last_update_at_changed == false && - close_at_changed == false && - sla_changed == false && - calendar_changed == false && - escalation_calculation['escalation_disabled'] == escalation_disabled - return false - end + if !force && + preferences[:escalation_calculation] && + first_response_at_changed == false && + last_update_at_changed == false && + close_at_changed == false && + sla_changed == false && + calendar_changed == false && + escalation_calculation['escalation_disabled'] == escalation_disabled + return false end # reset escalation attributes diff --git a/app/models/ticket/screen_options.rb b/app/models/ticket/screen_options.rb index df2c0d07e..3edf07851 100644 --- a/app/models/ticket/screen_options.rb +++ b/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| diff --git a/app/models/ticket/subject.rb b/app/models/ticket/subject.rb index 2ab98943c..802b688c2 100644 --- a/app/models/ticket/subject.rb +++ b/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! diff --git a/app/models/token.rb b/app/models/token.rb index b7acdb2d3..5a1de2b4d 100644 --- a/app/models/token.rb +++ b/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 token.persistent && user.active == false - end + return if !data[:inactive_user] && token.persistent && user.active == false # add permission check - if data[:permission] - return if !token.permissions?(data[:permission]) - end + return if data[:permission] && !token.permissions?(data[:permission]) # return token user user diff --git a/app/models/transaction/notification.rb b/app/models/transaction/notification.rb index 022abab92..672db04c6 100644 --- a/app/models/transaction/notification.rb +++ b/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 diff --git a/app/models/translation.rb b/app/models/translation.rb index 41b665fc4..868c9e802 100644 --- a/app/models/translation.rb +++ b/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 diff --git a/app/models/user.rb b/app/models/user.rb index b326acc31..f78aa5f8b 100644 --- a/app/models/user.rb +++ b/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 id - roles.each do |role| - check_notifications(role, false) - end + if @preferences_default.blank? && id + roles.each do |role| + check_notifications(role, false) 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['email'][0] == login - self.login = email - end + if changes && changes['email'] && changes['email'][0] == login + self.login = email 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) diff --git a/app/models/user/assets.rb b/app/models/user/assets.rb index 8c456b3d6..cfcb407a4 100644 --- a/app/models/user/assets.rb +++ b/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) diff --git a/app/models/user_device.rb b/app/models/user_device.rb index 834de2417..f63264fee 100644 --- a/app/models/user_device.rb +++ b/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 - end + return action(user_device.id, user_agent, ip, user_id, type) if user_device # create new device user_device = create!( diff --git a/app/policies/controllers/application_controller_policy.rb b/app/policies/controllers/application_controller_policy.rb index 378699a6c..dfaf2f0a4 100644 --- a/app/policies/controllers/application_controller_policy.rb +++ b/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 diff --git a/app/policies/ticket/article_policy.rb b/app/policies/ticket/article_policy.rb index 79bb19314..31309643b 100644 --- a/app/policies/ticket/article_policy.rb +++ b/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 diff --git a/config/environments/production.rb b/config/environments/production.rb index 2210cfbc8..873d86c41 100644 --- a/config/environments/production.rb +++ b/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 diff --git a/db/migrate/20170905140038_cti_log_preferences_migration.rb b/db/migrate/20170905140038_cti_log_preferences_migration.rb index 478ed41d0..69ac51a9a 100644 --- a/db/migrate/20170905140038_cti_log_preferences_migration.rb +++ b/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 diff --git a/db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb b/db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb index 642d2aae9..327b951ab 100644 --- a/db/migrate/20190408000001_issue_2541_fix_notification_email_without_body.rb +++ b/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? diff --git a/lib/auto_wizard.rb b/lib/auto_wizard.rb index 347e8ba2d..a12b8964b 100644 --- a/lib/auto_wizard.rb +++ b/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']['Ip'] - Calendar.init_setup(auto_wizard_hash['CalendarSetup']['Ip']) - end + if auto_wizard_hash['CalendarSetup'] && auto_wizard_hash['CalendarSetup']['Ip'] + Calendar.init_setup(auto_wizard_hash['CalendarSetup']['Ip']) end # load text modules - if auto_wizard_hash['TextModuleLocale'] - if auto_wizard_hash['TextModuleLocale']['Locale'] - begin - TextModule.load(auto_wizard_hash['TextModuleLocale']['Locale']) - rescue => e - Rails.logger.error "Unable to load text modules #{auto_wizard_hash['TextModuleLocale']['Locale']}: #{e.message}" - end + if auto_wizard_hash['TextModuleLocale'] && auto_wizard_hash['TextModuleLocale']['Locale'] + begin + TextModule.load(auto_wizard_hash['TextModuleLocale']['Locale']) + rescue => e + Rails.logger.error "Unable to load text modules #{auto_wizard_hash['TextModuleLocale']['Locale']}: #{e.message}" end end diff --git a/lib/calendar_subscriptions.rb b/lib/calendar_subscriptions.rb index 2569aa735..06689f10c 100644 --- a/lib/calendar_subscriptions.rb +++ b/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 diff --git a/lib/core_ext/activesupport/lib/active_support/tagged_logging/formatter.rb b/lib/core_ext/activesupport/lib/active_support/tagged_logging/formatter.rb index 87d0a7286..7341f1a16 100644 --- a/lib/core_ext/activesupport/lib/active_support/tagged_logging/formatter.rb +++ b/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 diff --git a/lib/core_ext/open-uri.rb b/lib/core_ext/open-uri.rb index 26457c35d..d6c46ef4d 100644 --- a/lib/core_ext/open-uri.rb +++ b/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 diff --git a/lib/core_ext/string.rb b/lib/core_ext/string.rb index 732f67991..79974cf89 100644 --- a/lib/core_ext/string.rb +++ b/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{]*)>(.+?)}m) do - "\n" + $2.html2text(true).gsub(/^(.*)$/, '> \1') + "\n" + "\n#{$2.html2text(true).gsub(/^(.*)$/, '> \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 diff --git a/lib/email_helper.rb b/lib/email_helper.rb index 4427079dc..0d78f11e4 100644 --- a/lib/email_helper.rb +++ b/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 diff --git a/lib/email_helper/probe.rb b/lib/email_helper/probe.rb index d5a0a4e85..5506a7f07 100644 --- a/lib/email_helper/probe.rb +++ b/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| diff --git a/lib/email_helper/verify.rb b/lib/email_helper/verify.rb index db28ca11c..0b80a54d7 100644 --- a/lib/email_helper/verify.rb +++ b/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' diff --git a/lib/excel_sheet.rb b/lib/excel_sheet.rb index ecf87b012..6a0fd54ec 100644 --- a/lib/excel_sheet.rb +++ b/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 diff --git a/lib/excel_sheet/ticket.rb b/lib/excel_sheet/ticket.rb index 3404431ca..cf2a802bd 100644 --- a/lib/excel_sheet/ticket.rb +++ b/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, diff --git a/lib/external_credential/twitter.rb b/lib/external_credential/twitter.rb index d4f63dd9b..960db462d 100644 --- a/lib/external_credential/twitter.rb +++ b/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 diff --git a/lib/facebook.rb b/lib/facebook.rb index a2cbe505d..aa09fe01e 100644 --- a/lib/facebook.rb +++ b/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 diff --git a/lib/html_sanitizer.rb b/lib/html_sanitizer.rb index 6316a5d96..c645a0656 100644 --- a/lib/html_sanitizer.rb +++ b/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['title'].blank? - node['title'] = node['href'] - end + if node.name == 'a' && !url_same?(node['href'], node.text) && node['title'].blank? + node['title'] = node['href'] 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[node.name].include?(key) + next if css_properties_whitelist.exclude?(node.name) + 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" diff --git a/lib/import/exchange/item_attributes.rb b/lib/import/exchange/item_attributes.rb index 2a20f2fcf..1a918b98a 100644 --- a/lib/import/exchange/item_attributes.rb +++ b/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}" diff --git a/lib/import/integration_base.rb b/lib/import/integration_base.rb index eeac68182..536a7f03d 100644 --- a/lib/import/integration_base.rb +++ b/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 diff --git a/lib/import/otrs.rb b/lib/import/otrs.rb index 2d75aaca2..50ab67ac4 100644 --- a/lib/import/otrs.rb +++ b/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 diff --git a/lib/import/otrs/history_factory.rb b/lib/import/otrs/history_factory.rb index 81f6c9483..729ee90f1 100644 --- a/lib/import/otrs/history_factory.rb +++ b/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 diff --git a/lib/import/otrs/requester.rb b/lib/import/otrs/requester.rb index d9f6415cd..f96f0872a 100644 --- a/lib/import/otrs/requester.rb +++ b/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 'PARAMS: ' + params.inspect + log "POST: #{url}" + log "PARAMS: #{params.inspect}" response = UserAgent.post( url, diff --git a/lib/import/otrs/state_factory.rb b/lib/import/otrs/state_factory.rb index f62f9b8ce..61bdea3ff 100644 --- a/lib/import/otrs/state_factory.rb +++ b/lib/import/otrs/state_factory.rb @@ -13,7 +13,7 @@ module Import def backup # rename states to handle not uniq issues ::Ticket::State.all.each do |state| - state.name = state.name + '_tmp' + state.name = "#{state.name}_tmp" state.save end end diff --git a/lib/import/otrs/sys_config_factory.rb b/lib/import/otrs/sys_config_factory.rb index 34b92933d..52efd0f03 100644 --- a/lib/import/otrs/sys_config_factory.rb +++ b/lib/import/otrs/sys_config_factory.rb @@ -24,7 +24,7 @@ module Import def direct_copy?(setting) cleaned_name = cleanup_name(setting['Key']) - return false if !direct_settings.include?(cleaned_name) + return false if direct_settings.exclude?(cleaned_name) internal_name = cleaned_name.underscore Setting.set(internal_name, setting['Value']) @@ -54,7 +54,7 @@ module Import def postmaster_default?(setting) relevant_configs = %w[PostmasterDefaultPriority PostmasterDefaultState PostmasterFollowUpState] - return false if !relevant_configs.include?(setting['Key']) + return false if relevant_configs.exclude?(setting['Key']) map = { 'PostmasterDefaultPriority' => :priority_default_create, diff --git a/lib/import/otrs/user.rb b/lib/import/otrs/user.rb index 62fc32917..b76c84e9d 100644 --- a/lib/import/otrs/user.rb +++ b/lib/import/otrs/user.rb @@ -102,7 +102,7 @@ module Import permissions ||= user['GroupIDs'][ queue['GroupID'].to_s ] next if !permissions - next if !permissions.include?('rw') + next if permissions.exclude?('rw') result.push queue['QueueID'] end @@ -172,7 +172,7 @@ module Import result = [] roles = Import::OTRS::Requester.load('Role') roles.each do |role| - next if !user['RoleIDs'].include?(role['ID']) + next if user['RoleIDs'].exclude?(role['ID']) result += groups_from_otrs_groups(role) end diff --git a/lib/knowledge_base/menu_item_update_action.rb b/lib/knowledge_base/menu_item_update_action.rb index 902aff9b1..f746a575c 100644 --- a/lib/knowledge_base/menu_item_update_action.rb +++ b/lib/knowledge_base/menu_item_update_action.rb @@ -33,7 +33,7 @@ class KnowledgeBase params .map { |location_params| update_location_using_params! knowledge_base, location_params } .map(&:reload) - .reduce(:+) + .sum end # Mass-update KB menu items in a given location @@ -77,8 +77,8 @@ class KnowledgeBase def remove_deleted @menu_items_data .select { |elem| elem[:_destroy] } - .map { |elem| elem[:id] } - .tap { |array| @kb_locale.menu_items.where(id: array).destroy_all } + .pluck(:id) + .tap { |array| @kb_locale.menu_items.where(id: array).destroy_all } end def all_ids_present? diff --git a/lib/knowledge_base/server_snippet.rb b/lib/knowledge_base/server_snippet.rb index 2e82bee22..0640a614f 100644 --- a/lib/knowledge_base/server_snippet.rb +++ b/lib/knowledge_base/server_snippet.rb @@ -8,7 +8,7 @@ class KnowledgeBase raise Exceptions::UnprocessableEntity, 'Custom address is not set' if @kb.custom_address_uri.nil? template_rewrite = host.present? ? template_full : template_path - template_rewrite + "\n" + template_original_url + "#{template_rewrite}\n#{template_original_url}" end def host diff --git a/lib/ldap.rb b/lib/ldap.rb index 4599f7203..f11c4bd7d 100644 --- a/lib/ldap.rb +++ b/lib/ldap.rb @@ -64,7 +64,7 @@ class Ldap # #=> # # @return [true] Returns always true - def search(filter, base: nil, scope: nil, attributes: nil) + def search(filter, base: nil, scope: nil, attributes: nil, &block) base ||= base_dn() scope ||= Net::LDAP::SearchScope_WholeSubtree @@ -75,9 +75,8 @@ class Ldap scope: scope, attributes: attributes, return_result: false, # improves performance - ) do |entry| - yield entry - end + &block + ) end # Checks if there are any entries for the given search criteria. diff --git a/lib/models.rb b/lib/models.rb index d0d3120b0..303e6e8ed 100644 --- a/lib/models.rb +++ b/lib/models.rb @@ -44,7 +44,7 @@ returns next if !model_class.respond_to? :table_name table_name = model_class.table_name # handle models where not table exists, pending migrations - next if !tables.include?(table_name) + next if tables.exclude?(table_name) model_object = model_class.new next if !model_object.respond_to? :attributes @@ -140,7 +140,7 @@ returns next if !model_attributes[:attributes] ref_attributes.each do |item| - next if !model_attributes[:attributes].include?(item) + next if model_attributes[:attributes].exclude?(item) count = model_class.where("#{item} = ?", object_id).count next if count.zero? && !include_zero @@ -154,7 +154,7 @@ returns end # find relations via reflections - list.each do |model_class, model_attributes| + list.each do |model_class, model_attributes| # rubocop:disable Style/CombinableLoops next if !model_attributes[:reflections] model_attributes[:reflections].each_value do |reflection_value| diff --git a/lib/notification_factory/renderer.rb b/lib/notification_factory/renderer.rb index f2aca9b91..ad34237da 100644 --- a/lib/notification_factory/renderer.rb +++ b/lib/notification_factory/renderer.rb @@ -25,7 +25,7 @@ examples how to use =end - def initialize(objects:, locale: nil, timezone: nil, template:, escape: true) + def initialize(objects:, template:, locale: nil, timezone: nil, escape: true) @objects = objects @locale = locale || Locale.default @timezone = timezone || Setting.get('timezone_default') diff --git a/lib/notification_factory/template.rb b/lib/notification_factory/template.rb index 945eba14d..f322dd7d3 100644 --- a/lib/notification_factory/template.rb +++ b/lib/notification_factory/template.rb @@ -20,7 +20,7 @@ examples how to use @template.gsub(/\#{\s*(.*?)\s*}/m) do # some browsers start adding HTML tags # fixes https://github.com/zammad/zammad/issues/385 - input_template = $1.gsub(/\A<.+?>\s*|\s*<.+?>\z/, '') + input_template = $1.gsub(/\A<.+?>\s*|\s*<.+?>\z/, '') # rubocop:disable Lint/OutOfRangeRegexpRef case input_template when /\At\('(.+?)'\)\z/m diff --git a/lib/search_index_backend.rb b/lib/search_index_backend.rb index 195dda2b9..6c8da006d 100644 --- a/lib/search_index_backend.rb +++ b/lib/search_index_backend.rb @@ -728,7 +728,7 @@ generate url for index or document access (only for internal use) # prepare url params params_string = '' if url_params.present? - params_string = '?' + url_params.map { |key, value| "#{key}=#{value}" }.join('&') + params_string = "?#{URI.encode_www_form(url_params)}" end url = Setting.get('es_url') @@ -755,7 +755,7 @@ generate url for index or document access (only for internal use) "#{url}#{params_string}" end - def self.humanized_error(verb:, url:, payload: nil, response:) + def self.humanized_error(verb:, url:, response:, payload: nil) prefix = "Unable to process #{verb} request to elasticsearch URL '#{url}'." suffix = "\n\nResponse:\n#{response.inspect}\n\n" @@ -782,7 +782,7 @@ generate url for index or document access (only for internal use) # add * on simple query like "somephrase23" def self.append_wildcard_to_simple_query(query) query.strip! - query += '*' if !query.include?(':') + query += '*' if query.exclude?(':') query end @@ -817,7 +817,7 @@ generate url for index or document access (only for internal use) } } - if (extension = options.dig(:query_extension)) + if (extension = options[:query_extension]) data[:query].deep_merge! extension.deep_dup end diff --git a/lib/search_knowledge_base_backend.rb b/lib/search_knowledge_base_backend.rb index 91d0360cc..2ad02d92e 100644 --- a/lib/search_knowledge_base_backend.rb +++ b/lib/search_knowledge_base_backend.rb @@ -207,7 +207,7 @@ class SearchKnowledgeBaseBackend end def visible_translation?(translation, visibility) - if !kb_locales_in(translation.category.knowledge_base_id).include? translation.kb_locale + if kb_locales_in(translation.category.knowledge_base_id).exclude?(translation.kb_locale) return false end diff --git a/lib/secure_mailing/smime/incoming.rb b/lib/secure_mailing/smime/incoming.rb index d1624a95d..1af48377b 100644 --- a/lib/secure_mailing/smime/incoming.rb +++ b/lib/secure_mailing/smime/incoming.rb @@ -6,6 +6,8 @@ class SecureMailing::SMIME::Incoming < SecureMailing::Backend::Handler OPENSSL_PKCS7_VERIFY_FLAGS = OpenSSL::PKCS7::NOVERIFY | OpenSSL::PKCS7::NOINTERN def initialize(mail) + super() + @mail = mail @content_type = @mail[:mail_instance].content_type end diff --git a/lib/secure_mailing/smime/outgoing.rb b/lib/secure_mailing/smime/outgoing.rb index 0535e8154..af231a2b7 100644 --- a/lib/secure_mailing/smime/outgoing.rb +++ b/lib/secure_mailing/smime/outgoing.rb @@ -1,6 +1,8 @@ class SecureMailing::SMIME::Outgoing < SecureMailing::Backend::Handler def initialize(mail, security) + super() + @mail = mail @security = security end diff --git a/lib/secure_mailing/smime/retry.rb b/lib/secure_mailing/smime/retry.rb index eaff22bac..60f2d3416 100644 --- a/lib/secure_mailing/smime/retry.rb +++ b/lib/secure_mailing/smime/retry.rb @@ -1,6 +1,7 @@ class SecureMailing::SMIME::Retry < SecureMailing::Backend::Handler def initialize(article) + super() @article = article end diff --git a/lib/sequencer/unit/common/mixin/dynamic_attribute.rb b/lib/sequencer/unit/common/mixin/dynamic_attribute.rb index 4481129c0..ab7009dcb 100644 --- a/lib/sequencer/unit/common/mixin/dynamic_attribute.rb +++ b/lib/sequencer/unit/common/mixin/dynamic_attribute.rb @@ -9,6 +9,8 @@ class Sequencer class << base def inherited(base) + super + base.extend(Forwardable) base.instance_delegate [:attribute] => base end diff --git a/lib/sequencer/unit/common/provider/named.rb b/lib/sequencer/unit/common/provider/named.rb index b2c3e1a51..984405237 100644 --- a/lib/sequencer/unit/common/provider/named.rb +++ b/lib/sequencer/unit/common/provider/named.rb @@ -11,6 +11,8 @@ class Sequencer end def self.inherited(base) + super + base.extend(ClassMethods) base.provides(base.named_provide) diff --git a/lib/sequencer/unit/import/exchange/attribute_examples.rb b/lib/sequencer/unit/import/exchange/attribute_examples.rb index 7af6e8438..5559ed6d6 100644 --- a/lib/sequencer/unit/import/exchange/attribute_examples.rb +++ b/lib/sequencer/unit/import/exchange/attribute_examples.rb @@ -20,7 +20,7 @@ class Sequencer break if extractor.enough end rescue NoMethodError => e - raise if !e.message.include?('Viewpoint::EWS::') + raise if e.message.exclude?('Viewpoint::EWS::') logger.error e logger.error "Skipping folder_id '#{folder_id}' due to unsupported entries." diff --git a/lib/sequencer/unit/import/zendesk/sub_sequence/mapped.rb b/lib/sequencer/unit/import/zendesk/sub_sequence/mapped.rb index 331140edd..8e1537448 100644 --- a/lib/sequencer/unit/import/zendesk/sub_sequence/mapped.rb +++ b/lib/sequencer/unit/import/zendesk/sub_sequence/mapped.rb @@ -11,6 +11,8 @@ class Sequencer end def inherited(base) + super + base.provides(base.resource_map) base.extend(Forwardable) diff --git a/lib/sequencer/unit/import/zendesk/sub_sequence/sub_object.rb b/lib/sequencer/unit/import/zendesk/sub_sequence/sub_object.rb index 454bf0b9c..021a9a1ae 100644 --- a/lib/sequencer/unit/import/zendesk/sub_sequence/sub_object.rb +++ b/lib/sequencer/unit/import/zendesk/sub_sequence/sub_object.rb @@ -9,6 +9,8 @@ class Sequencer uses :resource, :instance, :user_id, :model_class, :action def self.inherited(subclass) + super + subclass.prepend(::Sequencer::Unit::Import::Common::Model::Mixin::Skip::Action) subclass.skip_action(:skipped, :failed) end diff --git a/lib/sessions.rb b/lib/sessions.rb index 05ef88f54..0845c4f3b 100644 --- a/lib/sessions.rb +++ b/lib/sessions.rb @@ -399,9 +399,8 @@ broadcase also not to sender next if session[:user]['id'].blank? end - if sender_user_id - next if session[:user] && session[:user]['id'] && session[:user]['id'].to_i == sender_user_id.to_i - end + next if sender_user_id && session[:user] && session[:user]['id'] && session[:user]['id'].to_i == sender_user_id.to_i + Sessions.send(client_id, data) recipients.push client_id end @@ -634,7 +633,7 @@ delete spool messages ::Sessions.jobs(worker_node_id) sleep node_count - rescue Interrupt # rubocop:disable Layout/RescueEnsureAlignment + rescue Interrupt nil end end diff --git a/lib/sessions/backend/activity_stream.rb b/lib/sessions/backend/activity_stream.rb index 2901fb225..11b70eae1 100644 --- a/lib/sessions/backend/activity_stream.rb +++ b/lib/sessions/backend/activity_stream.rb @@ -2,7 +2,7 @@ class Sessions::Backend::ActivityStream < Sessions::Backend::Base attr_writer :user - def initialize(user, asset_lookup, client = nil, client_id = nil, ttl = 25) + def initialize(user, asset_lookup, client = nil, client_id = nil, ttl = 25) # rubocop:disable Lint/MissingSuper @user = user @client = client @client_id = client_id diff --git a/lib/sessions/backend/ticket_overview_list.rb b/lib/sessions/backend/ticket_overview_list.rb index 43102e9ce..2a96b7795 100644 --- a/lib/sessions/backend/ticket_overview_list.rb +++ b/lib/sessions/backend/ticket_overview_list.rb @@ -4,7 +4,7 @@ class Sessions::Backend::TicketOverviewList < Sessions::Backend::Base Cache.write("TicketOverviewPull::#{user_id}", { needed: true }) end - def initialize(user, asset_lookup, client = nil, client_id = nil, ttl = 7) + def initialize(user, asset_lookup, client = nil, client_id = nil, ttl = 7) # rubocop:disable Lint/MissingSuper @user = user @client = client @client_id = client_id diff --git a/lib/sessions/event/base.rb b/lib/sessions/event/base.rb index 1469be44a..4d8fed891 100644 --- a/lib/sessions/event/base.rb +++ b/lib/sessions/event/base.rb @@ -21,6 +21,7 @@ class Sessions::Event::Base end def self.inherited(subclass) + super subclass.instance_variable_set(:@database_connection, @database_connection) end @@ -131,9 +132,8 @@ class Sessions::Event::Base end def log(level, data, client_id = nil) - if !@options[:v] - return if level == 'debug' - end + return if !@options[:v] && level == 'debug' + if !client_id client_id = @client_id end diff --git a/lib/stats.rb b/lib/stats.rb index c691cbfca..6be0fdda6 100644 --- a/lib/stats.rb +++ b/lib/stats.rb @@ -37,7 +37,7 @@ returns backend = stats_item.state_current[:value] if !backend - raise 'Dashboard::Stats backend ' + stats_item.name + ' is not defined' + raise "Dashboard::Stats backend #{stats_item.name} is not defined" end require_dependency backend.to_filename diff --git a/lib/stats/ticket_channel_distribution.rb b/lib/stats/ticket_channel_distribution.rb index 88919852f..cad03db9c 100644 --- a/lib/stats/ticket_channel_distribution.rb +++ b/lib/stats/ticket_channel_distribution.rb @@ -40,7 +40,7 @@ class Stats::TicketChannelDistribution ) end - if Channel.where(area: 'Sms::Account').exists? + if Channel.exists?(area: 'Sms::Account') channels.push( { sender: 'sms', @@ -49,7 +49,7 @@ class Stats::TicketChannelDistribution ) end - if Channel.where(area: 'Twitter::Account').exists? + if Channel.exists?(area: 'Twitter::Account') channels.push( { sender: 'twitter', @@ -58,7 +58,7 @@ class Stats::TicketChannelDistribution ) end - if Channel.where(area: 'Facebook::Account').exists? + if Channel.exists?(area: 'Facebook::Account') channels.push( { sender: 'facebook', @@ -67,7 +67,7 @@ class Stats::TicketChannelDistribution ) end - if Channel.where(area: 'Telegram::Account').exists? + if Channel.exists?(area: 'Telegram::Account') channels.push( { sender: 'telegram', @@ -111,7 +111,7 @@ class Stats::TicketChannelDistribution end # append in percent - channels.each do |channel| + channels.each do |channel| # rubocop:disable Style/CombinableLoops count = result[channel[:sender].to_sym][:inbound] #puts "#{channel.inspect}:in/#{result.inspect}:#{count}" in_process_precent = if count.zero? diff --git a/lib/stats/ticket_load_measure.rb b/lib/stats/ticket_load_measure.rb index 0e1347b71..f3fc66037 100644 --- a/lib/stats/ticket_load_measure.rb +++ b/lib/stats/ticket_load_measure.rb @@ -37,7 +37,7 @@ class Stats::TicketLoadMeasure return result if !result.key?(:used_for_average) - if result[:total] < 1 || result[:average_per_agent] == 0.0 + if result[:total] < 1 || result[:average_per_agent].to_d == 0.0.to_d result[:state] = 'supergood' return result end diff --git a/lib/stats/ticket_reopen.rb b/lib/stats/ticket_reopen.rb index 6c1f9ac70..da18c7428 100644 --- a/lib/stats/ticket_reopen.rb +++ b/lib/stats/ticket_reopen.rb @@ -41,7 +41,7 @@ class Stats::TicketReopen return result if !result.key?(:used_for_average) - if result[:total] < 1 || result[:average_per_agent] == 0.0 + if result[:total] < 1 || result[:average_per_agent].to_d == 0.0.to_d result[:state] = 'supergood' return result end diff --git a/lib/tasks/zammad/setup/db_config.rake b/lib/tasks/zammad/setup/db_config.rake index 4f27bd69d..a3d96a40b 100644 --- a/lib/tasks/zammad/setup/db_config.rake +++ b/lib/tasks/zammad/setup/db_config.rake @@ -15,7 +15,7 @@ namespace :zammad do next if FileUtils.identical?(template, destination) printf 'config/database.yml: File exists. Overwrite? [y/N] ' - next if !STDIN.gets.chomp.casecmp('y').zero? + next if !$stdin.gets.chomp.casecmp('y').zero? end FileUtils.cp(template, destination) diff --git a/lib/telegram.rb b/lib/telegram.rb index c256870db..04fa6f971 100644 --- a/lib/telegram.rb +++ b/lib/telegram.rb @@ -65,10 +65,8 @@ returns # verify token bot = Telegram.check_token(token) - if !channel - if Telegram.bot_duplicate?(bot['id']) - raise Exceptions::UnprocessableEntity, 'Bot already exists!' - end + if !channel && Telegram.bot_duplicate?(bot['id']) + raise Exceptions::UnprocessableEntity, 'Bot already exists!' end if params[:group_id].blank? @@ -704,9 +702,7 @@ returns end # prevent multiple update - if !params[:edited_message] - return if Ticket::Article.exists?(message_id: Telegram.message_id(params)) - end + return if !params[:edited_message] && Ticket::Article.exists?(message_id: Telegram.message_id(params)) # update article if params[:edited_message] diff --git a/lib/twitter_sync.rb b/lib/twitter_sync.rb index d0850fc16..d50e7df36 100644 --- a/lib/twitter_sync.rb +++ b/lib/twitter_sync.rb @@ -549,8 +549,9 @@ create a tweet or direct message from an article # no changes in post is from page user it self if channel.options[:user][:id].to_s == user_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 diff --git a/lib/user_agent.rb b/lib/user_agent.rb index 9b4ffc713..9b3c79466 100644 --- a/lib/user_agent.rb +++ b/lib/user_agent.rb @@ -277,7 +277,7 @@ returns proxy_no = options['proxy_no'] || Setting.get('proxy_no') || '' proxy_no = proxy_no.split(',').map(&:strip) || [] proxy_no.push('localhost', '127.0.0.1', '::1') - if proxy.present? && !proxy_no.include?(uri.host.downcase) + if proxy.present? && proxy_no.exclude?(uri.host.downcase) if proxy =~ /^(.+?):(.+?)$/ proxy_host = $1 proxy_port = $2 @@ -349,7 +349,7 @@ returns end body = request.body if body - request_data[:content] += "\n" + body + request_data[:content] += "\n#{body}" end request_data[:content] = request_data[:content].slice(0, 8000) @@ -371,7 +371,7 @@ returns 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) end diff --git a/lib/user_context.rb b/lib/user_context.rb index 0533c667d..9fede8f8d 100644 --- a/lib/user_context.rb +++ b/lib/user_context.rb @@ -5,7 +5,7 @@ # to the underlying User instance in the Policy class UserContext < Delegator - def initialize(user, token) + def initialize(user, token) # rubocop:disable Lint/MissingSuper @user = user @token = token end diff --git a/lib/websocket_server.rb b/lib/websocket_server.rb index 98247750c..6a18d36b2 100644 --- a/lib/websocket_server.rb +++ b/lib/websocket_server.rb @@ -173,14 +173,12 @@ class WebsocketServer log 'notice', 'send data to client', client_id websocket_send(client_id, queue) rescue => e - log 'error', 'problem:' + e.inspect, client_id + log 'error', "problem:#{e.inspect}", client_id # disconnect client client[:error_count] += 1 - if client[:error_count] > 20 - if @clients.include? client_id - @clients.delete client_id - end + if client[:error_count] > 20 && @clients.include?(client_id) + @clients.delete client_id end end end @@ -210,9 +208,8 @@ class WebsocketServer end def self.log(level, data, client_id = '-') - if !@options[:v] - return if level == 'debug' - end + return if !@options[:v] && level == 'debug' + puts "#{Time.now.utc.iso8601}:client(#{client_id}) #{data}" # rubocop:disable Rails/Output #puts "#{Time.now.utc.iso8601}:#{ level }:client(#{ client_id }) #{ data }" end diff --git a/lib/zammad/application/initializer/db_preflight_check/postgresql.rb b/lib/zammad/application/initializer/db_preflight_check/postgresql.rb index da21ec4ce..17099844b 100644 --- a/lib/zammad/application/initializer/db_preflight_check/postgresql.rb +++ b/lib/zammad/application/initializer/db_preflight_check/postgresql.rb @@ -31,11 +31,11 @@ module Zammad alternate_dbs = %w[template0 template1 postgres] @connection ||= begin - PG.connect(db_config) - rescue PG::ConnectionBad - db_config[:dbname] = alternate_dbs.pop - retry if db_config[:dbname].present? - end + PG.connect(db_config) + rescue PG::ConnectionBad + db_config[:dbname] = alternate_dbs.pop + retry if db_config[:dbname].present? + end end # Adapted from ActiveRecord::ConnectionHandling#postgresql_connection diff --git a/lib/zammad/application/initializer/session_store.rb b/lib/zammad/application/initializer/session_store.rb index e7f3d5dad..af3a83eae 100644 --- a/lib/zammad/application/initializer/session_store.rb +++ b/lib/zammad/application/initializer/session_store.rb @@ -7,7 +7,7 @@ module Zammad class Initializer module SessionStore STORE_TYPE = :active_record_store # default: :cookie_store - SESSION_KEY = ('_zammad_session_' + Digest::MD5.hexdigest(Rails.root.to_s)[5..15]).freeze # default: '_zammad_session' + SESSION_KEY = "_zammad_session_#{Digest::MD5.hexdigest(Rails.root.to_s)[5..15]}".freeze # default: '_zammad_session' def self.perform Rails.application.config.session_store STORE_TYPE, diff --git a/script/scheduler.rb b/script/scheduler.rb index 47ad3da65..62b110658 100755 --- a/script/scheduler.rb +++ b/script/scheduler.rb @@ -3,7 +3,7 @@ begin load File.expand_path('../bin/spring', __dir__) rescue LoadError => e - raise if !e.message.include?('spring') + raise if e.message.exclude?('spring') end dir = File.expand_path(File.join(File.dirname(__FILE__), '..')) diff --git a/script/websocket-server.rb b/script/websocket-server.rb index f850f2a1c..51c47a205 100755 --- a/script/websocket-server.rb +++ b/script/websocket-server.rb @@ -3,7 +3,7 @@ begin load File.expand_path('../bin/spring', __dir__) rescue LoadError => e - raise if !e.message.include?('spring') + raise if e.message.exclude?('spring') end dir = File.expand_path(File.join(File.dirname(__FILE__), '..')) diff --git a/spec/db/migrate/20171023000001_fixed_store_upgrade_ror_45_spec.rb b/spec/db/migrate/20171023000001_fixed_store_upgrade_ror_45_spec.rb index 18e7415f9..94aaa95d3 100644 --- a/spec/db/migrate/20171023000001_fixed_store_upgrade_ror_45_spec.rb +++ b/spec/db/migrate/20171023000001_fixed_store_upgrade_ror_45_spec.rb @@ -8,7 +8,7 @@ RSpec.describe FixedStoreUpgradeRor45, type: :db_migration do context 'when DB contains `store`d attributes saved as unpermitted ActionController::Parameters' do before do - ActiveRecord::Base.connection.execute(<<~SQL.tap { |sql| sql.delete!('`') if !mysql? }) + ActiveRecord::Base.connection.execute(<<~SQL.tap { |sql| sql.delete!('`') if !mysql? }) # rubocop:disable Rails/SquishedSQLHeredocs INSERT INTO taskbars (`user_id`, `client_id`, `key`, `callback`, `state`, `params`, `prio`, `notify`, `active`, `preferences`, `last_contact`, `updated_at`, `created_at`) VALUES (#{user.id}, '123', diff --git a/spec/factories/history/type.rb b/spec/factories/history/type.rb index a6f1ea47d..dd290c1dd 100644 --- a/spec/factories/history/type.rb +++ b/spec/factories/history/type.rb @@ -7,8 +7,7 @@ FactoryBot.define do # (Faker::Verb.unique.exclude(:past_participle, [], History::Type.pluck(:name)), # but it's not available yet in the current release of Faker (1.9.1). Faker::Verb.unique - .instance_variable_get(:@previous_results) - .dig([:past_participle, []]) + .instance_variable_get(:@previous_results)[[:past_participle, []]] .merge(History::Type.pluck(:name)) Faker::Verb.unique.past_participle diff --git a/spec/factories/text_module.rb b/spec/factories/text_module.rb index 75dfcb29b..ebbdcf5a9 100644 --- a/spec/factories/text_module.rb +++ b/spec/factories/text_module.rb @@ -1,6 +1,6 @@ FactoryBot.define do factory :text_module do - name { 'text module ' + Faker::Number.unique.number(3) } + name { "text module #{Faker::Number.unique.number(3)}" } keywords { Faker::Superhero.prefix } content { Faker::Lorem.sentence } updated_by_id { 1 } diff --git a/spec/factories/ticket/state.rb b/spec/factories/ticket/state.rb index ada43b2df..e202cc5bc 100644 --- a/spec/factories/ticket/state.rb +++ b/spec/factories/ticket/state.rb @@ -7,8 +7,7 @@ FactoryBot.define do # (Faker::Verb.unique.exclude(:past_participle, [], Ticket::State.pluck(:name)), # but it's not available yet in the current release of Faker (1.9.1). Faker::Verb.unique - .instance_variable_get(:@previous_results) - .dig([:past_participle, []]) + .instance_variable_get(:@previous_results)[[:past_participle, []]] .merge(Ticket::State.pluck(:name)) Faker::Verb.unique.past_participle diff --git a/spec/factories/ticket/state_type.rb b/spec/factories/ticket/state_type.rb index ba8e08996..3068ff050 100644 --- a/spec/factories/ticket/state_type.rb +++ b/spec/factories/ticket/state_type.rb @@ -7,8 +7,7 @@ FactoryBot.define do # (Faker::Verb.unique.exclude(:past_participle, [], Ticket::StateType.pluck(:name)), # but it's not available yet in the current release of Faker (1.9.1). Faker::Verb.unique - .instance_variable_get(:@previous_results) - .dig([:past_participle, []]) + .instance_variable_get(:@previous_results)[[:past_participle, []]] .merge(Ticket::StateType.pluck(:name)) Faker::Verb.unique.past_participle diff --git a/spec/factories/type_lookup.rb b/spec/factories/type_lookup.rb index 9a5ae2aec..c9babf441 100644 --- a/spec/factories/type_lookup.rb +++ b/spec/factories/type_lookup.rb @@ -7,8 +7,7 @@ FactoryBot.define do # (Faker::Verb.unique.exclude(:past_participle, [], Ticket::StateType.pluck(:name)), # but it's not available yet in the current release of Faker (1.9.1). Faker::Verb.unique - .instance_variable_get(:@previous_results) - .dig([:base, []]) + .instance_variable_get(:@previous_results)[[:base, []]] .merge(TypeLookup.pluck(:name)) Faker::Verb.unique.past_participle diff --git a/spec/jobs/issue_2715_fix_broken_twitter_urls_job_spec.rb b/spec/jobs/issue_2715_fix_broken_twitter_urls_job_spec.rb index e8a7d0dbe..aedb1fbe2 100644 --- a/spec/jobs/issue_2715_fix_broken_twitter_urls_job_spec.rb +++ b/spec/jobs/issue_2715_fix_broken_twitter_urls_job_spec.rb @@ -38,7 +38,7 @@ RSpec.describe Issue2715FixBrokenTwitterUrlsJob, type: :job do end def urls_of(article) - article.reload.preferences[:links].map { |link| link[:url] } + article.reload.preferences[:links].pluck(:url) end end end diff --git a/spec/lib/core_ext/string_spec.rb b/spec/lib/core_ext/string_spec.rb index a86943b53..f700b1514 100644 --- a/spec/lib/core_ext/string_spec.rb +++ b/spec/lib/core_ext/string_spec.rb @@ -556,7 +556,7 @@ RSpec.describe String do context 'performance tests' do let(:filler) do - %(

some word some url and the end.

\n) * 11 + "\n" + %(#{%(

some word some url and the end.

\n) * 11}\n) end it 'converts a 1076-byte unicode file in under 2s' do diff --git a/spec/lib/search_knowledge_base_backend_spec.rb b/spec/lib/search_knowledge_base_backend_spec.rb index a928eafed..e026ce461 100644 --- a/spec/lib/search_knowledge_base_backend_spec.rb +++ b/spec/lib/search_knowledge_base_backend_spec.rb @@ -46,7 +46,7 @@ RSpec.describe SearchKnowledgeBaseBackend do published_answer configure_elasticsearch(required: true, rebuild: true) if elasticsearch first_result = instance.search(published_answer.translations.first.title, user: user).first - expect(first_result.dig(:id)).to be_a(Integer) + expect(first_result[:id]).to be_a(Integer) end end diff --git a/spec/lib/sequencer/unit/import/common/model/associations/assign_spec.rb b/spec/lib/sequencer/unit/import/common/model/associations/assign_spec.rb index cb0cfa5db..adc2453b4 100644 --- a/spec/lib/sequencer/unit/import/common/model/associations/assign_spec.rb +++ b/spec/lib/sequencer/unit/import/common/model/associations/assign_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Sequencer::Unit::Import::Common::Model::Associations::Assign, seq let(:instance) { create(:user) } let(:action) { :created } let(:associations) do - alt_org = Organization.where('id <> ?', instance.organization_id.to_i).pluck(:id).sample + alt_org = Organization.where.not(id: instance.organization_id.to_i).pluck(:id).sample { organization_id: alt_org } end diff --git a/spec/models/application_model/can_assets_examples.rb b/spec/models/application_model/can_assets_examples.rb index b5047be27..0abf2b7a8 100644 --- a/spec/models/application_model/can_assets_examples.rb +++ b/spec/models/application_model/can_assets_examples.rb @@ -6,7 +6,7 @@ RSpec.shared_examples 'ApplicationModel::CanAssets' do |associations: [], select describe '#assets (for supplying model data to front-end framework)' do shared_examples 'own asset attributes' do it 'returns a hash with own asset attributes' do - expect(subject.assets({}).dig(described_class.to_app_model)) + expect(subject.assets({})[described_class.to_app_model]) .to include(subject.id => hash_including(subject.attributes_with_association_ids)) end end diff --git a/spec/models/application_model/can_creates_and_updates_examples.rb b/spec/models/application_model/can_creates_and_updates_examples.rb index b250698b5..ea28c9818 100644 --- a/spec/models/application_model/can_creates_and_updates_examples.rb +++ b/spec/models/application_model/can_creates_and_updates_examples.rb @@ -42,7 +42,7 @@ RSpec.shared_examples 'ApplicationModel::CanCreatesAndUpdates' do end context 'when given an invalid #name' do - let(:name) { described_class.pluck(:name).max + 'foo' } + let(:name) { "#{described_class.pluck(:name).max}foo" } it 'attempts to create a new record' do allow(described_class).to receive(:create) @@ -68,7 +68,7 @@ RSpec.shared_examples 'ApplicationModel::CanCreatesAndUpdates' do end context 'when given an invalid #login' do - let(:login) { described_class.pluck(:login).max + 'foo' } + let(:login) { "#{described_class.pluck(:login).max}foo" } it 'attempts to create a new record' do allow(described_class).to receive(:create) @@ -94,7 +94,7 @@ RSpec.shared_examples 'ApplicationModel::CanCreatesAndUpdates' do end context 'when given an invalid #email' do - let(:email) { described_class.pluck(:email).max + 'foo' } + let(:email) { "#{described_class.pluck(:email).max}foo" } it 'attempts to create a new record' do allow(described_class).to receive(:create) @@ -121,7 +121,7 @@ RSpec.shared_examples 'ApplicationModel::CanCreatesAndUpdates' do end context 'when given an invalid #source or #locale' do - let(:source) { described_class.pluck(:source).max + 'foo' } + let(:source) { "#{described_class.pluck(:source).max}foo" } let(:locale) { record.locale } it 'attempts to create a new record' do @@ -172,7 +172,7 @@ RSpec.shared_examples 'ApplicationModel::CanCreatesAndUpdates' do end context 'when given an invalid #name' do - let(:name) { described_class.pluck(:name).max + 'foo' } + let(:name) { "#{described_class.pluck(:name).max}foo" } it 'attempts to create a new record' do allow(described_class).to receive(:create) @@ -193,7 +193,7 @@ RSpec.shared_examples 'ApplicationModel::CanCreatesAndUpdates' do end context 'when given an invalid #login' do - let(:login) { described_class.pluck(:login).max + 'foo' } + let(:login) { "#{described_class.pluck(:login).max}foo" } it 'attempts to create a new record' do allow(described_class).to receive(:create) @@ -214,7 +214,7 @@ RSpec.shared_examples 'ApplicationModel::CanCreatesAndUpdates' do end context 'when given an invalid #email' do - let(:email) { described_class.pluck(:email).max + 'foo' } + let(:email) { "#{described_class.pluck(:email).max}foo" } it 'attempts to create a new record' do allow(described_class).to receive(:create) diff --git a/spec/models/channel/driver/imap_spec.rb b/spec/models/channel/driver/imap_spec.rb index b080ae542..dab1a77be 100644 --- a/spec/models/channel/driver/imap_spec.rb +++ b/spec/models/channel/driver/imap_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Channel::Driver::Imap do result = described_class.new.fetch(params, nil, 'check') - expect(result.dig(:result)).to eq 'ok' + expect(result[:result]).to eq 'ok' end end diff --git a/spec/models/channel/driver/twitter_spec.rb b/spec/models/channel/driver/twitter_spec.rb index 56ff4ff5a..5c6a88a69 100644 --- a/spec/models/channel/driver/twitter_spec.rb +++ b/spec/models/channel/driver/twitter_spec.rb @@ -237,7 +237,7 @@ RSpec.describe Channel::Driver::Twitter do ] end - let(:user_ids) { payload[:users].values.map { |u| u[:id] } } + let(:user_ids) { payload[:users].values.pluck(:id) } it 'creates a new article' do expect { channel.process(payload) } @@ -336,7 +336,7 @@ RSpec.describe Channel::Driver::Twitter do ] end - let(:user_ids) { payload[:users].values.map { |u| u[:id] } } + let(:user_ids) { payload[:users].values.pluck(:id) } it 'creates a new article' do expect { channel.process(payload) } @@ -490,7 +490,7 @@ RSpec.describe Channel::Driver::Twitter do let(:twitter_prefs) do { - 'mention_ids' => payload[:tweet_create_events].first[:entities][:user_mentions].map { |um| um[:id] }, + 'mention_ids' => payload[:tweet_create_events].first[:entities][:user_mentions].pluck(:id), 'geo' => payload[:tweet_create_events].first[:geo].to_h, 'retweeted' => payload[:tweet_create_events].first[:retweeted], 'possibly_sensitive' => payload[:tweet_create_events].first[:possibly_sensitive], @@ -522,7 +522,7 @@ RSpec.describe Channel::Driver::Twitter do context 'when message mentions multiple users' do let(:payload_file) { Rails.root.join('test/data/twitter/webhook_events/tweet_create-user_mention_multiple.yml') } - let(:mentionees) { "@#{payload[:tweet_create_events].first[:entities][:user_mentions].map { |um| um[:screen_name] }.join(', @')}" } + let(:mentionees) { "@#{payload[:tweet_create_events].first[:entities][:user_mentions].pluck(:screen_name).join(', @')}" } it 'records all mentionees in comma-separated "to" attribute' do expect { channel.process(payload) } @@ -642,7 +642,7 @@ RSpec.describe Channel::Driver::Twitter do let(:twitter_prefs) do { - 'mention_ids' => payload[:tweet_create_events].first[:entities][:user_mentions].map { |um| um[:id] }, + 'mention_ids' => payload[:tweet_create_events].first[:entities][:user_mentions].pluck(:id), 'geo' => payload[:tweet_create_events].first[:geo].to_h, 'retweeted' => payload[:tweet_create_events].first[:retweeted], 'possibly_sensitive' => payload[:tweet_create_events].first[:possibly_sensitive], diff --git a/spec/models/concerns/has_history_examples.rb b/spec/models/concerns/has_history_examples.rb index e0bafe75f..95e78d0e8 100644 --- a/spec/models/concerns/has_history_examples.rb +++ b/spec/models/concerns/has_history_examples.rb @@ -25,61 +25,61 @@ RSpec.shared_examples 'HasHistory' do |history_relation_object: nil| end end - describe 'of #active' do + describe 'of #active', if: described_class.attribute_names.include?('active') do let(:attribute) { 'active' } let(:new_value) { !subject.active } let(:attributes) { { 'value_from' => old_value.to_s, 'value_to' => new_value.to_s } } - include_examples 'attribute update' if described_class.attribute_names.include?('active') + include_examples 'attribute update' end - describe 'of #body' do + describe 'of #body', if: described_class.attribute_names.include?('body') do let(:attribute) { 'body' } let(:new_value) { 'Lorem ipsum dolor' } let(:attributes) { { 'value_from' => old_value, 'value_to' => new_value } } - include_examples 'attribute update' if described_class.attribute_names.include?('body') + include_examples 'attribute update' end - describe 'of #email' do + describe 'of #email', if: described_class.attribute_names.include?('email') do let(:attribute) { 'email' } let(:new_value) { Faker::Internet.email } let(:attributes) { { 'value_from' => old_value, 'value_to' => new_value } } - include_examples 'attribute update' if described_class.attribute_names.include?('email') + include_examples 'attribute update' end - describe 'of #lastname' do + describe 'of #lastname', if: described_class.attribute_names.include?('lastname') do let(:attribute) { 'lastname' } let(:new_value) { 'Foo' } let(:attributes) { { 'value_from' => old_value, 'value_to' => new_value } } - include_examples 'attribute update' if described_class.attribute_names.include?('lastname') + include_examples 'attribute update' end - describe 'of #name' do + describe 'of #name', if: described_class.attribute_names.include?('name') do let(:attribute) { 'name' } let(:new_value) { 'Foo' } let(:attributes) { { 'value_from' => old_value, 'value_to' => new_value } } - include_examples 'attribute update' if described_class.attribute_names.include?('name') + include_examples 'attribute update' end - describe 'of #state' do + describe 'of #state', if: described_class.attribute_names.include?('state_id') do let(:attribute) { 'state' } let(:new_value) { state_class.where.not(id: old_value.id).first } let(:state_class) { "#{described_class.name}::State".constantize } let(:attributes) { { 'value_from' => old_value.name, 'value_to' => new_value.name } } - include_examples 'attribute update' if described_class.attribute_names.include?('state_id') + include_examples 'attribute update' end - describe 'of #title' do + describe 'of #title', if: described_class.attribute_names.include?('title') do let(:attribute) { 'title' } let(:new_value) { 'foo' } let(:attributes) { { 'value_from' => old_value, 'value_to' => new_value } } - include_examples 'attribute update' if described_class.attribute_names.include?('title') + include_examples 'attribute update' end context 'when validations or callbacks prevent update' do @@ -89,11 +89,11 @@ RSpec.shared_examples 'HasHistory' do |history_relation_object: nil| end end - describe 'of #owner' do + describe 'of #owner', if: described_class.attribute_names.include?('owner_id') do let(:attribute) { 'owner' } let(:new_value) { create(:customer) } # Ticket#owner is restricted to active agents of the same group - include_examples 'failed attribute update' if described_class.attribute_names.include?('owner_id') + include_examples 'failed attribute update' end end end diff --git a/spec/models/email_address_spec.rb b/spec/models/email_address_spec.rb index 2e6d364cb..f9a95143f 100644 --- a/spec/models/email_address_spec.rb +++ b/spec/models/email_address_spec.rb @@ -2,10 +2,10 @@ require 'rails_helper' require 'models/concerns/has_collection_update_examples' RSpec.describe EmailAddress, type: :model do - it_behaves_like 'HasCollectionUpdate', collection_factory: :email_address - subject(:email_address) { create(:email_address) } + it_behaves_like 'HasCollectionUpdate', collection_factory: :email_address + describe 'Attributes:' do describe '#active' do subject(:email_address) do diff --git a/spec/models/job_spec.rb b/spec/models/job_spec.rb index 36437e284..bd1b23ec0 100644 --- a/spec/models/job_spec.rb +++ b/spec/models/job_spec.rb @@ -2,10 +2,10 @@ require 'rails_helper' require 'models/application_model_examples' RSpec.describe Job, type: :model do - it_behaves_like 'ApplicationModel', can_assets: { selectors: %i[condition perform] } - subject(:job) { create(:job) } + it_behaves_like 'ApplicationModel', can_assets: { selectors: %i[condition perform] } + describe 'Class methods:' do describe '.run' do let!(:executable_jobs) { jobs.select(&:executable?).select(&:in_timeplan?) } diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index 3caa67977..6fe43a093 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -8,6 +8,8 @@ require 'models/concerns/has_object_manager_attributes_validation_examples' require 'models/concerns/has_taskbars_examples' RSpec.describe Organization, type: :model do + subject(:organization) { create(:organization) } + it_behaves_like 'ApplicationModel', can_assets: { associations: :members } it_behaves_like 'CanCsvImport', unique_attributes: 'name' it_behaves_like 'HasHistory' @@ -16,8 +18,6 @@ RSpec.describe Organization, type: :model do it_behaves_like 'HasObjectManagerAttributesValidation' it_behaves_like 'HasTaskbars' - subject(:organization) { create(:organization) } - describe 'Class methods:' do describe '.where_or_cis' do it 'finds instance by querying multiple attributes case insensitive' do diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb index fe87e8b88..94b118aa3 100644 --- a/spec/models/role_spec.rb +++ b/spec/models/role_spec.rb @@ -6,14 +6,14 @@ require 'models/concerns/has_collection_update_examples' require 'models/concerns/has_ticket_create_screen_impact_examples' RSpec.describe Role do + subject(:role) { create(:role) } + it_behaves_like 'ApplicationModel' it_behaves_like 'CanBeImported' it_behaves_like 'HasGroups', group_access_factory: :role it_behaves_like 'HasCollectionUpdate', collection_factory: :role it_behaves_like 'HasTicketCreateScreenImpact', create_screen_factory: :role - subject(:role) { create(:role) } - describe 'Default state' do describe 'of whole table:' do it 'has three records ("Admin", "Agent", and "Customer")' do diff --git a/spec/models/ticket/article_spec.rb b/spec/models/ticket/article_spec.rb index 2583cfc59..e0ba48892 100644 --- a/spec/models/ticket/article_spec.rb +++ b/spec/models/ticket/article_spec.rb @@ -6,14 +6,14 @@ require 'models/concerns/has_history_examples' require 'models/concerns/has_object_manager_attributes_validation_examples' RSpec.describe Ticket::Article, type: :model do + subject(:article) { create(:ticket_article) } + it_behaves_like 'ApplicationModel' it_behaves_like 'CanBeImported' it_behaves_like 'CanCsvImport' it_behaves_like 'HasHistory' it_behaves_like 'HasObjectManagerAttributesValidation' - subject(:article) { create(:ticket_article) } - describe 'Callbacks, Observers, & Async Transactions -' do describe 'NULL byte handling (via ChecksAttributeValuesAndLength concern):' do it 'removes them from #subject on creation, if necessary (postgres doesn’t like them)' do @@ -290,7 +290,7 @@ RSpec.describe Ticket::Article, type: :model do end context 'with NULL bytes' do - let(:body) { "\u0000" + 'a' * 2_000_000 } + let(:body) { "\u0000#{'a' * 2_000_000}" } it 'still removes them, if necessary (postgres doesn’t like them)' do expect(article).to be_persisted diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index 5d19a716d..14f921130 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -9,6 +9,8 @@ require 'models/concerns/has_xss_sanitized_note_examples' require 'models/concerns/has_object_manager_attributes_validation_examples' RSpec.describe Ticket, type: :model do + subject(:ticket) { create(:ticket) } + it_behaves_like 'ApplicationModel' it_behaves_like 'CanBeImported' it_behaves_like 'CanCsvImport' @@ -18,8 +20,6 @@ RSpec.describe Ticket, type: :model do it_behaves_like 'HasXssSanitizedNote', model_factory: :ticket it_behaves_like 'HasObjectManagerAttributesValidation' - subject(:ticket) { create(:ticket) } - describe 'Class methods:' do describe '.selectors' do # https://github.com/zammad/zammad/issues/1769 @@ -131,7 +131,7 @@ RSpec.describe Ticket, type: :model do context 'and deleting the origin ticket' do it 'adds reference number and title to the target ticket' do expect { ticket.destroy } - .to change { target_ticket.history_get.find { |elem| elem.fetch('type') == 'received_merge' }.dig('value_from') } + .to change { target_ticket.history_get.find { |elem| elem.fetch('type') == 'received_merge' }['value_from'] } .to("##{ticket.number} #{ticket.title}") end end @@ -140,7 +140,7 @@ RSpec.describe Ticket, type: :model do context 'and deleting the target ticket' do it 'adds reference number and title to the origin ticket' do expect { target_ticket.destroy } - .to change { ticket.history_get.find { |elem| elem.fetch('type') == 'merged_into' }.dig('value_to') } + .to change { ticket.history_get.find { |elem| elem.fetch('type') == 'merged_into' }['value_to'] } .to("##{target_ticket.number} #{target_ticket.title}") end end diff --git a/spec/policies/controllers/knowledge_base/categories_controller_policy_spec.rb b/spec/policies/controllers/knowledge_base/categories_controller_policy_spec.rb index 61aa9ee9f..df02fa47f 100644 --- a/spec/policies/controllers/knowledge_base/categories_controller_policy_spec.rb +++ b/spec/policies/controllers/knowledge_base/categories_controller_policy_spec.rb @@ -1,10 +1,10 @@ require 'rails_helper' describe Controllers::KnowledgeBase::CategoriesControllerPolicy do - include_context 'basic Knowledge Base' - subject { described_class.new(user, record) } + include_context 'basic Knowledge Base' + let(:record_class) { KnowledgeBase::CategoriesController } let(:record) do diff --git a/spec/requests/integration/monitoring_spec.rb b/spec/requests/integration/monitoring_spec.rb index 958896fe7..cd4a9bd84 100644 --- a/spec/requests/integration/monitoring_spec.rb +++ b/spec/requests/integration/monitoring_spec.rb @@ -191,7 +191,7 @@ RSpec.describe 'Monitoring', type: :request do Store.add( object: 'User', o_id: 1, - data: string + '123', + data: "#{string}123", filename: 'filename2.txt', created_by_id: 1, ) diff --git a/spec/requests/integration/placetel_spec.rb b/spec/requests/integration/placetel_spec.rb index 9f9a2f14e..0697f66e9 100644 --- a/spec/requests/integration/placetel_spec.rb +++ b/spec/requests/integration/placetel_spec.rb @@ -1,5 +1,7 @@ require 'rails_helper' +# rubocop:disable Style/CombinableLoops + RSpec.describe 'Integration Placetel', type: :request do let(:agent) do @@ -616,3 +618,5 @@ RSpec.describe 'Integration Placetel', type: :request do end end end + +# rubocop:enable Style/CombinableLoops diff --git a/spec/requests/integration/sipgate_spec.rb b/spec/requests/integration/sipgate_spec.rb index 6a8f9cb52..98013065e 100644 --- a/spec/requests/integration/sipgate_spec.rb +++ b/spec/requests/integration/sipgate_spec.rb @@ -1,5 +1,7 @@ require 'rails_helper' +# rubocop:disable Style/CombinableLoops + RSpec.describe 'Integration Sipgate', type: :request do let(:agent) do @@ -488,3 +490,5 @@ RSpec.describe 'Integration Sipgate', type: :request do end end end + +# rubocop:enable Style/CombinableLoops diff --git a/spec/support/assets_matchers.rb b/spec/support/assets_matchers.rb index 7aea5df79..4c4a6d300 100644 --- a/spec/support/assets_matchers.rb +++ b/spec/support/assets_matchers.rb @@ -13,12 +13,12 @@ RSpec::Matchers.define :include_assets_of do failure_message do |actual| list = expected_array.reject { |elem| find_assets_of(elem, actual) } - "Expected hash to include, but not included:\n" + items_for_message(list) + "Expected hash to include, but not included:\n#{items_for_message(list)}" end failure_message_when_negated do |actual| list = expected_array.select { |elem| find_assets_of(elem, actual) } - "Expected hash to not include, but was included:\n" + items_for_message(list) + "Expected hash to not include, but was included:\n#{items_for_message(list)}" end def items_for_message(items) diff --git a/spec/support/capybara/common_actions.rb b/spec/support/capybara/common_actions.rb index 684cb95ab..8b2d0a91b 100644 --- a/spec/support/capybara/common_actions.rb +++ b/spec/support/capybara/common_actions.rb @@ -227,12 +227,10 @@ module CommonActions # Executes action inside of modal. Makes sure modal has opened and closes # # @param timeout [Integer] seconds to wait - def in_modal(timeout: 4) + def in_modal(timeout: 4, &block) modal_ready(timeout: timeout) - within '.modal' do - yield - end + within('.modal', &block) modal_disappear(timeout: timeout) end diff --git a/spec/support/db_migration.rb b/spec/support/db_migration.rb index 9640e6a2c..aceadd163 100644 --- a/spec/support/db_migration.rb +++ b/spec/support/db_migration.rb @@ -115,7 +115,7 @@ module DbMigrationHelper # remove_foreign_key(:online_notifications, :users) # # @return [nil] - def respond_to_missing?(*) + def respond_to_missing?(*) # rubocop:disable Lint/MissingSuper true end diff --git a/test/browser/admin_role_test.rb b/test/browser/admin_role_test.rb index e42a8b11c..d8a5dba0e 100644 --- a/test/browser/admin_role_test.rb +++ b/test/browser/admin_role_test.rb @@ -11,10 +11,10 @@ class AdminRoleTest < TestCase tasks_close_all() rand = rand(99_999_999).to_s - login = 'agent-role-' + rand - firstname = 'Role' + rand - lastname = 'Module' + rand - email = 'agent-role-' + rand + '@example.com' + login = "agent-role-#{rand}" + firstname = "Role#{rand}" + lastname = "Module#{rand}" + email = "agent-role-#{rand}@example.com" password = 'agentpw' user_create( diff --git a/test/browser/agent_organization_profile_test.rb b/test/browser/agent_organization_profile_test.rb index 9b5572bd7..c058f4c7a 100644 --- a/test/browser/agent_organization_profile_test.rb +++ b/test/browser/agent_organization_profile_test.rb @@ -3,8 +3,8 @@ require 'browser_test_helper' class AgentOrganizationProfileTest < TestCase def test_org_profile # work in one browser window - message = '1 ' + rand(99_999_999).to_s - note = 'some note ' + rand(99_999_999).to_s + message = "1 #{rand(99_999_999)}" + note = "some note #{rand(99_999_999)}" @browser = browser_instance login( @@ -97,8 +97,8 @@ class AgentOrganizationProfileTest < TestCase data: { customer: 'nico', group: 'Users', - title: 'org profile check ' + message, - body: 'org profile check ' + message, + title: "org profile check #{message}", + body: "org profile check #{message}", }, ) @@ -108,12 +108,12 @@ class AgentOrganizationProfileTest < TestCase ) watch_for( css: '.active .profile-window', - value: 'org profile check ' + message, + value: "org profile check #{message}", ) tasks_close_all() # work with two browser windows - message = 'comment 1 ' + rand(99_999_999_999_999_999).to_s + message = "comment 1 #{rand(99_999_999_999_999_999)}" # use current session browser1 = @browser diff --git a/test/browser/agent_ticket_attachment_test.rb b/test/browser/agent_ticket_attachment_test.rb index 31fe35d4a..e109147db 100644 --- a/test/browser/agent_ticket_attachment_test.rb +++ b/test/browser/agent_ticket_attachment_test.rb @@ -219,13 +219,13 @@ class AgentTicketAttachmentTest < TestCase browser: browser2, ) - random = 'ticket-actions-6-test-' + rand(999_999).to_s - user_email = random + '@example.com' + random = "ticket-actions-6-test-#{rand(999_999)}" + user_email = "#{random}@example.com" user_create( browser: browser2, data: { - firstname: 'Action6 Firstname' + random, - lastname: 'Action6 Lastname' + random, + firstname: "Action6 Firstname#{random}", + lastname: "Action6 Lastname#{random}", email: user_email, password: 'some-pass', }, diff --git a/test/browser/agent_ticket_email_signature_test.rb b/test/browser/agent_ticket_email_signature_test.rb index 3738363e9..5258a8e26 100644 --- a/test/browser/agent_ticket_email_signature_test.rb +++ b/test/browser/agent_ticket_email_signature_test.rb @@ -4,13 +4,13 @@ class AgentTicketEmailSignatureTest < TestCase def test_agent_signature_check suffix = rand(99_999_999_999_999_999).to_s - signature_name1 = 'sig name 1 äöüß ' + suffix - signature_body1 = "--\nsig body 1 äöüß " + suffix - signature_name2 = 'sig name 2 äöüß ' + suffix - signature_body2 = "--\nsig body 2 äöüß " + suffix - group_name1 = 'group name 1 ' + suffix - group_name2 = 'group name 2 ' + suffix - group_name3 = 'group name 3 ' + suffix + signature_name1 = "sig name 1 äöüß #{suffix}" + signature_body1 = "--\nsig body 1 äöüß #{suffix}" + signature_name2 = "sig name 2 äöüß #{suffix}" + signature_body2 = "--\nsig body 2 äöüß #{suffix}" + group_name1 = "group name 1 #{suffix}" + group_name2 = "group name 2 #{suffix}" + group_name3 = "group name 3 #{suffix}" @browser = browser_instance login( diff --git a/test/browser/agent_ticket_overview_level0_test.rb b/test/browser/agent_ticket_overview_level0_test.rb index d23c784d5..1d0fe1f19 100644 --- a/test/browser/agent_ticket_overview_level0_test.rb +++ b/test/browser/agent_ticket_overview_level0_test.rb @@ -44,25 +44,25 @@ class AgentTicketOverviewLevel0Test < TestCase # select both via bulk action click( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"] + .icon-checkbox.icon-unchecked), fast: true, ) # scroll to reply - needed for chrome scroll_to( position: 'top', - css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"] + .icon-checkbox.icon-unchecked), ) click( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"] + .icon-checkbox.icon-unchecked), fast: true, ) exists( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"][type="checkbox"]:checked', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"][type="checkbox"]:checked), ) exists( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"][type="checkbox"]:checked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"][type="checkbox"]:checked), ) # select close state & submit @@ -78,15 +78,15 @@ class AgentTicketOverviewLevel0Test < TestCase ) watch_for_disappear( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"]), timeout: 6, ) exists_not( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"]), ) exists_not( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"]), ) # remember current overview count @@ -261,25 +261,25 @@ class AgentTicketOverviewLevel0Test < TestCase # select both via bulk action click( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"] + .icon-checkbox.icon-unchecked), fast: true, ) # scroll to reply - needed for chrome scroll_to( position: 'top', - css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"] + .icon-checkbox.icon-unchecked), ) click( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"] + .icon-checkbox.icon-unchecked), fast: true, ) exists( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"][type="checkbox"]:checked', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"][type="checkbox"]:checked), ) exists( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"][type="checkbox"]:checked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"][type="checkbox"]:checked), ) exists( @@ -320,15 +320,15 @@ class AgentTicketOverviewLevel0Test < TestCase ) watch_for_disappear( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"]), timeout: 12, ) exists_not( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"]), ) exists_not( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"]), ) # get new overview count @@ -410,25 +410,25 @@ class AgentTicketOverviewLevel0Test < TestCase # select both via bulk action click( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"] + .icon-checkbox.icon-unchecked), fast: true, ) # scroll to reply - needed for chrome scroll_to( position: 'top', - css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"] + .icon-checkbox.icon-unchecked), ) click( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"] + .icon-checkbox.icon-unchecked), fast: true, ) exists( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"][type="checkbox"]:checked', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"][type="checkbox"]:checked), ) exists( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"][type="checkbox"]:checked', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"][type="checkbox"]:checked), ) select( @@ -449,15 +449,15 @@ class AgentTicketOverviewLevel0Test < TestCase ) watch_for_disappear( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"]), timeout: 12, ) exists_not( - css: '.content.active table tr td input[value="' + ticket1[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket1[:id]}"]), ) exists_not( - css: '.content.active table tr td input[value="' + ticket2[:id] + '"]', + css: %(.content.active table tr td input[value="#{ticket2[:id]}"]), ) # get new overview count @@ -562,7 +562,7 @@ class AgentTicketOverviewLevel0Test < TestCase # first select the ticket that we have change rights to check( - css: '.content.active table tr td input[value="' + can_change_ticket[:id] + '"]', + css: %(.content.active table tr td input[value="#{can_change_ticket[:id]}"]), ) # check that the bulk action form appears @@ -574,10 +574,10 @@ class AgentTicketOverviewLevel0Test < TestCase # then select the ticket that we do not have change rights to scroll_to( position: 'top', - css: '.content.active table tr td input[value="' + cannot_change_ticket[:id] + '"] + .icon-checkbox.icon-unchecked', + css: %(.content.active table tr td input[value="#{cannot_change_ticket[:id]}"] + .icon-checkbox.icon-unchecked), ) check( - css: '.content.active table tr td input[value="' + cannot_change_ticket[:id] + '"]', + css: %(.content.active table tr td input[value="#{cannot_change_ticket[:id]}"]), ) # check that the bulk action form disappears @@ -588,7 +588,7 @@ class AgentTicketOverviewLevel0Test < TestCase # de-select the ticket that we do not have change rights to uncheck( - css: '.content.active table tr td input[value="' + cannot_change_ticket[:id] + '"]', + css: %(.content.active table tr td input[value="#{cannot_change_ticket[:id]}"]), fast: true, ) @@ -600,7 +600,7 @@ class AgentTicketOverviewLevel0Test < TestCase # de-select the ticket that we have change rights to uncheck( - css: '.content.active table tr td input[value="' + can_change_ticket[:id] + '"]', + css: %(.content.active table tr td input[value="#{can_change_ticket[:id]}"]), fast: true, ) diff --git a/test/browser/agent_ticket_overview_level1_test.rb b/test/browser/agent_ticket_overview_level1_test.rb index 310c1a22a..4bb7e8959 100644 --- a/test/browser/agent_ticket_overview_level1_test.rb +++ b/test/browser/agent_ticket_overview_level1_test.rb @@ -2,8 +2,8 @@ require 'browser_test_helper' class AgentTicketOverviewLevel1Test < TestCase def test_i - name1 = 'name_low_' + rand(999_999).to_s - name2 = 'name_high_' + rand(999_999).to_s + name1 = "name_low_#{rand(999_999)}" + name2 = "name_high_#{rand(999_999)}" browser1 = browser_instance login( diff --git a/test/browser/agent_ticket_tag_test.rb b/test/browser/agent_ticket_tag_test.rb index 4ddbcff5d..88a5c8724 100644 --- a/test/browser/agent_ticket_tag_test.rb +++ b/test/browser/agent_ticket_tag_test.rb @@ -378,22 +378,22 @@ class AgentTicketTagTest < TestCase set( css: '.content.active .js-create input[name="name"]', - value: tag_prefix + ' A', + value: "#{tag_prefix} A", ) click(css: '.content.active .js-create .js-submit') set( css: '.content.active .js-create input[name="name"]', - value: tag_prefix + ' a', + value: "#{tag_prefix} a", ) click(css: '.content.active .js-create .js-submit') set( css: '.content.active .js-create input[name="name"]', - value: tag_prefix + ' B', + value: "#{tag_prefix} B", ) click(css: '.content.active .js-create .js-submit') set( css: '.content.active .js-create input[name="name"]', - value: tag_prefix + ' C', + value: "#{tag_prefix} C", ) click(css: '.content.active .js-create .js-submit') diff --git a/test/browser/agent_ticket_text_module_test.rb b/test/browser/agent_ticket_text_module_test.rb index 22572d84d..d40c9c6c1 100644 --- a/test/browser/agent_ticket_text_module_test.rb +++ b/test/browser/agent_ticket_text_module_test.rb @@ -2,8 +2,8 @@ require 'browser_test_helper' class AgentTicketTextModuleTest < TestCase def test_text_modules - random = 'text_module_test_' + rand(99_999_999).to_s - random2 = 'text_module_test_' + rand(99_999_999).to_s + random = "text_module_test_#{rand(99_999_999)}" + random2 = "text_module_test_#{rand(99_999_999)}" @browser = browser_instance login( @@ -16,16 +16,16 @@ class AgentTicketTextModuleTest < TestCase # create new text modules text_module_create( data: { - name: 'some name' + random, + name: "some name#{random}", keywords: random, - content: 'some content' + random, + content: "some content#{random}", }, ) text_module_create( data: { - name: 'some name' + random2, + name: "some name#{random2}", keywords: random2, - content: 'some content' + random2, + content: "some content#{random2}", }, ) @@ -36,7 +36,7 @@ class AgentTicketTextModuleTest < TestCase set( css: '.active div[data-name=body]', - value: 'test ::' + random, + value: "test ::#{random}", ) watch_for( css: '.active .shortcut', @@ -50,18 +50,18 @@ class AgentTicketTextModuleTest < TestCase watch_for( css: '.active div[data-name=body]', - value: 'some content' + random, + value: "some content#{random}", ) tasks_close_all() # test with two browser windows - random = 'text_II_module_test_' + rand(99_999_999).to_s + random = "text_II_module_test_#{rand(99_999_999)}" user_rand = rand(99_999_999).to_s - login = 'agent-text-module-' + user_rand - firstname = 'Text' + user_rand - lastname = 'Module' + user_rand - email = 'agent-text-module-' + user_rand + '@example.com' + login = "agent-text-module-#{user_rand}" + firstname = "Text#{user_rand}" + lastname = "Module#{user_rand}" + email = "agent-text-module-#{user_rand}@example.com" password = 'agentpw' # use current session @@ -98,7 +98,7 @@ class AgentTicketTextModuleTest < TestCase text_module_create( browser: browser1, data: { - name: 'some name' + random, + name: "some name#{random}", keywords: random, content: "some content \#{ticket.customer.lastname}#{random}", }, @@ -120,7 +120,7 @@ class AgentTicketTextModuleTest < TestCase set( browser: browser2, css: '.active div[data-name=body]', - value: 'test ::' + random, + value: "test ::#{random}", ) watch_for( browser: browser2, @@ -139,7 +139,7 @@ class AgentTicketTextModuleTest < TestCase watch_for( browser: browser2, css: '.active div[data-name=body]', - value: 'some content -' + random, + value: "some content -#{random}", ) ticket_customer_select( @@ -151,7 +151,7 @@ class AgentTicketTextModuleTest < TestCase set( browser: browser2, css: '.active div[data-name=body]', - value: '::' + random, + value: "::#{random}", ) sendkey( browser: browser2, @@ -165,7 +165,7 @@ class AgentTicketTextModuleTest < TestCase watch_for( browser: browser2, css: '.active div[data-name=body]', - value: 'some content Braun' + random, + value: "some content Braun#{random}", ) # verify zoom @@ -194,7 +194,7 @@ class AgentTicketTextModuleTest < TestCase set( browser: browser2, css: '.active div[data-name=body]', - value: '::' + random, + value: "::#{random}", no_click: true, ) sendkey( @@ -210,7 +210,7 @@ class AgentTicketTextModuleTest < TestCase watch_for( browser: browser2, css: '.active div[data-name=body]', - value: 'some content Braun' + random, + value: "some content Braun#{random}", ) # change customer @@ -245,7 +245,7 @@ class AgentTicketTextModuleTest < TestCase set( browser: browser2, css: '.active div[data-name=body]', - value: '::' + random, + value: "::#{random}", no_click: true, ) @@ -262,7 +262,7 @@ class AgentTicketTextModuleTest < TestCase watch_for( browser: browser2, css: '.active div[data-name=body]', - value: 'some content ' + lastname, + value: "some content #{lastname}", ) end end diff --git a/test/browser/agent_ticket_zoom_hide_test.rb b/test/browser/agent_ticket_zoom_hide_test.rb index 260b8787c..8819eb245 100644 --- a/test/browser/agent_ticket_zoom_hide_test.rb +++ b/test/browser/agent_ticket_zoom_hide_test.rb @@ -56,7 +56,7 @@ class AgentTicketZoomHideTest < TestCase # Now go to a previous ticket and confirm that the modal disappears location( - url: browser_url + '/#ticket/zoom/1', + url: "#{browser_url}/#ticket/zoom/1", ) sleep 2 modal_disappear() diff --git a/test/browser/agent_user_manage_test.rb b/test/browser/agent_user_manage_test.rb index e7cba02fd..136e2f778 100644 --- a/test/browser/agent_user_manage_test.rb +++ b/test/browser/agent_user_manage_test.rb @@ -2,7 +2,7 @@ require 'browser_test_helper' class AgentUserManageTest < TestCase def test_agent_customer_ticket_create - customer_user_email = 'customer-test-' + rand(999_999).to_s + '@example.com' + customer_user_email = "customer-test-#{rand(999_999)}@example.com" firstname = 'Customer Firstname' lastname = 'Customer Lastname' fullname = "#{firstname} #{lastname} <#{customer_user_email}>" @@ -130,7 +130,7 @@ class AgentUserManageTest < TestCase end def test_agent_customer_ticket_zoom - customer_user_email = 'customer-test-' + rand(999_999).to_s + '@example.com' + customer_user_email = "customer-test-#{rand(999_999)}@example.com" firstname = 'Customer Firstname' lastname = 'Customer Lastname' fullname = "#{firstname} #{lastname} <#{customer_user_email}>" diff --git a/test/browser/agent_user_profile_test.rb b/test/browser/agent_user_profile_test.rb index 078557d7d..f58fb9da7 100644 --- a/test/browser/agent_user_profile_test.rb +++ b/test/browser/agent_user_profile_test.rb @@ -2,7 +2,7 @@ require 'browser_test_helper' class AgentUserProfileTest < TestCase def test_user_profile - message = '1 ' + rand(99_999_999).to_s + message = "1 #{rand(99_999_999)}" @browser = browser_instance login( @@ -93,8 +93,8 @@ class AgentUserProfileTest < TestCase data: { customer: 'nico', group: 'Users', - title: 'user profile check ' + message, - body: 'user profile check ' + message, + title: "user profile check #{message}", + body: "user profile check #{message}", }, ) @@ -102,12 +102,12 @@ class AgentUserProfileTest < TestCase user_open_by_search(value: 'Braun') watch_for( css: '.active .profile-window', - value: 'user profile check ' + message, + value: "user profile check #{message}", ) tasks_close_all() # work with two browser windows - message = 'comment 1 ' + rand(99_999_999_999_999_999).to_s + message = "comment 1 #{rand(99_999_999_999_999_999)}" # use current session browser1 = @browser diff --git a/test/browser/maintenance_session_message_test.rb b/test/browser/maintenance_session_message_test.rb index 3464db478..763fbad4b 100644 --- a/test/browser/maintenance_session_message_test.rb +++ b/test/browser/maintenance_session_message_test.rb @@ -86,12 +86,12 @@ class MaintenanceSessionMessageTest < TestCase set( browser: browser1, css: '.content.active .js-Message input[name="head"]', - value: title_html + ' #2', + value: "#{title_html} #2", ) set( browser: browser1, css: '.content.active .js-Message .js-textarea[data-name="message"]', - value: message_html + ' #2', + value: "#{message_html} #2", ) click( @@ -103,12 +103,12 @@ class MaintenanceSessionMessageTest < TestCase watch_for( browser: browser2, css: '.modal', - value: title_text + ' #2', + value: "#{title_text} #2", ) watch_for( browser: browser2, css: '.modal', - value: message_text + ' #2', + value: "#{message_text} #2", ) match_not( @@ -136,12 +136,12 @@ class MaintenanceSessionMessageTest < TestCase set( browser: browser1, css: '.content.active .js-Message input[name="head"]', - value: title_html + ' #3', + value: "#{title_html} #3", ) set( browser: browser1, css: '.content.active .js-Message .js-textarea[data-name="message"]', - value: message_html + ' #3', + value: "#{message_html} #3", ) click( browser: browser1, @@ -156,12 +156,12 @@ class MaintenanceSessionMessageTest < TestCase watch_for( browser: browser2, css: '.modal', - value: title_text + ' #3', + value: "#{title_text} #3", ) watch_for( browser: browser2, css: '.modal', - value: message_text + ' #3', + value: "#{message_text} #3", ) watch_for( browser: browser2, diff --git a/test/browser/manage_test.rb b/test/browser/manage_test.rb index 8be1f0d3c..fdade3e24 100644 --- a/test/browser/manage_test.rb +++ b/test/browser/manage_test.rb @@ -3,7 +3,7 @@ require 'browser_test_helper' class ManageTest < TestCase def test_user random = "manage-test-#{rand(999_999)}" - user_email = random + '@example.com' + user_email = "#{random}@example.com" # user management @browser = browser_instance diff --git a/test/browser/signup_password_change_and_reset_test.rb b/test/browser/signup_password_change_and_reset_test.rb index 67ea1ef72..067609eb5 100644 --- a/test/browser/signup_password_change_and_reset_test.rb +++ b/test/browser/signup_password_change_and_reset_test.rb @@ -136,7 +136,7 @@ class SignupPasswordChangeAndResetTest < TestCase logout() # reset password (not possible) - location(url: browser_url + '/#password_reset_verify/not_existing_token') + location(url: "#{browser_url}/#password_reset_verify/not_existing_token") watch_for( css: 'body', @@ -150,7 +150,7 @@ class SignupPasswordChangeAndResetTest < TestCase url: browser_url, ) - location(url: browser_url + '/#password_reset') + location(url: "#{browser_url}/#password_reset") sleep 1 match_not( diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index 50b224d93..ae05150f9 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -1320,7 +1320,7 @@ set type of task (closeTab, closeNextInOverview, stayOnTab) if !fallback verify_task(params, true) end - raise 'ERROR: ' + e.inspect + raise "ERROR: #{e.inspect}" end true end @@ -1757,7 +1757,7 @@ wait untill text in selector disabppears instance = params[:browser] || @browser - element = instance.find_elements(css: params[:css] + ' input[name="customer_id_completion"]')[0] + element = instance.find_elements(css: %(#{params[:css]} input[name="customer_id_completion"]))[0] element.click element.clear @@ -2378,7 +2378,7 @@ wait untill text in selector disabppears #element.send_keys(:tab) instance.execute_script('$(".content.active .ticketZoom-header .js-objectTitle").focus()') - instance.execute_script('$(".content.active .ticketZoom-header .js-objectTitle").text("' + data[:title] + '")') + instance.execute_script(%($(".content.active .ticketZoom-header .js-objectTitle").text("#{data[:title]}"))) instance.execute_script('$(".content.active .ticketZoom-header .js-objectTitle").blur()') instance.execute_script('$(".content.active .ticketZoom-header .js-objectTitle").trigger("blur")') # { @@ -3660,37 +3660,35 @@ wait untill text in selector disabppears end instance.find_elements(css: '.modal button.js-submit')[0].click modal_disappear(browser: instance) - 11.times do - element = instance.find_elements(css: 'body')[0] - text = element.text - if text.match?(/#{Regexp.quote(data[:name])}/) - assert(true, 'group created') - modal_disappear(browser: instance) # wait until modal has gone - # add member - data[:member]&.each do |member| - instance.find_elements(css: 'a[href="#manage"]')[0].click - sleep 1 - instance.find_elements(css: '.content.active a[href="#manage/users"]')[0].click - sleep 3 - element = instance.find_elements(css: '.content.active [name="search"]')[0] - element.clear - element.send_keys(member[:login]) - sleep 3 - #instance.find_elements(:css => '.content.active table [data-id]')[0].click - instance.execute_script('$(".content.active table [data-id] td").first().click()') - modal_ready(browser: instance) - #instance.find_elements(:css => 'label:contains(" ' + action[:name] + '")')[0].click - instance.execute_script('$(".js-groupList tr:contains(\"' + data[:name] + '\") .js-groupListItem[value=' + member[:access] + ']").prop("checked", true)') - instance.find_elements(css: '.modal button.js-submit')[0].click - modal_disappear(browser: instance) - end + element = instance.find_elements(css: 'body')[0] + text = element.text + if text.match?(/#{Regexp.quote(data[:name])}/) + assert(true, 'group created') + modal_disappear(browser: instance) # wait until modal has gone + + # add member + data[:member]&.each do |member| + instance.find_elements(css: 'a[href="#manage"]')[0].click + sleep 1 + instance.find_elements(css: '.content.active a[href="#manage/users"]')[0].click + sleep 3 + element = instance.find_elements(css: '.content.active [name="search"]')[0] + element.clear + element.send_keys(member[:login]) + sleep 3 + #instance.find_elements(:css => '.content.active table [data-id]')[0].click + instance.execute_script('$(".content.active table [data-id] td").first().click()') + modal_ready(browser: instance) + #instance.find_elements(:css => 'label:contains(" ' + action[:name] + '")')[0].click + instance.execute_script(%($(".js-groupList tr:contains(\\"#{data[:name]}\\") .js-groupListItem[value=#{member[:access]}]").prop("checked", true))) + instance.find_elements(css: '.modal button.js-submit')[0].click + modal_disappear(browser: instance) end - sleep 1 - return true end - screenshot(browser: instance, comment: 'group_create_failed') - raise 'group creation failed' + + sleep 1 + true end =begin @@ -3866,37 +3864,35 @@ wait untill text in selector disabppears instance.find_elements(css: '.modal button.js-submit')[0].click modal_disappear(browser: instance) - 11.times do - element = instance.find_elements(css: 'body')[0] - text = element.text - if text.match?(/#{Regexp.quote(data[:name])}/) - assert(true, 'role created') - modal_disappear(browser: instance) # wait until modal has gone - # add member - data[:member]&.each do |login| - instance.find_elements(css: 'a[href="#manage"]')[0].click - sleep 1 - instance.find_elements(css: '.content.active a[href="#manage/users"]')[0].click - sleep 3 - element = instance.find_elements(css: '.content.active [name="search"]')[0] - element.clear - element.send_keys(login) - sleep 3 - #instance.find_elements(:css => '.content.active table [data-id]')[0].click - instance.execute_script('$(".content.active table [data-id] td").first().click()') - sleep 3 - #instance.find_elements(:css => 'label:contains(" ' + action[:name] + '")')[0].click - instance.execute_script('$(\'label:contains(" ' + data[:name] + '")\').first().click()') - instance.find_elements(css: '.modal button.js-submit')[0].click - modal_disappear(browser: instance) - end + element = instance.find_elements(css: 'body')[0] + text = element.text + if text.match?(/#{Regexp.quote(data[:name])}/) + assert(true, 'role created') + modal_disappear(browser: instance) # wait until modal has gone + + # add member + data[:member]&.each do |login| + instance.find_elements(css: 'a[href="#manage"]')[0].click + sleep 1 + instance.find_elements(css: '.content.active a[href="#manage/users"]')[0].click + sleep 3 + element = instance.find_elements(css: '.content.active [name="search"]')[0] + element.clear + element.send_keys(login) + sleep 3 + #instance.find_elements(:css => '.content.active table [data-id]')[0].click + instance.execute_script('$(".content.active table [data-id] td").first().click()') + sleep 3 + #instance.find_elements(:css => 'label:contains(" ' + action[:name] + '")')[0].click + instance.execute_script(%($('label:contains(" #{data[:name]}")').first().click())) + instance.find_elements(css: '.modal button.js-submit')[0].click + modal_disappear(browser: instance) end - sleep 1 - return true end - screenshot(browser: instance, comment: 'role_create_failed') - raise 'role creation failed' + + sleep 1 + true end =begin @@ -3935,7 +3931,7 @@ wait untill text in selector disabppears css: '.content.active a[href="#manage/roles"]', mute_log: true, ) - instance.execute_script('$(\'.content.active table tr td:contains(" ' + data[:name] + '")\').first().click()') + instance.execute_script(%($('.content.active table tr td:contains(" #{data[:name]}")').first().click())) modal_ready(browser: instance) element = instance.find_elements(css: '.modal input[name=name]')[0] @@ -3991,37 +3987,35 @@ wait untill text in selector disabppears instance.find_elements(css: '.modal button.js-submit')[0].click modal_disappear(browser: instance) - 11.times do - element = instance.find_elements(css: 'body')[0] - text = element.text - if text.match?(/#{Regexp.quote(data[:name])}/) - assert(true, 'role created') - modal_disappear(browser: instance) # wait until modal has gone - # add member - data[:member]&.each do |login| - instance.find_elements(css: 'a[href="#manage"]')[0].click - sleep 1 - instance.find_elements(css: '.content.active a[href="#manage/users"]')[0].click - sleep 3 - element = instance.find_elements(css: '.content.active [name="search"]')[0] - element.clear - element.send_keys(login) - sleep 3 - #instance.find_elements(:css => '.content.active table [data-id]')[0].click - instance.execute_script('$(".content.active table [data-id] td").first().click()') - sleep 3 - #instance.find_elements(:css => 'label:contains(" ' + action[:name] + '")')[0].click - instance.execute_script('$(\'label:contains(" ' + data[:name] + '")\').first().click()') - instance.find_elements(css: '.modal button.js-submit')[0].click - modal_disappear(browser: instance) - end + element = instance.find_elements(css: 'body')[0] + text = element.text + if text.match?(/#{Regexp.quote(data[:name])}/) + assert(true, 'role created') + modal_disappear(browser: instance) # wait until modal has gone + + # add member + data[:member]&.each do |login| + instance.find_elements(css: 'a[href="#manage"]')[0].click + sleep 1 + instance.find_elements(css: '.content.active a[href="#manage/users"]')[0].click + sleep 3 + element = instance.find_elements(css: '.content.active [name="search"]')[0] + element.clear + element.send_keys(login) + sleep 3 + #instance.find_elements(:css => '.content.active table [data-id]')[0].click + instance.execute_script('$(".content.active table [data-id] td").first().click()') + sleep 3 + #instance.find_elements(:css => 'label:contains(" ' + action[:name] + '")')[0].click + instance.execute_script(%($('label:contains(" #{data[:name]}")').first().click())) + instance.find_elements(css: '.modal button.js-submit')[0].click + modal_disappear(browser: instance) end - sleep 1 - return true end - screenshot(browser: instance, comment: 'role_edit_failed') - raise 'role edit failed' + + sleep 1 + true end =begin @@ -4647,7 +4641,7 @@ wait untill text in selector disabppears def fetch_settings url = URI.parse(browser_url) - req = Net::HTTP::Get.new(browser_url + '/api/v1/settings/') + req = Net::HTTP::Get.new("#{browser_url}/api/v1/settings/") req.basic_auth('master@example.com', 'test') res = Net::HTTP.start(url.host, url.port) do |http| diff --git a/test/integration/email_deliver_test.rb b/test/integration/email_deliver_test.rb index 5fd60e1f6..767fc9b74 100644 --- a/test/integration/email_deliver_test.rb +++ b/test/integration/email_deliver_test.rb @@ -209,7 +209,7 @@ class EmailDeliverTest < ActiveSupport::TestCase assert(Delayed::Job.where(attempts: 1).none?) Scheduler.worker(true) - assert(Delayed::Job.where(attempts: 1).exists?) + assert(Delayed::Job.exists?(attempts: 1)) ticket1.reload article2_lookup = Ticket::Article.find(article2.id) @@ -233,7 +233,7 @@ class EmailDeliverTest < ActiveSupport::TestCase travel 26.seconds assert(Delayed::Job.where(attempts: 2).none?) Scheduler.worker(true) - assert(Delayed::Job.where(attempts: 2).exists?) + assert(Delayed::Job.exists?(attempts: 2)) ticket1.reload article2_lookup = Ticket::Article.find(article2.id) @@ -258,7 +258,7 @@ class EmailDeliverTest < ActiveSupport::TestCase travel 51.seconds assert(Delayed::Job.where(attempts: 3).none?) Scheduler.worker(true) - assert(Delayed::Job.where(attempts: 3).exists?) + assert(Delayed::Job.exists?(attempts: 3)) ticket1.reload article2_lookup = Ticket::Article.find(article2.id) diff --git a/test/integration/facebook_browser_test.rb b/test/integration/facebook_browser_test.rb index cc4671176..04a5c542b 100644 --- a/test/integration/facebook_browser_test.rb +++ b/test/integration/facebook_browser_test.rb @@ -143,7 +143,7 @@ class FacebookBrowserTest < TestCase value: 'Dashboard', ) - select(css: '.content.active .modal [name="pages::' + page_id + '::group_id"]', value: 'Users') + select(css: %(.content.active .modal [name="pages::#{page_id}::group_id"]), value: 'Users') sleep 1 click(css: '.content.active .modal .js-submit') sleep 5 diff --git a/test/integration/package_test.rb b/test/integration/package_test.rb index 0385228a5..8a1d50d36 100644 --- a/test/integration/package_test.rb +++ b/test/integration/package_test.rb @@ -307,7 +307,7 @@ class PackageTest < ActiveSupport::TestCase begin package = Package.install(string: test[:zpm]) rescue => e - puts 'ERROR: ' + e.inspect + puts "ERROR: #{e.inspect}" end if test[:result] assert(package, 'install package not successful') @@ -350,10 +350,10 @@ class PackageTest < ActiveSupport::TestCase end when 'auto_install' if test[:zpm] - if !File.exist?(Rails.root.to_s + '/auto_install/') - Dir.mkdir(Rails.root.to_s + '/auto_install/', 0o755) + if !File.exist?(Rails.root.join('auto_install')) + Dir.mkdir(Rails.root.join('auto_install'), 0o755) end - location = Rails.root.to_s + '/auto_install/unittest.zpm' + location = Rails.root.join('auto_install/unittest.zpm') file = File.new(location, 'wb') file.write(test[:zpm]) file.close diff --git a/test/unit/chat_test.rb b/test/unit/chat_test.rb index 1b46f0d40..029dac52a 100644 --- a/test/unit/chat_test.rb +++ b/test/unit/chat_test.rb @@ -51,7 +51,7 @@ class ChatTest < ActiveSupport::TestCase skip "Can't properly disconnect while Spring is in use." if defined?(Spring) - class DummyWs + class DummyWs # rubocop:disable Lint/ConstantDefinitionInBlock def send(msg) Rails.logger.info "WS send: #{msg}" end diff --git a/test/unit/session_enhanced_test.rb b/test/unit/session_enhanced_test.rb index 71cf3a294..41480fc31 100644 --- a/test/unit/session_enhanced_test.rb +++ b/test/unit/session_enhanced_test.rb @@ -177,7 +177,7 @@ class SessionEnhancedTest < ActiveSupport::TestCase roles = Role.where(name: ['Agent']) groups = Group.all organization = Organization.create( - name: 'SomeOrg::' + rand(999_999).to_s, active: true, + name: "SomeOrg::#{rand(999_999)}", active: true, updated_by_id: 1, created_by_id: 1, ) diff --git a/test/unit/ticket_test.rb b/test/unit/ticket_test.rb index 057058670..ccd23157c 100644 --- a/test/unit/ticket_test.rb +++ b/test/unit/ticket_test.rb @@ -266,7 +266,7 @@ class TicketTest < ActiveSupport::TestCase test 'ticket process_pending' do # close all other pending close tickets first - Ticket.where('pending_time IS NOT NULL').each do |ticket| + Ticket.where.not(pending_time: nil).each do |ticket| ticket.state = Ticket::State.lookup(name: 'closed') ticket.save! end diff --git a/test/unit/user_csv_import_test.rb b/test/unit/user_csv_import_test.rb index 7ee152cbb..1b0d63210 100644 --- a/test/unit/user_csv_import_test.rb +++ b/test/unit/user_csv_import_test.rb @@ -131,8 +131,8 @@ class UserCsvImportTest < ActiveSupport::TestCase assert_equal(user1.email, 'user-simple-import1@example.com') assert_equal(user1.active, true) assert_equal(user1.updated_at, user1_1.updated_at) - user2 = user2 - user2_1 = User.find_by(login: 'user-simple-import2') + user2_1 = user2 + user2 = User.find_by(login: 'user-simple-import2') assert(user2) assert_equal(user2.login, 'user-simple-import2') assert_equal(user2.firstname, 'firstname-simple-import2') @@ -168,8 +168,8 @@ class UserCsvImportTest < ActiveSupport::TestCase assert_equal(user1.email, 'user-simple-import1@example.com') assert_equal(user1.active, true) assert_not_equal(user1.updated_at, user1_1.updated_at) - user2 = user2 - user2_1 = User.find_by(login: 'user-simple-import2') + user2_1 = user2 + user2 = User.find_by(login: 'user-simple-import2') assert(user2) assert_equal(user2.login, 'user-simple-import2') assert_equal(user2.firstname, 'firstname-simple-import2')