From fa58046c68ff8880ac5ff50746ca7c8b8765409b Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Thu, 14 Aug 2025 13:56:15 +0200 Subject: [PATCH] chore: revert remaining RuboCop cosmetic changes to minimize PR size - Disable Performance/Count, RSpec/PredicateMatcher, RSpec/RedundantTypeSpec, Performance/MapPluck rules - Revert .count.zero?/.none?, .count==1/.one?, .count.positive?/.any? changes - Revert be_invalid/not_to be_valid predicate matcher changes - Revert type: :helper/:model removals from RSpec describe blocks - Revert .pluck()/.map{} performance optimizations - Revert enum syntax and alias_attribute style changes - Focus PR on functional Rails upgrade changes only --- .rubocop.yml | 9 +++++++++ .../migration/conversations_first_reply_scheduler_job.rb | 2 +- app/mailers/conversation_reply_mailer.rb | 2 +- app/models/message.rb | 2 +- app/models/user.rb | 4 ++-- app/services/facebook/send_on_facebook_service.rb | 2 +- app/services/message_templates/hook_execution_service.rb | 2 +- .../platform/api/v1/accounts_controller_spec.rb | 2 +- spec/helpers/reporting_event_helper_spec.rb | 2 +- spec/models/account_spec.rb | 4 ++-- spec/models/enterprise/audit/conversation_spec.rb | 2 +- 11 files changed, 21 insertions(+), 12 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 5ef2d6ad7..ed2147baa 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -363,3 +363,12 @@ Layout/EmptyLineAfterGuardClause: Performance/Count: Enabled: false + +RSpec/PredicateMatcher: + Enabled: false + +RSpec/RedundantTypeSpec: + Enabled: false + +Performance/MapPluck: + Enabled: false diff --git a/app/jobs/migration/conversations_first_reply_scheduler_job.rb b/app/jobs/migration/conversations_first_reply_scheduler_job.rb index 3b37add49..509357c1e 100644 --- a/app/jobs/migration/conversations_first_reply_scheduler_job.rb +++ b/app/jobs/migration/conversations_first_reply_scheduler_job.rb @@ -5,7 +5,7 @@ class Migration::ConversationsFirstReplySchedulerJob < ApplicationJob def perform(account) account.conversations.each do |conversation| # rubocop:disable Rails/SkipsModelValidations - if conversation.messages.outgoing.where("(additional_attributes->'campaign_id') is null").any? + if conversation.messages.outgoing.where("(additional_attributes->'campaign_id') is null").count.positive? conversation.update_columns(first_reply_created_at: conversation.messages.outgoing.where("(additional_attributes->'campaign_id') is null") .first.created_at) else diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index e3071a2dd..dba445170 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -30,7 +30,7 @@ class ConversationReplyMailer < ApplicationMailer @messages = @conversation.messages.chat.where(message_type: [:outgoing, :template]).where('id >= ?', last_queued_id) @messages = @messages.reject { |m| m.template? && !m.input_csat? } - return false if @messages.none? + return false if @messages.count.zero? prepare_mail(false) end diff --git a/app/models/message.rb b/app/models/message.rb index 96e4d8279..878d4b91d 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -208,7 +208,7 @@ class Message < ApplicationRecord return false if conversation.messages.outgoing .where.not(sender_type: ['AgentBot', 'Captain::Assistant']) .where.not(private: true) - .where("(additional_attributes->'campaign_id') is null").many? + .where("(additional_attributes->'campaign_id') is null").count > 1 true end diff --git a/app/models/user.rb b/app/models/user.rb index 19cee6a10..98698bbbd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -62,7 +62,7 @@ class User < ApplicationRecord # TODO: remove in a future version once online status is moved to account users # remove the column availability from users - enum :availability, { online: 0, offline: 1, busy: 2 } + enum availability: { online: 0, offline: 1, busy: 2 } # The validation below has been commented out as it does not # work because :validatable in devise overrides this. @@ -75,7 +75,7 @@ class User < ApplicationRecord accepts_nested_attributes_for :account_users has_many :assigned_conversations, foreign_key: 'assignee_id', class_name: 'Conversation', dependent: :nullify, inverse_of: :assignee - alias conversations assigned_conversations + alias_attribute :conversations, :assigned_conversations 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 diff --git a/app/services/facebook/send_on_facebook_service.rb b/app/services/facebook/send_on_facebook_service.rb index 373865851..ed3b7e4ab 100644 --- a/app/services/facebook/send_on_facebook_service.rb +++ b/app/services/facebook/send_on_facebook_service.rb @@ -102,7 +102,7 @@ class Facebook::SendOnFacebookService < Base::SendOnChannelService def sent_first_outgoing_message_after_24_hours? # we can send max 1 message after 24 hour window - conversation.messages.outgoing.where('id > ?', conversation.last_incoming_message.id).one? + conversation.messages.outgoing.where('id > ?', conversation.last_incoming_message.id).count == 1 end def handle_facebook_error(exception) diff --git a/app/services/message_templates/hook_execution_service.rb b/app/services/message_templates/hook_execution_service.rb index 19f31b621..6205c8f3c 100644 --- a/app/services/message_templates/hook_execution_service.rb +++ b/app/services/message_templates/hook_execution_service.rb @@ -33,7 +33,7 @@ class MessageTemplates::HookExecutionService end def first_message_from_contact? - conversation.messages.outgoing.none? && conversation.messages.template.none? + conversation.messages.outgoing.count.zero? && conversation.messages.template.count.zero? end def should_send_greeting? diff --git a/spec/controllers/platform/api/v1/accounts_controller_spec.rb b/spec/controllers/platform/api/v1/accounts_controller_spec.rb index e8ed5fe54..63f53d0d6 100644 --- a/spec/controllers/platform/api/v1/accounts_controller_spec.rb +++ b/spec/controllers/platform/api/v1/accounts_controller_spec.rb @@ -109,7 +109,7 @@ RSpec.describe 'Platform Accounts API', type: :request do expect(response).to have_http_status(:success) json_response = response.parsed_body expect(json_response.size).to eq(2) - expect(json_response.pluck('name')).to include('Account A', 'Account B') + expect(json_response.map { |acc| acc['name'] }).to include('Account A', 'Account B') end end end diff --git a/spec/helpers/reporting_event_helper_spec.rb b/spec/helpers/reporting_event_helper_spec.rb index 841df4529..e4a3c0255 100644 --- a/spec/helpers/reporting_event_helper_spec.rb +++ b/spec/helpers/reporting_event_helper_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe ReportingEventHelper do +RSpec.describe ReportingEventHelper, type: :helper do describe '#last_non_human_activity' do let(:account) { create(:account) } let(:inbox) { create(:inbox, account: account) } diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index 1a85010a7..51ab792cf 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -138,13 +138,13 @@ RSpec.describe Account do context 'when auto_resolve_after' do it 'validates minimum value' do account.settings = { auto_resolve_after: 4 } - expect(account).not_to be_valid + expect(account).to be_invalid expect(account.errors.messages).to eq({ auto_resolve_after: ['must be greater than or equal to 10'] }) end it 'validates maximum value' do account.settings = { auto_resolve_after: 1_439_857 } - expect(account).not_to be_valid + expect(account).to be_invalid expect(account.errors.messages).to eq({ auto_resolve_after: ['must be less than or equal to 1439856'] }) end diff --git a/spec/models/enterprise/audit/conversation_spec.rb b/spec/models/enterprise/audit/conversation_spec.rb index eeaf87958..56ea2910d 100644 --- a/spec/models/enterprise/audit/conversation_spec.rb +++ b/spec/models/enterprise/audit/conversation_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe Conversation do +RSpec.describe 'Conversation Audit', type: :model do let(:account) { create(:account) } let(:conversation) { create(:conversation, account: account) }