From fbeccbc85ce5e18e7f72afd5277cf6f79c69b3e9 Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Thu, 12 Apr 2018 16:57:37 +0200 Subject: [PATCH] Updated to latest rubocop version. --- .rubocop.yml | 103 +++++++----------- .rubocop_todo.yml | 80 ++++++++++++++ Gemfile.lock | 17 ++- Rakefile | 2 +- .../external_credentials_controller.rb | 1 - app/models/activity_stream.rb | 10 +- app/models/application_model/has_cache.rb | 4 +- app/models/avatar.rb | 2 +- app/models/channel.rb | 2 +- app/models/chat/session.rb | 7 +- app/models/concerns/can_csv_import.rb | 2 +- app/models/history.rb | 8 ++ app/models/karma/activity_log.rb | 2 +- app/models/link.rb | 7 +- app/models/object_manager/attribute.rb | 12 +- app/models/online_notification.rb | 6 +- app/models/organization.rb | 7 +- app/models/overview.rb | 2 +- app/models/recent_view.rb | 3 + app/models/scheduler.rb | 2 +- app/models/setting.rb | 2 +- app/models/store.rb | 12 +- app/models/tag.rb | 12 +- app/models/ticket.rb | 27 +++-- app/models/ticket/article.rb | 21 ++-- app/models/ticket/state.rb | 9 +- app/models/ticket/state_type.rb | 3 +- app/models/ticket/time_accounting.rb | 4 +- app/models/user.rb | 27 ++--- bin/bundle | 2 +- bin/setup | 2 +- config/application.rb | 2 +- config/boot.rb | 2 +- config/environment.rb | 2 +- config/routes.rb | 2 +- ...160217000001_object_manager_update_user.rb | 3 +- ...01_organization_domain_based_assignment.rb | 3 +- ...3000001_fixed_admin_user_permission_920.rb | 7 +- db/migrate/20170912123300_remove_network.rb | 3 +- ...hange_exchange_external_sync_identifier.rb | 7 +- db/seeds/object_manager_attributes.rb | 1 - lib/core_ext/open-uri.rb | 6 +- lib/email_helper/verify.rb | 6 +- lib/fill_db.rb | 1 + lib/html_sanitizer.rb | 2 +- lib/import/exchange/item_attributes.rb | 2 +- lib/import/zendesk/object_attribute/base.rb | 1 + lib/ldap.rb | 17 +-- lib/ldap/guid.rb | 2 +- lib/notification_factory/mailer.rb | 2 +- lib/password_hash.rb | 3 +- lib/sequencer/units/attributes.rb | 2 +- lib/service/image/zammad.rb | 25 ++--- lib/session_helper.rb | 2 +- lib/sessions/backend/collections.rb | 2 +- script/rails | 4 +- spec/models/import_job_spec.rb | 2 - spec/models/scheduler_spec.rb | 1 + spec/rails_helper.rb | 2 +- test/browser/admin_object_manager_test.rb | 3 +- test/browser_test_helper.rb | 12 +- ...ject_manager_attributes_controller_test.rb | 1 - test/integration/object_manager_test.rb | 1 - test/integration_test_helper.rb | 2 +- test/test_helper.rb | 2 +- test/unit/email_process_auto_response_test.rb | 1 - ...s_bounce_delivery_permanent_failed_test.rb | 1 - test/unit/email_process_bounce_follow_test.rb | 1 - .../notification_factory_renderer_test.rb | 1 - .../notification_factory_template_test.rb | 1 - test/unit/organization_assets_test.rb | 16 +-- test/unit/ticket_article_dos_test.rb | 20 ++-- test/unit/ticket_trigger_test.rb | 1 - test/unit/user_assets_test.rb | 16 +-- 74 files changed, 350 insertions(+), 243 deletions(-) create mode 100644 .rubocop_todo.yml diff --git a/.rubocop.yml b/.rubocop.yml index 6e91d3296..4420a6b76 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,10 @@ # Default enabled cops # https://github.com/bbatsov/rubocop/blob/master/config/enabled.yml +inherit_from: .rubocop_todo.yml + AllCops: + DisplayCopNames: true Exclude: - 'bin/rails' - 'bin/rake' @@ -35,9 +38,13 @@ Style/IfUnlessModifier: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#if-as-a-modifier' Enabled: false -Style/TrailingCommaInLiteral: - Description: 'Checks for trailing comma in array and hash literals.' - StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-trailing-array-commas' +Style/TrailingCommaInArrayLiteral: + Description: 'Checks for trailing comma in array literals.' + StyleGuide: '#no-trailing-array-commas' + Enabled: false + +Style/TrailingCommaInHashLiteral: + Description: 'Checks for trailing comma in hash literals.' Enabled: false Style/TrailingCommaInArguments: @@ -67,9 +74,12 @@ Style/MethodCallWithoutArgsParentheses: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-args-no-parens' Enabled: false -Layout/SpaceInsideBrackets: - Description: 'No spaces after [ or before ].' - StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-spaces-braces' +Layout/SpaceInsideReferenceBrackets: + Description: 'Checks the spacing inside referential brackets.' + Enabled: false + +Layout/SpaceInsideArrayLiteralBrackets: + Description: 'Checks the spacing inside array literal brackets.' Enabled: false Style/DefWithParentheses: @@ -167,68 +177,31 @@ Naming/VariableNumber: Description: 'Use the configured style when numbering variables.' Enabled: false -# 2.0 - -Metrics/PerceivedComplexity: +Naming/UncommunicativeMethodParamName: Description: >- - A complexity metric geared towards measuring complexity for a - human reader. - Enabled: false + Checks for method parameter names that contain capital letters, + end in numbers, or do not meet a minimal length. + Enabled: true + AllowedNames: e, id, _, ip -Metrics/AbcSize: - Description: >- - A calculated magnitude based on number of assignments, - branches, and conditions. - Enabled: false +Lint/BooleanSymbol: + Description: 'Check for `:true` and `:false` symbols.' + Enabled: true + Exclude: + - "db/seeds/object_manager_attributes.rb" + - "test/integration/object_manager_attributes_controller_test.rb" + - "test/integration/object_manager_test.rb" -Metrics/CyclomaticComplexity: - Description: >- - A complexity metric that is strongly correlated to the number - of test cases needed to validate a method. - Enabled: false - -Metrics/BlockNesting: - Description: 'Avoid excessive block nesting' - StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count' - Enabled: false - -Metrics/ModuleLength: - Description: 'Avoid modules longer than 100 lines of code.' - Enabled: false - -Metrics/BlockLength: - Enabled: false - -Lint/RescueWithoutErrorClass: - Enabled: false - -Rails/ApplicationRecord: - Enabled: false - -# TODO - -Rails/HasManyOrHasOneDependent: - Enabled: false - -Style/DateTime: - Enabled: false - -Style/Documentation: - Description: 'Document classes and non-namespace modules.' - Enabled: false - -Lint/UselessAssignment: - Enabled: false - -Layout/ExtraSpacing: - Description: 'Do not use unnecessary spacing.' - Enabled: false - -# Broken!!!! Generates broken code since "String".downcase == "strinG".downcase is not equals "String".casecmp("strinG") but "String".casecmp("strinG") == 0 !!! -Performance/Casecmp: - Description: 'Use `casecmp` rather than `downcase ==`.' - Reference: 'https://github.com/JuanitoFatas/fast-ruby#stringcasecmp-vs-stringdowncase---code' - Enabled: false +Lint/InterpolationCheck: + Description: 'Raise warning for interpolation in single q strs' + Enabled: true + Exclude: + - "test/unit/email_process_auto_response_test.rb" + - "test/unit/email_process_bounce_delivery_permanent_failed_test.rb" + - "test/unit/email_process_bounce_follow_test.rb" + - "test/unit/notification_factory_renderer_test.rb" + - "test/unit/notification_factory_template_test.rb" + - "test/unit/ticket_trigger_test.rb" # RSpec tests Style/NumericPredicate: diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 000000000..9ead11a0b --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,80 @@ +# 10.0 + +Metrics/PerceivedComplexity: + Description: >- + A complexity metric geared towards measuring complexity for a + human reader. + Enabled: false + +Metrics/AbcSize: + Description: >- + A calculated magnitude based on number of assignments, + branches, and conditions. + Enabled: false + +Metrics/CyclomaticComplexity: + Description: >- + A complexity metric that is strongly correlated to the number + of test cases needed to validate a method. + Enabled: false + +Metrics/BlockNesting: + Description: 'Avoid excessive block nesting' + StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#three-is-the-number-thou-shalt-count' + Enabled: false + +Metrics/ModuleLength: + Description: 'Avoid modules longer than 100 lines of code.' + Enabled: false + +Metrics/BlockLength: + Enabled: false + +Style/RescueStandardError: + Description: 'Avoid rescuing without specifying an error class.' + Enabled: false + +# TODO + +Rails/ApplicationRecord: + Description: 'Check that models subclass ApplicationRecord.' + Enabled: false + +Rails/CreateTableWithTimestamps: + Description: >- + Checks the migration for which timestamps are not included + when creating a new table. + Enabled: false + +Rails/HasManyOrHasOneDependent: + Description: 'Define the dependent option to the has_many and has_one associations.' + StyleGuide: 'https://github.com/bbatsov/rails-style-guide#has_many-has_one-dependent-option' + Enabled: false + +Style/DateTime: + Description: 'Use Date or Time over DateTime.' + StyleGuide: '#date--time' + Enabled: false + +Style/Documentation: + Description: 'Document classes and non-namespace modules.' + Enabled: false + +Lint/UselessAssignment: + Enabled: false + +Layout/ExtraSpacing: + Description: 'Do not use unnecessary spacing.' + Enabled: false + +# Broken!!!! Generates broken code since "String".downcase == "strinG".downcase is not equals "String".casecmp("strinG") but "String".casecmp("strinG") == 0 !!! +Performance/Casecmp: + Description: 'Use `casecmp` rather than `downcase ==`.' + Reference: 'https://github.com/JuanitoFatas/fast-ruby#stringcasecmp-vs-stringdowncase---code' + Enabled: false + +# Can be removed after introduction of Sequencer for OTRS migration +Lint/MissingCopEnableDirective: + Enabled: true + Exclude: + - "lib/import/**/*" diff --git a/Gemfile.lock b/Gemfile.lock index 3db4fe6b6..3b6ded1ba 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -68,7 +68,7 @@ GEM argon2 (1.1.4) ffi (~> 1.9) ffi-compiler (~> 0.1) - ast (2.3.0) + ast (2.4.0) autoprefixer-rails (7.1.6) execjs biz (1.7.0) @@ -284,9 +284,9 @@ GEM omniauth-weibo-oauth2 (0.4.5) omniauth (~> 1.5) omniauth-oauth2 (>= 1.4.0) - parallel (1.12.0) - parser (2.4.0.2) - ast (~> 2.3) + parallel (1.12.1) + parser (2.5.0.5) + ast (~> 2.4.0) pg (0.21.0) pluginator (1.5.0) power_assert (1.1.1) @@ -328,8 +328,7 @@ GEM method_source rake (>= 0.8.7) thor (>= 0.18.1, < 2.0) - rainbow (2.2.2) - rake + rainbow (3.0.0) raindrops (0.19.0) rake (12.3.1) rb-fsevent (0.10.2) @@ -358,11 +357,11 @@ GEM rspec-mocks (~> 3.7.0) rspec-support (~> 3.7.0) rspec-support (3.7.0) - rubocop (0.51.0) + rubocop (0.54.0) parallel (~> 1.10) - parser (>= 2.3.3.1, < 3.0) + parser (>= 2.5) powerpack (~> 0.1) - rainbow (>= 2.2.2, < 3.0) + rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) unicode-display_width (~> 1.0, >= 1.0.1) ruby-progressbar (1.9.0) diff --git a/Rakefile b/Rakefile index 6c03f17e3..9e4caa9ae 100755 --- a/Rakefile +++ b/Rakefile @@ -2,6 +2,6 @@ # Add your own tasks in files placed in lib/tasks ending in .rake, # for example lib/tasks/capistrano.rake, and they will automatically be available to Rake. -require File.expand_path('../config/application', __FILE__) +require File.expand_path('config/application', __dir__) Zammad::Application.load_tasks diff --git a/app/controllers/external_credentials_controller.rb b/app/controllers/external_credentials_controller.rb index e79d32579..5b68e9f3c 100644 --- a/app/controllers/external_credentials_controller.rb +++ b/app/controllers/external_credentials_controller.rb @@ -26,7 +26,6 @@ class ExternalCredentialsController < ApplicationController def app_verify attributes = ExternalCredential.app_verify(params) render json: { attributes: attributes }, status: :ok - return rescue => e render json: { error: e.message }, status: :ok end diff --git a/app/models/activity_stream.rb b/app/models/activity_stream.rb index d79c70ee0..31d75d665 100644 --- a/app/models/activity_stream.rb +++ b/app/models/activity_stream.rb @@ -5,8 +5,16 @@ class ActivityStream < ApplicationModel include ActivityStream::Assets self.table_name = 'activity_streams' + + # rubocop:disable Rails/InverseOf belongs_to :object, class_name: 'ObjectLookup', foreign_key: 'activity_stream_object_id' - belongs_to :type, class_name: 'TypeLookup', foreign_key: 'activity_stream_type_id' + belongs_to :type, class_name: 'TypeLookup', foreign_key: 'activity_stream_type_id' + # rubocop:enable Rails/InverseOf + + # the noop is needed since Layout/EmptyLines detects + # the block commend below wrongly as the measurement of + # the wanted indentation of the rubocop re-enabling above + def noop; end =begin diff --git a/app/models/application_model/has_cache.rb b/app/models/application_model/has_cache.rb index 892f4b2ca..9a6f285dc 100644 --- a/app/models/application_model/has_cache.rb +++ b/app/models/application_model/has_cache.rb @@ -11,9 +11,9 @@ module ApplicationModel::HasCache after_destroy :cache_delete end - def cache_update(o) + def cache_update(other) cache_delete if respond_to?('cache_delete') - o.cache_delete if o.respond_to?('cache_delete') + other.cache_delete if other.respond_to?('cache_delete') true end diff --git a/app/models/avatar.rb b/app/models/avatar.rb index fb4b2793b..3600288ae 100644 --- a/app/models/avatar.rb +++ b/app/models/avatar.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Avatar < ApplicationModel - belongs_to :object_lookup, class_name: 'ObjectLookup' + belongs_to :object_lookup =begin diff --git a/app/models/channel.rb b/app/models/channel.rb index 1503e0455..4e0d62477 100644 --- a/app/models/channel.rb +++ b/app/models/channel.rb @@ -4,7 +4,7 @@ class Channel < ApplicationModel load 'channel/assets.rb' include Channel::Assets - belongs_to :group, class_name: 'Group' + belongs_to :group store :options store :preferences diff --git a/app/models/chat/session.rb b/app/models/chat/session.rb index cf83beea4..93c42223f 100644 --- a/app/models/chat/session.rb +++ b/app/models/chat/session.rb @@ -8,10 +8,13 @@ class Chat::Session < ApplicationModel load 'chat/session/assets.rb' include Chat::Session::Assets - has_many :messages, class_name: 'Chat::Message', foreign_key: 'chat_session_id' + # rubocop:disable Rails/InverseOf + has_many :messages, class_name: 'Chat::Message', foreign_key: 'chat_session_id' + # rubocop:enable Rails/InverseOf before_create :generate_session_id - store :preferences + + store :preferences def generate_session_id self.session_id = Digest::MD5.hexdigest(Time.zone.now.to_s + rand(99_999_999_999_999).to_s) diff --git a/app/models/concerns/can_csv_import.rb b/app/models/concerns/can_csv_import.rb index ea8e49c80..273f013a3 100644 --- a/app/models/concerns/can_csv_import.rb +++ b/app/models/concerns/can_csv_import.rb @@ -173,7 +173,7 @@ returns record.with_lock do record.associations_from_param(attributes) - record.update_attributes!(clean_params) + record.update!(clean_params) end rescue => e errors.push "Line #{line_count}: #{e.message}" diff --git a/app/models/history.rb b/app/models/history.rb index 74a9bf6ef..f2ff24c19 100644 --- a/app/models/history.rb +++ b/app/models/history.rb @@ -5,9 +5,17 @@ class History < ApplicationModel include History::Assets self.table_name = 'histories' + + # rubocop:disable Rails/InverseOf belongs_to :history_type, class_name: 'History::Type' belongs_to :history_object, class_name: 'History::Object' belongs_to :history_attribute, class_name: 'History::Attribute' + # rubocop:enable Rails/InverseOf + + # the noop is needed since Layout/EmptyLines detects + # the block commend below wrongly as the measurement of + # the wanted indentation of the rubocop re-enabling above + def noop; end =begin diff --git a/app/models/karma/activity_log.rb b/app/models/karma/activity_log.rb index f36f97f7a..bfdfd611b 100644 --- a/app/models/karma/activity_log.rb +++ b/app/models/karma/activity_log.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Karma::ActivityLog < ApplicationModel - belongs_to :object_lookup, class_name: 'ObjectLookup' + belongs_to :object_lookup self.table_name = 'karma_activity_logs' diff --git a/app/models/link.rb b/app/models/link.rb index 0eca0cac6..0361ce474 100644 --- a/app/models/link.rb +++ b/app/models/link.rb @@ -1,8 +1,11 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Link < ApplicationModel - belongs_to :link_type, class_name: 'Link::Type' - belongs_to :link_object, class_name: 'Link::Object' + + # rubocop:disable Rails/InverseOf + belongs_to :link_type, class_name: 'Link::Type' + belongs_to :link_object, class_name: 'Link::Object' + # rubocop:enable Rails/InverseOf after_destroy :touch_link_references diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index 0cb2f3d5a..bccea00cf 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -4,11 +4,13 @@ class ObjectManager::Attribute < ApplicationModel self.table_name = 'object_manager_attributes' - belongs_to :object_lookup, class_name: 'ObjectLookup' - validates :name, presence: true - store :screens - store :data_option - store :data_option_new + belongs_to :object_lookup + + validates :name, presence: true + + store :screens + store :data_option + store :data_option_new =begin diff --git a/app/models/online_notification.rb b/app/models/online_notification.rb index 3e64937ac..ae05ed875 100644 --- a/app/models/online_notification.rb +++ b/app/models/online_notification.rb @@ -4,9 +4,11 @@ class OnlineNotification < ApplicationModel load 'online_notification/assets.rb' include OnlineNotification::Assets - belongs_to :type, class_name: 'TypeLookup', foreign_key: 'type_lookup_id' - belongs_to :object, class_name: 'ObjectLookup', foreign_key: 'object_lookup_id' belongs_to :user + # rubocop:disable Rails/InverseOf + belongs_to :object, class_name: 'ObjectLookup', foreign_key: 'object_lookup_id' + belongs_to :type, class_name: 'TypeLookup', foreign_key: 'type_lookup_id' + # rubocop:enable Rails/InverseOf after_create :notify_clients_after_change after_update :notify_clients_after_change diff --git a/app/models/organization.rb b/app/models/organization.rb index 93833ec63..d75f7b4b4 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -15,12 +15,15 @@ class Organization < ApplicationModel load 'organization/search_index.rb' include Organization::SearchIndex - has_many :members, class_name: 'User' - validates :name, presence: true + # rubocop:disable Rails/InverseOf + has_many :members, class_name: 'User' + # rubocop:enable Rails/InverseOf before_create :domain_cleanup before_update :domain_cleanup + validates :name, presence: true + activity_stream_permission 'admin.role' private diff --git a/app/models/overview.rb b/app/models/overview.rb index dd1b3aa0a..d45140736 100644 --- a/app/models/overview.rb +++ b/app/models/overview.rb @@ -77,7 +77,7 @@ class Overview < ApplicationModel while check count += 1 exists = Overview.find_by(link: local_lookup_link) - if exists && exists.id != id # rubocop:disable Style/SafeNavigation + if exists && exists.id != id local_lookup_link = "#{local_link}_#{count}" else check = false diff --git a/app/models/recent_view.rb b/app/models/recent_view.rb index 714259fce..9d6363cf9 100644 --- a/app/models/recent_view.rb +++ b/app/models/recent_view.rb @@ -4,7 +4,10 @@ class RecentView < ApplicationModel load 'recent_view/assets.rb' include RecentView::Assets + # rubocop:disable Rails/InverseOf + belongs_to :ticket, foreign_key: 'o_id' belongs_to :object, class_name: 'ObjectLookup', foreign_key: 'recent_view_object_id' + # rubocop:enable Rails/InverseOf after_create :notify_clients after_update :notify_clients diff --git a/app/models/scheduler.rb b/app/models/scheduler.rb index 0c8b726c8..9b4c17c93 100644 --- a/app/models/scheduler.rb +++ b/app/models/scheduler.rb @@ -311,7 +311,7 @@ class Scheduler < ApplicationModel sleep wait logger.debug { '*** worker thread loop' } else - format "*** #{count} jobs processed at %.4f j/s, %d failed ...\n", count / realtime, result.last + format "*** #{count} jobs processed at %.4f j/s, %d failed ...\n", jps: count / realtime, failed: result.last end end diff --git a/app/models/setting.rb b/app/models/setting.rb index c2ad31123..73058483c 100644 --- a/app/models/setting.rb +++ b/app/models/setting.rb @@ -163,7 +163,7 @@ reload config settings # convert state into hash to be able to store it as store def state_check return true if !state - return true if state && state.respond_to?('has_key?') && state.key?(:value) + return true if state.try(:key?, :value) self.state_current = { value: state } true end diff --git a/app/models/store.rb b/app/models/store.rb index 45200c455..03354f47a 100644 --- a/app/models/store.rb +++ b/app/models/store.rb @@ -4,10 +4,14 @@ class Store < ApplicationModel load 'store/object.rb' load 'store/file.rb' - store :preferences - belongs_to :store_object, class_name: 'Store::Object' - belongs_to :store_file, class_name: 'Store::File' - validates :filename, presence: true + # rubocop:disable Rails/InverseOf + belongs_to :store_object, class_name: 'Store::Object' + belongs_to :store_file, class_name: 'Store::File' + # rubocop:enable Rails/InverseOf + + validates :filename, presence: true + + store :preferences =begin diff --git a/app/models/tag.rb b/app/models/tag.rb index 1e977df2f..8410b2aa3 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -1,8 +1,16 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Tag < ApplicationModel - belongs_to :tag_object, class_name: 'Tag::Object' - belongs_to :tag_item, class_name: 'Tag::Item' + + # rubocop:disable Rails/InverseOf + belongs_to :tag_object, class_name: 'Tag::Object' + belongs_to :tag_item, class_name: 'Tag::Item' + # rubocop:enable Rails/InverseOf + + # the noop is needed since Layout/EmptyLines detects + # the block commend below wrongly as the measurement of + # the wanted indentation of the rubocop re-enabling above + def noop; end =begin diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 1dc8320ca..c226fd07f 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -55,18 +55,21 @@ class Ticket < ApplicationModel :article_count, :preferences - belongs_to :group, class_name: 'Group' - has_many :articles, class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update, dependent: :destroy - has_many :ticket_time_accounting, class_name: 'Ticket::TimeAccounting', dependent: :destroy - belongs_to :organization, class_name: 'Organization' - belongs_to :state, class_name: 'Ticket::State' - belongs_to :priority, class_name: 'Ticket::Priority' - belongs_to :owner, class_name: 'User' - belongs_to :customer, class_name: 'User' - belongs_to :created_by, class_name: 'User' - belongs_to :updated_by, class_name: 'User' - belongs_to :create_article_type, class_name: 'Ticket::Article::Type' - belongs_to :create_article_sender, class_name: 'Ticket::Article::Sender' + belongs_to :group + belongs_to :organization + has_many :articles, class_name: 'Ticket::Article', after_add: :cache_update, after_remove: :cache_update, dependent: :destroy, inverse_of: :ticket + has_many :ticket_time_accounting, class_name: 'Ticket::TimeAccounting', dependent: :destroy, inverse_of: :ticket + + # rubocop:disable Rails/InverseOf + belongs_to :state, class_name: 'Ticket::State' + belongs_to :priority, class_name: 'Ticket::Priority' + belongs_to :owner, class_name: 'User' + belongs_to :customer, class_name: 'User' + belongs_to :created_by, class_name: 'User' + belongs_to :updated_by, class_name: 'User' + belongs_to :create_article_type, class_name: 'Ticket::Article::Type' + belongs_to :create_article_sender, class_name: 'Ticket::Article::Sender' + # rubocop:enable Rails/InverseOf self.inheritance_column = nil diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index 187472085..e3cec702e 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -10,18 +10,23 @@ class Ticket::Article < ApplicationModel load 'ticket/article/assets.rb' include Ticket::Article::Assets - belongs_to :ticket - has_one :ticket_time_accounting, class_name: 'Ticket::TimeAccounting', foreign_key: :ticket_article_id, dependent: :destroy - belongs_to :type, class_name: 'Ticket::Article::Type' - belongs_to :sender, class_name: 'Ticket::Article::Sender' - belongs_to :created_by, class_name: 'User' - belongs_to :updated_by, class_name: 'User' - belongs_to :origin_by, class_name: 'User' - store :preferences + belongs_to :ticket + has_one :ticket_time_accounting, class_name: 'Ticket::TimeAccounting', foreign_key: :ticket_article_id, dependent: :destroy, inverse_of: :ticket_article + + # rubocop:disable Rails/InverseOf + belongs_to :type, class_name: 'Ticket::Article::Type' + belongs_to :sender, class_name: 'Ticket::Article::Sender' + belongs_to :created_by, class_name: 'User' + belongs_to :updated_by, class_name: 'User' + belongs_to :origin_by, class_name: 'User' + # rubocop:enable Rails/InverseOf + before_create :check_subject, :check_body, :check_message_id_md5 before_update :check_subject, :check_body, :check_message_id_md5 after_destroy :store_delete + store :preferences + sanitized_html :body activity_stream_permission 'ticket.agent' diff --git a/app/models/ticket/state.rb b/app/models/ticket/state.rb index 9f75ef380..6b126269b 100644 --- a/app/models/ticket/state.rb +++ b/app/models/ticket/state.rb @@ -2,13 +2,16 @@ class Ticket::State < ApplicationModel include ChecksLatestChangeObserved + belongs_to :state_type, class_name: 'Ticket::StateType', inverse_of: :states + # rubocop:disable Rails/InverseOf + belongs_to :next_state, class_name: 'Ticket::State' + # rubocop:enable Rails/InverseOf + after_create :ensure_defaults after_update :ensure_defaults after_destroy :ensure_defaults - belongs_to :state_type, class_name: 'Ticket::StateType' - belongs_to :next_state, class_name: 'Ticket::State' - validates :name, presence: true + validates :name, presence: true attr_accessor :callback_loop diff --git a/app/models/ticket/state_type.rb b/app/models/ticket/state_type.rb index 454b2727e..a1bbb65f4 100644 --- a/app/models/ticket/state_type.rb +++ b/app/models/ticket/state_type.rb @@ -2,6 +2,7 @@ class Ticket::StateType < ApplicationModel include ChecksLatestChangeObserved - has_many :states, class_name: 'Ticket::State' + has_many :states, class_name: 'Ticket::State', inverse_of: :state_type + validates :name, presence: true end diff --git a/app/models/ticket/time_accounting.rb b/app/models/ticket/time_accounting.rb index 6e2a73d3c..599606642 100644 --- a/app/models/ticket/time_accounting.rb +++ b/app/models/ticket/time_accounting.rb @@ -1,8 +1,8 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Ticket::TimeAccounting < ApplicationModel - belongs_to :ticket - belongs_to :ticket_article, class_name: 'Ticket::Article' + belongs_to :ticket + belongs_to :ticket_article, class_name: 'Ticket::Article', inverse_of: :ticket_time_accounting after_create :ticket_time_unit_update after_update :ticket_time_unit_update diff --git a/app/models/user.rb b/app/models/user.rb index 6d78906bb..868920364 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -39,6 +39,12 @@ class User < ApplicationModel load 'user/search_index.rb' include User::SearchIndex + has_and_belongs_to_many :roles, after_add: %i[cache_update check_notifications], after_remove: :cache_update, before_add: %i[validate_agent_limit_by_role validate_roles], before_remove: :last_admin_check_by_role, class_name: 'Role' + has_and_belongs_to_many :organizations, after_add: :cache_update, after_remove: :cache_update, class_name: 'Organization' + has_many :tokens, after_add: :cache_update, after_remove: :cache_update + has_many :authorizations, after_add: :cache_update, after_remove: :cache_update + belongs_to :organization, inverse_of: :members + before_validation :check_name, :check_email, :check_login, :check_mail_delivery_failed, :ensure_uniq_email, :ensure_password, :ensure_roles, :ensure_identifier before_create :check_preferences_default, :validate_ooo, :domain_based_assignment, :set_locale before_update :check_preferences_default, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute @@ -46,14 +52,7 @@ class User < ApplicationModel after_update :avatar_for_email_check before_destroy :avatar_destroy, :user_device_destroy, :cit_caller_id_destroy, :task_destroy - has_and_belongs_to_many :roles, after_add: %i[cache_update check_notifications], after_remove: :cache_update, before_add: %i[validate_agent_limit_by_role validate_roles], before_remove: :last_admin_check_by_role, class_name: 'Role' - has_and_belongs_to_many :organizations, after_add: :cache_update, after_remove: :cache_update, class_name: 'Organization' - #has_many :permissions, class_name: 'Permission', through: :roles, class_name: 'Role' - has_many :tokens, after_add: :cache_update, after_remove: :cache_update - has_many :authorizations, after_add: :cache_update, after_remove: :cache_update - belongs_to :organization, class_name: 'Organization' - - store :preferences + store :preferences activity_stream_permission 'admin.user' @@ -804,12 +803,12 @@ returns true end - def check_notifications(o, should_save = true) + def check_notifications(other, should_save = true) default = Rails.configuration.preferences_default_by_permission return if !default default.deep_stringify_keys! has_changed = false - o.permissions.each do |permission| + other.permissions.each do |permission| next if !default[permission.name] default[permission.name].each do |key, value| next if preferences[key] @@ -871,11 +870,7 @@ returns if (firstname.blank? && lastname.present?) || (firstname.present? && lastname.blank?) # "Lastname, Firstname" - used_name = if firstname.blank? - lastname - else - firstname - end + used_name = firstname.presence || lastname name = used_name.split(', ', 2) if name.count == 2 if name[0].present? @@ -948,7 +943,7 @@ returns check = true while check exists = User.find_by(login: login) - if exists && exists.id != id # rubocop:disable Style/SafeNavigation + if exists && exists.id != id self.login = "#{login}#{rand(999)}" else check = false diff --git a/bin/bundle b/bin/bundle index 66e9889e8..f19acf5b5 100755 --- a/bin/bundle +++ b/bin/bundle @@ -1,3 +1,3 @@ #!/usr/bin/env ruby -ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__) +ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) load Gem.bin_path('bundler', 'bundle') diff --git a/bin/setup b/bin/setup index 016034e94..319f93a92 100755 --- a/bin/setup +++ b/bin/setup @@ -2,7 +2,7 @@ require 'pathname' # path to your application root. -APP_ROOT = Pathname.new File.expand_path('../../', __FILE__) +APP_ROOT = Pathname.new File.expand_path('..', __dir__) Dir.chdir APP_ROOT do # This script is a starting point to setup your application. diff --git a/config/application.rb b/config/application.rb index 5945668f0..4fe500369 100644 --- a/config/application.rb +++ b/config/application.rb @@ -1,4 +1,4 @@ -require File.expand_path('../boot', __FILE__) +require File.expand_path('boot', __dir__) require 'rails/all' diff --git a/config/boot.rb b/config/boot.rb index 6b750f00b..30f5120df 100644 --- a/config/boot.rb +++ b/config/boot.rb @@ -1,3 +1,3 @@ -ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../../Gemfile', __FILE__) +ENV['BUNDLE_GEMFILE'] ||= File.expand_path('../Gemfile', __dir__) require 'bundler/setup' # Set up gems listed in the Gemfile. diff --git a/config/environment.rb b/config/environment.rb index ee8d90dc6..0b8bdd828 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -1,5 +1,5 @@ # Load the Rails application. -require File.expand_path('../application', __FILE__) +require File.expand_path('application', __dir__) # Initialize the Rails application. Rails.application.initialize! diff --git a/config/routes.rb b/config/routes.rb index d523fcc7f..3baffc146 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -8,7 +8,7 @@ Rails.application.routes.draw do root to: 'init#index', via: :get # load routes from external files - dir = File.expand_path('../', __FILE__) + dir = File.expand_path(__dir__) files = Dir.glob( "#{dir}/routes/*.rb" ) files.each do |file| if Rails.configuration.cache_classes diff --git a/db/migrate/20160217000001_object_manager_update_user.rb b/db/migrate/20160217000001_object_manager_update_user.rb index 5d307ae1e..5a9dded3c 100644 --- a/db/migrate/20160217000001_object_manager_update_user.rb +++ b/db/migrate/20160217000001_object_manager_update_user.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/BooleanSymbol class ObjectManagerUpdateUser < ActiveRecord::Migration[4.2] def up @@ -569,6 +568,7 @@ class ObjectManagerUpdateUser < ActiveRecord::Migration[4.2] position: 1400, ) + # rubocop:disable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, object: 'User', @@ -607,6 +607,7 @@ class ObjectManagerUpdateUser < ActiveRecord::Migration[4.2] to_delete: false, position: 1490, ) + # rubocop:enable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, diff --git a/db/migrate/20161112000001_organization_domain_based_assignment.rb b/db/migrate/20161112000001_organization_domain_based_assignment.rb index afce02e32..176d8d538 100644 --- a/db/migrate/20161112000001_organization_domain_based_assignment.rb +++ b/db/migrate/20161112000001_organization_domain_based_assignment.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/BooleanSymbol class OrganizationDomainBasedAssignment < ActiveRecord::Migration[4.2] def up # return if it's a new setup @@ -8,6 +7,7 @@ class OrganizationDomainBasedAssignment < ActiveRecord::Migration[4.2] add_column :organizations, :domain_assignment, :boolean, null: false, default: false add_index :organizations, [:domain] + # rubocop:disable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, object: 'Organization', @@ -46,6 +46,7 @@ class OrganizationDomainBasedAssignment < ActiveRecord::Migration[4.2] updated_by_id: 1, created_by_id: 1, ) + # rubocop:enable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, diff --git a/db/migrate/20170403000001_fixed_admin_user_permission_920.rb b/db/migrate/20170403000001_fixed_admin_user_permission_920.rb index c1387387b..a0b3199a8 100644 --- a/db/migrate/20170403000001_fixed_admin_user_permission_920.rb +++ b/db/migrate/20170403000001_fixed_admin_user_permission_920.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/BooleanSymbol class FixedAdminUserPermission920 < ActiveRecord::Migration[4.2] def up @@ -331,6 +330,7 @@ class FixedAdminUserPermission920 < ActiveRecord::Migration[4.2] position: 100, ) + # rubocop:disable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, object: 'TicketArticle', @@ -363,6 +363,7 @@ class FixedAdminUserPermission920 < ActiveRecord::Migration[4.2] to_delete: false, position: 200, ) + # rubocop:enable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, @@ -487,6 +488,7 @@ class FixedAdminUserPermission920 < ActiveRecord::Migration[4.2] position: 1400, ) + # rubocop:disable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, object: 'User', @@ -523,6 +525,7 @@ class FixedAdminUserPermission920 < ActiveRecord::Migration[4.2] to_delete: false, position: 1490, ) + # rubocop:enable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, @@ -597,6 +600,7 @@ class FixedAdminUserPermission920 < ActiveRecord::Migration[4.2] position: 1800, ) + # rubocop:disable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, object: 'Organization', @@ -672,6 +676,7 @@ class FixedAdminUserPermission920 < ActiveRecord::Migration[4.2] to_delete: false, position: 1410, ) + # rubocop:enable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, diff --git a/db/migrate/20170912123300_remove_network.rb b/db/migrate/20170912123300_remove_network.rb index fa8f29434..fe0473729 100644 --- a/db/migrate/20170912123300_remove_network.rb +++ b/db/migrate/20170912123300_remove_network.rb @@ -1,10 +1,10 @@ -# rubocop:disable Rails/ReversibleMigration class RemoveNetwork < ActiveRecord::Migration[5.0] # rewinds db/migrate/20120101000020_create_network.rb def change return if !ActiveRecord::Base.connection.table_exists? 'networks' + # rubocop:disable Rails/ReversibleMigration drop_table :networks drop_table :network_category_types drop_table :network_privacies @@ -15,5 +15,6 @@ class RemoveNetwork < ActiveRecord::Migration[5.0] drop_table :network_item_plus drop_table :network_category_subscriptions drop_table :network_item_subscriptions + # rubocop:enable Rails/ReversibleMigration end end diff --git a/db/migrate/20180108000001_change_exchange_external_sync_identifier.rb b/db/migrate/20180108000001_change_exchange_external_sync_identifier.rb index 7e49a6213..4c2de5e57 100644 --- a/db/migrate/20180108000001_change_exchange_external_sync_identifier.rb +++ b/db/migrate/20180108000001_change_exchange_external_sync_identifier.rb @@ -3,8 +3,11 @@ class ChangeExchangeExternalSyncIdentifier < ActiveRecord::Migration[5.1] # return if it's a new setup to avoid running the migration return if !Setting.find_by(name: 'system_init_done') - # rubocop:disable Rails/SkipsModelValidations - ExternalSync.where(source: 'EWS::FolderContact').update_all(source: 'Exchange::FolderContact') + ExternalSync.where( + source: 'EWS::FolderContact' + ).update_all( # rubocop:disable Rails/SkipsModelValidations + source: 'Exchange::FolderContact' + ) end end diff --git a/db/seeds/object_manager_attributes.rb b/db/seeds/object_manager_attributes.rb index 560b3a210..fd0f0df44 100644 --- a/db/seeds/object_manager_attributes.rb +++ b/db/seeds/object_manager_attributes.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/BooleanSymbol ObjectManager::Attribute.add( force: true, object: 'Ticket', diff --git a/lib/core_ext/open-uri.rb b/lib/core_ext/open-uri.rb index 2a262b467..9d9db6602 100644 --- a/lib/core_ext/open-uri.rb +++ b/lib/core_ext/open-uri.rb @@ -1,11 +1,9 @@ -# rubocop:disable Naming/FileName -# rubocop:disable Style/CommentedKeyword -if Kernel.respond_to?(:open_uri_original_open) +if Kernel.respond_to?(:open_uri_original_open) # rubocop:disable Naming/FileName module Kernel private # see: https://github.com/ruby/ruby/pull/1675 - def open(name, *rest, &block) # :doc: + def open(name, *rest, &block) if name.respond_to?(:open) && name.method(:open).parameters.present? name.open(*rest, &block) elsif name.respond_to?(:to_str) && diff --git a/lib/email_helper/verify.rb b/lib/email_helper/verify.rb index 933e522de..25c42c958 100644 --- a/lib/email_helper/verify.rb +++ b/lib/email_helper/verify.rb @@ -56,11 +56,7 @@ or def self.email(params) # send verify email - subject = if params[:subject].blank? - '#' + rand(99_999_999_999).to_s - else - params[:subject] - end + subject = params[:subject].presence || '#' + rand(99_999_999_999).to_s result = EmailHelper::Probe.outbound(params[:outbound], params[:sender], subject) if result[:result] != 'ok' result[:source] = 'outbound' diff --git a/lib/fill_db.rb b/lib/fill_db.rb index a02bd298f..fb3fa7fdf 100644 --- a/lib/fill_db.rb +++ b/lib/fill_db.rb @@ -205,3 +205,4 @@ or if you only want to create 100 tickets end end end +# rubocop:enable Rails/Output diff --git a/lib/html_sanitizer.rb b/lib/html_sanitizer.rb index 750a019f9..3fc361c69 100644 --- a/lib/html_sanitizer.rb +++ b/lib/html_sanitizer.rb @@ -168,7 +168,7 @@ satinize html string based on whiltelist end # remove attributes if not whitelisted - node.each do |attribute, _value| # rubocop:disable Performance/HashEachMethods + node.each do |attribute, _value| attribute_name = attribute.downcase next if attributes_whitelist[:all].include?(attribute_name) || (attributes_whitelist[node.name]&.include?(attribute_name)) node.delete(attribute) diff --git a/lib/import/exchange/item_attributes.rb b/lib/import/exchange/item_attributes.rb index 2d8c375af..4eb111bfa 100644 --- a/lib/import/exchange/item_attributes.rb +++ b/lib/import/exchange/item_attributes.rb @@ -11,7 +11,7 @@ module Import end def extract - @attributes ||= begin + @extract ||= begin properties = @resource.get_all_properties! result = normalize(properties) flattened = flatten(result) diff --git a/lib/import/zendesk/object_attribute/base.rb b/lib/import/zendesk/object_attribute/base.rb index a444a1c3f..b1ae02c9d 100644 --- a/lib/import/zendesk/object_attribute/base.rb +++ b/lib/import/zendesk/object_attribute/base.rb @@ -21,6 +21,7 @@ module Import rescue => e # rubocop:disable Style/SpecialGlobalVars raise $!, "Problem with ObjectManager Attribute '#{name}': #{$!}", $!.backtrace + # rubocop:enable Style/SpecialGlobalVars end def attribute_config(object, name, attribute) diff --git a/lib/ldap.rb b/lib/ldap.rb index 7c0623a9e..fbb090fdc 100644 --- a/lib/ldap.rb +++ b/lib/ldap.rb @@ -11,7 +11,7 @@ require 'net/ldap/entry' # @return [String] the base dn used while initializing the connection class Ldap - attr_reader :connection, :base_dn, :host, :port, :ssl + attr_reader :base_dn, :host, :port, :ssl # Initializes a LDAP connection. # @@ -35,7 +35,8 @@ class Ldap @config = Setting.get('ldap_config') end - connect + # connect on initialization + connection end # Requests the rootDSE (the root of the directory data tree on a directory server). @@ -46,7 +47,7 @@ class Ldap # # @return [Hash{String => Array}] The found RootDSEs. def preferences - @connection.search_root_dse.to_h + connection.search_root_dse.to_h end # Performs a LDAP search and yields over the found LDAP entries. @@ -68,7 +69,7 @@ class Ldap base ||= base_dn() scope ||= Net::LDAP::SearchScope_WholeSubtree - @connection.search( + connection.search( base: base, filter: filter, scope: scope, @@ -114,15 +115,15 @@ class Ldap counter end - private - - def connect + def connection @connection ||= begin attributes_from_config binded_connection end end + private + def binded_connection # ldap connect ldap = Net::LDAP.new(connection_params) @@ -175,7 +176,7 @@ class Ldap # fallback to default # port if none given - @port ||= 389 + @port ||= 389 # rubocop:disable Naming/MemoizedInstanceVariableName end def parse_host_url diff --git a/lib/ldap/guid.rb b/lib/ldap/guid.rb index e82c54e95..562aceb2a 100644 --- a/lib/ldap/guid.rb +++ b/lib/ldap/guid.rb @@ -76,7 +76,7 @@ class Ldap # # @return [String] The GUID equivalent of the HEX string. def string - oracle_raw16(guid.unpack('H*').first, dashify: true) + oracle_raw16(guid.unpack1('H*'), dashify: true) end private diff --git a/lib/notification_factory/mailer.rb b/lib/notification_factory/mailer.rb index 6472543f8..cbbb76cc7 100644 --- a/lib/notification_factory/mailer.rb +++ b/lib/notification_factory/mailer.rb @@ -172,7 +172,7 @@ returns ) # rebuild subject - if data[:main_object] && data[:main_object].respond_to?(:subject_build) + if data[:main_object].respond_to?(:subject_build) result[:subject] = data[:main_object].subject_build(result[:subject]) end diff --git a/lib/password_hash.rb b/lib/password_hash.rb index 831a8a2e8..0882d789a 100644 --- a/lib/password_hash.rb +++ b/lib/password_hash.rb @@ -3,8 +3,7 @@ module PasswordHash include ApplicationLib - # rubocop:disable Style/ModuleFunction - extend self + extend self # rubocop:disable Style/ModuleFunction def crypt(password) argon2.create(password) diff --git a/lib/sequencer/units/attributes.rb b/lib/sequencer/units/attributes.rb index 9c5a72136..a649d8f84 100644 --- a/lib/sequencer/units/attributes.rb +++ b/lib/sequencer/units/attributes.rb @@ -53,7 +53,7 @@ class Sequencer end def __setobj__(declarations) - @attributes ||= begin + @attributes ||= begin # rubocop:disable Naming/MemoizedInstanceVariableName attributes = Hash.new do |hash, key| hash[key] = Sequencer::Units::Attribute.new end diff --git a/lib/service/image/zammad.rb b/lib/service/image/zammad.rb index dcae93943..312e2c7fd 100644 --- a/lib/service/image/zammad.rb +++ b/lib/service/image/zammad.rb @@ -2,11 +2,10 @@ class Service::Image::Zammad - # rubocop:disable Style/ClassVars - @@api_host = 'https://images.zammad.com' - @@open_timeout = 4 - @@read_timeout = 6 - @@total_timeout = 6 + API_HOST = 'https://images.zammad.com'.freeze + OPEN_TIMEOUT = 4 + READ_TIMEOUT = 6 + TOTAL_TIMEOUT = 6 def self.user(email) raise Exceptions::UnprocessableEntity, 'no email given' if email.blank? @@ -17,14 +16,14 @@ class Service::Image::Zammad # fetch image response = UserAgent.post( - "#{@@api_host}/api/v1/person/image", + "#{API_HOST}/api/v1/person/image", { email: email, }, { - open_timeout: @@open_timeout, - read_timeout: @@read_timeout, - total_timeout: @@total_timeout, + open_timeout: OPEN_TIMEOUT, + read_timeout: READ_TIMEOUT, + total_timeout: TOTAL_TIMEOUT, }, ) if !response.success? @@ -50,14 +49,14 @@ class Service::Image::Zammad # fetch org logo response = UserAgent.post( - "#{@@api_host}/api/v1/organization/image", + "#{API_HOST}/api/v1/organization/image", { domain: domain }, { - open_timeout: @@open_timeout, - read_timeout: @@read_timeout, - total_timeout: @@total_timeout, + open_timeout: OPEN_TIMEOUT, + read_timeout: READ_TIMEOUT, + total_timeout: TOTAL_TIMEOUT, }, ) if !response.success? diff --git a/lib/session_helper.rb b/lib/session_helper.rb index ef72786d6..3beea0f80 100644 --- a/lib/session_helper.rb +++ b/lib/session_helper.rb @@ -5,7 +5,7 @@ module SessionHelper default_collection = {} # load collections to deliver from external files - dir = File.expand_path('../../', __FILE__) + dir = File.expand_path('..', __dir__) files = Dir.glob( "#{dir}/app/controllers/sessions/collection_*.rb") files.each do |file| load file diff --git a/lib/sessions/backend/collections.rb b/lib/sessions/backend/collections.rb index 794fbe5c4..c74812a15 100644 --- a/lib/sessions/backend/collections.rb +++ b/lib/sessions/backend/collections.rb @@ -28,7 +28,7 @@ class Sessions::Backend::Collections < Sessions::Backend::Base backends = [] # load collections to deliver from external files - dir = File.expand_path('../../../../', __FILE__) + dir = File.expand_path('../../..', __dir__) files = Dir.glob("#{dir}/lib/sessions/backend/collections/*.rb") files.each do |file| file.gsub!("#{dir}/lib/", '') diff --git a/script/rails b/script/rails index f8da2cffd..f653fc8de 100755 --- a/script/rails +++ b/script/rails @@ -1,6 +1,6 @@ #!/usr/bin/env ruby # This command will automatically be run when you run "rails" with Rails 3 gems installed from the root of your application. -APP_PATH = File.expand_path('../../config/application', __FILE__) -require File.expand_path('../../config/boot', __FILE__) +APP_PATH = File.expand_path('../config/application', __dir__) +require File.expand_path('../config/boot', __dir__) require 'rails/commands' diff --git a/spec/models/import_job_spec.rb b/spec/models/import_job_spec.rb index 0393cf416..0f5f88f34 100644 --- a/spec/models/import_job_spec.rb +++ b/spec/models/import_job_spec.rb @@ -57,7 +57,6 @@ RSpec.describe ImportJob do payload: {}, delay: false ) - end.not_to change { Delayed::Job.count } @@ -72,7 +71,6 @@ RSpec.describe ImportJob do name: test_backend_name, payload: {}, ) - end.not_to change { Delayed::Job.count } diff --git a/spec/models/scheduler_spec.rb b/spec/models/scheduler_spec.rb index 78c820cec..aef9511cc 100644 --- a/spec/models/scheduler_spec.rb +++ b/spec/models/scheduler_spec.rb @@ -14,6 +14,7 @@ RSpec.describe Scheduler do def self.reschedule=(reschedule) @reschedule = reschedule end + # rubocop:enable Style/TrivialAccessors def self.reschedule?(_delayed_job) @reschedule || false diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 0e350a2ec..2445b5d34 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,6 +1,6 @@ # This file is copied to spec/ when you run 'rails generate rspec:install' ENV['RAILS_ENV'] ||= 'test' -require File.expand_path('../../config/environment', __FILE__) +require File.expand_path('../config/environment', __dir__) # Prevent database truncation if the environment is production abort('The Rails environment is running in production mode!') if Rails.env.production? diff --git a/test/browser/admin_object_manager_test.rb b/test/browser/admin_object_manager_test.rb index 691cb5a55..d54222884 100644 --- a/test/browser/admin_object_manager_test.rb +++ b/test/browser/admin_object_manager_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/BooleanSymbol require 'browser_test_helper' class AdminObjectManagerTest < TestCase @@ -248,6 +247,7 @@ class AdminObjectManagerTest < TestCase }, ) + # rubocop:disable Lint/BooleanSymbol object_manager_attribute_create( data: { name: 'browser_test7', @@ -262,6 +262,7 @@ class AdminObjectManagerTest < TestCase }, }, ) + # rubocop:enable Lint/BooleanSymbol watch_for( css: '.content.active', diff --git a/test/browser_test_helper.rb b/test/browser_test_helper.rb index edb20d00d..6d2f18a16 100644 --- a/test/browser_test_helper.rb +++ b/test/browser_test_helper.rb @@ -1,10 +1,12 @@ ENV['RAILS_ENV'] = 'test' -# rubocop:disable HandleExceptions, ClassVars, NonLocalExitFromIterator, Style/GuardClause -require File.expand_path('../../config/environment', __FILE__) +# rubocop:disable HandleExceptions, NonLocalExitFromIterator, Style/GuardClause, Lint/MissingCopEnableDirective +require File.expand_path('../config/environment', __dir__) require 'selenium-webdriver' class TestCase < Test::Unit::TestCase - @@debug = true + + DEBUG = true + def browser ENV['BROWSER'] || 'firefox' end @@ -60,7 +62,7 @@ class TestCase < Test::Unit::TestCase local_browser = browser_instance_remote break rescue - wait_until_ready = rand(9) + 5 + wait_until_ready = rand(5..13) sleep wait_until_ready log('browser_instance', { rescure: true, count: count, sleep: wait_until_ready }) end @@ -3696,7 +3698,7 @@ wait untill text in selector disabppears rescue # failed to get logs end - return if !@@debug + return if !DEBUG return if params[:mute_log] puts "#{Time.zone.now}/#{method}: #{params.inspect}" end diff --git a/test/integration/object_manager_attributes_controller_test.rb b/test/integration/object_manager_attributes_controller_test.rb index 470292093..f209c6684 100644 --- a/test/integration/object_manager_attributes_controller_test.rb +++ b/test/integration/object_manager_attributes_controller_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/BooleanSymbol require 'test_helper' require 'rake' diff --git a/test/integration/object_manager_test.rb b/test/integration/object_manager_test.rb index 47d9db91a..75d53be6d 100644 --- a/test/integration/object_manager_test.rb +++ b/test/integration/object_manager_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/BooleanSymbol require 'test_helper' class ObjectManagerTest < ActiveSupport::TestCase diff --git a/test/integration_test_helper.rb b/test/integration_test_helper.rb index ca23a6622..9ded0c718 100644 --- a/test/integration_test_helper.rb +++ b/test/integration_test_helper.rb @@ -1,5 +1,5 @@ ENV['RAILS_ENV'] = 'test' -require File.expand_path('../../config/environment', __FILE__) +require File.expand_path('../config/environment', __dir__) require 'rails/test_help' require 'cache' diff --git a/test/test_helper.rb b/test/test_helper.rb index 51bb095e5..4cd429615 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,5 +1,5 @@ ENV['RAILS_ENV'] = 'test' -require File.expand_path('../../config/environment', __FILE__) +require File.expand_path('../config/environment', __dir__) require 'rails/test_help' require 'cache' require 'simplecov' diff --git a/test/unit/email_process_auto_response_test.rb b/test/unit/email_process_auto_response_test.rb index d37c1f1d7..c22bf9b84 100644 --- a/test/unit/email_process_auto_response_test.rb +++ b/test/unit/email_process_auto_response_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/InterpolationCheck require 'test_helper' class EmailProcessAutoResponseTest < ActiveSupport::TestCase diff --git a/test/unit/email_process_bounce_delivery_permanent_failed_test.rb b/test/unit/email_process_bounce_delivery_permanent_failed_test.rb index 75d5b62ce..98701ab08 100644 --- a/test/unit/email_process_bounce_delivery_permanent_failed_test.rb +++ b/test/unit/email_process_bounce_delivery_permanent_failed_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/InterpolationCheck require 'test_helper' class EmailProcessBounceDeliveryPermanentFailedTest < ActiveSupport::TestCase diff --git a/test/unit/email_process_bounce_follow_test.rb b/test/unit/email_process_bounce_follow_test.rb index 0deed2d91..967e1cf03 100644 --- a/test/unit/email_process_bounce_follow_test.rb +++ b/test/unit/email_process_bounce_follow_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/InterpolationCheck require 'test_helper' class EmailProcessBounceFollowUpTest < ActiveSupport::TestCase diff --git a/test/unit/notification_factory_renderer_test.rb b/test/unit/notification_factory_renderer_test.rb index 74e33362c..1bca33044 100644 --- a/test/unit/notification_factory_renderer_test.rb +++ b/test/unit/notification_factory_renderer_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/InterpolationCheck require 'test_helper' class NotificationFactoryRendererTest < ActiveSupport::TestCase diff --git a/test/unit/notification_factory_template_test.rb b/test/unit/notification_factory_template_test.rb index 604ed0e62..38ca7367c 100644 --- a/test/unit/notification_factory_template_test.rb +++ b/test/unit/notification_factory_template_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/InterpolationCheck require 'test_helper' class NotificationFactoryTemplateTest < ActiveSupport::TestCase diff --git a/test/unit/organization_assets_test.rb b/test/unit/organization_assets_test.rb index 48c89a735..42081cffe 100644 --- a/test/unit/organization_assets_test.rb +++ b/test/unit/organization_assets_test.rb @@ -139,18 +139,18 @@ class OrganizationAssetsTest < ActiveSupport::TestCase assert_not(Organization.find_by(id: org_new.id)) end - def diff(o1, o2) - return true if o1 == o2 + def diff(object1, object2) + return true if object1 == object2 %w[updated_at created_at].each do |item| - if o1[item] - o1[item] = o1[item].to_s + if object1[item] + object1[item] = object1[item].to_s end - if o2[item] - o2[item] = o2[item].to_s + if object2[item] + object2[item] = object2[item].to_s end end - return true if (o1.to_a - o2.to_a).blank? - #puts "ERROR: difference \n1: #{o1.inspect}\n2: #{o2.inspect}\ndiff: #{(o1.to_a - o2.to_a).inspect}" + return true if (object1.to_a - object2.to_a).blank? + #puts "ERROR: difference \n1: #{object1.inspect}\n2: #{object2.inspect}\ndiff: #{(object1.to_a - object2.to_a).inspect}" false end diff --git a/test/unit/ticket_article_dos_test.rb b/test/unit/ticket_article_dos_test.rb index f22ba38ec..180790ec1 100644 --- a/test/unit/ticket_article_dos_test.rb +++ b/test/unit/ticket_article_dos_test.rb @@ -3,6 +3,10 @@ require 'test_helper' class TicketArticleDos < ActiveSupport::TestCase + def two_mio_random_chars + @two_mio_random_chars ||= Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join + end + test 'check body size' do org_community = Organization.create_if_not_exists( @@ -36,7 +40,7 @@ class TicketArticleDos < ActiveSupport::TestCase type_id: Ticket::Article::Type.find_by(name: 'phone').id, sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, from: 'Zammad Feedback ', - body: Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join, + body: two_mio_random_chars, internal: false, updated_by_id: 1, created_by_id: 1, @@ -55,7 +59,7 @@ class TicketArticleDos < ActiveSupport::TestCase type_id: Ticket::Article::Type.find_by(name: 'phone').id, sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, from: 'Zammad Feedback ', - body: "\u0000#{Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join}", + body: "\u0000#{two_mio_random_chars}", internal: false, updated_by_id: 1, created_by_id: 1, @@ -78,7 +82,7 @@ class TicketArticleDos < ActiveSupport::TestCase type_id: Ticket::Article::Type.find_by(name: 'phone').id, sender_id: Ticket::Article::Sender.find_by(name: 'Customer').id, from: 'Zammad Feedback ', - body: "\u0000#{Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join}", + body: "\u0000#{two_mio_random_chars}", internal: false, updated_by_id: 1, created_by_id: 1, @@ -89,11 +93,13 @@ class TicketArticleDos < ActiveSupport::TestCase test 'check body size / cut if email' do - email_raw_string = "From: me@example.com -To: customer@example.com -Subject: some new subject + email_raw_string = <<-MAIL.strip_indent + From: me@example.com + To: customer@example.com + Subject: some new subject -Some Text" + Array.new(2_000_000) { [*'0'..'9', *'a'..'z', ' ', ' ', ' ', '. '].sample }.join + Some Text#{two_mio_random_chars} + MAIL ticket_p, article_p, user_p, mail = Channel::EmailParser.new.process({}, email_raw_string) assert_equal(1_500_000, article_p.body.length) diff --git a/test/unit/ticket_trigger_test.rb b/test/unit/ticket_trigger_test.rb index f2c68cacc..d19fd0da6 100644 --- a/test/unit/ticket_trigger_test.rb +++ b/test/unit/ticket_trigger_test.rb @@ -1,4 +1,3 @@ -# rubocop:disable Lint/InterpolationCheck require 'test_helper' class TicketTriggerTest < ActiveSupport::TestCase diff --git a/test/unit/user_assets_test.rb b/test/unit/user_assets_test.rb index 432cdee94..8d79547c3 100644 --- a/test/unit/user_assets_test.rb +++ b/test/unit/user_assets_test.rb @@ -139,18 +139,18 @@ class UserAssetsTest < ActiveSupport::TestCase assert_not(Organization.find_by(id: org2.id)) end - def diff(o1, o2) - return true if o1 == o2 + def diff(object1, object2) + return true if object1 == object2 %w[updated_at created_at].each do |item| - if o1[item] - o1[item] = o1[item].to_s + if object1[item] + object1[item] = object1[item].to_s end - if o2[item] - o2[item] = o2[item].to_s + if object2[item] + object2[item] = object2[item].to_s end end - return true if (o1.to_a - o2.to_a).blank? - #puts "ERROR: difference \n1: #{o1.inspect}\n2: #{o2.inspect}\ndiff: #{(o1.to_a - o2.to_a).inspect}" + return true if (object1.to_a - object2.to_a).blank? + #puts "ERROR: difference \n1: #{object1.inspect}\n2: #{object2.inspect}\ndiff: #{(object1.to_a - object2.to_a).inspect}" false end