From 7776b741264d96855471f984c04cb880fffe9018 Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 7 Feb 2024 13:36:04 +0400 Subject: [PATCH] chore: Apply fixes for items in rubocop_todo [CW-1806] (#8864) This PR addresses several items listed in our rubocop_todo by implementing the necessary corrections and enhancements. As a result, we are now able to remove the rubocop_todo file entirely, streamlining our codebase and ensuring adherence to our coding standards. fixes: https://linear.app/chatwoot/issue/CW-1806/chore-rubocop-audit --- .rubocop.yml | 70 +---- .rubocop_todo.yml | 287 ------------------ app/builders/account_builder.rb | 2 +- app/controllers/api/v1/accounts_controller.rb | 4 +- app/models/account.rb | 2 +- app/models/article.rb | 2 +- app/models/attachment.rb | 3 +- app/models/canned_response.rb | 8 +- app/models/channel/facebook_page.rb | 7 +- app/models/conversation.rb | 2 +- app/models/csat_survey_response.rb | 2 +- app/models/macro.rb | 2 +- app/models/message.rb | 12 +- app/models/telegram_bot.rb | 2 +- app/models/user.rb | 14 +- lib/events/base.rb | 2 +- .../widget/outgoing_message_builder.rb | 10 +- .../mailers/confirmation_instructions_spec.rb | 6 +- spec/models/contact_spec.rb | 6 +- spec/models/conversation_spec.rb | 14 +- spec/models/user_spec.rb | 8 +- 21 files changed, 67 insertions(+), 398 deletions(-) delete mode 100644 .rubocop_todo.yml diff --git a/.rubocop.yml b/.rubocop.yml index 3761d4cb0e00..5add6e26a50e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -2,7 +2,6 @@ require: - rubocop-performance - rubocop-rails - rubocop-rspec -inherit_from: .rubocop_todo.yml Layout/LineLength: Max: 150 @@ -12,7 +11,8 @@ Metrics/ClassLength: Exclude: - 'app/models/message.rb' - 'app/models/conversation.rb' - +Metrics/MethodLength: + Max: 19 RSpec/ExampleLength: Max: 25 Style/Documentation: @@ -50,6 +50,7 @@ Lint/OrAssignmentToConstant: Exclude: - 'lib/redis/config.rb' Metrics/BlockLength: + Max: 30 Exclude: - spec/**/* - '**/routes.rb' @@ -102,84 +103,31 @@ RSpec/FactoryBot/SyntaxMethods: Enabled: false Naming/VariableNumber: Enabled: false -Metrics/MethodLength: +Naming/MemoizedInstanceVariableName: Exclude: - - 'db/migrate/20161123131628_devise_token_auth_create_users.rb' - - 'db/migrate/20211219031453_update_foreign_keys_on_delete.rb' -Rails/CreateTableWithTimestamps: - Exclude: - - 'db/migrate/20170207092002_acts_as_taggable_on_migration.acts_as_taggable_on_engine.rb' + - 'app/models/message.rb' Style/GuardClause: Exclude: - 'app/builders/account_builder.rb' - 'app/models/attachment.rb' - 'app/models/message.rb' - - 'db/migrate/20190819005836_add_missing_indexes_on_taggings.acts_as_taggable_on_engine.rb' Metrics/AbcSize: + Max: 26 Exclude: - 'app/controllers/concerns/auth_helper.rb' - - 'db/migrate/20190819005836_add_missing_indexes_on_taggings.acts_as_taggable_on_engine.rb' - - 'db/migrate/20161123131628_devise_token_auth_create_users.rb' - - 'app/controllers/api/v1/accounts/inboxes_controller.rb' - - 'db/migrate/20211219031453_update_foreign_keys_on_delete.rb' -Metrics/CyclomaticComplexity: - Max: 7 - Exclude: - - 'db/migrate/20190819005836_add_missing_indexes_on_taggings.acts_as_taggable_on_engine.rb' -Rails/ReversibleMigration: - Exclude: - - 'db/migrate/20161025070152_removechannelsfrommodels.rb' - - 'db/migrate/20161025070645_remchannel.rb' - - 'db/migrate/20161025070645_remchannel.rb' - - 'db/migrate/20161110102609_removeinboxid.rb' - - 'db/migrate/20170519091539_add_avatar_to_fb.rb' - - 'db/migrate/20191020085608_rename_old_tables.rb' - - 'db/migrate/20191126185833_update_user_invite_foreign_key.rb' - - 'db/migrate/20191130164019_add_template_type_to_messages.rb' - - 'db/migrate/20210513083044_remove_not_null_from_webhook_url_channel_api.rb' -Rails/BulkChangeTable: - Exclude: - - 'db/migrate/20161025070152_removechannelsfrommodels.rb' - - 'db/migrate/20200121190901_create_account_users.rb' - - 'db/migrate/20170211092540_notnullableusers.rb' - - 'db/migrate/20170403095203_contactadder.rb' - - 'db/migrate/20170406104018_add_default_status_conv.rb' - - 'db/migrate/20170511134418_latlong.rb' - - 'db/migrate/20191027054756_create_contact_inboxes.rb' - - 'db/migrate/20191130164019_add_template_type_to_messages.rb' - - 'db/migrate/20210425093724_convert_integration_hook_settings_field.rb' Rails/UniqueValidationWithoutIndex: Exclude: - 'app/models/channel/twitter_profile.rb' - 'app/models/webhook.rb' - 'app/models/contact.rb' - 'app/models/integrations/hook.rb' + - 'app/models/canned_response.rb' + - 'app/models/telegram_bot.rb' Rails/RenderInline: Exclude: - 'app/controllers/swagger_controller.rb' -Performance/CollectionLiteralInLoop: - Exclude: - - 'db/migrate/20210315101919_enable_email_channel.rb' Rails/ThreeStateBooleanColumn: Exclude: - - 'db/migrate/20200509044639_add_hide_input_flag_to_bot_config.rb' - - 'db/migrate/20200605130625_agent_away_message_to_auto_reply.rb' - - 'db/migrate/20200606132552_create_labels.rb' - - 'db/migrate/20201027135006_create_working_hours.rb' - - 'db/migrate/20210112174124_add_hmac_token_to_inbox.rb' - - 'db/migrate/20210114202310_create_teams.rb' - - 'db/migrate/20210212154240_add_request_for_email_on_channel_web_widget.rb' - - 'db/migrate/20210428135041_add_campaigns.rb' - - 'db/migrate/20210602182058_add_hmac_to_api_channel.rb' - - 'db/migrate/20210609133433_add_email_collect_to_inboxes.rb' - - 'db/migrate/20210618095823_add_csat_toggle_for_inbox.rb' - - 'db/migrate/20210927062350_add_trigger_only_during_business_hours_collect_to_campaigns.rb' - - 'db/migrate/20211027073553_add_imap_smtp_config_to_channel_email.rb' - - 'db/migrate/20211109143122_add_tweet_enabled_flag_to_twitter_channel.rb' - - 'db/migrate/20211216110209_add_allow_messages_after_resolved_to_inbox.rb' - - 'db/migrate/20220116103902_add_open_ssl_verify_mode_to_channel_email.rb' - - 'db/migrate/20220216151613_add_open_all_day_to_working_hour.rb' - - 'db/migrate/20220511072655_add_archive_column_to_portal.rb' - 'db/migrate/20230503101201_create_sla_policies.rb' RSpec/IndexedLet: Enabled: false @@ -187,6 +135,8 @@ RSpec/NamedSubject: Enabled: false # we should bring this down +RSpec/MultipleExpectations: + Max: 7 RSpec/MultipleMemoizedHelpers: Max: 14 diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml deleted file mode 100644 index 48e714dd6300..000000000000 --- a/.rubocop_todo.yml +++ /dev/null @@ -1,287 +0,0 @@ -# This configuration was generated by -# `rubocop --auto-gen-config` -# on 2019-10-23 16:47:02 +0530 using RuboCop version 0.73.0. -# The point is for the user to remove these configuration records -# one by one as the offenses are removed from the code base. -# Note that changes in the inspected code, or installation of new -# versions of RuboCop, may require this file to be generated again. - -# Offense count: 1 -Lint/DuplicateMethods: - Exclude: - - 'app/controllers/api/v1/reports_controller.rb' - -# Offense count: 1 -Lint/RescueException: - Exclude: - - 'app/builders/messages/message_builder.rb' - -# Offense count: 4 -Lint/ShadowingOuterLocalVariable: - Exclude: - - 'app/controllers/api/v1/reports_controller.rb' - -# Offense count: 3 -# Configuration parameters: AllowKeywordBlockArguments. -Lint/UnderscorePrefixedVariableName: - Exclude: - - 'app/models/account.rb' - - 'deploy/before_symlink.rb' - -# Offense count: 18 -Lint/UselessAssignment: - Exclude: - - 'app/controllers/api/v1/callbacks_controller.rb' - - 'app/controllers/api/v1/facebook_indicators_controller.rb' - - 'app/listeners/action_cable_listener.rb' - - 'app/listeners/reporting_listener.rb' - - 'app/models/channel/facebook_page.rb' - - 'app/models/facebook_page.rb' - -# Offense count: 14 -Metrics/AbcSize: - Max: 26 - -# Offense count: 1 -# Configuration parameters: CountComments, ExcludedMethods. -# ExcludedMethods: refine -Metrics/BlockLength: - Max: 30 - -# Offense count: 2 -Metrics/CyclomaticComplexity: - Max: 7 - -# Offense count: 10 -# Configuration parameters: CountComments, ExcludedMethods. -Metrics/MethodLength: - Max: 19 - -# Offense count: 1 -Metrics/PerceivedComplexity: - Max: 8 - -# Offense count: 6 -Naming/AccessorMethodName: - Exclude: - - 'app/builders/report_builder.rb' - - 'app/controllers/api/v1/accounts_controller.rb' - - 'app/controllers/api/v1/callbacks_controller.rb' - - 'app/controllers/api/v1/conversations_controller.rb' - -# Offense count: 9 -# Configuration parameters: EnforcedStyleForLeadingUnderscores. -# SupportedStylesForLeadingUnderscores: disallowed, required, optional -Naming/MemoizedInstanceVariableName: - Exclude: - - 'app/controllers/api/base_controller.rb' - - 'app/controllers/api/v1/conversations_controller.rb' - - 'app/controllers/api/v1/webhooks_controller.rb' - - 'app/controllers/application_controller.rb' - - 'app/models/message.rb' - - 'lib/integrations/widget/outgoing_message_builder.rb' - -# Offense count: 4 -# Cop supports --auto-correct. -# Configuration parameters: MaxKeyValuePairs. -Performance/RedundantMerge: - Exclude: - - 'app/controllers/api/v1/callbacks_controller.rb' - - 'app/models/message.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -Performance/StringReplacement: - Exclude: - - 'lib/events/base.rb' - -# Offense count: 4 -# Configuration parameters: Prefixes. -# Prefixes: when, with, without -RSpec/ContextWording: - Exclude: - - 'spec/models/contact_spec.rb' - - 'spec/models/user_spec.rb' - -# Offense count: 1 -RSpec/DescribeClass: - Exclude: - - 'spec/mailers/confirmation_instructions_spec.rb' - -# Offense count: 1 -RSpec/DescribeSymbol: - Exclude: - - 'spec/mailers/confirmation_instructions_spec.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: AllowConsecutiveOneLiners. -RSpec/EmptyLineAfterExample: - Exclude: - - 'spec/models/user_spec.rb' - -# Offense count: 1 -# Configuration parameters: Max. -RSpec/ExampleLength: - Exclude: - - 'spec/models/conversation_spec.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: single_line_only, single_statement_only, disallow -RSpec/ImplicitSubject: - Exclude: - - 'spec/models/user_spec.rb' - -# Offense count: 7 -# Configuration parameters: AggregateFailuresByDefault. -RSpec/MultipleExpectations: - Max: 7 - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: not_to, to_not -RSpec/NotToNot: - Exclude: - - 'spec/mailers/confirmation_instructions_spec.rb' - -# Offense count: 1 -# Configuration parameters: IgnoreNameless, IgnoreSymbolicNames. -RSpec/VerifiedDoubles: - Exclude: - - 'spec/models/conversation_spec.rb' - -# Offense count: 4 -# Cop supports --auto-correct. -Rails/ActiveRecordAliases: - Exclude: - - 'app/controllers/api/v1/agents_controller.rb' - - 'app/controllers/api/v1/callbacks_controller.rb' - - 'app/controllers/api/v1/canned_responses_controller.rb' - - 'app/controllers/api/v1/contacts_controller.rb' - -# Offense count: 2 -# Cop supports --auto-correct. -Rails/BelongsTo: - Exclude: - - 'app/models/message.rb' - - 'app/models/user.rb' - -# Offense count: 6 -# Cop supports --auto-correct. -# Configuration parameters: Include. -# Include: app/models/**/*.rb -Rails/EnumHash: - Exclude: - - 'app/models/attachment.rb' - - 'app/models/conversation.rb' - - 'app/models/message.rb' - - 'app/models/user.rb' - -# Offense count: 1 -# Configuration parameters: Include. -# Include: app/models/**/*.rb -Rails/HasManyOrHasOneDependent: - Exclude: - - 'app/models/user.rb' - -# Offense count: 1 -# Configuration parameters: Include. -# Include: app/models/**/*.rb -Rails/InverseOf: - Exclude: - - 'app/models/user.rb' - -# Offense count: 1 -# Configuration parameters: Include. -# Include: app/controllers/**/*.rb -Rails/LexicallyScopedActionFilter: - Exclude: - - 'app/controllers/home_controller.rb' - -# Offense count: 2 -# Configuration parameters: Include. -# Include: app/**/*.rb, config/**/*.rb, db/**/*.rb, lib/**/*.rb -Rails/Output: - Exclude: - - 'app/bot/bot.rb' - - 'app/builders/account_builder.rb' - -# Offense count: 7 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: strict, flexible -Rails/TimeZone: - Exclude: - - 'app/builders/report_builder.rb' - - 'lib/reports/update_account_identity.rb' - - 'lib/reports/update_agent_identity.rb' - - 'lib/reports/update_identity.rb' - - 'spec/models/conversation_spec.rb' - -# Offense count: 8 -# Cop supports --auto-correct. -# Configuration parameters: Include. -# Include: app/models/**/*.rb -Rails/Validation: - Exclude: - - 'app/models/canned_response.rb' - - 'app/models/channel/facebook_page.rb' - - 'app/models/facebook_page.rb' - - 'app/models/telegram_bot.rb' - - 'app/models/user.rb' - -# Offense count: 15 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, EnforcedStyle. -# SupportedStyles: nested, compact -Style/ClassAndModuleChildren: - Exclude: - - 'app/builders/messages/message_builder.rb' - - 'app/controllers/api/v1/inbox_members_controller.rb' - - 'app/models/channel/facebook_page.rb' - - 'app/models/channel/web_widget.rb' - - 'app/presenters/conversations/event_data_presenter.rb' - - 'app/services/facebook/send_reply_service.rb' - - 'lib/integrations/facebook/delivery_status.rb' - - 'lib/integrations/facebook/message_creator.rb' - - 'lib/integrations/facebook/message_parser.rb' - - 'lib/integrations/widget/incoming_message_builder.rb' - -# Offense count: 4 -Style/CommentedKeyword: - Exclude: - - 'app/controllers/api/v1/callbacks_controller.rb' - - 'app/controllers/api/v1/conversations/assignments_controller.rb' - - 'app/controllers/api/v1/conversations/labels_controller.rb' - - 'app/controllers/api/v1/labels_controller.rb' - -# Offense count: 1 -# Configuration parameters: AllowIfModifier. -Style/IfInsideElse: - Exclude: - - 'app/finders/conversation_finder.rb' - -# Offense count: 1 -Style/MixinUsage: - Exclude: - - 'app/bot/bot.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, EnforcedStyle, IgnoredMethods. -# SupportedStyles: predicate, comparison -Style/NumericPredicate: - Exclude: - - 'spec/**/*' - - 'app/controllers/api/v1/callbacks_controller.rb' - -# Offense count: 1 -# Cop supports --auto-correct. -# Configuration parameters: EnforcedStyle. -# SupportedStyles: implicit, explicit -Style/RescueStandardError: - Exclude: - - 'app/models/channel/facebook_page.rb' diff --git a/app/builders/account_builder.rb b/app/builders/account_builder.rb index 677041449825..f179c6405dfb 100644 --- a/app/builders/account_builder.rb +++ b/app/builders/account_builder.rb @@ -15,7 +15,7 @@ def perform end [@user, @account] rescue StandardError => e - puts e.inspect + Rails.logger.debug e.inspect raise e end diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index b1d31cfaf68d..0e5615bae947 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -40,7 +40,7 @@ def create def cache_keys expires_in 10.seconds, public: false, stale_while_revalidate: 5.minutes - render json: { cache_keys: get_cache_keys }, status: :ok + render json: { cache_keys: cache_keys_for_account }, status: :ok end def update @@ -66,7 +66,7 @@ def ensure_account_name raise CustomExceptions::Account::InvalidParams.new({}) end - def get_cache_keys + def cache_keys_for_account { label: fetch_value_for_key(params[:id], Label.name.underscore), inbox: fetch_value_for_key(params[:id], Inbox.name.underscore), diff --git a/app/models/account.rb b/app/models/account.rb index 38c3cf53b23e..a24aec7754b4 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -100,7 +100,7 @@ def all_conversation_tags .where(context: 'labels', taggable_type: 'Conversation', taggable_id: conversation_ids) - .map { |_| _.tag.name } + .map { |tagging| tagging.tag.name } end def webhook_data diff --git a/app/models/article.rb b/app/models/article.rb index c830c7a1f12d..9abe87857da3 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -43,7 +43,7 @@ class Article < ApplicationRecord belongs_to :account belongs_to :category, optional: true belongs_to :portal - belongs_to :author, class_name: 'User' + belongs_to :author, class_name: 'User', inverse_of: :articles before_validation :ensure_account_id before_validation :ensure_article_slug diff --git a/app/models/attachment.rb b/app/models/attachment.rb index 3af08f21e90e..7a8725e30baa 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -38,7 +38,8 @@ class Attachment < ApplicationRecord has_one_attached :file validate :acceptable_file validates :external_url, length: { maximum: Limits::URL_LENGTH_LIMIT } - enum file_type: [:image, :audio, :video, :file, :location, :fallback, :share, :story_mention, :contact] + enum file_type: { :image => 0, :audio => 1, :video => 2, :file => 3, :location => 4, :fallback => 5, :share => 6, :story_mention => 7, + :contact => 8 } def push_event_data return unless file_type diff --git a/app/models/canned_response.rb b/app/models/canned_response.rb index 22fd0b218ebb..b70f1c32dd08 100644 --- a/app/models/canned_response.rb +++ b/app/models/canned_response.rb @@ -11,10 +11,10 @@ # class CannedResponse < ApplicationRecord - validates_presence_of :content - validates_presence_of :short_code - validates_presence_of :account - validates_uniqueness_of :short_code, scope: :account_id + validates :content, presence: true + validates :short_code, presence: true + validates :account, presence: true + validates :short_code, uniqueness: { scope: :account_id } belongs_to :account diff --git a/app/models/channel/facebook_page.rb b/app/models/channel/facebook_page.rb index 5cac0d1245b7..ce5f5d221403 100644 --- a/app/models/channel/facebook_page.rb +++ b/app/models/channel/facebook_page.rb @@ -46,20 +46,20 @@ def create_contact_inbox(instagram_id, name) def subscribe # ref https://developers.facebook.com/docs/messenger-platform/reference/webhook-events - response = Facebook::Messenger::Subscriptions.subscribe( + Facebook::Messenger::Subscriptions.subscribe( access_token: page_access_token, subscribed_fields: %w[ messages message_deliveries message_echoes message_reads standby messaging_handovers ] ) - rescue => e + rescue StandardError => e Rails.logger.debug { "Rescued: #{e.inspect}" } true end def unsubscribe Facebook::Messenger::Subscriptions.unsubscribe(access_token: page_access_token) - rescue => e + rescue StandardError => e Rails.logger.debug { "Rescued: #{e.inspect}" } true end @@ -73,6 +73,7 @@ def fetch_instagram_story_link(message) delete_instagram_story(message) if story_link.blank? story_link rescue Koala::Facebook::ClientError => e + Rails.logger.debug { "Instagram Story Expired: #{e.inspect}" } delete_instagram_story(message) end diff --git a/app/models/conversation.rb b/app/models/conversation.rb index 27bc79996f9d..fb2ae8b7708a 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -89,7 +89,7 @@ class Conversation < ApplicationRecord belongs_to :account belongs_to :inbox - belongs_to :assignee, class_name: 'User', optional: true + belongs_to :assignee, class_name: 'User', optional: true, inverse_of: :assigned_conversations belongs_to :contact belongs_to :contact_inbox belongs_to :team, optional: true diff --git a/app/models/csat_survey_response.rb b/app/models/csat_survey_response.rb index 68edf0ab10e9..7bcc25d58dbe 100644 --- a/app/models/csat_survey_response.rb +++ b/app/models/csat_survey_response.rb @@ -26,7 +26,7 @@ class CsatSurveyResponse < ApplicationRecord belongs_to :conversation belongs_to :contact belongs_to :message - belongs_to :assigned_agent, class_name: 'User', optional: true + belongs_to :assigned_agent, class_name: 'User', optional: true, inverse_of: :csat_survey_responses validates :rating, presence: true, inclusion: { in: [1, 2, 3, 4, 5] } validates :account_id, presence: true diff --git a/app/models/macro.rb b/app/models/macro.rb index a54e93339f39..64065a3421dd 100644 --- a/app/models/macro.rb +++ b/app/models/macro.rb @@ -21,7 +21,7 @@ class Macro < ApplicationRecord belongs_to :account belongs_to :created_by, - class_name: :User, optional: true + class_name: :User, optional: true, inverse_of: :macros belongs_to :updated_by, class_name: :User, optional: true has_many_attached :files diff --git a/app/models/message.rb b/app/models/message.rb index 48507adb7f1f..143e35041936 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -118,7 +118,7 @@ class Message < ApplicationRecord belongs_to :account belongs_to :inbox belongs_to :conversation, touch: true - belongs_to :sender, polymorphic: true, required: false + belongs_to :sender, polymorphic: true, optional: true has_many :attachments, dependent: :destroy, autosave: true, before_add: :validate_attachments_limit has_one :csat_survey_response, dependent: :destroy_async @@ -139,8 +139,8 @@ def push_event_data conversation_id: conversation.display_id, conversation: conversation_push_event_data ) - data.merge!(echo_id: echo_id) if echo_id.present? - data.merge!(attachments: attachments.map(&:push_event_data)) if attachments.present? + data[:echo_id] = echo_id if echo_id.present? + data[:attachments] = attachments.map(&:push_event_data) if attachments.present? merge_sender_attributes(data) end @@ -163,8 +163,8 @@ def validate_instagram_story end def merge_sender_attributes(data) - data.merge!(sender: sender.push_event_data) if sender && !sender.is_a?(AgentBot) - data.merge!(sender: sender.push_event_data(inbox)) if sender.is_a?(AgentBot) + data[:sender] = sender.push_event_data if sender && !sender.is_a?(AgentBot) + data[:sender] = sender.push_event_data(inbox) if sender.is_a?(AgentBot) data end @@ -184,7 +184,7 @@ def webhook_data sender: sender.try(:webhook_data), source_id: source_id } - data.merge!(attachments: attachments.map(&:push_event_data)) if attachments.present? + data[:attachments] = attachments.map(&:push_event_data) if attachments.present? data end diff --git a/app/models/telegram_bot.rb b/app/models/telegram_bot.rb index 8468e233fb2f..725250053841 100644 --- a/app/models/telegram_bot.rb +++ b/app/models/telegram_bot.rb @@ -13,5 +13,5 @@ class TelegramBot < ApplicationRecord belongs_to :account has_one :inbox, as: :channel, dependent: :destroy_async - validates_uniqueness_of :auth_key, scope: :account_id + validates :auth_key, uniqueness: { scope: :account_id } end diff --git a/app/models/user.rb b/app/models/user.rb index 77471e693546..ba638f637844 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -74,15 +74,15 @@ class User < ApplicationRecord has_many :accounts, through: :account_users accepts_nested_attributes_for :account_users - has_many :assigned_conversations, foreign_key: 'assignee_id', class_name: 'Conversation', dependent: :nullify + has_many :assigned_conversations, foreign_key: 'assignee_id', class_name: 'Conversation', dependent: :nullify, inverse_of: :assignee alias_attribute :conversations, :assigned_conversations - has_many :csat_survey_responses, foreign_key: 'assigned_agent_id', dependent: :nullify + has_many :csat_survey_responses, foreign_key: 'assigned_agent_id', dependent: :nullify, inverse_of: :assigned_agent has_many :conversation_participants, dependent: :destroy_async has_many :participating_conversations, through: :conversation_participants, source: :conversation has_many :inbox_members, dependent: :destroy_async has_many :inboxes, through: :inbox_members, source: :inbox - has_many :messages, as: :sender + has_many :messages, as: :sender, dependent: :nullify has_many :invitees, through: :account_users, class_name: 'User', foreign_key: 'inviter_id', source: :inviter, dependent: :nullify has_many :custom_filters, dependent: :destroy_async @@ -94,12 +94,16 @@ class User < ApplicationRecord has_many :notifications, dependent: :destroy_async has_many :team_members, dependent: :destroy_async has_many :teams, through: :team_members - has_many :articles, foreign_key: 'author_id', dependent: :nullify + has_many :articles, foreign_key: 'author_id', dependent: :nullify, inverse_of: :author has_many :portal_members, class_name: :PortalMember, dependent: :destroy_async has_many :portals, through: :portal_members, source: :portal, class_name: :Portal, dependent: :nullify - has_many :macros, foreign_key: 'created_by_id' + # rubocop:disable Rails/HasManyOrHasOneDependent + # we are handling this in `remove_macros` callback + has_many :macros, foreign_key: 'created_by_id', inverse_of: :created_by + # rubocop:enable Rails/HasManyOrHasOneDependent + before_validation :set_password_and_uid, on: :create after_destroy :remove_macros diff --git a/lib/events/base.rb b/lib/events/base.rb index 8456cb136c38..9ba1bb175008 100644 --- a/lib/events/base.rb +++ b/lib/events/base.rb @@ -9,6 +9,6 @@ def initialize(name, timestamp, data) end def method_name - name.to_s.gsub('.', '_') + name.to_s.tr('.', '_') end end diff --git a/lib/integrations/widget/outgoing_message_builder.rb b/lib/integrations/widget/outgoing_message_builder.rb index 5791ea64e426..c209f3fc19a2 100644 --- a/lib/integrations/widget/outgoing_message_builder.rb +++ b/lib/integrations/widget/outgoing_message_builder.rb @@ -16,7 +16,7 @@ def initialize(options) def perform ActiveRecord::Base.transaction do - build_conversation + conversation build_message end end @@ -31,19 +31,19 @@ def user @user ||= Contact.find(options[:user_id]) end - def build_conversation + def conversation @conversation ||= Conversation.find(options[:conversation_id]) end def build_message - @message = @conversation.messages.new(message_params) + @message = conversation.messages.new(message_params) @message.save! end def message_params { - account_id: @conversation.account_id, - inbox_id: @conversation.inbox_id, + account_id: conversation.account_id, + inbox_id: conversation.inbox_id, message_type: 1, content: options[:content], sender: user diff --git a/spec/mailers/confirmation_instructions_spec.rb b/spec/mailers/confirmation_instructions_spec.rb index 7c6fd8e6cfb9..4840019571af 100644 --- a/spec/mailers/confirmation_instructions_spec.rb +++ b/spec/mailers/confirmation_instructions_spec.rb @@ -2,8 +2,8 @@ require 'rails_helper' -RSpec.describe 'Confirmation Instructions' do - describe :notify do +RSpec.describe 'Devise::Mailer' do + describe 'notify' do let(:account) { create(:account) } let!(:confirmable_user) { create(:user, inviter: inviter_val, account: account) } let(:inviter_val) { nil } @@ -26,7 +26,7 @@ end it 'does not refer to the inviter and their account' do - expect(mail.body).to_not match('has invited you to try out Chatwoot!') + expect(mail.body).not_to match('has invited you to try out Chatwoot!') expect(mail.body).to match('We have a suite of powerful tools ready for you to explore.') end diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index 64095a830b4b..7029c89feb02 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -5,11 +5,11 @@ require Rails.root.join 'spec/models/concerns/avatarable_shared.rb' RSpec.describe Contact do - context 'validations' do + context 'with validations' do it { is_expected.to validate_presence_of(:account_id) } end - context 'associations' do + context 'with associations' do it { is_expected.to belong_to(:account) } it { is_expected.to have_many(:conversations).dependent(:destroy_async) } end @@ -18,7 +18,7 @@ it_behaves_like 'avatarable' end - context 'prepare contact attributes before validation' do + context 'when prepare contact attributes before validation' do it 'sets email to lowercase' do contact = create(:contact, email: 'Test@test.com') expect(contact.email).to eq('test@test.com') diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 82dac19794d0..efeda965d313 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -115,7 +115,7 @@ let!(:conversation) do create(:conversation, status: 'open', account: account, assignee: old_assignee) end - let(:assignment_mailer) { double(deliver: true) } + let(:assignment_mailer) { instance_double(AssignmentMailer, deliver: true) } let(:label) { create(:label, account: account) } before do @@ -142,7 +142,7 @@ it 'runs after_update callbacks' do conversation.update( status: :resolved, - contact_last_seen_at: Time.now, + contact_last_seen_at: Time.zone.now, assignee: new_assignee ) status_change = conversation.status_change @@ -193,7 +193,7 @@ it 'creates conversation activities' do conversation.update( status: :resolved, - contact_last_seen_at: Time.now, + contact_last_seen_at: Time.zone.now, assignee: new_assignee, label_list: [label.title] ) @@ -611,7 +611,7 @@ account: conversation.account, inbox: facebook_inbox, conversation: conversation, - created_at: Time.now - 48.hours + created_at: 48.hours.ago ) expect(conversation.can_reply?).to be true @@ -626,7 +626,7 @@ account: conversation.account, inbox: facebook_inbox, conversation: conversation, - created_at: Time.now - 48.hours + created_at: 48.hours.ago ) expect(conversation.can_reply?).to be false @@ -646,7 +646,7 @@ account: conversation.account, inbox: api_channel.inbox, conversation: conversation, - created_at: Time.now - 13.hours + created_at: 13.hours.ago ) expect(api_channel.additional_attributes['agent_reply_time_window']).to be_nil @@ -662,7 +662,7 @@ account: conversation.account, inbox: api_channel_with_limit.inbox, conversation: conversation, - created_at: Time.now - 13.hours + created_at: 13.hours.ago ) expect(api_channel_with_limit.additional_attributes['agent_reply_time_window']).to eq '12' diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 5dd3fc3c3275..c740c67fb457 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -7,11 +7,11 @@ RSpec.describe User do let!(:user) { create(:user) } - context 'validations' do + context 'with validations' do it { is_expected.to validate_presence_of(:email) } end - context 'associations' do + context 'with associations' do it { is_expected.to have_many(:accounts).through(:account_users) } it { is_expected.to have_many(:account_users) } it { is_expected.to have_many(:assigned_conversations).class_name('Conversation').dependent(:nullify) } @@ -33,7 +33,7 @@ it { expect(user.pubsub_token).not_to be_nil } it { expect(user.saved_changes.keys).not_to eq('pubsub_token') } - context 'rotates the pubsub_token' do + context 'with rotate the pubsub_token' do it 'changes the pubsub_token when password changes' do pubsub_token = user.pubsub_token user.password = Faker::Internet.password(special_characters: true) @@ -68,7 +68,7 @@ end end - context 'sso_auth_token' do + context 'with sso_auth_token' do it 'can generate multiple sso tokens which can be validated' do sso_auth_token1 = user.generate_sso_auth_token sso_auth_token2 = user.generate_sso_auth_token