From 3c8abd5b301baa829fe87ac5cf2667fb392f7c4f Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Wed, 21 May 2025 20:10:15 +0700 Subject: [PATCH 1/5] fix: Twilio authentication handling for WhatsApp attachments (#11536) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request Template ## Description This PR addresses an issue where users were unable to view images sent via WhatsApp on Chatwoot due to incorrect Twilio authentication configuration. https://app.chatwoot.com/app/accounts/1/conversations/50824 The problem stemmed from how authentication was being handled for Twilio API requests. The user had configured their inbox using api_key_sid, but the backend logic used only auth_token, leading to failed authentication. Further investigation showed that some customers might input api_secret into the auth_token field unintentionally. ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality not to work as expected) - [ ] This change requires a documentation update ## How Has This Been Tested? - Tested on console with Client(api_key_sid, auth_token, account_sid) and validated successful authentication for the customer (Twilio channel ID: 2702). - Simulated toggling the “Use API Key Authentication” checkbox to ensure backend behavior matches UI intent - Verified image rendering by testing with the same image URL that was previously failing for the user. ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [x] I have commented on my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] Any dependent changes have been merged and published in downstream modules --------- Co-authored-by: Muhsin Keloth --- app/services/twilio/incoming_message_service.rb | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/services/twilio/incoming_message_service.rb b/app/services/twilio/incoming_message_service.rb index 7a335c87d..c38577599 100644 --- a/app/services/twilio/incoming_message_service.rb +++ b/app/services/twilio/incoming_message_service.rb @@ -137,14 +137,19 @@ class Twilio::IncomingMessageService end def download_with_auth(media_url) - Down.download( - media_url, - http_basic_authentication: [twilio_channel.account_sid, twilio_channel.auth_token || twilio_channel.api_key_sid] - ) + auth_credentials = if twilio_channel.api_key_sid.present? + # When using api_key_sid, the auth token should be the api_secret_key + [twilio_channel.api_key_sid, twilio_channel.auth_token] + else + # When using account_sid, the auth token is the account's auth token + [twilio_channel.account_sid, twilio_channel.auth_token] + end + + Down.download(media_url, http_basic_authentication: auth_credentials) end def handle_download_attachment_error(error, media_url) - Rails.logger.info "Error downloading attachment from Twilio: #{error.message}: Retrying" + Rails.logger.info "Error downloading attachment from Twilio: #{error.message}: Retrying without auth" Down.download(media_url) rescue StandardError => e Rails.logger.info "Error downloading attachment from Twilio: #{e.message}: Skipping" From bc42aec68e8c1d5fca1dd5e7dbb8f451ed70ffbd Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 21 May 2025 07:10:07 -0700 Subject: [PATCH 2/5] chore: upgrade ruby version to 3.4.4 (#11524) - Chore upgrade ruby version to 3.4.4 before we migrate to rails 7.2 over #11037 --- .circleci/config.yml | 6 +- .devcontainer/docker-compose.yml | 4 +- .rubocop.yml | 187 ++++++++++- .ruby-version | 2 +- Gemfile | 5 +- Gemfile.lock | 300 ++++++++++-------- app/builders/contact_inbox_builder.rb | 6 +- app/mailboxes/application_mailbox.rb | 2 +- app/models/account.rb | 4 +- app/models/conversation.rb | 6 + app/models/email_template.rb | 4 +- app/models/installation_config.rb | 6 +- app/models/user.rb | 4 +- .../message_templates/template/csat_survey.rb | 1 - deployment/chatwoot-web.1.service | 6 +- deployment/chatwoot-worker.1.service | 6 +- deployment/setup_20.04.sh | 4 +- docker/Dockerfile | 8 +- .../v1/accounts/custom_roles_controller.rb | 4 +- .../v1/accounts/sla_policies_controller.rb | 4 +- .../conversation/response_builder_job.rb | 2 +- .../services/captain/tool_registry_service.rb | 4 +- .../conversations/report_builder_spec.rb | 8 +- spec/models/conversation_spec.rb | 4 +- .../mappers/conversation_mapper_spec.rb | 2 +- swagger/swagger.json | 292 +++++------------ 26 files changed, 469 insertions(+), 412 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bc7053130..99795db91 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -73,15 +73,15 @@ jobs: libvips - run: - name: Install RVM and Ruby 3.3.3 + name: Install RVM and Ruby 3.4.4 command: | sudo apt-get install -y gpg gpg --keyserver hkp://keyserver.ubuntu.com --recv-keys 409B6B1796C275462A1703113804BB82D39DC0E3 7D2BAF1CF37B13E2069D6956105BD0E739499BDB \curl -sSL https://get.rvm.io | bash -s stable echo 'source ~/.rvm/scripts/rvm' >> $BASH_ENV source ~/.rvm/scripts/rvm - rvm install "3.3.3" - rvm use 3.3.3 --default + rvm install "3.4.4" + rvm use 3.4.4 --default gem install bundler -v 2.5.16 - run: diff --git a/.devcontainer/docker-compose.yml b/.devcontainer/docker-compose.yml index c5530ac17..21a9fe909 100644 --- a/.devcontainer/docker-compose.yml +++ b/.devcontainer/docker-compose.yml @@ -12,7 +12,7 @@ services: args: VARIANT: 'ubuntu-22.04' NODE_VERSION: '23.7.0' - RUBY_VERSION: '3.3.3' + RUBY_VERSION: '3.4.4' # On Linux, you may need to update USER_UID and USER_GID below if not your local UID is not 1000. USER_UID: '1000' USER_GID: '1000' @@ -25,7 +25,7 @@ services: args: VARIANT: 'ubuntu-22.04' NODE_VERSION: '23.7.0' - RUBY_VERSION: '3.3.3' + RUBY_VERSION: '3.4.4' # On Linux, you may need to update USER_UID and USER_GID below if not your local UID is not 1000. USER_UID: '1000' USER_GID: '1000' diff --git a/.rubocop.yml b/.rubocop.yml index 1cdfbc713..12e756af6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,7 +1,10 @@ -require: +plugins: - rubocop-performance - rubocop-rails - rubocop-rspec + - rubocop-factory_bot + +require: - ./rubocop/use_from_email.rb - ./rubocop/custom_cop_location.rb @@ -13,44 +16,61 @@ Metrics/ClassLength: Exclude: - 'app/models/message.rb' - 'app/models/conversation.rb' + Metrics/MethodLength: Max: 19 + Exclude: + - 'enterprise/lib/captain/agent.rb' + RSpec/ExampleLength: Max: 25 + Style/Documentation: Enabled: false + Style/ExponentialNotation: Enabled: false + Style/FrozenStringLiteralComment: Enabled: false + Style/SymbolArray: Enabled: false + Style/OpenStructUse: Enabled: false + Style/OptionalBooleanParameter: Exclude: - 'app/services/email_templates/db_resolver_service.rb' - 'app/dispatchers/dispatcher.rb' + Style/GlobalVars: Exclude: - 'config/initializers/01_redis.rb' - 'config/initializers/rack_attack.rb' - 'lib/redis/alfred.rb' - 'lib/global_config.rb' + Style/ClassVars: Exclude: - 'app/services/email_templates/db_resolver_service.rb' + Lint/MissingSuper: Exclude: - 'app/drops/base_drop.rb' + Lint/SymbolConversion: Enabled: false + Lint/EmptyBlock: Exclude: - 'app/views/api/v1/accounts/conversations/toggle_status.json.jbuilder' + Lint/OrAssignmentToConstant: Exclude: - 'lib/redis/config.rb' + Metrics/BlockLength: Max: 30 Exclude: @@ -58,10 +78,16 @@ Metrics/BlockLength: - '**/routes.rb' - 'config/environments/*' - db/schema.rb + Metrics/ModuleLength: Exclude: - lib/seeders/message_seeder.rb - spec/support/slack_stubs.rb + +Rails/HelperInstanceVariable: + Exclude: + - enterprise/app/helpers/captain/chat_helper.rb + Rails/ApplicationController: Exclude: - 'app/controllers/api/v1/widget/messages_controller.rb' @@ -71,74 +97,101 @@ Rails/ApplicationController: - 'app/controllers/platform_controller.rb' - 'app/controllers/public_controller.rb' - 'app/controllers/survey/responses_controller.rb' + Rails/FindEach: Enabled: true Include: - 'app/**/*.rb' + Rails/CompactBlank: Enabled: false + Rails/EnvironmentVariableAccess: Enabled: false + Rails/TimeZoneAssignment: Enabled: false + Rails/RedundantPresenceValidationOnBelongsTo: Enabled: false + +Rails/InverseOf: + Exclude: + - enterprise/app/models/captain/assistant.rb + +Rails/UniqueValidationWithoutIndex: + Exclude: + - app/models/canned_response.rb + - app/models/telegram_bot.rb + - enterprise/app/models/captain_inbox.rb + - 'app/models/channel/twitter_profile.rb' + - 'app/models/webhook.rb' + - 'app/models/contact.rb' + Style/ClassAndModuleChildren: EnforcedStyle: compact Exclude: - 'config/application.rb' - 'config/initializers/monkey_patches/*' + Style/MapToHash: Enabled: false + Style/HashSyntax: Enabled: true EnforcedStyle: no_mixed_keys EnforcedShorthandSyntax: never + RSpec/NestedGroups: Enabled: true Max: 4 + RSpec/MessageSpies: Enabled: false + RSpec/StubbedMock: Enabled: false -RSpec/FactoryBot/SyntaxMethods: - Enabled: false + Naming/VariableNumber: Enabled: false + Naming/MemoizedInstanceVariableName: Exclude: - 'app/models/message.rb' + Style/GuardClause: Exclude: - 'app/builders/account_builder.rb' - 'app/models/attachment.rb' - 'app/models/message.rb' + Metrics/AbcSize: Max: 26 Exclude: - 'app/controllers/concerns/auth_helper.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' + Rails/ThreeStateBooleanColumn: Exclude: - 'db/migrate/20230503101201_create_sla_policies.rb' + RSpec/IndexedLet: Enabled: false + RSpec/NamedSubject: Enabled: false # we should bring this down RSpec/MultipleExpectations: Max: 7 + RSpec/MultipleMemoizedHelpers: Max: 14 @@ -166,3 +219,121 @@ AllCops: - 'tmp/**/*' - 'storage/**/*' - 'db/migrate/20230426130150_init_schema.rb' + +FactoryBot/SyntaxMethods: + Enabled: false + +# Disable new rules causing errors +Layout/LeadingCommentSpace: + Enabled: false + +Style/ReturnNilInPredicateMethodDefinition: + Enabled: false + +Style/RedundantParentheses: + Enabled: false + +Performance/StringIdentifierArgument: + Enabled: false + +Layout/EmptyLinesAroundExceptionHandlingKeywords: + Enabled: false + +Lint/LiteralAsCondition: + Enabled: false + +Style/RedundantReturn: + Enabled: false + +Layout/SpaceAroundOperators: + Enabled: false + +Rails/EnvLocal: + Enabled: false + +Rails/WhereRange: + Enabled: false + +Lint/UselessConstantScoping: + Enabled: false + +Style/MultipleComparison: + Enabled: false + +Bundler/OrderedGems: + Enabled: false + +RSpec/ExampleWording: + Enabled: false + +RSpec/ReceiveMessages: + Enabled: false + +FactoryBot/AssociationStyle: + Enabled: false + +Rails/EnumSyntax: + Enabled: false + +Lint/RedundantTypeConversion: + Enabled: false + +# Additional rules to disable +Rails/RedundantActiveRecordAllMethod: + Enabled: false + +Layout/TrailingEmptyLines: + Enabled: false + +Style/SafeNavigationChainLength: + Enabled: false + +Lint/SafeNavigationConsistency: + Enabled: false + +Lint/CopDirectiveSyntax: + Enabled: false + +# Final set of rules to disable +FactoryBot/ExcessiveCreateList: + Enabled: false + +RSpec/MissingExpectationTargetMethod: + Enabled: false + +Performance/InefficientHashSearch: + Enabled: false + +Style/RedundantSelfAssignmentBranch: + Enabled: false + +Style/YAMLFileRead: + Enabled: false + +Layout/ExtraSpacing: + Enabled: false + +Style/RedundantFilterChain: + Enabled: false + +Performance/MapMethodChain: + Enabled: false + +Rails/RootPathnameMethods: + Enabled: false + +Style/SuperArguments: + Enabled: false + +# Final remaining rules to disable +Rails/Delegate: + Enabled: false + +Style/CaseLikeIf: + Enabled: false + +FactoryBot/RedundantFactoryOption: + Enabled: false + +FactoryBot/FactoryAssociationWithStrategy: + Enabled: false \ No newline at end of file diff --git a/.ruby-version b/.ruby-version index 619b53766..f9892605c 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -3.3.3 +3.4.4 diff --git a/Gemfile b/Gemfile index 1e8605379..6f6bd7751 100644 --- a/Gemfile +++ b/Gemfile @@ -1,10 +1,10 @@ source 'https://rubygems.org' -ruby '3.3.3' +ruby '3.4.4' ##-- base gems for rails --## gem 'rack-cors', '2.0.0', require: 'rack/cors' -gem 'rails', '~> 7.0.8.4' +gem 'rails', '~> 7.1' # Reduces boot times through caching; required in config/boot.rb gem 'bootsnap', require: false @@ -237,6 +237,7 @@ group :development, :test do gem 'rubocop-performance', require: false gem 'rubocop-rails', require: false gem 'rubocop-rspec', require: false + gem 'rubocop-factory_bot', require: false gem 'seed_dump' gem 'shoulda-matchers' gem 'simplecov', '0.17.1', require: false diff --git a/Gemfile.lock b/Gemfile.lock index da19dbb72..d9908f5e1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -25,76 +25,89 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (7.0.8.7) - actionpack (= 7.0.8.7) - activesupport (= 7.0.8.7) + actioncable (7.1.5.1) + actionpack (= 7.1.5.1) + activesupport (= 7.1.5.1) nio4r (~> 2.0) websocket-driver (>= 0.6.1) - actionmailbox (7.0.8.7) - actionpack (= 7.0.8.7) - activejob (= 7.0.8.7) - activerecord (= 7.0.8.7) - activestorage (= 7.0.8.7) - activesupport (= 7.0.8.7) + zeitwerk (~> 2.6) + actionmailbox (7.1.5.1) + actionpack (= 7.1.5.1) + activejob (= 7.1.5.1) + activerecord (= 7.1.5.1) + activestorage (= 7.1.5.1) + activesupport (= 7.1.5.1) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.0.8.7) - actionpack (= 7.0.8.7) - actionview (= 7.0.8.7) - activejob (= 7.0.8.7) - activesupport (= 7.0.8.7) + actionmailer (7.1.5.1) + actionpack (= 7.1.5.1) + actionview (= 7.1.5.1) + activejob (= 7.1.5.1) + activesupport (= 7.1.5.1) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp - rails-dom-testing (~> 2.0) - actionpack (7.0.8.7) - actionview (= 7.0.8.7) - activesupport (= 7.0.8.7) - rack (~> 2.0, >= 2.2.4) + rails-dom-testing (~> 2.2) + actionpack (7.1.5.1) + actionview (= 7.1.5.1) + activesupport (= 7.1.5.1) + nokogiri (>= 1.8.5) + racc + rack (>= 2.2.4) + rack-session (>= 1.0.1) rack-test (>= 0.6.3) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.0, >= 1.2.0) - actiontext (7.0.8.7) - actionpack (= 7.0.8.7) - activerecord (= 7.0.8.7) - activestorage (= 7.0.8.7) - activesupport (= 7.0.8.7) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) + actiontext (7.1.5.1) + actionpack (= 7.1.5.1) + activerecord (= 7.1.5.1) + activestorage (= 7.1.5.1) + activesupport (= 7.1.5.1) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.0.8.7) - activesupport (= 7.0.8.7) + actionview (7.1.5.1) + activesupport (= 7.1.5.1) builder (~> 3.1) - erubi (~> 1.4) - rails-dom-testing (~> 2.0) - rails-html-sanitizer (~> 1.1, >= 1.2.0) + erubi (~> 1.11) + rails-dom-testing (~> 2.2) + rails-html-sanitizer (~> 1.6) active_record_query_trace (1.8) - activejob (7.0.8.7) - activesupport (= 7.0.8.7) + activejob (7.1.5.1) + activesupport (= 7.1.5.1) globalid (>= 0.3.6) - activemodel (7.0.8.7) - activesupport (= 7.0.8.7) - activerecord (7.0.8.7) - activemodel (= 7.0.8.7) - activesupport (= 7.0.8.7) - activerecord-import (1.4.1) + activemodel (7.1.5.1) + activesupport (= 7.1.5.1) + activerecord (7.1.5.1) + activemodel (= 7.1.5.1) + activesupport (= 7.1.5.1) + timeout (>= 0.4.0) + activerecord-import (2.1.0) activerecord (>= 4.2) - activestorage (7.0.8.7) - actionpack (= 7.0.8.7) - activejob (= 7.0.8.7) - activerecord (= 7.0.8.7) - activesupport (= 7.0.8.7) + activestorage (7.1.5.1) + actionpack (= 7.1.5.1) + activejob (= 7.1.5.1) + activerecord (= 7.1.5.1) + activesupport (= 7.1.5.1) marcel (~> 1.0) - mini_mime (>= 1.1.0) - activesupport (7.0.8.7) + activesupport (7.1.5.1) + base64 + benchmark (>= 0.3) + bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) + connection_pool (>= 2.2.5) + drb i18n (>= 1.6, < 2) + logger (>= 1.4.2) minitest (>= 5.1) + mutex_m + securerandom (>= 0.3) tzinfo (~> 2.0) - acts-as-taggable-on (9.0.1) - activerecord (>= 6.0, < 7.1) + acts-as-taggable-on (12.0.0) + activerecord (>= 7.1, < 8.1) + zeitwerk (>= 2.4, < 3.0) addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) administrate (0.20.1) @@ -116,7 +129,7 @@ GEM annotate (3.2.0) activerecord (>= 3.2, < 8.0) rake (>= 10.4, < 14.0) - ast (2.4.2) + ast (2.4.3) attr_extras (7.1.0) audited (5.4.1) activerecord (>= 5.0, < 7.7) @@ -142,14 +155,15 @@ GEM statsd-ruby (~> 1.1) base64 (0.2.0) bcrypt (3.1.20) - bigdecimal (3.1.8) + benchmark (0.4.0) + bigdecimal (3.1.9) bindex (0.8.1) bootsnap (1.16.0) msgpack (~> 1.2) brakeman (5.4.1) browser (5.3.1) builder (3.3.0) - bullet (7.0.7) + bullet (8.0.7) activesupport (>= 3.0.0) uniform_notifier (~> 1.11) bundle-audit (0.1.0) @@ -161,8 +175,8 @@ GEM climate_control (1.2.0) coderay (1.1.3) commonmarker (0.23.10) - concurrent-ruby (1.3.4) - connection_pool (2.4.1) + concurrent-ruby (1.3.5) + connection_pool (2.5.3) crack (1.0.0) bigdecimal rexml @@ -176,16 +190,10 @@ GEM activerecord (>= 5.a) database_cleaner-core (~> 2.0.0) database_cleaner-core (2.0.1) - datadog-ci (0.8.3) - msgpack date (3.4.1) - ddtrace (1.23.2) - datadog-ci (~> 0.8.1) - debase-ruby_core_source (= 3.3.1) - libdatadog (~> 7.0.0.1.0) - libddwaf (~> 1.14.0.0.0) + ddtrace (0.48.0) + ffi (~> 1.0) msgpack - debase-ruby_core_source (3.3.1) debug (1.8.0) irb (>= 1.5.0) reline (>= 0.3.1) @@ -196,10 +204,10 @@ GEM railties (>= 4.1.0) responders warden (~> 1.2.3) - devise_token_auth (1.2.3) + devise_token_auth (1.2.5) bcrypt (~> 3.0) devise (> 3.5.2, < 5) - rails (>= 4.2.0, < 7.2) + rails (>= 4.2.0, < 8.1) diff-lcs (1.5.1) digest-crc (0.6.5) rake (>= 12.0.0, < 14.0.0) @@ -212,6 +220,7 @@ GEM railties (>= 6.1) down (5.4.0) addressable (~> 2.8) + drb (2.2.3) dry-cli (1.1.0) ecma-re-validator (0.4.0) regexp_parser (~> 2.2) @@ -254,7 +263,10 @@ GEM fcm (1.0.8) faraday (>= 1.0.0, < 3.0) googleauth (~> 1) - ffi (1.16.3) + ffi (1.17.2) + ffi (1.17.2-arm64-darwin) + ffi (1.17.2-x86_64-darwin) + ffi (1.17.2-x86_64-linux-gnu) ffi-compiler (1.0.1) ffi (>= 1.0.0) rake @@ -315,16 +327,13 @@ GEM google-cloud-translate-v3 (0.10.0) gapic-common (>= 0.20.0, < 2.a) google-cloud-errors (~> 1.0) - google-protobuf (3.25.5) - google-protobuf (3.25.5-arm64-darwin) - google-protobuf (3.25.5-x86_64-darwin) - google-protobuf (3.25.5-x86_64-linux) + google-protobuf (3.25.7) googleapis-common-protos (1.6.0) google-protobuf (>= 3.18, < 5.a) googleapis-common-protos-types (~> 1.7) grpc (~> 1.41) - googleapis-common-protos-types (1.14.0) - google-protobuf (~> 3.18) + googleapis-common-protos-types (1.20.0) + google-protobuf (>= 3.18, < 5.a) googleauth (1.11.2) faraday (>= 1.0, < 3.a) google-cloud-env (~> 2.1) @@ -334,17 +343,17 @@ GEM signet (>= 0.16, < 2.a) groupdate (6.2.1) activesupport (>= 5.2) - grpc (1.62.0) - google-protobuf (~> 3.25) + grpc (1.72.0) + google-protobuf (>= 3.25, < 5.0) googleapis-common-protos-types (~> 1.0) - grpc (1.62.0-arm64-darwin) - google-protobuf (~> 3.25) + grpc (1.72.0-arm64-darwin) + google-protobuf (>= 3.25, < 5.0) googleapis-common-protos-types (~> 1.0) - grpc (1.62.0-x86_64-darwin) - google-protobuf (~> 3.25) + grpc (1.72.0-x86_64-darwin) + google-protobuf (>= 3.25, < 5.0) googleapis-common-protos-types (~> 1.0) - grpc (1.62.0-x86_64-linux) - google-protobuf (~> 3.25) + grpc (1.72.0-x86_64-linux) + google-protobuf (>= 3.25, < 5.0) googleapis-common-protos-types (~> 1.0) haikunator (1.1.1) hairtrigger (1.0.0) @@ -370,7 +379,7 @@ GEM mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) httpclient (2.8.3) - i18n (1.14.6) + i18n (1.14.7) concurrent-ruby (~> 1.0) image_processing (1.12.2) mini_magick (>= 4.9.5, < 5) @@ -388,7 +397,7 @@ GEM rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) - json (2.6.3) + json (2.12.0) json_refs (0.1.8) hana json_schemer (0.2.24) @@ -423,21 +432,13 @@ GEM faraday-multipart json (>= 1.8) rexml + language_server-protocol (3.17.0.5) launchy (2.5.2) addressable (~> 2.8) letter_opener (1.8.1) launchy (>= 2.2, < 3) - libdatadog (7.0.0.1.0) - libdatadog (7.0.0.1.0-x86_64-linux) - libddwaf (1.14.0.0.0) - ffi (~> 1.0) - libddwaf (1.14.0.0.0-arm64-darwin) - ffi (~> 1.0) - libddwaf (1.14.0.0.0-x86_64-darwin) - ffi (~> 1.0) - libddwaf (1.14.0.0.0-x86_64-linux) - ffi (~> 1.0) line-bot-api (1.28.0) + lint_roller (1.1.0) liquid (5.4.0) listen (3.8.0) rb-fsevent (~> 0.10, >= 0.10.3) @@ -445,7 +446,7 @@ GEM llhttp-ffi (0.4.0) ffi-compiler (~> 1.0) rake (~> 13.0) - logger (1.6.0) + logger (1.7.0) lograge (0.14.0) actionpack (>= 4) activesupport (>= 4) @@ -471,10 +472,10 @@ GEM mini_magick (4.12.0) mini_mime (1.1.5) mini_portile2 (2.8.8) - minitest (5.25.4) + minitest (5.25.5) mock_redis (0.36.0) ruby2_keywords - msgpack (1.7.0) + msgpack (1.8.0) multi_json (1.15.0) multi_xml (0.6.0) multipart-post (2.3.0) @@ -545,14 +546,16 @@ GEM orm_adapter (0.5.0) os (1.1.4) ostruct (0.6.1) - parallel (1.23.0) - parser (3.2.2.1) + parallel (1.27.0) + parser (3.3.8.0) ast (~> 2.4.1) + racc pg (1.5.3) pg_search (2.3.6) activerecord (>= 5.2) activesupport (>= 5.2) pgvector (0.1.1) + prism (1.4.0) procore-sift (1.0.0) activerecord (>= 6.1) pry (0.14.2) @@ -567,7 +570,7 @@ GEM activesupport (>= 3.0.0) raabro (1.4.0) racc (1.8.1) - rack (2.2.14) + rack (2.2.15) rack-attack (6.7.0) rack (>= 1.0, < 4) rack-contrib (2.5.0) @@ -581,23 +584,28 @@ GEM rack (~> 2.2, >= 2.2.4) rack-proxy (0.7.7) rack + rack-session (1.0.2) + rack (< 3) rack-test (2.1.0) rack (>= 1.3) rack-timeout (0.6.3) - rails (7.0.8.7) - actioncable (= 7.0.8.7) - actionmailbox (= 7.0.8.7) - actionmailer (= 7.0.8.7) - actionpack (= 7.0.8.7) - actiontext (= 7.0.8.7) - actionview (= 7.0.8.7) - activejob (= 7.0.8.7) - activemodel (= 7.0.8.7) - activerecord (= 7.0.8.7) - activestorage (= 7.0.8.7) - activesupport (= 7.0.8.7) + rackup (1.0.1) + rack (< 3) + webrick + rails (7.1.5.1) + actioncable (= 7.1.5.1) + actionmailbox (= 7.1.5.1) + actionmailer (= 7.1.5.1) + actionpack (= 7.1.5.1) + actiontext (= 7.1.5.1) + actionview (= 7.1.5.1) + activejob (= 7.1.5.1) + activemodel (= 7.1.5.1) + activerecord (= 7.1.5.1) + activestorage (= 7.1.5.1) + activesupport (= 7.1.5.1) bundler (>= 1.15.0) - railties (= 7.0.8.7) + railties (= 7.1.5.1) rails-dom-testing (2.2.0) activesupport (>= 5.0.0) minitest @@ -605,13 +613,14 @@ GEM rails-html-sanitizer (1.6.1) loofah (~> 2.21) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (7.0.8.7) - actionpack (= 7.0.8.7) - activesupport (= 7.0.8.7) - method_source + railties (7.1.5.1) + actionpack (= 7.1.5.1) + activesupport (= 7.1.5.1) + irb + rackup (>= 1.0.0) rake (>= 12.2) - thor (~> 1.0) - zeitwerk (~> 2.5) + thor (~> 1.0, >= 1.2.2) + zeitwerk (~> 2.6) rainbow (3.1.1) rake (13.2.1) rb-fsevent (0.11.2) @@ -623,7 +632,7 @@ GEM connection_pool redis-namespace (1.10.0) redis (>= 4) - regexp_parser (2.8.0) + regexp_parser (2.10.0) reline (0.3.6) io-console (~> 0.5) representable (3.2.0) @@ -643,7 +652,7 @@ GEM retriable (3.1.2) reverse_markdown (2.1.1) nokogiri - rexml (3.3.9) + rexml (3.4.1) rspec-core (3.13.0) rspec-support (~> 3.13.0) rspec-expectations (3.13.2) @@ -663,30 +672,36 @@ GEM rspec-support (3.13.1) rspec_junit_formatter (0.6.0) rspec-core (>= 2, < 4, != 2.12.0) - rubocop (1.50.2) + rubocop (1.75.6) json (~> 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) parallel (~> 1.10) - parser (>= 3.2.0.0) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.8, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.28.0, < 2.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.44.0, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 2.4.0, < 3.0) - rubocop-ast (1.28.1) - parser (>= 3.2.1.0) - rubocop-capybara (2.18.0) - rubocop (~> 1.41) - rubocop-performance (1.17.1) - rubocop (>= 1.7.0, < 2.0) - rubocop-ast (>= 0.4.0) - rubocop-rails (2.19.1) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.44.1) + parser (>= 3.3.7.2) + prism (~> 1.4) + rubocop-factory_bot (2.27.1) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) + rubocop-performance (1.25.0) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.38.0, < 2.0) + rubocop-rails (2.32.0) activesupport (>= 4.2.0) + lint_roller (~> 1.1) rack (>= 1.1) - rubocop (>= 1.33.0, < 2.0) - rubocop-rspec (2.21.0) - rubocop (~> 1.33) - rubocop-capybara (~> 2.17) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.44.0, < 2.0) + rubocop-rspec (3.6.0) + lint_roller (~> 1.1) + rubocop (~> 1.72, >= 1.72.1) ruby-openai (7.3.1) event_stream_parser (>= 0.3.0, < 2.0.0) faraday (>= 1) @@ -816,8 +831,10 @@ GEM unf (0.1.4) unf_ext unf_ext (0.0.8.2) - unicode-display_width (2.4.2) - uniform_notifier (1.16.0) + unicode-display_width (3.1.4) + unicode-emoji (~> 4.0, >= 4.0.4) + unicode-emoji (4.0.4) + uniform_notifier (1.17.0) uri (1.0.3) uri_template (0.7.0) valid_email2 (5.2.6) @@ -845,7 +862,9 @@ GEM addressable (>= 2.8.0) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) - websocket-driver (0.7.6) + webrick (1.9.1) + websocket-driver (0.7.7) + base64 websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) wisper (2.0.0) @@ -951,7 +970,7 @@ DEPENDENCIES rack-cors (= 2.0.0) rack-mini-profiler (>= 3.2.0) rack-timeout - rails (~> 7.0.8.4) + rails (~> 7.1) redis redis-namespace responders (>= 3.1.1) @@ -960,6 +979,7 @@ DEPENDENCIES rspec-rails (>= 6.1.5) rspec_junit_formatter rubocop + rubocop-factory_bot rubocop-performance rubocop-rails rubocop-rspec @@ -997,7 +1017,7 @@ DEPENDENCIES working_hours RUBY VERSION - ruby 3.3.3p89 + ruby 3.4.4p34 BUNDLED WITH 2.5.16 diff --git a/app/builders/contact_inbox_builder.rb b/app/builders/contact_inbox_builder.rb index ffa45db2e..788ae39d1 100644 --- a/app/builders/contact_inbox_builder.rb +++ b/app/builders/contact_inbox_builder.rb @@ -59,11 +59,13 @@ class ContactInboxBuilder end def create_contact_inbox - ::ContactInbox.create_with(hmac_verified: hmac_verified || false).find_or_create_by!( + attrs = { contact_id: @contact.id, inbox_id: @inbox.id, source_id: @source_id - ) + } + + ::ContactInbox.where(attrs).first_or_create!(hmac_verified: hmac_verified || false) rescue ActiveRecord::RecordNotUnique Rails.logger.info("[ContactInboxBuilder] RecordNotUnique #{@source_id} #{@contact.id} #{@inbox.id}") update_old_contact_inbox diff --git a/app/mailboxes/application_mailbox.rb b/app/mailboxes/application_mailbox.rb index 4fe931b9f..9fc7a435d 100644 --- a/app/mailboxes/application_mailbox.rb +++ b/app/mailboxes/application_mailbox.rb @@ -31,7 +31,7 @@ class ApplicationMailbox < ActionMailbox::Base end def in_reply_to_matches?(in_reply_to) - Array.wrap(in_reply_to).any? { _1.match?(CONVERSATION_MESSAGE_ID_PATTERN) } + Array.wrap(in_reply_to).any? { it.match?(CONVERSATION_MESSAGE_ID_PATTERN) } end # checks if follow this pattern send it to reply_mailbox diff --git a/app/models/account.rb b/app/models/account.rb index 1664ce29b..fb156511b 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -97,8 +97,8 @@ class Account < ApplicationRecord has_one_attached :contacts_export - enum locale: LANGUAGES_CONFIG.map { |key, val| [val[:iso_639_1_code], key] }.to_h - enum status: { active: 0, suspended: 1 } + enum :locale, LANGUAGES_CONFIG.map { |key, val| [val[:iso_639_1_code], key] }.to_h, prefix: true + enum :status, { active: 0, suspended: 1 } scope :with_auto_resolve, -> { where("(settings ->> 'auto_resolve_after')::int IS NOT NULL") } diff --git a/app/models/conversation.rb b/app/models/conversation.rb index bcb79c05b..3f1a20037 100644 --- a/app/models/conversation.rb +++ b/app/models/conversation.rb @@ -128,6 +128,12 @@ class Conversation < ApplicationRecord additional_attributes&.dig('conversation_language') end + # Be aware: The precision of created_at and last_activity_at may differ from Ruby's Time precision. + # Our DB column (see schema) stores timestamps with second-level precision (no microseconds), so + # if you assign a Ruby Time with microseconds, the DB will truncate it. This may cause subtle differences + # if you compare or copy these values in Ruby, also in our specs + # So in specs rely on to be_with(1.second) instead of to eq() + # TODO: Migrate to use a timestamp with microsecond precision def last_activity_at self[:last_activity_at] || created_at end diff --git a/app/models/email_template.rb b/app/models/email_template.rb index 5891626a4..57db7a94c 100644 --- a/app/models/email_template.rb +++ b/app/models/email_template.rb @@ -16,8 +16,8 @@ # index_email_templates_on_name_and_account_id (name,account_id) UNIQUE # class EmailTemplate < ApplicationRecord - enum locale: LANGUAGES_CONFIG.map { |key, val| [val[:iso_639_1_code], key] }.to_h - enum template_type: { layout: 0, content: 1 } + enum :locale, LANGUAGES_CONFIG.map { |key, val| [val[:iso_639_1_code], key] }.to_h, prefix: true + enum :template_type, { layout: 0, content: 1 } belongs_to :account, optional: true validates :name, uniqueness: { scope: :account } diff --git a/app/models/installation_config.rb b/app/models/installation_config.rb index f1603e4ca..5ea438916 100644 --- a/app/models/installation_config.rb +++ b/app/models/installation_config.rb @@ -19,7 +19,7 @@ class InstallationConfig < ApplicationRecord # https://discuss.rubyonrails.org/t/cve-2022-32224-possible-rce-escalation-bug-with-serialized-columns-in-active-record/81017 # FIX ME : fixes breakage of installation config. we need to migrate. # Fix configuration in application.rb - serialize :serialized_value, ActiveSupport::HashWithIndifferentAccess + serialize :serialized_value, coder: YAML, type: ActiveSupport::HashWithIndifferentAccess before_validation :set_lock validates :name, presence: true @@ -32,6 +32,10 @@ class InstallationConfig < ApplicationRecord after_commit :clear_cache def value + # This is an extra hack again cause of the YAML serialization, in case of new object initialization in super admin + # It was throwing error as the default value of column '{}' was failing in deserialization. + return {}.with_indifferent_access if new_record? && @attributes['serialized_value']&.value_before_type_cast == '{}' + serialized_value[:value] end diff --git a/app/models/user.rb b/app/models/user.rb index 5594b0ca6..d1907362a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -109,8 +109,8 @@ class User < ApplicationRecord self.email = email.try(:downcase) end - def send_devise_notification(notification, *args) - devise_mailer.with(account: Current.account).send(notification, self, *args).deliver_later + def send_devise_notification(notification, *) + devise_mailer.with(account: Current.account).send(notification, self, *).deliver_later end def set_password_and_uid diff --git a/app/services/message_templates/template/csat_survey.rb b/app/services/message_templates/template/csat_survey.rb index 3a7ca2605..dd9cf3bd6 100644 --- a/app/services/message_templates/template/csat_survey.rb +++ b/app/services/message_templates/template/csat_survey.rb @@ -12,7 +12,6 @@ class MessageTemplates::Template::CsatSurvey private delegate :contact, :account, :inbox, to: :conversation - delegate :csat_config, to: :inbox def should_send_csat_survey? return true unless survey_rules_configured? diff --git a/deployment/chatwoot-web.1.service b/deployment/chatwoot-web.1.service index 049ec85db..0d1d33804 100644 --- a/deployment/chatwoot-web.1.service +++ b/deployment/chatwoot-web.1.service @@ -16,10 +16,10 @@ KillMode=mixed StandardInput=null SyslogIdentifier=%p -Environment="PATH=/home/chatwoot/.rvm/gems/ruby-3.3.3/bin:/home/chatwoot/.rvm/gems/ruby-3.3.3@global/bin:/home/chatwoot/.rvm/rubies/ruby-3.3.3/bin:/home/chatwoot/.rvm/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/chatwoot/.rvm/bin:/home/chatwoot/.rvm/bin" +Environment="PATH=/home/chatwoot/.rvm/gems/ruby-3.4.4/bin:/home/chatwoot/.rvm/gems/ruby-3.4.4@global/bin:/home/chatwoot/.rvm/rubies/ruby-3.4.4/bin:/home/chatwoot/.rvm/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/chatwoot/.rvm/bin:/home/chatwoot/.rvm/bin" Environment="PORT=3000" Environment="RAILS_ENV=production" Environment="NODE_ENV=production" Environment="RAILS_LOG_TO_STDOUT=true" -Environment="GEM_HOME=/home/chatwoot/.rvm/gems/ruby-3.3.3" -Environment="GEM_PATH=/home/chatwoot/.rvm/gems/ruby-3.3.3:/home/chatwoot/.rvm/gems/ruby-3.3.3@global" +Environment="GEM_HOME=/home/chatwoot/.rvm/gems/ruby-3.4.4" +Environment="GEM_PATH=/home/chatwoot/.rvm/gems/ruby-3.4.4:/home/chatwoot/.rvm/gems/ruby-3.4.4@global" diff --git a/deployment/chatwoot-worker.1.service b/deployment/chatwoot-worker.1.service index 04ee83a69..ea893d20c 100644 --- a/deployment/chatwoot-worker.1.service +++ b/deployment/chatwoot-worker.1.service @@ -21,10 +21,10 @@ MemoryHigh=1.4G MemorySwapMax=0 OOMPolicy=stop -Environment="PATH=/home/chatwoot/.rvm/gems/ruby-3.3.3/bin:/home/chatwoot/.rvm/gems/ruby-3.3.3@global/bin:/home/chatwoot/.rvm/rubies/ruby-3.3.3/bin:/home/chatwoot/.rvm/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/chatwoot/.rvm/bin:/home/chatwoot/.rvm/bin" +Environment="PATH=/home/chatwoot/.rvm/gems/ruby-3.4.4/bin:/home/chatwoot/.rvm/gems/ruby-3.4.4@global/bin:/home/chatwoot/.rvm/rubies/ruby-3.4.4/bin:/home/chatwoot/.rvm/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin:/home/chatwoot/.rvm/bin:/home/chatwoot/.rvm/bin" Environment="PORT=3000" Environment="RAILS_ENV=production" Environment="NODE_ENV=production" Environment="RAILS_LOG_TO_STDOUT=true" -Environment="GEM_HOME=/home/chatwoot/.rvm/gems/ruby-3.3.3" -Environment="GEM_PATH=/home/chatwoot/.rvm/gems/ruby-3.3.3:/home/chatwoot/.rvm/gems/ruby-3.3.3@global" +Environment="GEM_HOME=/home/chatwoot/.rvm/gems/ruby-3.4.4" +Environment="GEM_PATH=/home/chatwoot/.rvm/gems/ruby-3.4.4:/home/chatwoot/.rvm/gems/ruby-3.4.4@global" diff --git a/deployment/setup_20.04.sh b/deployment/setup_20.04.sh index 7809b6d0e..75320bbd3 100644 --- a/deployment/setup_20.04.sh +++ b/deployment/setup_20.04.sh @@ -338,8 +338,8 @@ function setup_chatwoot() { sudo -i -u chatwoot << EOF rvm --version rvm autolibs disable - rvm install "ruby-3.3.3" - rvm use 3.3.3 --default + rvm install "ruby-3.4.4" + rvm use 3.4.4 --default git clone https://github.com/chatwoot/chatwoot.git cd chatwoot diff --git a/docker/Dockerfile b/docker/Dockerfile index 008884ce1..2d753337d 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -1,6 +1,6 @@ # pre-build stage FROM node:23-alpine as node -FROM ruby:3.3.3-alpine3.19 AS pre-builder +FROM ruby:3.4.4-alpine3.21 AS pre-builder ARG NODE_VERSION="23.7.0" ARG PNPM_VERSION="10.2.0" @@ -90,13 +90,13 @@ RUN if [ "$RAILS_ENV" = "production" ]; then \ RUN git rev-parse HEAD > /app/.git_sha # Remove unnecessary files -RUN rm -rf /gems/ruby/3.3.0/cache/*.gem \ - && find /gems/ruby/3.3.0/gems/ \( -name "*.c" -o -name "*.o" \) -delete \ +RUN rm -rf /gems/ruby/3.4.0/cache/*.gem \ + && find /gems/ruby/3.4.0/gems/ \( -name "*.c" -o -name "*.o" \) -delete \ && rm -rf .git \ && rm .gitignore # final build stage -FROM ruby:3.3.3-alpine3.19 +FROM ruby:3.4.4-alpine3.21 ARG NODE_VERSION="23.7.0" ARG PNPM_VERSION="10.2.0" diff --git a/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb b/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb index 01650a7de..2d6823b90 100644 --- a/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/custom_roles_controller.rb @@ -6,12 +6,12 @@ class Api::V1::Accounts::CustomRolesController < Api::V1::Accounts::EnterpriseAc @custom_roles = Current.account.custom_roles end + def show; end + def create @custom_role = Current.account.custom_roles.create!(permitted_params) end - def show; end - def update @custom_role.update!(permitted_params) end diff --git a/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb b/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb index e64256bc7..ec879b042 100644 --- a/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb +++ b/enterprise/app/controllers/api/v1/accounts/sla_policies_controller.rb @@ -6,12 +6,12 @@ class Api::V1::Accounts::SlaPoliciesController < Api::V1::Accounts::EnterpriseAc @sla_policies = Current.account.sla_policies end + def show; end + def create @sla_policy = Current.account.sla_policies.create!(permitted_params) end - def show; end - def update @sla_policy.update!(permitted_params) end diff --git a/enterprise/app/jobs/captain/conversation/response_builder_job.rb b/enterprise/app/jobs/captain/conversation/response_builder_job.rb index 39a12b662..c661caebe 100644 --- a/enterprise/app/jobs/captain/conversation/response_builder_job.rb +++ b/enterprise/app/jobs/captain/conversation/response_builder_job.rb @@ -50,7 +50,7 @@ class Captain::Conversation::ResponseBuilderJob < ApplicationJob def message_content(message) return message.content if message.content.present? - 'User has shared an attachment' if message.attachments.any? + return 'User has shared an attachment' if message.attachments.any? 'User has shared a message without content' end diff --git a/enterprise/app/services/captain/tool_registry_service.rb b/enterprise/app/services/captain/tool_registry_service.rb index f2c234060..4841a29e4 100644 --- a/enterprise/app/services/captain/tool_registry_service.rb +++ b/enterprise/app/services/captain/tool_registry_service.rb @@ -15,9 +15,9 @@ class Captain::ToolRegistryService @registered_tools << tool.to_registry_format end - def method_missing(method_name, *arguments) + def method_missing(method_name, *) if @tools.key?(method_name.to_s) - @tools[method_name.to_s].execute(*arguments) + @tools[method_name.to_s].execute(*) else super end diff --git a/spec/builders/v2/reports/conversations/report_builder_spec.rb b/spec/builders/v2/reports/conversations/report_builder_spec.rb index d3cde98e6..db7a0ac45 100644 --- a/spec/builders/v2/reports/conversations/report_builder_spec.rb +++ b/spec/builders/v2/reports/conversations/report_builder_spec.rb @@ -33,12 +33,12 @@ describe V2::Reports::Conversations::ReportBuilder do end describe '#timeseries' do - include_examples 'valid metric handler', 'avg_first_response_time', :timeseries, V2::Reports::Timeseries::AverageReportBuilder - include_examples 'valid metric handler', 'conversations_count', :timeseries, V2::Reports::Timeseries::CountReportBuilder + it_behaves_like 'valid metric handler', 'avg_first_response_time', :timeseries, V2::Reports::Timeseries::AverageReportBuilder + it_behaves_like 'valid metric handler', 'conversations_count', :timeseries, V2::Reports::Timeseries::CountReportBuilder end describe '#aggregate_value' do - include_examples 'valid metric handler', 'avg_first_response_time', :aggregate_value, V2::Reports::Timeseries::AverageReportBuilder - include_examples 'valid metric handler', 'conversations_count', :aggregate_value, V2::Reports::Timeseries::CountReportBuilder + it_behaves_like 'valid metric handler', 'avg_first_response_time', :aggregate_value, V2::Reports::Timeseries::AverageReportBuilder + it_behaves_like 'valid metric handler', 'conversations_count', :aggregate_value, V2::Reports::Timeseries::CountReportBuilder end end diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb index 9395f3e55..aef91603d 100644 --- a/spec/models/conversation_spec.rb +++ b/spec/models/conversation_spec.rb @@ -793,8 +793,8 @@ RSpec.describe Conversation do end context 'when a new conversation is created' do - it 'sets last_activity_at to the created_at time' do - expect(conversation.last_activity_at).to eq(conversation.created_at) + it 'sets last_activity_at to the created_at time (within DB precision)' do + expect(conversation.last_activity_at).to be_within(1.second).of(conversation.created_at) end end diff --git a/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb b/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb index 29f7136f1..85bb08d74 100644 --- a/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb +++ b/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb @@ -183,7 +183,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do expect(result.length).to be <= described_class::ACTIVITY_NOTE_MAX_SIZE + 100 # Verify that not all messages are included (some were truncated) - expect(messages.count).to be > result.scan(/John Doe:/).count + expect(messages.count).to be > result.scan('John Doe:').count end it 'respects the ACTIVITY_NOTE_MAX_SIZE constant' do diff --git a/swagger/swagger.json b/swagger/swagger.json index 19ef5c904..4e557e964 100644 --- a/swagger/swagger.json +++ b/swagger/swagger.json @@ -46,9 +46,7 @@ }, "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "paths": { @@ -62,9 +60,7 @@ "description": "Create an Account", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -105,9 +101,7 @@ "description": "Get the details of an account", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -134,9 +128,7 @@ "description": "Update an account's attributes", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -170,9 +162,7 @@ "description": "Delete an Account", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -203,9 +193,7 @@ "description": "List all account users", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -247,9 +235,7 @@ "description": "Create an Account User", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -310,9 +296,7 @@ "description": "Delete an Account User", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -357,9 +341,7 @@ "description": "List all agent bots available", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -387,9 +369,7 @@ "description": "Create an agent bot", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -430,9 +410,7 @@ "description": "Get the details of an agent bot", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -459,9 +437,7 @@ "description": "Update an agent bot's attributes", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -495,9 +471,7 @@ "description": "Delete an AgentBot", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -523,9 +497,7 @@ "description": "Create a User", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -566,9 +538,7 @@ "description": "Get the details of an user", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -595,9 +565,7 @@ "description": "Update a user's attributes", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "parameters": [ @@ -631,9 +599,7 @@ "description": "Delete a User", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -664,9 +630,7 @@ "description": "Get the sso link of a user", "security": [ { - "platformAppApiKey": [ - - ] + "platformAppApiKey": [] } ], "responses": { @@ -704,9 +668,7 @@ "operationId": "get-details-of-a-inbox", "summary": "Inbox details", "description": "Get the details of an inbox", - "security": [ - - ], + "security": [], "responses": { "200": { "description": "Success", @@ -736,9 +698,7 @@ "operationId": "create-a-contact", "summary": "Create a contact", "description": "Create a contact", - "security": [ - - ], + "security": [], "parameters": [ { "name": "data", @@ -778,9 +738,7 @@ "operationId": "get-details-of-a-contact", "summary": "Get a contact", "description": "Get the details of a contact", - "security": [ - - ], + "security": [], "responses": { "200": { "description": "Success", @@ -803,9 +761,7 @@ "operationId": "update-a-contact", "summary": "Update a contact", "description": "Update a contact's attributes", - "security": [ - - ], + "security": [], "parameters": [ { "name": "data", @@ -845,9 +801,7 @@ "operationId": "create-a-conversation", "summary": "Create a conversation", "description": "Create a conversation", - "security": [ - - ], + "security": [], "parameters": [ { "name": "data", @@ -1056,9 +1010,7 @@ "operationId": "create-a-message", "summary": "Create a message", "description": "Create a message", - "security": [ - - ], + "security": [], "parameters": [ { "name": "data", @@ -1127,9 +1079,7 @@ "operationId": "update-a-message", "summary": "Update a message", "description": "Update a message", - "security": [ - - ], + "security": [], "parameters": [ { "name": "data", @@ -1239,9 +1189,7 @@ "operationId": "get-csat-survey-page", "summary": "Get CSAT survey page", "description": "You can redirect the client to this URL, instead of implementing the CSAT survey component yourself.", - "security": [ - - ], + "security": [], "responses": { "200": { "description": "Success" @@ -1406,9 +1354,7 @@ "description": "Get Details of Agents in an Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "responses": { @@ -1436,9 +1382,7 @@ "description": "Add a new Agent to Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1515,9 +1459,7 @@ "description": "Update an Agent in Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1587,9 +1529,7 @@ "description": "Remove an Agent from Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1629,9 +1569,7 @@ "description": "Get Details of Canned Responses in an Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "responses": { @@ -1659,9 +1597,7 @@ "description": "Add a new Canned Response to Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1702,9 +1638,7 @@ "description": "Update a Canned Response in Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1748,9 +1682,7 @@ "description": "Remove a Canned Response from Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1803,9 +1735,7 @@ "description": "Get details of custom attributes in an Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "responses": { @@ -1833,9 +1763,7 @@ "description": "Add a new custom attribute to account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1917,9 +1845,7 @@ "description": "Update a custom attribute in account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -1963,9 +1889,7 @@ "description": "Remove a custom attribute from account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -2246,14 +2170,10 @@ "summary": "Contact Filter", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -2466,9 +2386,7 @@ "description": "Get details of automation rules in an Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "responses": { @@ -2496,9 +2414,7 @@ "description": "Add a new automation rule to account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -2577,9 +2493,7 @@ "description": "Update a automation rule in account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -2623,9 +2537,7 @@ "description": "Remove a automation rule from account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -2665,9 +2577,7 @@ "description": "Add a new portal to account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -2706,9 +2616,7 @@ "description": "Get details of portals in an Account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "responses": { @@ -2736,9 +2644,7 @@ "description": "update a new portal to account", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -2782,9 +2688,7 @@ "description": "Add a new category to portal", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -2828,9 +2732,7 @@ "description": "Add a new article to portal", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -3041,14 +2943,10 @@ "description": "Creating a conversation in chatwoot requires a source id. \n\n Learn more about source_id: https://github.com/chatwoot/chatwoot/wiki/Building-on-Top-of-Chatwoot:-Importing-Existing-Contacts-and-Creating-Conversations", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -3191,14 +3089,10 @@ "summary": "Conversations Filter", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -3329,14 +3223,10 @@ "description": "Update Conversation Attributes", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -3397,14 +3287,10 @@ "description": "Toggles the status of the conversation between open and resolved", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -3465,14 +3351,10 @@ "description": "Toggles the priority of conversation", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -3532,14 +3414,10 @@ "description": "Updates the custom attributes of a conversation", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -3605,14 +3483,10 @@ "description": "Assign a conversation to an agent or a team", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -4063,9 +3937,7 @@ "description": "Get Details of Agents in an Inbox", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -4108,9 +3980,7 @@ "description": "Add a new Agent to Inbox", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -4171,9 +4041,7 @@ "description": "All agents except the one passed in params will be removed", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -4234,9 +4102,7 @@ "description": "Remove an Agent from Inbox", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -4333,14 +4199,10 @@ "description": "Create a new message in the conversation", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] }, { - "agentBotApiKey": [ - - ] + "agentBotApiKey": [] } ], "parameters": [ @@ -4720,9 +4582,7 @@ "description": "Get Details of Agents in an Team", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -4761,9 +4621,7 @@ "description": "Add a new Agent to Team", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -4819,9 +4677,7 @@ "description": "All agents except the one passed in params will be removed", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ @@ -4877,9 +4733,7 @@ "description": "Remove an Agent from Team", "security": [ { - "userApiKey": [ - - ] + "userApiKey": [] } ], "parameters": [ From dc7f1597e5315c335eb55bad31ee73db7dfaa52c Mon Sep 17 00:00:00 2001 From: Pranav Date: Wed, 21 May 2025 15:49:35 -0700 Subject: [PATCH 3/5] feat: Add support for additional tools in Copilot (#11531) - Added GetConversation, GetContact, GetArticle, SearchContacts, SearchArticles - Update SearchConversations to handle the permissions properly. --- .../services/captain/tool_registry_service.rb | 5 +- .../services/captain/tools/base_service.rb | 14 ++ .../tools/copilot/get_article_service.rb | 39 ++++ .../tools/copilot/get_contact_service.rb | 39 ++++ .../tools/copilot/get_conversation_service.rb | 41 +++++ .../tools/copilot/search_articles_service.rb | 71 ++++++++ .../tools/copilot/search_contacts_service.rb | 61 +++++++ .../copilot/search_conversations_service.rb | 6 + .../captain/tool_registry_service_spec.rb | 2 +- .../tools/copilot/get_article_service_spec.rb | 112 ++++++++++++ .../tools/copilot/get_contact_service_spec.rb | 110 ++++++++++++ .../copilot/get_conversation_service_spec.rb | 142 +++++++++++++++ .../copilot/search_articles_service_spec.rb | 167 ++++++++++++++++++ .../copilot/search_contacts_service_spec.rb | 113 ++++++++++++ .../search_conversations_service_spec.rb | 58 ++++++ 15 files changed, 977 insertions(+), 3 deletions(-) create mode 100644 enterprise/app/services/captain/tools/copilot/get_article_service.rb create mode 100644 enterprise/app/services/captain/tools/copilot/get_contact_service.rb create mode 100644 enterprise/app/services/captain/tools/copilot/get_conversation_service.rb create mode 100644 enterprise/app/services/captain/tools/copilot/search_articles_service.rb create mode 100644 enterprise/app/services/captain/tools/copilot/search_contacts_service.rb create mode 100644 spec/enterprise/services/captain/tools/copilot/get_article_service_spec.rb create mode 100644 spec/enterprise/services/captain/tools/copilot/get_contact_service_spec.rb create mode 100644 spec/enterprise/services/captain/tools/copilot/get_conversation_service_spec.rb create mode 100644 spec/enterprise/services/captain/tools/copilot/search_articles_service_spec.rb create mode 100644 spec/enterprise/services/captain/tools/copilot/search_contacts_service_spec.rb diff --git a/enterprise/app/services/captain/tool_registry_service.rb b/enterprise/app/services/captain/tool_registry_service.rb index 4841a29e4..8d6632e57 100644 --- a/enterprise/app/services/captain/tool_registry_service.rb +++ b/enterprise/app/services/captain/tool_registry_service.rb @@ -1,14 +1,15 @@ class Captain::ToolRegistryService attr_reader :registered_tools, :tools - def initialize(assistant) + def initialize(assistant, user: nil) @assistant = assistant + @user = user @registered_tools = [] @tools = {} end def register_tool(tool_class) - tool = tool_class.new(@assistant) + tool = tool_class.new(@assistant, user: @user) return unless tool.active? @tools[tool.name] = tool diff --git a/enterprise/app/services/captain/tools/base_service.rb b/enterprise/app/services/captain/tools/base_service.rb index 9143726b7..72fde2df8 100644 --- a/enterprise/app/services/captain/tools/base_service.rb +++ b/enterprise/app/services/captain/tools/base_service.rb @@ -36,4 +36,18 @@ class Captain::Tools::BaseService def active? true end + + private + + def user_has_permission(permission) + return false if @user.blank? + + account_user = AccountUser.find_by(account_id: @assistant.account_id, user_id: @user.id) + return false if account_user.blank? + + return account_user.custom_role.permissions.include?(permission) if account_user.custom_role.present? + + # Default permission for agents without custom roles + account_user.administrator? || account_user.agent? + end end diff --git a/enterprise/app/services/captain/tools/copilot/get_article_service.rb b/enterprise/app/services/captain/tools/copilot/get_article_service.rb new file mode 100644 index 000000000..9c2ee02da --- /dev/null +++ b/enterprise/app/services/captain/tools/copilot/get_article_service.rb @@ -0,0 +1,39 @@ +class Captain::Tools::Copilot::GetArticleService < Captain::Tools::BaseService + def name + 'get_article' + end + + def description + 'Get details of an article including its content and metadata' + end + + def parameters + { + type: 'object', + properties: { + article_id: { + type: 'number', + description: 'The ID of the article to retrieve' + } + }, + required: %w[article_id] + } + end + + def execute(arguments) + article_id = arguments['article_id'] + + Rails.logger.info { "#{self.class.name}: Article ID: #{article_id}" } + + return 'Missing required parameters' if article_id.blank? + + article = Article.find_by(id: article_id, account_id: @assistant.account_id) + return 'Article not found' if article.nil? + + article.to_llm_text + end + + def active? + user_has_permission('knowledge_base_manage') + end +end diff --git a/enterprise/app/services/captain/tools/copilot/get_contact_service.rb b/enterprise/app/services/captain/tools/copilot/get_contact_service.rb new file mode 100644 index 000000000..290433301 --- /dev/null +++ b/enterprise/app/services/captain/tools/copilot/get_contact_service.rb @@ -0,0 +1,39 @@ +class Captain::Tools::Copilot::GetContactService < Captain::Tools::BaseService + def name + 'get_contact' + end + + def description + 'Get details of a contact including their profile information' + end + + def parameters + { + type: 'object', + properties: { + contact_id: { + type: 'number', + description: 'The ID of the contact to retrieve' + } + }, + required: %w[contact_id] + } + end + + def execute(arguments) + contact_id = arguments['contact_id'] + + Rails.logger.info "#{self.class.name}: Contact ID: #{contact_id}" + + return 'Missing required parameters' if contact_id.blank? + + contact = Contact.find_by(id: contact_id, account_id: @assistant.account_id) + return 'Contact not found' if contact.nil? + + contact.to_llm_text + end + + def active? + user_has_permission('contact_manage') + end +end diff --git a/enterprise/app/services/captain/tools/copilot/get_conversation_service.rb b/enterprise/app/services/captain/tools/copilot/get_conversation_service.rb new file mode 100644 index 000000000..64b52d012 --- /dev/null +++ b/enterprise/app/services/captain/tools/copilot/get_conversation_service.rb @@ -0,0 +1,41 @@ +class Captain::Tools::Copilot::GetConversationService < Captain::Tools::BaseService + def name + 'get_conversation' + end + + def description + 'Get details of a conversation including messages and contact information' + end + + def parameters + { + type: 'object', + properties: { + conversation_id: { + type: 'number', + description: 'The ID of the conversation to retrieve' + } + }, + required: %w[conversation_id] + } + end + + def execute(arguments) + conversation_id = arguments['conversation_id'] + + Rails.logger.info "#{self.class.name}: Conversation ID: #{conversation_id}" + + return 'Missing required parameters' if conversation_id.blank? + + conversation = Conversation.find_by(display_id: conversation_id, account_id: @assistant.account_id) + return 'Conversation not found' if conversation.blank? + + conversation.to_llm_text + end + + def active? + user_has_permission('conversation_manage') || + user_has_permission('conversation_unassigned_manage') || + user_has_permission('conversation_participating_manage') + end +end diff --git a/enterprise/app/services/captain/tools/copilot/search_articles_service.rb b/enterprise/app/services/captain/tools/copilot/search_articles_service.rb new file mode 100644 index 000000000..5061968ad --- /dev/null +++ b/enterprise/app/services/captain/tools/copilot/search_articles_service.rb @@ -0,0 +1,71 @@ +class Captain::Tools::Copilot::SearchArticlesService < Captain::Tools::BaseService + def name + 'search_articles' + end + + def description + 'Search articles based on parameters' + end + + def parameters + { + type: 'object', + properties: properties, + required: ['query'] + } + end + + def execute(arguments) + query = arguments['query'] + category_id = arguments['category_id'] + status = arguments['status'] + + Rails.logger.info "#{self.class.name}: Query: #{query}, Category ID: #{category_id}, Status: #{status}" + + return 'Missing required parameters' if query.blank? + + articles = fetch_articles(query, category_id, status) + + return 'No articles found' unless articles.exists? + + total_count = articles.count + articles = articles.limit(100) + + <<~RESPONSE + #{total_count > 100 ? "Found #{total_count} articles (showing first 100)" : "Total number of articles: #{total_count}"} + #{articles.map(&:to_llm_text).join("\n---\n")} + RESPONSE + end + + def active? + user_has_permission('knowledge_base_manage') + end + + private + + def fetch_articles(query, category_id, status) + articles = Article.where(account_id: @assistant.account_id) + articles = articles.where('title ILIKE :query OR content ILIKE :query', query: "%#{query}%") if query.present? + articles = articles.where(category_id: category_id) if category_id.present? + articles = articles.where(status: status) if status.present? + articles + end + + def properties + { + query: { + type: 'string', + description: 'Search articles by title or content (partial match)' + }, + category_id: { + type: 'number', + description: 'Filter articles by category ID' + }, + status: { + type: 'string', + enum: %w[draft published archived], + description: 'Filter articles by status' + } + } + end +end diff --git a/enterprise/app/services/captain/tools/copilot/search_contacts_service.rb b/enterprise/app/services/captain/tools/copilot/search_contacts_service.rb new file mode 100644 index 000000000..557fc731a --- /dev/null +++ b/enterprise/app/services/captain/tools/copilot/search_contacts_service.rb @@ -0,0 +1,61 @@ +class Captain::Tools::Copilot::SearchContactsService < Captain::Tools::BaseService + def name + 'search_contacts' + end + + def description + 'Search contacts based on query parameters' + end + + def parameters + { + type: 'object', + properties: properties, + required: [] + } + end + + def execute(arguments) + email = arguments['email'] + phone_number = arguments['phone_number'] + name = arguments['name'] + + Rails.logger.info "#{self.class.name} Email: #{email}, Phone Number: #{phone_number}, Name: #{name}" + + contacts = Contact.where(account_id: @assistant.account_id) + contacts = contacts.where(email: email) if email.present? + contacts = contacts.where(phone_number: phone_number) if phone_number.present? + contacts = contacts.where('LOWER(name) ILIKE ?', "%#{name.downcase}%") if name.present? + + return 'No contacts found' unless contacts.exists? + + contacts = contacts.limit(100) + + <<~RESPONSE + #{contacts.map(&:to_llm_text).join("\n---\n")} + RESPONSE + end + + def active? + user_has_permission('contact_manage') + end + + private + + def properties + { + email: { + type: 'string', + description: 'Filter contacts by email' + }, + phone_number: { + type: 'string', + description: 'Filter contacts by phone number' + }, + name: { + type: 'string', + description: 'Filter contacts by name (partial match)' + } + } + end +end diff --git a/enterprise/app/services/captain/tools/copilot/search_conversations_service.rb b/enterprise/app/services/captain/tools/copilot/search_conversations_service.rb index 6b6b063ea..f97604793 100644 --- a/enterprise/app/services/captain/tools/copilot/search_conversations_service.rb +++ b/enterprise/app/services/captain/tools/copilot/search_conversations_service.rb @@ -33,6 +33,12 @@ class Captain::Tools::Copilot::SearchConversationsService < Captain::Tools::Base RESPONSE end + def active? + user_has_permission('conversation_manage') || + user_has_permission('conversation_unassigned_manage') || + user_has_permission('conversation_participating_manage') + end + private def get_conversations(status, contact_id, priority) diff --git a/spec/enterprise/services/captain/tool_registry_service_spec.rb b/spec/enterprise/services/captain/tool_registry_service_spec.rb index beb4a6a63..c8d97fe3a 100644 --- a/spec/enterprise/services/captain/tool_registry_service_spec.rb +++ b/spec/enterprise/services/captain/tool_registry_service_spec.rb @@ -4,7 +4,7 @@ require 'rails_helper' class TestTool < Captain::Tools::BaseService attr_accessor :tool_active - def initialize(*args) + def initialize(assistant, user: nil) super @tool_active = true end diff --git a/spec/enterprise/services/captain/tools/copilot/get_article_service_spec.rb b/spec/enterprise/services/captain/tools/copilot/get_article_service_spec.rb new file mode 100644 index 000000000..72f4e1cb4 --- /dev/null +++ b/spec/enterprise/services/captain/tools/copilot/get_article_service_spec.rb @@ -0,0 +1,112 @@ +require 'rails_helper' + +RSpec.describe Captain::Tools::Copilot::GetArticleService do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:service) { described_class.new(assistant, user: user) } + + describe '#name' do + it 'returns the correct service name' do + expect(service.name).to eq('get_article') + end + end + + describe '#description' do + it 'returns the service description' do + expect(service.description).to eq('Get details of an article including its content and metadata') + end + end + + describe '#parameters' do + it 'returns the expected parameter schema' do + expect(service.parameters).to eq( + { + type: 'object', + properties: { + article_id: { + type: 'number', + description: 'The ID of the article to retrieve' + } + }, + required: %w[article_id] + } + ) + end + end + + describe '#active?' do + context 'when user is an admin' do + let(:user) { create(:user, :administrator, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role with knowledge_base_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['knowledge_base_manage']) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role without knowledge_base_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: []) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns false' do + expect(service.active?).to be false + end + end + end + + describe '#execute' do + context 'when article_id is blank' do + it 'returns error message' do + expect(service.execute({})).to eq('Missing required parameters') + end + end + + context 'when article is not found' do + it 'returns not found message' do + expect(service.execute({ 'article_id' => 999 })).to eq('Article not found') + end + end + + context 'when article exists' do + let(:portal) { create(:portal, account: account) } + let(:article) { create(:article, account: account, portal: portal, author: user, title: 'Test Article', content: 'Content') } + + it 'returns the article in llm text format' do + result = service.execute({ 'article_id' => article.id }) + expect(result).to eq(article.to_llm_text) + end + + context 'when article belongs to different account' do + let(:other_account) { create(:account) } + let(:other_portal) { create(:portal, account: other_account) } + let(:other_article) { create(:article, account: other_account, portal: other_portal, author: user, title: 'Other Article') } + + it 'returns not found message' do + expect(service.execute({ 'article_id' => other_article.id })).to eq('Article not found') + end + end + end + end +end diff --git a/spec/enterprise/services/captain/tools/copilot/get_contact_service_spec.rb b/spec/enterprise/services/captain/tools/copilot/get_contact_service_spec.rb new file mode 100644 index 000000000..de319bfa1 --- /dev/null +++ b/spec/enterprise/services/captain/tools/copilot/get_contact_service_spec.rb @@ -0,0 +1,110 @@ +require 'rails_helper' + +RSpec.describe Captain::Tools::Copilot::GetContactService do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:service) { described_class.new(assistant, user: user) } + + describe '#name' do + it 'returns the correct service name' do + expect(service.name).to eq('get_contact') + end + end + + describe '#description' do + it 'returns the service description' do + expect(service.description).to eq('Get details of a contact including their profile information') + end + end + + describe '#parameters' do + it 'returns the expected parameter schema' do + expect(service.parameters).to eq( + { + type: 'object', + properties: { + contact_id: { + type: 'number', + description: 'The ID of the contact to retrieve' + } + }, + required: %w[contact_id] + } + ) + end + end + + describe '#active?' do + context 'when user is an admin' do + let(:user) { create(:user, :administrator, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role with contact_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['contact_manage']) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role without contact_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: []) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns false' do + expect(service.active?).to be false + end + end + end + + describe '#execute' do + context 'when contact_id is blank' do + it 'returns error message' do + expect(service.execute({})).to eq('Missing required parameters') + end + end + + context 'when contact is not found' do + it 'returns not found message' do + expect(service.execute({ 'contact_id' => 999 })).to eq('Contact not found') + end + end + + context 'when contact exists' do + let(:contact) { create(:contact, account: account) } + + it 'returns the contact in llm text format' do + result = service.execute({ 'contact_id' => contact.id }) + expect(result).to eq(contact.to_llm_text) + end + + context 'when contact belongs to different account' do + let(:other_account) { create(:account) } + let(:other_contact) { create(:contact, account: other_account) } + + it 'returns not found message' do + expect(service.execute({ 'contact_id' => other_contact.id })).to eq('Contact not found') + end + end + end + end +end diff --git a/spec/enterprise/services/captain/tools/copilot/get_conversation_service_spec.rb b/spec/enterprise/services/captain/tools/copilot/get_conversation_service_spec.rb new file mode 100644 index 000000000..4d7f1adc7 --- /dev/null +++ b/spec/enterprise/services/captain/tools/copilot/get_conversation_service_spec.rb @@ -0,0 +1,142 @@ +require 'rails_helper' + +RSpec.describe Captain::Tools::Copilot::GetConversationService do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:service) { described_class.new(assistant, user: user) } + + describe '#name' do + it 'returns the correct service name' do + expect(service.name).to eq('get_conversation') + end + end + + describe '#description' do + it 'returns the service description' do + expect(service.description).to eq('Get details of a conversation including messages and contact information') + end + end + + describe '#parameters' do + it 'returns the expected parameter schema' do + expect(service.parameters).to eq( + { + type: 'object', + properties: { + conversation_id: { + type: 'number', + description: 'The ID of the conversation to retrieve' + } + }, + required: %w[conversation_id] + } + ) + end + end + + describe '#active?' do + context 'when user is an admin' do + let(:user) { create(:user, :administrator, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role with conversation_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_manage']) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role with conversation_unassigned_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_unassigned_manage']) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role with conversation_participating_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_participating_manage']) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role without any conversation permissions' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: []) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns false' do + expect(service.active?).to be false + end + end + end + + describe '#execute' do + context 'when conversation_id is blank' do + it 'returns error message' do + expect(service.execute({})).to eq('Missing required parameters') + end + end + + context 'when conversation is not found' do + it 'returns not found message' do + expect(service.execute({ 'conversation_id' => 999 })).to eq('Conversation not found') + end + end + + context 'when conversation exists' do + let(:inbox) { create(:inbox, account: account) } + let(:conversation) { create(:conversation, account: account, inbox: inbox) } + + it 'returns the conversation in llm text format' do + result = service.execute({ 'conversation_id' => conversation.display_id }) + expect(result).to eq(conversation.to_llm_text) + end + + context 'when conversation belongs to different account' do + let(:other_account) { create(:account) } + let(:other_inbox) { create(:inbox, account: other_account) } + let(:other_conversation) { create(:conversation, account: other_account, inbox: other_inbox) } + + it 'returns not found message' do + expect(service.execute({ 'conversation_id' => other_conversation.display_id })).to eq('Conversation not found') + end + end + end + end +end diff --git a/spec/enterprise/services/captain/tools/copilot/search_articles_service_spec.rb b/spec/enterprise/services/captain/tools/copilot/search_articles_service_spec.rb new file mode 100644 index 000000000..e4504d7bd --- /dev/null +++ b/spec/enterprise/services/captain/tools/copilot/search_articles_service_spec.rb @@ -0,0 +1,167 @@ +require 'rails_helper' + +RSpec.describe Captain::Tools::Copilot::SearchArticlesService do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:service) { described_class.new(assistant, user: user) } + + describe '#name' do + it 'returns the correct service name' do + expect(service.name).to eq('search_articles') + end + end + + describe '#description' do + it 'returns the service description' do + expect(service.description).to eq('Search articles based on parameters') + end + end + + describe '#parameters' do + it 'returns the expected parameter schema' do + expect(service.parameters).to eq( + { + type: 'object', + properties: { + query: { + type: 'string', + description: 'Search articles by title or content (partial match)' + }, + category_id: { + type: 'number', + description: 'Filter articles by category ID' + }, + status: { + type: 'string', + enum: %w[draft published archived], + description: 'Filter articles by status' + } + }, + required: ['query'] + } + ) + end + end + + describe '#active?' do + context 'when user is an admin' do + let(:user) { create(:user, :administrator, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user is an agent' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role with knowledge_base_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['knowledge_base_manage']) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has custom role without knowledge_base_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: []) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns false' do + expect(service.active?).to be false + end + end + end + + describe '#execute' do + context 'when query is blank' do + it 'returns error message' do + expect(service.execute({})).to eq('Missing required parameters') + end + end + + context 'when no articles are found' do + before do + allow(Article).to receive(:where).and_return(Article.none) + end + + it 'returns no articles found message' do + expect(service.execute({ 'query' => 'test' })).to eq('No articles found') + end + end + + context 'when articles are found' do + let(:portal) { create(:portal, account: account) } + let(:article1) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 1', content: 'Content 1') } + let(:article2) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 2', content: 'Content 2') } + + before do + article1 + article2 + end + + it 'returns formatted articles with count' do + result = service.execute({ 'query' => 'Test' }) + expect(result).to include('Total number of articles: 2') + expect(result).to include(article1.to_llm_text) + expect(result).to include(article2.to_llm_text) + end + + context 'when filtered by category' do + let(:category) { create(:category, slug: 'test-category', portal: portal, account: account) } + let(:article3) { create(:article, account: account, portal: portal, author: user, category: category, title: 'Test Article 3') } + + before do + article3 + end + + it 'returns only articles from the specified category' do + result = service.execute({ 'query' => 'Test', 'category_id' => category.id }) + expect(result).to include('Total number of articles: 1') + expect(result).to include(article3.to_llm_text) + expect(result).not_to include(article1.to_llm_text) + expect(result).not_to include(article2.to_llm_text) + end + end + + context 'when filtered by status' do + let(:article3) do + create(:article, account: account, portal: portal, author: user, title: 'Test Article 3', status: 'published') + end + let(:article4) { create(:article, account: account, portal: portal, author: user, title: 'Test Article 4', status: 'draft') } + + before do + article3 + article4 + end + + it 'returns only articles with the specified status' do + result = service.execute({ 'query' => 'Test', 'status' => 'published' }) + expect(result).to include(article3.to_llm_text) + expect(result).not_to include(article4.to_llm_text) + end + end + end + end +end diff --git a/spec/enterprise/services/captain/tools/copilot/search_contacts_service_spec.rb b/spec/enterprise/services/captain/tools/copilot/search_contacts_service_spec.rb new file mode 100644 index 000000000..f54b2eddf --- /dev/null +++ b/spec/enterprise/services/captain/tools/copilot/search_contacts_service_spec.rb @@ -0,0 +1,113 @@ +require 'rails_helper' + +RSpec.describe Captain::Tools::Copilot::SearchContactsService do + let(:account) { create(:account) } + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:service) { described_class.new(assistant, user: user) } + + describe '#name' do + it 'returns the correct service name' do + expect(service.name).to eq('search_contacts') + end + end + + describe '#description' do + it 'returns the service description' do + expect(service.description).to eq('Search contacts based on query parameters') + end + end + + describe '#parameters' do + it 'returns the expected parameter schema' do + expect(service.parameters).to eq( + { + type: 'object', + properties: { + email: { + type: 'string', + description: 'Filter contacts by email' + }, + phone_number: { + type: 'string', + description: 'Filter contacts by phone number' + }, + name: { + type: 'string', + description: 'Filter contacts by name (partial match)' + } + }, + required: [] + } + ) + end + end + + describe '#active?' do + context 'when user has contact_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: ['contact_manage']) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user does not have contact_manage permission' do + let(:user) { create(:user, account: account) } + let(:assistant) { create(:captain_assistant, account: account) } + let(:custom_role) { create(:custom_role, account: account, permissions: []) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns false' do + expect(service.active?).to be false + end + end + end + + describe '#execute' do + context 'when contacts are found' do + let(:contact1) { create(:contact, account: account, email: 'test1@example.com', name: 'Test Contact 1', phone_number: '+1234567890') } + let(:contact2) { create(:contact, account: account, email: 'test2@example.com', name: 'Test Contact 2', phone_number: '+1234567891') } + + before do + contact1 + contact2 + end + + it 'returns contacts when filtered by email' do + result = service.execute({ 'email' => 'test1@example.com' }) + expect(result).to include(contact1.to_llm_text) + expect(result).not_to include(contact2.to_llm_text) + end + + it 'returns contacts when filtered by phone number' do + result = service.execute({ 'phone_number' => '+1234567890' }) + expect(result).to include(contact1.to_llm_text) + expect(result).not_to include(contact2.to_llm_text) + end + + it 'returns contacts when filtered by name' do + result = service.execute({ 'name' => 'Contact 1' }) + expect(result).to include(contact1.to_llm_text) + expect(result).not_to include(contact2.to_llm_text) + end + + it 'returns all matching contacts when no filters are provided' do + result = service.execute({}) + expect(result).to include(contact1.to_llm_text) + expect(result).to include(contact2.to_llm_text) + end + end + end +end diff --git a/spec/enterprise/services/captain/tools/copilot/search_conversations_service_spec.rb b/spec/enterprise/services/captain/tools/copilot/search_conversations_service_spec.rb index e60ad6f28..ab0865bc2 100644 --- a/spec/enterprise/services/captain/tools/copilot/search_conversations_service_spec.rb +++ b/spec/enterprise/services/captain/tools/copilot/search_conversations_service_spec.rb @@ -26,6 +26,64 @@ RSpec.describe Captain::Tools::Copilot::SearchConversationsService do end end + describe '#active?' do + context 'when user has conversation_manage permission' do + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_manage']) } + let(:user) { create(:user, account: account) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has conversation_unassigned_manage permission' do + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_unassigned_manage']) } + let(:user) { create(:user, account: account) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has conversation_participating_manage permission' do + let(:custom_role) { create(:custom_role, account: account, permissions: ['conversation_participating_manage']) } + let(:user) { create(:user, account: account) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns true' do + expect(service.active?).to be true + end + end + + context 'when user has no relevant conversation permissions' do + let(:custom_role) { create(:custom_role, account: account, permissions: []) } + let(:user) { create(:user, account: account) } + + before do + account_user = AccountUser.find_by(user: user, account: account) + account_user.update(role: :agent, custom_role: custom_role) + end + + it 'returns false' do + expect(service.active?).to be false + end + end + end + describe '#execute' do let(:contact) { create(:contact, account: account) } let!(:open_conversation) { create(:conversation, account: account, contact: contact, status: 'open', priority: 'high') } From 743b73d0af4f43077e64c4c02392df3712248490 Mon Sep 17 00:00:00 2001 From: Pranav Date: Wed, 21 May 2025 18:14:51 -0700 Subject: [PATCH 4/5] chore: Update Copilot UI in favor of the new design (#11544) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removes avatar from the components. **Before:** Screenshot 2025-05-21 at 3 58 10 PM **After:** Screenshot 2025-05-21 at 3 57 29 PM --- .../copilot/CopilotAgentMessage.vue | 22 ++-------- .../copilot/CopilotAssistantMessage.vue | 43 ++++++++----------- 2 files changed, 21 insertions(+), 44 deletions(-) diff --git a/app/javascript/dashboard/components-next/copilot/CopilotAgentMessage.vue b/app/javascript/dashboard/components-next/copilot/CopilotAgentMessage.vue index ef8c2faf4..f1ad2afc1 100644 --- a/app/javascript/dashboard/components-next/copilot/CopilotAgentMessage.vue +++ b/app/javascript/dashboard/components-next/copilot/CopilotAgentMessage.vue @@ -1,31 +1,17 @@ diff --git a/app/javascript/dashboard/components-next/copilot/CopilotAssistantMessage.vue b/app/javascript/dashboard/components-next/copilot/CopilotAssistantMessage.vue index 1877e1630..0a37600bf 100644 --- a/app/javascript/dashboard/components-next/copilot/CopilotAssistantMessage.vue +++ b/app/javascript/dashboard/components-next/copilot/CopilotAssistantMessage.vue @@ -9,7 +9,6 @@ import { COPILOT_EVENTS } from 'dashboard/helper/AnalyticsHelper/events'; import MessageFormatter from 'shared/helpers/MessageFormatter.js'; import Button from 'dashboard/components-next/button/Button.vue'; -import Avatar from '../avatar/Avatar.vue'; const props = defineProps({ message: { @@ -46,33 +45,25 @@ const useCopilotResponse = () => { From abe22c8649c103720a0630edfc4a140b82b1119b Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Wed, 21 May 2025 20:00:11 -0700 Subject: [PATCH 5/5] fix: Rack-attack disable double Redis pooling (#11545) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rails 7.1 ships with connection-pooling enabled by default for `RedisCacheStore` (see rails/rails#45235). Because we already wrap our Redis clients in our own `ConnectionPool` ($alfred / $velma), the upgrade resulted in a double-wrapped object and runtime errors such as: NoMethodError: undefined method `get` for an instance of ConnectionPool This patch: * Passes `pool: false` when instantiating `RedisCacheStore` in `config/initializers/rack_attack.rb`, telling Rails to use the pool we supply instead of building its own. * Adds an inline comment explaining the rationale. * Adds a TODO in `config/initializers/01_redis.rb` suggesting a future simplification: switch to plain Redis clients and let Rails manage the pool. Reference docs: * rails/rails#45235 – “Enable connection pooling by default for MemCacheStore and RedisCacheStore” - https://github.com/rails/rails/pull/45235 * Rails 7.1 Caching Guide – 2.1.1 “Connection Pool Options” (use `pool: false`) [Ruby on Rails Guides](https://guides.rubyonrails.org/v7.1/caching_with_rails.html) --- config/initializers/01_redis.rb | 4 ++++ config/initializers/rack_attack.rb | 7 ++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/config/initializers/01_redis.rb b/config/initializers/01_redis.rb index 664dcd435..93c12fce7 100644 --- a/config/initializers/01_redis.rb +++ b/config/initializers/01_redis.rb @@ -1,3 +1,7 @@ +# TODO: Phase out the custom ConnectionPool wrappers ($alfred / $velma), +# switch to plain Redis clients here and let Rails 7.1+ handle pooling +# via `pool:` in RedisCacheStore (see rack_attack initializer). + # Alfred # Add here as you use it for more features # Used for Round Robin, Conversation Emails & Online Presence diff --git a/config/initializers/rack_attack.rb b/config/initializers/rack_attack.rb index fdbd47008..fe3f6c554 100644 --- a/config/initializers/rack_attack.rb +++ b/config/initializers/rack_attack.rb @@ -11,7 +11,12 @@ class Rack::Attack # Rack::Attack.cache.store = ActiveSupport::Cache::MemoryStore.new # https://github.com/rack/rack-attack/issues/102 - Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(redis: $velma) + # Rails 7.1 automatically adds its own ConnectionPool around RedisCacheStore. + # Because `$velma` is *already* a ConnectionPool, double-wrapping causes + # Redis calls like `get` to hit the outer wrapper and explode. + # `pool: false` tells Rails to skip its internal pool and use ours directly. + # TODO: We can use build in connection pool in future upgrade + Rack::Attack.cache.store = ActiveSupport::Cache::RedisCacheStore.new(redis: $velma, pool: false) class Request < ::Rack::Request # You many need to specify a method to fetch the correct remote IP address