From 667cd1ba1f84017cbf2452cf43b4f6ab7511fe39 Mon Sep 17 00:00:00 2001 From: Linas Baublys Date: Fri, 24 Apr 2026 14:11:30 +0300 Subject: [PATCH 01/32] fix: widget et translation (#14119) Added missing et translations for widget --------- Co-authored-by: Sojan Jose --- app/javascript/widget/i18n/locale/et.json | 168 +++++++++++----------- 1 file changed, 84 insertions(+), 84 deletions(-) diff --git a/app/javascript/widget/i18n/locale/et.json b/app/javascript/widget/i18n/locale/et.json index c3d6ddfc1..e4a1f135f 100644 --- a/app/javascript/widget/i18n/locale/et.json +++ b/app/javascript/widget/i18n/locale/et.json @@ -1,153 +1,153 @@ { "COMPONENTS": { "FILE_BUBBLE": { - "DOWNLOAD": "Download", - "UPLOADING": "Uploading..." + "DOWNLOAD": "Laadi alla", + "UPLOADING": "Üleslaadimine..." }, "FORM_BUBBLE": { - "SUBMIT": "Submit" + "SUBMIT": "Saada" }, "MESSAGE_BUBBLE": { - "RETRY": "Send message again", - "ERROR_MESSAGE": "Couldn't send, try again" + "RETRY": "Saada sõnum uuesti", + "ERROR_MESSAGE": "Saatmine ebaõnnestus, proovi uuesti" } }, "THUMBNAIL": { "AUTHOR": { - "NOT_AVAILABLE": "Not available" + "NOT_AVAILABLE": "Pole saadaval" } }, "TEAM_AVAILABILITY": { - "ONLINE": "We are online", - "OFFLINE": "We are away at the moment", - "BACK_AS_SOON_AS_POSSIBLE": "We will be back as soon as possible" + "ONLINE": "Oleme võrgus", + "OFFLINE": "Oleme hetkel eemal", + "BACK_AS_SOON_AS_POSSIBLE": "Oleme tagasi esimesel võimalusel" }, "REPLY_TIME": { - "IN_A_FEW_MINUTES": "Typically replies in a few minutes", - "IN_A_FEW_HOURS": "Typically replies in a few hours", - "IN_A_DAY": "Typically replies in a day", - "BACK_IN_HOURS": "We will be back online in {n} hour | We will be back online in {n} hours", - "BACK_IN_MINUTES": "We will be back online in {time} minutes", - "BACK_AT_TIME": "We will be back online at {time}", - "BACK_ON_DAY": "We will be back online on {day}", - "BACK_TOMORROW": "We will be back online tomorrow", - "BACK_IN_SOME_TIME": "We will be back online in some time" + "IN_A_FEW_MINUTES": "Tavaliselt vastame mõne minuti jooksul", + "IN_A_FEW_HOURS": "Tavaliselt vastame mõne tunni jooksul", + "IN_A_DAY": "Tavaliselt vastame päeva jooksul", + "BACK_IN_HOURS": "Oleme tagasi {n} tunni pärast | Oleme tagasi {n} tunni pärast", + "BACK_IN_MINUTES": "Oleme tagasi {time} minuti pärast", + "BACK_AT_TIME": "Oleme tagasi kell {time}", + "BACK_ON_DAY": "Oleme tagasi {day}", + "BACK_TOMORROW": "Oleme tagasi homme", + "BACK_IN_SOME_TIME": "Oleme mõne aja pärast tagasi" }, "DAY_NAMES": { - "SUNDAY": "Sunday", - "MONDAY": "Monday", - "TUESDAY": "Tuesday", - "WEDNESDAY": "Wednesday", - "THURSDAY": "Thursday", - "FRIDAY": "Friday", - "SATURDAY": "Saturday" + "SUNDAY": "Pühapäev", + "MONDAY": "Esmaspäev", + "TUESDAY": "Teisipäev", + "WEDNESDAY": "Kolmapäev", + "THURSDAY": "Neljapäev", + "FRIDAY": "Reede", + "SATURDAY": "Laupäev" }, - "START_CONVERSATION": "Start Conversation", - "END_CONVERSATION": "End Conversation", - "CONTINUE_CONVERSATION": "Continue conversation", - "YOU": "You", - "START_NEW_CONVERSATION": "Start a new conversation", - "VIEW_UNREAD_MESSAGES": "You have unread messages", + "START_CONVERSATION": "Alusta vestlust", + "END_CONVERSATION": "Lõpeta vestlus", + "CONTINUE_CONVERSATION": "Jätka vestlust", + "YOU": "Sina", + "START_NEW_CONVERSATION": "Alusta uut vestlust", + "VIEW_UNREAD_MESSAGES": "Sul on lugemata sõnumeid", "UNREAD_VIEW": { - "VIEW_MESSAGES_BUTTON": "See new messages", - "CLOSE_MESSAGES_BUTTON": "Close", - "COMPANY_FROM": "from", + "VIEW_MESSAGES_BUTTON": "Vaata uusi sõnumeid", + "CLOSE_MESSAGES_BUTTON": "Sulge", + "COMPANY_FROM": "saatjalt", "BOT": "Bot" }, "BUBBLE": { - "LABEL": "Chat with us" + "LABEL": "Vestle meiega" }, - "POWERED_BY": "Powered by Chatwoot", - "EMAIL_PLACEHOLDER": "Please enter your email", - "CHAT_PLACEHOLDER": "Type your message", - "TODAY": "Today", - "YESTERDAY": "Yesterday", + "POWERED_BY": "Toetab Chatwoot", + "EMAIL_PLACEHOLDER": "Palun sisesta oma e-post", + "CHAT_PLACEHOLDER": "Kirjuta oma sõnum", + "TODAY": "Täna", + "YESTERDAY": "Eile", "PRE_CHAT_FORM": { "FIELDS": { "FULL_NAME": { - "LABEL": "Full Name", - "PLACEHOLDER": "Please enter your full name", - "REQUIRED_ERROR": "Full Name is required" + "LABEL": "Täisnimi", + "PLACEHOLDER": "Palun sisesta oma täisnimi", + "REQUIRED_ERROR": "Täisnimi on kohustuslik" }, "EMAIL_ADDRESS": { - "LABEL": "Email Address", - "PLACEHOLDER": "Please enter your email address", - "REQUIRED_ERROR": "Email Address is required", - "VALID_ERROR": "Please enter a valid email address" + "LABEL": "E-posti aadress", + "PLACEHOLDER": "Palun sisesta oma e-posti aadress", + "REQUIRED_ERROR": "E-posti aadress on kohustuslik", + "VALID_ERROR": "Palun sisesta kehtiv e-posti aadress" }, "PHONE_NUMBER": { - "LABEL": "Phone Number", - "PLACEHOLDER": "Please enter your phone number", - "REQUIRED_ERROR": "Phone Number is required", - "DIAL_CODE_VALID_ERROR": "Please select a country code", - "VALID_ERROR": "Please enter a valid phone number", - "DROPDOWN_EMPTY": "No results found", - "DROPDOWN_SEARCH": "Search country" + "LABEL": "Telefoninumber", + "PLACEHOLDER": "Palun sisesta oma telefoninumber", + "REQUIRED_ERROR": "Telefoninumber on kohustuslik", + "DIAL_CODE_VALID_ERROR": "Palun vali riigikood", + "VALID_ERROR": "Palun sisesta kehtiv telefoninumber", + "DROPDOWN_EMPTY": "Tulemusi ei leitud", + "DROPDOWN_SEARCH": "Otsi riiki" }, "MESSAGE": { - "LABEL": "Message", - "PLACEHOLDER": "Please enter your message", - "ERROR": "Message too short" + "LABEL": "Sõnum", + "PLACEHOLDER": "Palun sisesta oma sõnum", + "ERROR": "Sõnum on liiga lühike" } }, - "CAMPAIGN_HEADER": "Please provide your name and email before starting the conversation", - "IS_REQUIRED": "is required", - "REQUIRED": "Required", - "REGEX_ERROR": "Please provide a valid input" + "CAMPAIGN_HEADER": "Palun sisesta enne vestluse alustamist oma nimi ja e-post", + "IS_REQUIRED": "on kohustuslik", + "REQUIRED": "Kohustuslik", + "REGEX_ERROR": "Palun sisesta korrektne väärtus" }, - "FILE_SIZE_LIMIT": "File exceeds the {MAXIMUM_FILE_UPLOAD_SIZE} attachment limit", + "FILE_SIZE_LIMIT": "Fail ületab {MAXIMUM_FILE_UPLOAD_SIZE} manuse limiidi", "CHAT_FORM": { "INVALID": { - "FIELD": "Invalid field" + "FIELD": "Vigane väli" } }, "EMOJI": { - "PLACEHOLDER": "Search emojis", - "NOT_FOUND": "No emoji match your search", - "ARIA_LABEL": "Emoji picker" + "PLACEHOLDER": "Otsi emotikone", + "NOT_FOUND": "Ühtegi emotikoni ei leitud", + "ARIA_LABEL": "Emotikonide valija" }, "CSAT": { - "TITLE": "Rate your conversation", - "SUBMITTED_TITLE": "Thank you for submitting the rating", - "PLACEHOLDER": "Tell us more..." + "TITLE": "Hinda oma vestlust", + "SUBMITTED_TITLE": "Täname hinnangu eest", + "PLACEHOLDER": "Räägi meile rohkem..." }, "EMAIL_TRANSCRIPT": { - "BUTTON_TEXT": "Request a conversation transcript", - "SEND_EMAIL_SUCCESS": "The chat transcript was sent successfully", - "SEND_EMAIL_ERROR": "There was an error, please try again" + "BUTTON_TEXT": "Taotle vestluse koopiat", + "SEND_EMAIL_SUCCESS": "Vestluse koopia saadeti edukalt", + "SEND_EMAIL_ERROR": "Tekkis viga, palun proovi uuesti" }, "INTEGRATIONS": { "DYTE": { - "CLICK_HERE_TO_JOIN": "Click here to join", - "LEAVE_THE_ROOM": "Leave the call" + "CLICK_HERE_TO_JOIN": "Klõpsa siia liitumiseks", + "LEAVE_THE_ROOM": "Lahku kõnest" } }, "PORTAL": { - "POPULAR_ARTICLES": "Popular Articles", - "VIEW_ALL_ARTICLES": "View all articles", - "IFRAME_LOAD_ERROR": "There was an error loading the article, please refresh the page and try again." + "POPULAR_ARTICLES": "Populaarsed artiklid", + "VIEW_ALL_ARTICLES": "Vaata kõiki artikleid", + "IFRAME_LOAD_ERROR": "Artikli laadimisel tekkis viga, palun värskenda lehte ja proovi uuesti." }, "ATTACHMENTS": { "image": { - "CONTENT": "Picture message" + "CONTENT": "Pildisõnum" }, "audio": { - "CONTENT": "Audio message" + "CONTENT": "Helisõnum" }, "video": { - "CONTENT": "Video message" + "CONTENT": "Videosõnum" }, "file": { - "CONTENT": "File Attachment" + "CONTENT": "Faili manus" }, "location": { - "CONTENT": "Location" + "CONTENT": "Asukoht" }, "fallback": { - "CONTENT": "has shared a url" + "CONTENT": "jagas URL-i" } }, "FOOTER_REPLY_TO": { - "REPLY_TO": "Replying to:" + "REPLY_TO": "Vastus sõnumile:" } } From 9a89e1f5227bd9cce9dfbcb69b6a60321c79bb03 Mon Sep 17 00:00:00 2001 From: Kenta Ishizaki <153918146+55728@users.noreply.github.com> Date: Fri, 24 Apr 2026 20:33:03 +0900 Subject: [PATCH 02/32] fix: Strip UTF-8 BOM in DataImportJob#csv_reader before parsing CSV (#14126) ## Description `DataImportJob#csv_reader` reads CSV data with `force_encoding('UTF-8')` but does not strip the UTF-8 Byte Order Mark (`EF BB BF`). If a CSV file containing a BOM is imported, the first header key is prefixed with `\uFEFF`, which causes key mismatches in `DataImport::ContactManager` when the first column is one of the recognized keys (`:email`, `:identifier`, `:phone_number`, `:name`). This was identified during review of #14123 (see #14124 for the tracking issue). Fixes #14124 ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Added a new fixture (`spec/fixtures/data_import/with_bom.csv`) containing a UTF-8 BOM followed by valid contact data. - Added a new spec (`will strip UTF-8 BOM and import contacts correctly`) that imports the BOM fixture and verifies that `name`, `email`, and `phone_number` are all correctly parsed. - All existing examples in `spec/jobs/data_import_job_spec.rb` continue to pass. ## 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 --- app/jobs/data_import_job.rb | 1 + spec/fixtures/data_import/with_bom.csv | 2 ++ spec/jobs/data_import_job_spec.rb | 14 ++++++++++++++ 3 files changed, 17 insertions(+) create mode 100644 spec/fixtures/data_import/with_bom.csv diff --git a/app/jobs/data_import_job.rb b/app/jobs/data_import_job.rb index d8bbeb992..4149ee16f 100644 --- a/app/jobs/data_import_job.rb +++ b/app/jobs/data_import_job.rb @@ -106,6 +106,7 @@ class DataImportJob < ApplicationJob raw_data = file.read utf8_data = raw_data.force_encoding('UTF-8') clean_data = utf8_data.valid_encoding? ? utf8_data : utf8_data.encode('UTF-16le', invalid: :replace, replace: '').encode('UTF-8') + clean_data = clean_data.delete_prefix("\xEF\xBB\xBF") CSV.new(StringIO.new(clean_data), headers: true) end diff --git a/spec/fixtures/data_import/with_bom.csv b/spec/fixtures/data_import/with_bom.csv new file mode 100644 index 000000000..8b1850620 --- /dev/null +++ b/spec/fixtures/data_import/with_bom.csv @@ -0,0 +1,2 @@ +name,email,phone_number +Ahmed,ahmed@example.com,+971501234567 diff --git a/spec/jobs/data_import_job_spec.rb b/spec/jobs/data_import_job_spec.rb index 19178b177..88b268ef6 100644 --- a/spec/jobs/data_import_job_spec.rb +++ b/spec/jobs/data_import_job_spec.rb @@ -91,6 +91,20 @@ RSpec.describe DataImportJob do expect(invalid_data_import.account.contacts.first.name).to eq(csv_data[0]['name'].encode('UTF-8', 'binary', invalid: :replace, undef: :replace, replace: '')) end + + it 'will strip UTF-8 BOM and import contacts correctly' do + bom_data_import = create(:data_import, + import_file: Rack::Test::UploadedFile.new(Rails.root.join('spec/fixtures/data_import/with_bom.csv'), + 'text/csv')) + + described_class.perform_now(bom_data_import) + expect(bom_data_import.account.contacts.count).to eq(1) + + contact = bom_data_import.account.contacts.first + expect(contact.name).to eq('Ahmed') + expect(contact.email).to eq('ahmed@example.com') + expect(contact.phone_number).to eq('+971501234567') + end end context 'when the data contains existing records' do From c5fb8d73cc1a4305a527ac3b77641cd99d65486f Mon Sep 17 00:00:00 2001 From: Kenta Ishizaki <153918146+55728@users.noreply.github.com> Date: Fri, 24 Apr 2026 21:36:25 +0900 Subject: [PATCH 03/32] fix: Prepend UTF-8 BOM to contact CSV export for non-ASCII character support (#14123) ## Description Spreadsheet applications such as Microsoft Excel do not auto-detect UTF-8 encoding when opening CSV files. This causes non-ASCII characters (Arabic, Japanese, Chinese, Korean, etc.) to appear garbled in the exported contacts CSV. This PR prepends the UTF-8 Byte Order Mark (`EF BB BF`) to the CSV output in `Account::ContactsExportJob`, which signals to spreadsheet applications that the file is UTF-8 encoded. Fixes: #13998 --- app/jobs/account/contacts_export_job.rb | 7 ++++++- spec/jobs/account/contacts_export_job_spec.rb | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/jobs/account/contacts_export_job.rb b/app/jobs/account/contacts_export_job.rb index 952795604..a66928a79 100644 --- a/app/jobs/account/contacts_export_job.rb +++ b/app/jobs/account/contacts_export_job.rb @@ -42,8 +42,13 @@ class Account::ContactsExportJob < ApplicationJob def attach_export_file(csv_data) return if csv_data.blank? + # Prepend UTF-8 BOM so that spreadsheet applications (e.g. Excel) + # correctly recognise the file encoding for non-ASCII characters + # such as Arabic, Japanese, and Chinese. + bom = "\xEF\xBB\xBF" + @account.contacts_export.attach( - io: StringIO.new(csv_data), + io: StringIO.new("#{bom}#{csv_data}"), filename: "#{@account.name}_#{@account.id}_contacts.csv", content_type: 'text/csv' ) diff --git a/spec/jobs/account/contacts_export_job_spec.rb b/spec/jobs/account/contacts_export_job_spec.rb index 9b9d74675..561015ea7 100644 --- a/spec/jobs/account/contacts_export_job_spec.rb +++ b/spec/jobs/account/contacts_export_job_spec.rb @@ -85,6 +85,13 @@ RSpec.describe Account::ContactsExportJob do expect(phone_numbers).to include('+910808080818', '+910808080808') end + it 'prepends UTF-8 BOM to the exported CSV for spreadsheet compatibility' do + described_class.perform_now(account.id, user.id, [], {}) + + raw = account.contacts_export.download + expect(raw.bytes[0..2]).to eq([0xEF, 0xBB, 0xBF]) + end + it 'returns all resolved contacts as results when filter is not prvoided' do create(:contact, account: account, email: nil, phone_number: nil) described_class.perform_now(account.id, user.id, %w[id name email column_not_present], {}) From 661608c0b1a393c93cb1225dfb32233461e609fc Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Fri, 24 Apr 2026 18:59:45 +0530 Subject: [PATCH 04/32] fix: [CW-6931] Harden external downloads against SSRF [avatar from url job] (#14153) This routes external downloads used by avatar sync through SafeFetch. It closes the SSRF exposure from raw Down.download paths, preserves provider-specific auth and header flows, and adds regression coverage for blocked internal URLs plus authenticated downloads. Fixes # (issue): [CW-6931](https://linear.app/chatwoot/issue/CW-6931/avatarwidget-url-ssrf-downdownload-unprotected-unauth) --- app/jobs/avatar/avatar_from_url_job.rb | 59 +++++-- app/models/concerns/avatarable.rb | 5 +- lib/safe_fetch.rb | 56 +++++-- spec/jobs/avatar/avatar_from_url_job_spec.rb | 154 +++++++++++++++---- spec/lib/safe_fetch_spec.rb | 58 ++++++- spec/models/contact_spec.rb | 7 + 6 files changed, 274 insertions(+), 65 deletions(-) diff --git a/app/jobs/avatar/avatar_from_url_job.rb b/app/jobs/avatar/avatar_from_url_job.rb index 929e76597..49bf25803 100644 --- a/app/jobs/avatar/avatar_from_url_job.rb +++ b/app/jobs/avatar/avatar_from_url_job.rb @@ -9,27 +9,17 @@ class Avatar::AvatarFromUrlJob < ApplicationJob include UrlHelper queue_as :purgable - MAX_DOWNLOAD_SIZE = 15 * 1024 * 1024 + ALLOWED_CONTENT_TYPES = Avatarable::ALLOWED_AVATAR_CONTENT_TYPES + MAX_DOWNLOAD_SIZE = 15.megabytes RATE_LIMIT_WINDOW = 1.minute def perform(avatarable, avatar_url) - return unless avatarable.respond_to?(:avatar) - return unless url_valid?(avatar_url) + return unless syncable_avatar?(avatarable, avatar_url) - return unless should_sync_avatar?(avatarable, avatar_url) - - avatar_file = Down.download(avatar_url, max_size: MAX_DOWNLOAD_SIZE) - raise Down::Error, 'Invalid file' unless valid_file?(avatar_file) - - avatarable.avatar.attach( - io: avatar_file, - filename: avatar_file.original_filename, - content_type: avatar_file.content_type - ) - - rescue Down::NotFound - Rails.logger.info "AvatarFromUrlJob: avatar not found at #{avatar_url}" - rescue Down::Error => e + fetch_and_attach_avatar(avatarable, avatar_url) + rescue SafeFetch::HttpError => e + log_http_error(avatar_url, e) + rescue SafeFetch::Error => e Rails.logger.error "AvatarFromUrlJob error for #{avatar_url}: #{e.class} - #{e.message}" ensure update_avatar_sync_attributes(avatarable, avatar_url) @@ -37,6 +27,41 @@ class Avatar::AvatarFromUrlJob < ApplicationJob private + def syncable_avatar?(avatarable, avatar_url) + avatarable.respond_to?(:avatar) && + url_valid?(avatar_url) && + should_sync_avatar?(avatarable, avatar_url) + end + + def fetch_and_attach_avatar(avatarable, avatar_url) + SafeFetch.fetch( + avatar_url, + max_bytes: MAX_DOWNLOAD_SIZE, + allowed_content_type_prefixes: [], + allowed_content_types: ALLOWED_CONTENT_TYPES + ) do |avatar_file| + attach_avatar(avatarable, avatar_file) + end + end + + def attach_avatar(avatarable, avatar_file) + raise SafeFetch::FetchError, 'Invalid file' unless valid_file?(avatar_file) + + avatarable.avatar.attach( + io: avatar_file.tempfile, + filename: avatar_file.original_filename, + content_type: avatar_file.content_type + ) + end + + def log_http_error(avatar_url, error) + if error.message.start_with?('404') + Rails.logger.info "AvatarFromUrlJob: avatar not found at #{avatar_url}" + else + Rails.logger.error "AvatarFromUrlJob error for #{avatar_url}: #{error.class} - #{error.message}" + end + end + def should_sync_avatar?(avatarable, avatar_url) # Only Contacts are rate-limited and hash-gated. return true unless avatarable.is_a?(Contact) diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb index 94ca55037..3057f8d44 100644 --- a/app/models/concerns/avatarable.rb +++ b/app/models/concerns/avatarable.rb @@ -4,6 +4,8 @@ module Avatarable extend ActiveSupport::Concern include Rails.application.routes.url_helpers + ALLOWED_AVATAR_CONTENT_TYPES = %w[image/jpeg image/png image/gif image/webp].freeze + included do has_one_attached :avatar validate :acceptable_avatar, if: -> { avatar.changed? } @@ -30,7 +32,6 @@ module Avatarable errors.add(:avatar, 'is too big') if avatar.byte_size > 15.megabytes - acceptable_types = ['image/jpeg', 'image/png', 'image/gif'].freeze - errors.add(:avatar, 'filetype not supported') unless acceptable_types.include?(avatar.content_type) + errors.add(:avatar, 'filetype not supported') unless ALLOWED_AVATAR_CONTENT_TYPES.include?(avatar.content_type) end end diff --git a/lib/safe_fetch.rb b/lib/safe_fetch.rb index e6635c9c3..2264b2850 100644 --- a/lib/safe_fetch.rb +++ b/lib/safe_fetch.rb @@ -6,7 +6,11 @@ module SafeFetch DEFAULT_READ_TIMEOUT = 20 DEFAULT_MAX_BYTES_FALLBACK_MB = 40 - Result = Data.define(:tempfile, :filename, :content_type) + Result = Data.define(:tempfile, :filename, :content_type) do + def original_filename + filename + end + end class Error < StandardError; end class InvalidUrlError < Error; end @@ -18,19 +22,15 @@ module SafeFetch def self.fetch(url, max_bytes: nil, - allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES) + allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES, + allowed_content_types: []) raise ArgumentError, 'block required' unless block_given? effective_max_bytes = max_bytes || default_max_bytes - uri = parse_and_validate_url!(url) - filename = filename_for(uri) + filename = filename_for(parse_and_validate_url!(url)) tempfile = Tempfile.new('chatwoot-safe-fetch', binmode: true) - - response = stream_to_tempfile(url, tempfile, effective_max_bytes, allowed_content_type_prefixes) - raise HttpError, "#{response.code} #{response.message}" unless response.is_a?(Net::HTTPSuccess) - - tempfile.rewind - yield Result.new(tempfile: tempfile, filename: filename, content_type: response['content-type']) + response = fetch_response(url, tempfile, effective_max_bytes, allowed_content_type_prefixes, allowed_content_types) + yield build_result(tempfile, filename, response) rescue SsrfFilter::InvalidUriScheme, URI::InvalidURIError => e raise InvalidUrlError, e.message rescue SsrfFilter::Error, Resolv::ResolvError => e @@ -44,18 +44,23 @@ module SafeFetch class << self private - def stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes) + def fetch_response(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types) + stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types) + end + + def stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types) response = nil bytes_written = 0 SsrfFilter.get( url, + request_proc: ->(request) { apply_url_basic_auth(request) }, http_options: { open_timeout: DEFAULT_OPEN_TIMEOUT, read_timeout: DEFAULT_READ_TIMEOUT } ) do |res| response = res next unless res.is_a?(Net::HTTPSuccess) - unless allowed_content_type?(res['content-type'], allowed_content_type_prefixes) + unless allowed_content_type?(res['content-type'], allowed_content_type_prefixes, allowed_content_types) raise UnsupportedContentTypeError, "content-type not allowed: #{res['content-type']}" end @@ -74,6 +79,14 @@ module SafeFetch File.basename(uri.path).presence || "download-#{Time.current.to_i}-#{SecureRandom.hex(4)}" end + def build_result(tempfile, filename, response) + raise HttpError, "#{response.code} #{response.message}" unless response.is_a?(Net::HTTPSuccess) + + tempfile.rewind + content_type = normalized_content_type(response['content-type']) + Result.new(tempfile: tempfile, filename: filename, content_type: content_type) + end + def default_max_bytes limit_mb = GlobalConfigService.load('MAXIMUM_FILE_UPLOAD_SIZE', DEFAULT_MAX_BYTES_FALLBACK_MB).to_i limit_mb = DEFAULT_MAX_BYTES_FALLBACK_MB if limit_mb <= 0 @@ -88,11 +101,24 @@ module SafeFetch uri end - def allowed_content_type?(value, prefixes) - mime = value.to_s.split(';').first&.strip&.downcase + def allowed_content_type?(value, prefixes, content_types) + mime = normalized_content_type(value) return false if mime.blank? - prefixes.any? { |prefix| mime.start_with?(prefix) } + prefixes.any? { |prefix| mime.start_with?(prefix) } || content_types.include?(mime) + end + + def normalized_content_type(value) + value.to_s.split(';').first&.strip&.downcase + end + + def apply_url_basic_auth(request) + uri = request.uri + return if uri.user.blank? + + username = URI.decode_uri_component(uri.user) + password = URI.decode_uri_component(uri.password.to_s) + request.basic_auth(username, password) end end end diff --git a/spec/jobs/avatar/avatar_from_url_job_spec.rb b/spec/jobs/avatar/avatar_from_url_job_spec.rb index 8db3769ad..e85c46390 100644 --- a/spec/jobs/avatar/avatar_from_url_job_spec.rb +++ b/spec/jobs/avatar/avatar_from_url_job_spec.rb @@ -1,9 +1,13 @@ require 'rails_helper' RSpec.describe Avatar::AvatarFromUrlJob do - let(:file) { fixture_file_upload(Rails.root.join('spec/assets/avatar.png'), 'image/png') } let(:valid_url) { 'https://example.com/avatar.png' } + before do + allow(Resolv).to receive(:getaddresses).and_call_original + allow(Resolv).to receive(:getaddresses).with('example.com').and_return(['93.184.216.34']) + end + it 'enqueues the job' do contact = create(:contact) expect { described_class.perform_later(contact, 'https://example.com/avatar.png') } @@ -14,7 +18,13 @@ RSpec.describe Avatar::AvatarFromUrlJob do let(:avatarable) { create(:contact) } it 'attaches and updates sync attributes' do - expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE).and_return(file) + stub_request(:get, valid_url) + .to_return( + status: 200, + body: File.read(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + described_class.perform_now(avatarable, valid_url) avatarable.reload expect(avatarable.avatar).to be_attached @@ -22,10 +32,71 @@ RSpec.describe Avatar::AvatarFromUrlJob do expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present end + it 'attaches webp avatars and updates sync attributes' do + webp_url = 'https://example.com/avatar.webp' + + stub_request(:get, webp_url) + .to_return( + status: 200, + body: File.read(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/webp' } + ) + + described_class.perform_now(avatarable, webp_url) + avatarable.reload + + expect(avatarable.avatar).to be_attached + expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(webp_url)) + expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present + end + + it 'attaches avatars with parameterized content type headers' do + parameterized_url = 'https://example.com/avatar-parameterized.png' + + stub_request(:get, parameterized_url) + .to_return( + status: 200, + body: File.read(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'IMAGE/PNG; charset=binary' } + ) + + described_class.perform_now(avatarable, parameterized_url) + avatarable.reload + + expect(avatarable.avatar).to be_attached + expect(avatarable.avatar.blob.content_type).to eq('image/png') + expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(parameterized_url)) + end + + it 'attaches avatars from URLs with embedded basic auth credentials' do + authenticated_url = 'https://user:pass@example.com/avatar-authenticated.png' + + stub_request(:get, 'https://example.com/avatar-authenticated.png') + .with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNz' }) + .to_return( + status: 200, + body: File.read(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + + described_class.perform_now(avatarable, authenticated_url) + avatarable.reload + + expect(avatarable.avatar).to be_attached + expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(authenticated_url)) + end + it 'returns early when rate limited' do ts = 30.seconds.ago.iso8601 avatarable.update(additional_attributes: { 'last_avatar_sync_at' => ts }) - expect(Down).not_to receive(:download) + + stub_request(:get, valid_url) + .to_return( + status: 200, + body: File.read(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + described_class.perform_now(avatarable, valid_url) avatarable.reload expect(avatarable.avatar).not_to be_attached @@ -33,21 +104,29 @@ RSpec.describe Avatar::AvatarFromUrlJob do expect(Time.zone.parse(avatarable.additional_attributes['last_avatar_sync_at'])) .to be > Time.zone.parse(ts) expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(valid_url)) + expect(WebMock).not_to have_requested(:get, valid_url) end it 'returns early when hash unchanged' do avatarable.update(additional_attributes: { 'avatar_url_hash' => Digest::SHA256.hexdigest(valid_url) }) - expect(Down).not_to receive(:download) + + stub_request(:get, valid_url) + .to_return( + status: 200, + body: File.read(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + described_class.perform_now(avatarable, valid_url) expect(avatarable.avatar).not_to be_attached avatarable.reload expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(valid_url)) + expect(WebMock).not_to have_requested(:get, valid_url) end it 'updates sync attributes even when URL is invalid' do invalid_url = 'invalid_url' - expect(Down).not_to receive(:download) described_class.perform_now(avatarable, invalid_url) avatarable.reload expect(avatarable.avatar).not_to be_attached @@ -56,17 +135,12 @@ RSpec.describe Avatar::AvatarFromUrlJob do end it 'updates sync attributes when file download is valid but content type is unsupported' do - temp_file = Tempfile.new(['invalid', '.xml']) - temp_file.write('content') - temp_file.rewind - - uploaded = ActionDispatch::Http::UploadedFile.new( - tempfile: temp_file, - filename: 'invalid.xml', - type: 'application/xml' - ) - - expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE).and_return(uploaded) + stub_request(:get, valid_url) + .to_return( + status: 200, + body: 'content', + headers: { 'Content-Type' => 'application/xml' } + ) described_class.perform_now(avatarable, valid_url) avatarable.reload @@ -74,9 +148,19 @@ RSpec.describe Avatar::AvatarFromUrlJob do expect(avatarable.avatar).not_to be_attached expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(valid_url)) + end - temp_file.close - temp_file.unlink + it 'updates sync attributes when the avatar URL is blocked by SSRF protection' do + blocked_url = 'http://127.0.0.1/avatar.png' + + expect do + described_class.perform_now(avatarable, blocked_url) + end.not_to raise_error + + avatarable.reload + expect(avatarable.avatar).not_to be_attached + expect(avatarable.additional_attributes['last_avatar_sync_at']).to be_present + expect(avatarable.additional_attributes['avatar_url_hash']).to eq(Digest::SHA256.hexdigest(blocked_url)) end end @@ -84,7 +168,13 @@ RSpec.describe Avatar::AvatarFromUrlJob do let(:avatarable) { create(:agent_bot) } it 'downloads and attaches avatar' do - expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE).and_return(file) + stub_request(:get, valid_url) + .to_return( + status: 200, + body: File.read(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + described_class.perform_now(avatarable, valid_url) expect(avatarable.avatar).to be_attached end @@ -93,22 +183,30 @@ RSpec.describe Avatar::AvatarFromUrlJob do # ref: https://github.com/chatwoot/chatwoot/issues/10449 it 'does not raise error when downloaded file has no filename (invalid content)' do contact = create(:contact) - temp_file = Tempfile.new(['invalid', '.xml']) - temp_file.write('content') - temp_file.rewind + invalid_file = Tempfile.new('avatar-without-name') - expect(Down).to receive(:download).with(valid_url, max_size: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE) - .and_return(ActionDispatch::Http::UploadedFile.new(tempfile: temp_file, type: 'application/xml')) + allow(SafeFetch).to receive(:fetch) + .with( + valid_url, + max_bytes: Avatar::AvatarFromUrlJob::MAX_DOWNLOAD_SIZE, + allowed_content_type_prefixes: [], + allowed_content_types: Avatar::AvatarFromUrlJob::ALLOWED_CONTENT_TYPES + ).and_yield( + SafeFetch::Result.new( + tempfile: invalid_file, + filename: nil, + content_type: 'image/png' + ) + ) expect { described_class.perform_now(contact, valid_url) }.not_to raise_error - - temp_file.close - temp_file.unlink + expect(contact.reload.avatar).not_to be_attached + ensure + invalid_file.close! end it 'skips sync attribute updates when URL is nil' do contact = create(:contact) - expect(Down).not_to receive(:download) expect { described_class.perform_now(contact, nil) }.not_to raise_error diff --git a/spec/lib/safe_fetch_spec.rb b/spec/lib/safe_fetch_spec.rb index d8d37115a..e2c513587 100644 --- a/spec/lib/safe_fetch_spec.rb +++ b/spec/lib/safe_fetch_spec.rb @@ -65,6 +65,23 @@ RSpec.describe SafeFetch do end end + context 'with embedded basic auth credentials' do + it 'passes decoded credentials to the request' do + authenticated_url = 'http://user+avatar%40example.com:p%40ss+word%3A1@example.com/image.png' + stub_request(:get, url) + .with(headers: { 'Authorization' => 'Basic dXNlcithdmF0YXJAZXhhbXBsZS5jb206cEBzcyt3b3JkOjE=' }) + .to_return( + status: 200, + body: File.new(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + + described_class.fetch(authenticated_url) do |result| + expect(result.content_type).to eq('image/png') + end + end + end + context 'with URL validation' do it 'raises InvalidUrlError for javascript: URLs' do expect { described_class.fetch('javascript:alert(1)') { nil } } @@ -153,14 +170,49 @@ RSpec.describe SafeFetch do expect { described_class.fetch(url) { nil } }.not_to raise_error end - it 'strips charset/boundary parameters before comparing' do + it 'normalizes parameters and casing before yielding content_type' do stub_request(:get, url).to_return( status: 200, body: 'x', - headers: { 'Content-Type' => 'image/png; charset=binary' } + headers: { 'Content-Type' => 'IMAGE/PNG; charset=binary' } ) - expect { described_class.fetch(url) { nil } }.not_to raise_error + described_class.fetch(url) do |result| + expect(result.content_type).to eq('image/png') + end + end + + it 'allows exact content-type matches when prefixes are empty' do + pdf_url = 'http://example.com/file.pdf' + stub_request(:get, pdf_url).to_return( + status: 200, + body: 'pdf-data', + headers: { 'Content-Type' => 'application/pdf' } + ) + + expect do + described_class.fetch( + pdf_url, + allowed_content_type_prefixes: [], + allowed_content_types: ['application/pdf'] + ) { nil } + end.not_to raise_error + end + + it 'rejects exact content-type mismatches when prefixes are empty' do + stub_request(:get, url).to_return( + status: 200, + body: 'x', + headers: { 'Content-Type' => 'image/webp' } + ) + + expect do + described_class.fetch( + url, + allowed_content_type_prefixes: [], + allowed_content_types: ['image/png'] + ) { nil } + end.to raise_error(described_class::UnsupportedContentTypeError) end it 'rejects when the content-type header is missing' do diff --git a/spec/models/contact_spec.rb b/spec/models/contact_spec.rb index f21a81978..159454eec 100644 --- a/spec/models/contact_spec.rb +++ b/spec/models/contact_spec.rb @@ -16,6 +16,13 @@ RSpec.describe Contact do describe 'concerns' do it_behaves_like 'avatarable' + + it 'accepts webp avatars' do + contact = build(:contact, account: create(:account)) + contact.avatar.attach(get_blob_for(Rails.root.join('spec/assets/avatar.png'), 'image/webp')) + + expect(contact).to be_valid + end end context 'when prepare contact attributes before validation' do From 4959a1ff1ecdc67c4bd70229dd9d5928908b91af Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Fri, 24 Apr 2026 19:56:01 +0530 Subject: [PATCH 05/32] style: [CW-6876] Updated designs for invite email (#14090) Improved the design for the invite emails --- .../devise/mailer/_confirmation_body.html.erb | 89 ++++++++++ .../mailer/confirmation_instructions.html.erb | 88 +++++++--- app/views/layouts/mailer/base.liquid | 153 +++++++++++------- .../mailer/confirmation_instructions.html.erb | 134 ++++++++++----- spec/enterprise/mailers/devise_mailer_spec.rb | 59 ++++--- .../mailers/confirmation_instructions_spec.rb | 40 ++++- 6 files changed, 414 insertions(+), 149 deletions(-) create mode 100644 app/views/devise/mailer/_confirmation_body.html.erb diff --git a/app/views/devise/mailer/_confirmation_body.html.erb b/app/views/devise/mailer/_confirmation_body.html.erb new file mode 100644 index 000000000..b256539f3 --- /dev/null +++ b/app/views/devise/mailer/_confirmation_body.html.erb @@ -0,0 +1,89 @@ + + + + <%= eyebrow %> + + + + + +

<%= heading %>

+ + + + +

Hi <%= recipient_name %>,

+ + + + +

<%= intro_text %>

+ + + + +

<%= supporting_text %>

+ + +<% if detail_rows.any? %> + + + + <% detail_rows.each_with_index do |(label, value), index| %> + + + + <% end %> +
+

+ <%= label %> +

+

<%= value %>

+
+ + +<% end %> +<% if action_url.present? %> + + + + + + +
+ <%= link_to( + action_text, + action_url, + style: 'display:block; width:100%; box-sizing:border-box; padding:12px 24px; font-size:14px; line-height:20px; font-weight:700; color:#FFFFFF; text-align:center; text-decoration:none;' + ) %> +
+ + + + +

+ If the button does not work, open + <%= link_to( + 'this secure link', + action_url, + style: 'color:#2781F6; text-decoration:none; font-weight:600;' + ) %>. +

+ + +<% elsif info_title.present? %> + + + + + + +
+

+ <%= info_title %> +

+

<%= info_text %>

+
+ + +<% end %> diff --git a/app/views/devise/mailer/confirmation_instructions.html.erb b/app/views/devise/mailer/confirmation_instructions.html.erb index 4a3b450a3..619c9f5ec 100644 --- a/app/views/devise/mailer/confirmation_instructions.html.erb +++ b/app/views/devise/mailer/confirmation_instructions.html.erb @@ -1,29 +1,65 @@ -

Hi <%= @resource.name %>,

+<% + brand_name = global_config['BRAND_NAME'] || 'Chatwoot' + recipient_name = @resource.name.presence || @resource.email + account_user = @resource&.account_users&.first + inviter = account_user&.inviter + account_name = account_user&.account&.name + invited_user = inviter.present? && @resource.unconfirmed_email.blank? -<% account_user = @resource&.account_users&.first %> + eyebrow = 'Welcome' + heading = 'Confirm your email to get started' + intro_text = + "Welcome to #{brand_name}. We just need to verify your email address before you can start using your account." + supporting_text = 'This only takes a moment.' + action_text = 'Confirm my account' + action_url = frontend_url('auth/confirmation', confirmation_token: @token) + info_title = nil + info_text = nil + detail_rows = [] + detail_rows << ['New email', @resource.unconfirmed_email] if @resource.unconfirmed_email.present? -<% if account_user&.inviter.present? && @resource.unconfirmed_email.blank? %> -

<%= account_user.inviter.name %>, with <%= account_user.account.name %>, has invited you to try out <%= global_config['BRAND_NAME'] || 'Chatwoot' %>.

-<% end %> + if @resource.unconfirmed_email.present? + eyebrow = 'Email update' + heading = 'Confirm your new email address' + intro_text = "We received a request to update the email address on your #{brand_name} account." + supporting_text = 'Confirm the new address below to finish the change.' + action_text = 'Confirm email address' + elsif @resource.confirmed? + eyebrow = 'Account ready' + heading = 'Your account is ready' + intro_text = "Your #{brand_name} account is already active." + supporting_text = 'Use the button below to sign in and continue where you left off.' + action_text = 'Open my account' + action_url = frontend_url('auth/sign_in') + detail_rows = [] + elsif invited_user + eyebrow = 'Workspace invitation' + heading = account_name.present? ? "You're invited to join #{account_name}" : "You're invited to try #{brand_name}" + intro_text = if account_name.present? + "#{inviter.name} invited you to join the #{account_name} workspace on #{brand_name}." + else + "#{inviter.name} invited you to try #{brand_name}." + end + supporting_text = 'Create your account to start collaborating with your team.' + action_text = 'Accept invitation' + action_url = frontend_url( + 'auth/password/edit', + reset_password_token: @resource.send(:set_reset_password_token) + ) + detail_rows = [['Invited by', inviter.name]] + detail_rows << ['Workspace', account_name] if account_name.present? + end +%> -<% if @resource.confirmed? %> -

You can login to your <%= global_config['BRAND_NAME'] || 'Chatwoot' %> account through the link below:

-<% else %> - <% if account_user&.inviter.blank? %> -

- Welcome to <%= global_config['BRAND_NAME'] || 'Chatwoot' %>! We have a suite of powerful tools ready for you to explore. Before that we quickly need to verify your email address to know it's really you. -

- <% end %> -

Please take a moment and click the link below and activate your account.

-<% end %> - - -<% if @resource.unconfirmed_email.present? %> -

<%= link_to 'Confirm my account', frontend_url('auth/confirmation', confirmation_token: @token) %>

-<% elsif @resource.confirmed? %> -

<%= link_to 'Login to my account', frontend_url('auth/sign_in') %>

-<% elsif account_user&.inviter.present? %> -

<%= link_to 'Confirm my account', frontend_url('auth/password/edit', reset_password_token: @resource.send(:set_reset_password_token)) %>

-<% else %> -

<%= link_to 'Confirm my account', frontend_url('auth/confirmation', confirmation_token: @token) %>

-<% end %> \ No newline at end of file +<%= render partial: 'devise/mailer/confirmation_body', locals: { + action_text: action_text, + action_url: action_url, + detail_rows: detail_rows, + eyebrow: eyebrow, + heading: heading, + info_text: info_text, + info_title: info_title, + intro_text: intro_text, + recipient_name: recipient_name, + supporting_text: supporting_text +} %> diff --git a/app/views/layouts/mailer/base.liquid b/app/views/layouts/mailer/base.liquid index 5fa07e139..e6c70b3e5 100644 --- a/app/views/layouts/mailer/base.liquid +++ b/app/views/layouts/mailer/base.liquid @@ -7,86 +7,129 @@ - - + {% assign brand_name = global_config['BRAND_NAME'] %} + {% if brand_name == nil %} + {% assign brand_name = 'Chatwoot' %} + {% endif %} + {% assign brand_url = global_config['BRAND_URL'] %} + + +
- diff --git a/enterprise/app/views/devise/mailer/confirmation_instructions.html.erb b/enterprise/app/views/devise/mailer/confirmation_instructions.html.erb index 91837f980..5032db646 100644 --- a/enterprise/app/views/devise/mailer/confirmation_instructions.html.erb +++ b/enterprise/app/views/devise/mailer/confirmation_instructions.html.erb @@ -1,45 +1,99 @@ -

Hi <%= @resource.name %>,

+<% + brand_name = global_config['BRAND_NAME'] || 'Chatwoot' + recipient_name = @resource.name.presence || @resource.email + account_user = @resource&.account_users&.first + inviter = account_user&.inviter + account_name = account_user&.account&.name + is_saml_account = account_user&.account&.saml_enabled? + invited_user = inviter.present? && @resource.unconfirmed_email.blank? -<% account_user = @resource&.account_users&.first %> -<% is_saml_account = account_user&.account&.saml_enabled? %> + eyebrow = 'Welcome' + heading = 'Confirm your email to get started' + intro_text = + "Welcome to #{brand_name}. We just need to verify your email address before you can start using your account." + supporting_text = 'This only takes a moment.' + action_text = 'Confirm my account' + action_url = frontend_url('auth/confirmation', confirmation_token: @token) + info_title = nil + info_text = nil + detail_rows = [] + detail_rows << ['New email', @resource.unconfirmed_email] if @resource.unconfirmed_email.present? -<% if account_user&.inviter.present? && @resource.unconfirmed_email.blank? %> - <% if is_saml_account %> -

<%= account_user.inviter.name %>, with <%= account_user.account.name %>, has invited you to access <%= global_config['BRAND_NAME'] || 'Chatwoot' %> via Single Sign-On (SSO).

-

Your organization uses SSO for secure authentication. You will not need a password to access your account.

- <% else %> -

<%= account_user.inviter.name %>, with <%= account_user.account.name %>, has invited you to try out <%= global_config['BRAND_NAME'] || 'Chatwoot' %>.

- <% end %> -<% end %> + if @resource.unconfirmed_email.present? + eyebrow = 'Email update' + heading = 'Confirm your new email address' + intro_text = "We received a request to update the email address on your #{brand_name} account." + supporting_text = 'Confirm the new address below to finish the change.' + action_text = 'Confirm email address' + elsif @resource.confirmed? + eyebrow = 'Account ready' -<% if @resource.confirmed? %> -

You can login to your <%= global_config['BRAND_NAME'] || 'Chatwoot' %> account through the link below:

-<% else %> - <% if account_user&.inviter.blank? %> -

- Welcome to <%= global_config['BRAND_NAME'] || 'Chatwoot' %>! We have a suite of powerful tools ready for you to explore. Before that we quickly need to verify your email address to know it's really you. -

- <% end %> - <% unless is_saml_account %> -

Please take a moment and click the link below and activate your account.

- <% end %> -<% end %> + if is_saml_account + heading = 'Your access is ready' + intro_text = "Your #{brand_name} access is already set up." + supporting_text = "Use your organization's Single Sign-On (SSO) portal to access #{brand_name}." + action_text = nil + action_url = nil + info_title = "Sign in with your organization's SSO" + info_text = + "You won't need a separate password for #{brand_name}. Start from your company identity provider portal." + detail_rows = [] + detail_rows << ['Workspace', account_name] if account_name.present? + detail_rows << ['Sign-in method', 'Single Sign-On (SSO)'] + else + heading = 'Your account is ready' + intro_text = "Your #{brand_name} account is already active." + supporting_text = 'Use the button below to sign in and continue where you left off.' + action_text = 'Open my account' + action_url = frontend_url('auth/sign_in') + detail_rows = [] + end + elsif invited_user + eyebrow = 'Workspace invitation' + heading = account_name.present? ? "You're invited to join #{account_name}" : "You're invited to try #{brand_name}" + if is_saml_account + intro_text = if account_name.present? + "#{inviter.name} invited you to access the #{account_name} workspace on #{brand_name}." + else + "#{inviter.name} invited you to access #{brand_name}." + end + supporting_text = + "Your organization uses Single Sign-On (SSO), so you won't need to create a separate password." + action_text = nil + action_url = nil + info_title = "Use your organization's SSO portal" + info_text = "Continue from your company identity provider portal to access #{brand_name}." + detail_rows = [['Invited by', inviter.name]] + detail_rows << ['Workspace', account_name] if account_name.present? + detail_rows << ['Sign-in method', 'Single Sign-On (SSO)'] + else + intro_text = if account_name.present? + "#{inviter.name} invited you to join the #{account_name} workspace on #{brand_name}." + else + "#{inviter.name} invited you to try #{brand_name}." + end + supporting_text = 'Create your account to start collaborating with your team.' + action_text = 'Accept invitation' + action_url = frontend_url( + 'auth/password/edit', + reset_password_token: @resource.send(:set_reset_password_token) + ) + detail_rows = [['Invited by', inviter.name]] + detail_rows << ['Workspace', account_name] if account_name.present? + end + end +%> -<% if @resource.unconfirmed_email.present? %> -

<%= link_to 'Confirm my account', frontend_url('auth/confirmation', confirmation_token: @token) %>

-<% elsif @resource.confirmed? %> - <% if is_saml_account %> -

You can now access your account by logging in through your organization's SSO portal.

- <% else %> -

<%= link_to 'Login to my account', frontend_url('auth/sign_in') %>

- <% end %> -<% elsif account_user&.inviter.present? %> - <% if is_saml_account %> -

You can access your account by logging in through your organization's SSO portal.

- <% else %> -

<%= link_to 'Confirm my account', frontend_url('auth/password/edit', reset_password_token: @resource.send(:set_reset_password_token)) %>

- <% end %> -<% else %> -

<%= link_to 'Confirm my account', frontend_url('auth/confirmation', confirmation_token: @token) %>

-<% end %> +<%= render partial: 'devise/mailer/confirmation_body', locals: { + action_text: action_text, + action_url: action_url, + detail_rows: detail_rows, + eyebrow: eyebrow, + heading: heading, + info_text: info_text, + info_title: info_title, + intro_text: intro_text, + recipient_name: recipient_name, + supporting_text: supporting_text +} %> diff --git a/spec/enterprise/mailers/devise_mailer_spec.rb b/spec/enterprise/mailers/devise_mailer_spec.rb index 286e863f7..61ce92047 100644 --- a/spec/enterprise/mailers/devise_mailer_spec.rb +++ b/spec/enterprise/mailers/devise_mailer_spec.rb @@ -8,12 +8,23 @@ RSpec.describe 'Devise::Mailer' do let!(:confirmable_user) { create(:user, inviter: inviter_val, account: account) } let(:inviter_val) { nil } let(:mail) { Devise::Mailer.confirmation_instructions(confirmable_user.reload, nil, {}) } + let(:mail_body) { CGI.unescapeHTML(mail.body.to_s) } before do confirmable_user.update!(confirmed_at: nil) confirmable_user.send(:generate_confirmation_token) end + context 'when brand name is intentionally blank' do + before do + create(:installation_config, name: 'BRAND_NAME', value: '') + end + + it 'preserves the blank brand override' do + expect(mail_body).not_to include('Chatwoot') + end + end + context 'with SAML enabled account' do let(:saml_settings) { create(:account_saml_settings, account: account) } @@ -21,12 +32,13 @@ RSpec.describe 'Devise::Mailer' do context 'when user has no inviter' do it 'shows standard welcome message without SSO references' do - expect(mail.body).to match('We have a suite of powerful tools ready for you to explore.') - expect(mail.body).not_to match('via Single Sign-On') + expect(mail_body).to include('Confirm your email to get started') + expect(mail_body).to include('We just need to verify your email address before you can start using your account.') + expect(mail_body).not_to include('Single Sign-On (SSO)') end - it 'does not show activation instructions for SAML accounts' do - expect(mail.body).not_to match('Please take a moment and click the link below and activate your account') + it 'shows the standard confirmation CTA' do + expect(mail_body).to include('Confirm my account') end it 'shows confirmation link' do @@ -38,22 +50,21 @@ RSpec.describe 'Devise::Mailer' do let(:inviter_val) { create(:user, :administrator, skip_confirmation: true, account: account) } it 'mentions SSO invitation' do - expect(mail.body).to match( - "#{CGI.escapeHTML(inviter_val.name)}, with #{CGI.escapeHTML(account.name)}, has invited you to access.*via Single Sign-On \\(SSO\\)" - ) + expect(mail_body).to include("You're invited to join #{account.name}") + expect(mail_body).to include("#{inviter_val.name} invited you to access the #{account.name} workspace on Chatwoot.") end it 'explains SSO authentication' do - expect(mail.body).to match('Your organization uses SSO for secure authentication') - expect(mail.body).to match('You will not need a password to access your account') + expect(mail_body).to include("Your organization uses Single Sign-On (SSO), so you won't need to create a separate password.") end it 'does not show standard invitation message' do - expect(mail.body).not_to match('has invited you to try out') + expect(mail_body).not_to include('invited you to join') + expect(mail_body).not_to include('Accept invitation') end it 'directs to SSO portal instead of password reset' do - expect(mail.body).to match('You can access your account by logging in through your organization\'s SSO portal') + expect(mail_body).to include("Use your organization's SSO portal") expect(mail.body).not_to include('app/auth/password/edit') end end @@ -66,7 +77,9 @@ RSpec.describe 'Devise::Mailer' do end it 'shows SSO login instructions' do - expect(mail.body).to match('You can now access your account by logging in through your organization\'s SSO portal') + expect(mail_body).to include('Your access is ready') + expect(mail_body).to include("Sign in with your organization's SSO") + expect(mail_body).to include("Use your organization's Single Sign-On (SSO) portal to access") expect(mail.body).not_to include('/auth/sign_in') end end @@ -79,6 +92,7 @@ RSpec.describe 'Devise::Mailer' do end it 'still shows confirmation link for email verification' do + expect(mail_body).to include('Confirm your new email address') expect(mail.body).to include('app/auth/confirmation?confirmation_token') expect(confirmable_user.unconfirmed_email.blank?).to be false end @@ -90,7 +104,8 @@ RSpec.describe 'Devise::Mailer' do end it 'shows SSO login instructions instead of regular login' do - expect(mail.body).to match('You can now access your account by logging in through your organization\'s SSO portal') + expect(mail_body).to include('Your access is ready') + expect(mail_body).to include("Sign in with your organization's SSO") expect(mail.body).not_to include('/auth/sign_in') end end @@ -101,9 +116,10 @@ RSpec.describe 'Devise::Mailer' do let(:inviter_val) { create(:user, :administrator, skip_confirmation: true, account: account) } it 'shows standard invitation without SSO references' do - expect(mail.body).to match('has invited you to try out Chatwoot') - expect(mail.body).not_to match('via Single Sign-On') - expect(mail.body).not_to match('SSO portal') + expect(mail_body).to include("You're invited to join #{account.name}") + expect(mail_body).to include("#{inviter_val.name} invited you to join the #{account.name} workspace on") + expect(mail_body).not_to include('Single Sign-On (SSO)') + expect(mail_body).not_to include("Use your organization's SSO portal") end it 'shows password reset link' do @@ -112,9 +128,10 @@ RSpec.describe 'Devise::Mailer' do end context 'when user has no inviter' do - it 'shows standard welcome message and activation instructions' do - expect(mail.body).to match('We have a suite of powerful tools ready for you to explore') - expect(mail.body).to match('Please take a moment and click the link below and activate your account') + it 'shows the standard confirmation state' do + expect(mail_body).to include('Confirm your email to get started') + expect(mail_body).to include('We just need to verify your email address before you can start using your account.') + expect(mail_body).to include('Confirm my account') end it 'shows confirmation link' do @@ -130,8 +147,9 @@ RSpec.describe 'Devise::Mailer' do end it 'shows regular login link' do + expect(mail_body).to include('Your account is ready') expect(mail.body).to include('/auth/sign_in') - expect(mail.body).not_to match('SSO portal') + expect(mail_body).not_to include('SSO portal') end end @@ -141,6 +159,7 @@ RSpec.describe 'Devise::Mailer' do end it 'shows confirmation link for email verification' do + expect(mail_body).to include('Confirm your new email address') expect(mail.body).to include('app/auth/confirmation?confirmation_token') expect(confirmable_user.unconfirmed_email.blank?).to be false end diff --git a/spec/mailers/confirmation_instructions_spec.rb b/spec/mailers/confirmation_instructions_spec.rb index 484001957..b82202c51 100644 --- a/spec/mailers/confirmation_instructions_spec.rb +++ b/spec/mailers/confirmation_instructions_spec.rb @@ -8,6 +8,7 @@ RSpec.describe 'Devise::Mailer' do let!(:confirmable_user) { create(:user, inviter: inviter_val, account: account) } let(:inviter_val) { nil } let(:mail) { Devise::Mailer.confirmation_instructions(confirmable_user.reload, nil, {}) } + let(:mail_body) { CGI.unescapeHTML(mail.body.to_s) } before do # to verify the token in email @@ -22,12 +23,26 @@ RSpec.describe 'Devise::Mailer' do end it 'uses the user\'s name' do - expect(mail.body).to match("Hi #{CGI.escapeHTML(confirmable_user.name)},") + expect(mail.body.to_s).to include("Hi #{CGI.escapeHTML(confirmable_user.name)},") + expect(mail_body).to include("Hi #{confirmable_user.name},") end - it 'does not refer to the inviter and their account' do - expect(mail.body).not_to match('has invited you to try out Chatwoot!') - expect(mail.body).to match('We have a suite of powerful tools ready for you to explore.') + context 'when the user name contains HTML' do + before do + confirmable_user.update!(name: 'Sony ') + end + + it 'escapes the name in the rendered email body' do + expect(mail.body.to_s).to include("Hi #{CGI.escapeHTML(confirmable_user.name)},") + expect(mail.body.to_s).not_to include("Hi #{confirmable_user.name},") + end + end + + it 'shows the default confirmation state' do + expect(mail_body).to include('Confirm your email to get started') + expect(mail_body).to include('Welcome to Chatwoot. We just need to verify your email address before you can start using your account.') + expect(mail_body).to include('Confirm my account') + expect(mail_body).not_to include('Workspace invitation') end it 'sends a confirmation link' do @@ -39,10 +54,10 @@ RSpec.describe 'Devise::Mailer' do let(:inviter_val) { create(:user, :administrator, skip_confirmation: true, account: account) } it 'refers to the inviter and their account' do - expect(mail.body).to match( - "#{CGI.escapeHTML(inviter_val.name)}, with #{CGI.escapeHTML(account.name)}, has invited you to try out Chatwoot." - ) - expect(mail.body).not_to match('We have a suite of powerful tools ready for you to explore.') + expect(mail_body).to include("You're invited to join #{account.name}") + expect(mail_body).to include("#{inviter_val.name} invited you to join the #{account.name} workspace on Chatwoot.") + expect(mail_body).to include('Accept invitation') + expect(mail_body).not_to include('Confirm your email to get started') end it 'sends a password reset link' do @@ -58,7 +73,10 @@ RSpec.describe 'Devise::Mailer' do it 'sends a confirmation link' do confirmation_mail = Devise::Mailer.confirmation_instructions(confirmable_user.reload, nil, {}) + confirmation_body = CGI.unescapeHTML(confirmation_mail.body.to_s) + expect(confirmation_body).to include('Confirm your new email address') + expect(confirmation_body).to include('New email') expect(confirmation_mail.body).to include('app/auth/confirmation?confirmation_token') expect(confirmation_mail.body).not_to include('app/auth/password/edit') expect(confirmable_user.unconfirmed_email.blank?).to be false @@ -73,7 +91,9 @@ RSpec.describe 'Devise::Mailer' do it 'sends a confirmation link' do confirmation_mail = Devise::Mailer.confirmation_instructions(confirmable_user.reload, nil, {}) + confirmation_body = CGI.unescapeHTML(confirmation_mail.body.to_s) + expect(confirmation_body).to include('Confirm your new email address') expect(confirmation_mail.body).to include('app/auth/confirmation?confirmation_token') expect(confirmation_mail.body).not_to include('app/auth/password/edit') expect(confirmable_user.unconfirmed_email.blank?).to be false @@ -88,6 +108,10 @@ RSpec.describe 'Devise::Mailer' do it 'send instructions with the link to login' do confirmation_mail = Devise::Mailer.confirmation_instructions(confirmable_user.reload, nil, {}) + confirmation_body = CGI.unescapeHTML(confirmation_mail.body.to_s) + + expect(confirmation_body).to include('Your account is ready') + expect(confirmation_body).to include('Open my account') expect(confirmation_mail.body).to include('/auth/sign_in') end end From 751c28d94d33c78b499cd6b0602b544dd0328839 Mon Sep 17 00:00:00 2001 From: Pranav Date: Fri, 24 Apr 2026 08:51:26 -0700 Subject: [PATCH 06/32] feat(ee): Add article translation via LLM in help center (#14136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the ability to translate help center articles to other languages using Captain's LLM infrastructure. Translated articles are created as drafts linked to the source article. Fixes https://linear.app/chatwoot/issue/CW-6901/translate-article-to-another-language **How to test** 1. Navigate to Help Center → Articles for a portal with multiple locales 2. Click the three-dot menu on any article → "Translate" 3. Select a target language and category → click Translate 4. Switch to the target locale — the translated article appears as a draft 5. Try translating the same article again — a warning shows the existing translation with a link to open it in a new tab 6. Click "Overwrite and translate" to replace the existing translation https://github.com/user-attachments/assets/1d2e991b-f0ac-403a-bcc1-2181b5731ea4 --- .../articles/bulk_actions_controller.rb | 19 ++ .../dashboard/api/helpCenter/articles.js | 7 + .../HelpCenter/ArticleCard/ArticleCard.vue | 22 +- .../Pages/ArticlePage/ArticleList.vue | 6 + .../Pages/ArticlePage/ArticlesPage.vue | 17 +- .../Pages/ArticlePage/BulkTranslateDialog.vue | 249 ++++++++++++++++++ .../dashboard/helper/portalHelper.js | 13 +- .../dashboard/i18n/locale/en/helpCenter.json | 18 ++ .../modules/helpCenterArticles/actions.js | 14 + config/locales/en.yml | 5 + config/routes.rb | 5 + .../articles/bulk_actions_controller.rb | 68 +++++ .../jobs/captain/articles/translate_job.rb | 59 +++++ .../llm/article_translation_service.rb | 62 +++++ .../articles/bulk_actions_controller_spec.rb | 179 +++++++++++++ .../captain/articles/translate_job_spec.rb | 134 ++++++++++ .../llm/article_translation_service_spec.rb | 67 +++++ 17 files changed, 939 insertions(+), 5 deletions(-) create mode 100644 app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb create mode 100644 app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/BulkTranslateDialog.vue create mode 100644 enterprise/app/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller.rb create mode 100644 enterprise/app/jobs/captain/articles/translate_job.rb create mode 100644 enterprise/app/services/captain/llm/article_translation_service.rb create mode 100644 spec/enterprise/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller_spec.rb create mode 100644 spec/enterprise/jobs/captain/articles/translate_job_spec.rb create mode 100644 spec/enterprise/services/captain/llm/article_translation_service_spec.rb diff --git a/app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb b/app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb new file mode 100644 index 000000000..584e3dbf2 --- /dev/null +++ b/app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb @@ -0,0 +1,19 @@ +class Api::V1::Accounts::Articles::BulkActionsController < Api::V1::Accounts::BaseController + before_action :portal + before_action :check_authorization + + def translate + head :not_implemented + end + + private + + def portal + @portal ||= Current.account.portals.find_by!(slug: params[:portal_id]) + end + + def check_authorization + authorize(Article, :create?) + end +end +Api::V1::Accounts::Articles::BulkActionsController.prepend_mod_with('Api::V1::Accounts::Articles::BulkActionsController') diff --git a/app/javascript/dashboard/api/helpCenter/articles.js b/app/javascript/dashboard/api/helpCenter/articles.js index 727340ed5..781570d0b 100644 --- a/app/javascript/dashboard/api/helpCenter/articles.js +++ b/app/javascript/dashboard/api/helpCenter/articles.js @@ -72,6 +72,13 @@ class ArticlesAPI extends PortalsAPI { category_slug: categorySlug, }); } + + bulkTranslate({ portalSlug, articleIds, locale, categoryId, force = false }) { + return axios.post( + `${this.url}/${portalSlug}/articles/bulk_actions/translate`, + { ids: articleIds, locale, category_id: categoryId, force } + ); + } } export default new ArticlesAPI(); diff --git a/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue b/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue index ce9e9db36..25eda255a 100644 --- a/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue +++ b/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue @@ -9,6 +9,9 @@ import { ARTICLE_STATUSES, } from 'dashboard/helper/portalHelper'; +import { useMapGetter } from 'dashboard/composables/store.js'; +import { useConfig } from 'dashboard/composables/useConfig'; +import { FEATURE_FLAGS } from 'dashboard/featureFlags'; import Icon from 'dashboard/components-next/icon/Icon.vue'; import CardLayout from 'dashboard/components-next/CardLayout.vue'; import DropdownMenu from 'dashboard/components-next/dropdown-menu/DropdownMenu.vue'; @@ -52,6 +55,21 @@ const { t } = useI18n(); const [showActionsDropdown, toggleDropdown] = useToggle(); +const currentAccountId = useMapGetter('getCurrentAccountId'); +const isFeatureEnabledonAccount = useMapGetter( + 'accounts/isFeatureEnabledonAccount' +); +const { isEnterprise } = useConfig(); + +const isTranslationAvailable = computed( + () => + isEnterprise && + isFeatureEnabledonAccount.value( + currentAccountId.value, + FEATURE_FLAGS.CAPTAIN_TASKS + ) +); + const articleMenuItems = computed(() => { const commonItems = Object.entries(ARTICLE_MENU_ITEMS).reduce( (acc, [key, item]) => { @@ -64,7 +82,9 @@ const articleMenuItems = computed(() => { const statusItems = ( ARTICLE_MENU_OPTIONS[props.status] || ARTICLE_MENU_OPTIONS[ARTICLE_STATUSES.PUBLISHED] - ).map(key => commonItems[key]); + ) + .filter(key => key !== 'translate' || isTranslationAvailable.value) + .map(key => commonItems[key]); return [...statusItems, commonItems.delete]; }); diff --git a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue index cc7c97000..c46a965bc 100644 --- a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue +++ b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticleList.vue @@ -22,6 +22,8 @@ const props = defineProps({ }, }); +const emit = defineEmits(['translateArticle']); + const { ARTICLE_STATUS_TYPES } = wootConstants; const router = useRouter(); @@ -152,6 +154,10 @@ const handleArticleAction = async (action, { status, id }) => { }; const updateArticle = ({ action, value, id }) => { + if (action === 'translate') { + emit('translateArticle', id); + return; + } const status = action !== 'delete' ? getArticleStatus(value) : null; handleArticleAction(action, { status, id }); }; diff --git a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue index e31d40d8a..f1e177505 100644 --- a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue +++ b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue @@ -1,5 +1,5 @@ + diff --git a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/BulkTranslateDialog.vue b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/BulkTranslateDialog.vue new file mode 100644 index 000000000..c2551830d --- /dev/null +++ b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/BulkTranslateDialog.vue @@ -0,0 +1,249 @@ + + + diff --git a/app/javascript/dashboard/helper/portalHelper.js b/app/javascript/dashboard/helper/portalHelper.js index 37d09337f..33a2a822a 100644 --- a/app/javascript/dashboard/helper/portalHelper.js +++ b/app/javascript/dashboard/helper/portalHelper.js @@ -91,6 +91,13 @@ export const ARTICLE_MENU_ITEMS = { action: 'archive', icon: 'i-lucide-archive-restore', }, + translate: { + label: + 'HELP_CENTER.ARTICLES_PAGE.ARTICLE_CARD.CARD.DROPDOWN_MENU.TRANSLATE', + value: 'translate', + action: 'translate', + icon: 'i-lucide-languages', + }, delete: { label: 'HELP_CENTER.ARTICLES_PAGE.ARTICLE_CARD.CARD.DROPDOWN_MENU.DELETE', value: 'delete', @@ -100,9 +107,9 @@ export const ARTICLE_MENU_ITEMS = { }; export const ARTICLE_MENU_OPTIONS = { - [ARTICLE_STATUSES.ARCHIVED]: ['publish', 'draft'], - [ARTICLE_STATUSES.DRAFT]: ['publish', 'archive'], - [ARTICLE_STATUSES.PUBLISHED]: ['draft', 'archive'], + [ARTICLE_STATUSES.ARCHIVED]: ['publish', 'draft', 'translate'], + [ARTICLE_STATUSES.DRAFT]: ['publish', 'archive', 'translate'], + [ARTICLE_STATUSES.PUBLISHED]: ['draft', 'archive', 'translate'], }; export const ARTICLE_TABS = { diff --git a/app/javascript/dashboard/i18n/locale/en/helpCenter.json b/app/javascript/dashboard/i18n/locale/en/helpCenter.json index ffeca222a..bb9bf2e99 100644 --- a/app/javascript/dashboard/i18n/locale/en/helpCenter.json +++ b/app/javascript/dashboard/i18n/locale/en/helpCenter.json @@ -525,6 +525,7 @@ "PUBLISH": "Publish", "DRAFT": "Draft", "ARCHIVE": "Archive", + "TRANSLATE": "Translate", "DELETE": "Delete" }, "STATUS": { @@ -579,6 +580,23 @@ "TITLE": "There are no articles in this category", "SUBTITLE": "Articles in this category will appear here" } + }, + "BULK_TRANSLATE": { + "TITLE": "Translate article | Translate {count} articles", + "DESCRIPTION": "Translate the selected article to another language. | Translate the selected articles to another language.", + "LOCALE_LABEL": "Target language", + "LOCALE_PLACEHOLDER": "Select a language", + "CATEGORY_LABEL": "Target category", + "CATEGORY_PLACEHOLDER": "Select a category", + "OPTIONAL": "(optional)", + "CONFIRM": "Translate", + "CONFIRM_OVERWRITE": "Overwrite and translate", + "DUPLICATE_WARNING": "A translation already exists for this article in the selected language. | Translations already exist for {count} articles in the selected language.", + "DUPLICATE_CONFIRM_HINT": "Click translate again to overwrite the existing translation.", + "API": { + "SUCCESS_MESSAGE": "Translation in progress. The article will appear as a draft once ready.", + "ERROR_MESSAGE": "Failed to start translation. Please try again." + } } }, "CATEGORY_PAGE": { diff --git a/app/javascript/dashboard/store/modules/helpCenterArticles/actions.js b/app/javascript/dashboard/store/modules/helpCenterArticles/actions.js index 13a2b4899..229a42a5d 100644 --- a/app/javascript/dashboard/store/modules/helpCenterArticles/actions.js +++ b/app/javascript/dashboard/store/modules/helpCenterArticles/actions.js @@ -166,4 +166,18 @@ export const actions = { throw error; } }, + + bulkTranslate: async ( + _, + { portalSlug, articleIds, locale, categoryId, force = false } + ) => { + const { data } = await articlesAPI.bulkTranslate({ + portalSlug, + articleIds, + locale, + categoryId, + force, + }); + return data; + }, }; diff --git a/config/locales/en.yml b/config/locales/en.yml index 1841db332..36f115bab 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -488,6 +488,11 @@ en: agent_capacity_policy: inbox_already_assigned: 'Inbox has already been assigned to this policy' portals: + articles: + captain_not_available: 'Translation requires Captain to be enabled for this account' + locale_not_available: 'Locale not available in this portal' + category_not_found: 'Category not found in this portal' + no_articles_found: 'No articles found to process' send_instructions: email_required: 'Email is required' invalid_email_format: 'Invalid email format' diff --git a/config/routes.rb b/config/routes.rb index 2461539ad..c6111d317 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -358,6 +358,11 @@ Rails.application.routes.draw do resources :categories do post :reorder, on: :collection end + namespace :articles do + resource :bulk_actions, only: [] do + post :translate + end + end resources :articles do post :reorder, on: :collection end diff --git a/enterprise/app/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller.rb b/enterprise/app/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller.rb new file mode 100644 index 000000000..87ad54d24 --- /dev/null +++ b/enterprise/app/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller.rb @@ -0,0 +1,68 @@ +module Enterprise::Api::V1::Accounts::Articles::BulkActionsController + def translate + return unless validate_translate_params? + + duplicates = find_existing_translations + if duplicates.any? && !ActiveModel::Type::Boolean.new.cast(permitted_params[:force]) + return render json: { + duplicate_articles: duplicates.map { |a| { id: a.id, title: a.title } } + }, status: :conflict + end + + @articles.find_each do |article| + Captain::Articles::TranslateJob.perform_later( + Current.account, article.id, @locale, @category&.id, Current.user + ) + end + + head :ok + end + + private + + def permitted_params + params.permit(:locale, :category_id, :force, ids: []) + end + + def validate_translate_params? + @locale = permitted_params[:locale] + @category = @portal.categories.find_by(id: permitted_params[:category_id], locale: @locale) + @articles = @portal.articles.where(id: permitted_params[:ids]) + + captain_available? && valid_locale? && valid_category? && valid_articles? + end + + def find_existing_translations + root_ids = @articles.map { |a| Article.find_root_article_id(a) } + @portal.articles.where(associated_article_id: root_ids, locale: @locale) + end + + def captain_available? + return true if Current.account.feature_enabled?('captain_tasks') + + render_could_not_create_error(I18n.t('portals.articles.captain_not_available')) + false + end + + def valid_locale? + return true if @portal.config['allowed_locales']&.include?(@locale) + + render_could_not_create_error(I18n.t('portals.articles.locale_not_available')) + false + end + + def valid_category? + return true if permitted_params[:category_id].blank? + return true if @category.present? + + render_could_not_create_error(I18n.t('portals.articles.category_not_found')) + false + end + + def valid_articles? + return true if @articles.any? + + render_could_not_create_error(I18n.t('portals.articles.no_articles_found')) + false + end +end diff --git a/enterprise/app/jobs/captain/articles/translate_job.rb b/enterprise/app/jobs/captain/articles/translate_job.rb new file mode 100644 index 000000000..c3524cbff --- /dev/null +++ b/enterprise/app/jobs/captain/articles/translate_job.rb @@ -0,0 +1,59 @@ +class Captain::Articles::TranslateJob < ApplicationJob + queue_as :low + + def perform(account, article_id, target_locale, target_category_id, user) + @account = account + @source_article = account.articles.find(article_id) + + target_language = language_name_for(target_locale) + + translated_title = translate(@source_article.title, target_language: target_language, type: :title) + translated_content = if @source_article.content.present? + translate(@source_article.content, target_language: target_language, type: :content) + else + @source_article.content + end + + existing = find_existing_translation(target_locale) + + if existing + existing.update!(title: translated_title, content: translated_content, description: @source_article.description) + else + create_translated_article(translated_title, translated_content, target_locale, target_category_id, user) + end + end + + private + + def translate(text, target_language:, type:) + response = Captain::Llm::ArticleTranslationService.new( + account: @account, text: text, target_language: target_language, type: type + ).perform + raise "Translation failed: #{response[:error]}" if response[:error] + + response[:message] + end + + def find_existing_translation(target_locale) + root_id = Article.find_root_article_id(@source_article) + @source_article.portal.articles.find_by(associated_article_id: root_id, locale: target_locale) + end + + def create_translated_article(translated_title, translated_content, target_locale, target_category_id, user) + @source_article.portal.articles.create!( + title: translated_title, + content: translated_content, + description: @source_article.description, + category_id: target_category_id, + locale: target_locale, + author_id: user.id, + status: :draft, + associated_article_id: Article.find_root_article_id(@source_article) + ) + end + + def language_name_for(locale_code) + language_map = YAML.load_file(Rails.root.join('config/languages/language_map.yml')) + language_map[locale_code] || locale_code + end +end diff --git a/enterprise/app/services/captain/llm/article_translation_service.rb b/enterprise/app/services/captain/llm/article_translation_service.rb new file mode 100644 index 000000000..5db26088e --- /dev/null +++ b/enterprise/app/services/captain/llm/article_translation_service.rb @@ -0,0 +1,62 @@ +class Captain::Llm::ArticleTranslationService < Captain::BaseTaskService + TYPES = %i[title content].freeze + + pattr_initialize [:account!, :text!, :target_language!, :type!] + + def perform + raise ArgumentError, "Invalid type: #{type}" unless TYPES.include?(type) + + response = make_api_call(model: translation_model, messages: messages) + return response if response[:error] + + response.merge(message: response[:message].strip) + end + + private + + def messages + [ + { role: 'system', content: system_prompt }, + { role: 'user', content: text } + ] + end + + def system_prompt + type == :title ? title_system_prompt : content_system_prompt + end + + def event_name + 'article_translation' + end + + def llm_credential + @llm_credential ||= system_llm_credential + end + + def translation_model + @translation_model ||= InstallationConfig.find_by(name: 'CAPTAIN_OPEN_AI_MODEL')&.value.presence || GPT_MODEL + end + + def title_system_prompt + <<~SYSTEM_PROMPT_MESSAGE + You are a professional translator. + Translate the following text to #{target_language}. + Return only the translated text, no explanations or extra formatting. + SYSTEM_PROMPT_MESSAGE + end + + def content_system_prompt + <<~SYSTEM_PROMPT_MESSAGE + You are a professional translator. Translate the following content to #{target_language}. + The content is markdown that may contain embedded HTML blocks. + Rules: + - Translate ONLY the visible text content (headings, paragraphs, list items, table cells, etc.). + - Preserve ALL markdown formatting exactly: headings (#), bold (**), italic (*), links, lists, code blocks, blockquotes, tables, horizontal rules. + - Preserve ALL HTML tags, attributes, and structure exactly as they are. + - Do NOT translate or modify: URLs, image src/alt attributes, link href values, class names, IDs, data attributes, code blocks, or any HTML attribute values. + - Keep all image tags (both markdown ![](url) and HTML ), iframes, and embedded media completely unchanged. + - Preserve all line breaks, blank lines, and whitespace patterns. + - Return ONLY the translated content, no wrapping or explanations. + SYSTEM_PROMPT_MESSAGE + end +end diff --git a/spec/enterprise/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller_spec.rb b/spec/enterprise/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller_spec.rb new file mode 100644 index 000000000..31f61d977 --- /dev/null +++ b/spec/enterprise/controllers/enterprise/api/v1/accounts/articles/bulk_actions_controller_spec.rb @@ -0,0 +1,179 @@ +require 'rails_helper' + +RSpec.describe 'Article Bulk Actions API', type: :request do + include ActiveJob::TestHelper + + let(:account) { create(:account) } + let(:admin) { create(:user, account: account, role: :administrator) } + let(:agent) { create(:user, account: account, role: :agent) } + let!(:portal) { create(:portal, name: 'test_portal', account: account, config: { allowed_locales: %w[en es fr] }) } + let!(:category_en) { create(:category, portal: portal, account: account, locale: 'en', slug: 'getting-started') } + let!(:category_es) { create(:category, portal: portal, account: account, locale: 'es', slug: 'primeros-pasos') } + let!(:article_one) { create(:article, category: category_en, portal: portal, account: account, author_id: admin.id) } + let!(:article_two) { create(:article, category: category_en, portal: portal, account: account, author_id: admin.id) } + + let(:translate_url) { "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles/bulk_actions/translate" } + + describe 'POST articles/bulk_actions/translate' do + context 'when unauthenticated' do + it 'returns unauthorized' do + post translate_url, params: { ids: [article_one.id], locale: 'es', category_id: category_es.id }, as: :json + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when authenticated as agent' do + it 'returns unauthorized' do + post translate_url, + headers: agent.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: category_es.id }, + as: :json + + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when captain is not enabled' do + it 'returns unprocessable entity' do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: category_es.id }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context 'when authenticated as admin' do + before do + account.enable_features!('captain_tasks') + end + + it 'enqueues translation jobs for each article' do + expect do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id, article_two.id], locale: 'es', category_id: category_es.id }, + as: :json + end.to have_enqueued_job(Captain::Articles::TranslateJob).exactly(2).times + + expect(response).to have_http_status(:ok) + end + + it 'enqueues job with correct arguments' do + expect do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: category_es.id }, + as: :json + end.to have_enqueued_job(Captain::Articles::TranslateJob).with( + account, article_one.id, 'es', category_es.id, admin + ) + + expect(response).to have_http_status(:ok) + end + + it 'returns unprocessable entity for invalid locale' do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'zh', category_id: category_es.id }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'returns unprocessable entity for invalid category' do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: 0 }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'returns unprocessable entity when category locale does not match requested locale' do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: category_en.id }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'enqueues job with nil category when category_id is omitted' do + expect do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es' }, + as: :json + end.to have_enqueued_job(Captain::Articles::TranslateJob).with( + account, article_one.id, 'es', nil, admin + ) + + expect(response).to have_http_status(:ok) + end + + it 'enqueues job with nil category when category_id is blank' do + expect do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: '' }, + as: :json + end.to have_enqueued_job(Captain::Articles::TranslateJob).with( + account, article_one.id, 'es', nil, admin + ) + + expect(response).to have_http_status(:ok) + end + + it 'returns unprocessable entity when no articles found' do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [0], locale: 'es', category_id: category_es.id }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + + context 'when translations already exist' do + let!(:existing_translation) do + create(:article, portal: portal, category: category_es, account: account, author_id: admin.id, + locale: 'es', associated_article_id: article_one.id) + end + + it 'returns conflict with duplicate articles' do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: category_es.id }, + as: :json + + expect(response).to have_http_status(:conflict) + body = response.parsed_body + expect(body['duplicate_articles'].length).to eq(1) + expect(body['duplicate_articles'].first['id']).to eq(existing_translation.id) + end + + it 'does not enqueue jobs when duplicates found without force' do + expect do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: category_es.id }, + as: :json + end.not_to have_enqueued_job(Captain::Articles::TranslateJob) + end + + it 'enqueues jobs when force is true' do + expect do + post translate_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], locale: 'es', category_id: category_es.id, force: true }, + as: :json + end.to have_enqueued_job(Captain::Articles::TranslateJob).exactly(1).times + + expect(response).to have_http_status(:ok) + end + end + end + end +end diff --git a/spec/enterprise/jobs/captain/articles/translate_job_spec.rb b/spec/enterprise/jobs/captain/articles/translate_job_spec.rb new file mode 100644 index 000000000..119c93b1c --- /dev/null +++ b/spec/enterprise/jobs/captain/articles/translate_job_spec.rb @@ -0,0 +1,134 @@ +require 'rails_helper' + +RSpec.describe Captain::Articles::TranslateJob, type: :job do + let(:account) { create(:account) } + let(:user) { create(:user, account: account, role: :administrator) } + let!(:portal) { create(:portal, account: account, config: { allowed_locales: %w[en es] }) } + let!(:category_en) { create(:category, portal: portal, account: account, locale: 'en', slug: 'getting-started') } + let!(:category_es) { create(:category, portal: portal, account: account, locale: 'es', slug: 'primeros-pasos') } + let!(:article) do + create(:article, portal: portal, category: category_en, account: account, author: user, + title: 'Getting Started', content: '# Welcome\nThis is a guide.') + end + + let(:title_service) { instance_double(Captain::Llm::ArticleTranslationService) } + let(:content_service) { instance_double(Captain::Llm::ArticleTranslationService) } + + before do + allow(Captain::Llm::ArticleTranslationService).to receive(:new).with(hash_including(type: :title)).and_return(title_service) + allow(Captain::Llm::ArticleTranslationService).to receive(:new).with(hash_including(type: :content)).and_return(content_service) + allow(title_service).to receive(:perform).and_return(message: 'Primeros pasos') + allow(content_service).to receive(:perform).and_return(message: '# Bienvenido\nEsta es una guía.') + end + + it 'queues on the low queue' do + expect { described_class.perform_later(account, article.id, 'es', category_es.id, user) } + .to have_enqueued_job.on_queue('low') + end + + it 'creates a translated article as draft' do + expect do + described_class.perform_now(account, article.id, 'es', category_es.id, user) + end.to change(Article, :count).by(1) + + translated = Article.last + expect(translated).to have_attributes( + title: 'Primeros pasos', + content: '# Bienvenido\nEsta es una guía.', + locale: 'es', + category_id: category_es.id, + author_id: user.id, + status: 'draft', + associated_article_id: article.id + ) + end + + it 'creates a translated article without a category when target_category_id is nil' do + expect do + described_class.perform_now(account, article.id, 'es', nil, user) + end.to change(Article, :count).by(1) + + translated = Article.last + expect(translated).to have_attributes( + title: 'Primeros pasos', + locale: 'es', + category_id: nil, + status: 'draft', + associated_article_id: article.id + ) + end + + it 'calls the translation service with the correct language' do + described_class.perform_now(account, article.id, 'es', category_es.id, user) + + expect(Captain::Llm::ArticleTranslationService).to have_received(:new).with( + account: account, text: 'Getting Started', target_language: 'Spanish', type: :title + ) + expect(Captain::Llm::ArticleTranslationService).to have_received(:new).with( + account: account, text: '# Welcome\nThis is a guide.', target_language: 'Spanish', type: :content + ) + end + + it 'uses language_map for locale name resolution' do + described_class.perform_now(account, article.id, 'pt_BR', category_es.id, user) + + expect(Captain::Llm::ArticleTranslationService).to have_received(:new).with( + hash_including(target_language: 'Portuguese (Brazil)', type: :title) + ) + end + + context 'when a translation already exists' do + let!(:existing_translation) do + create(:article, portal: portal, category: category_es, account: account, author: user, + title: 'Old title', content: 'Old content', locale: 'es', + associated_article_id: article.id) + end + + it 'updates the existing translation instead of creating a new one' do + expect do + described_class.perform_now(account, article.id, 'es', category_es.id, user) + end.not_to change(Article, :count) + + existing_translation.reload + expect(existing_translation).to have_attributes( + title: 'Primeros pasos', + content: '# Bienvenido\nEsta es una guía.', + description: article.description + ) + end + end + + context 'when the source article has blank content' do + let!(:draft_article) do + create(:article, portal: portal, category: category_en, account: account, author: user, + title: 'Empty draft', content: nil, status: :draft) + end + + it 'creates the translated article with the original blank content and skips the content LLM call' do + expect do + described_class.perform_now(account, draft_article.id, 'es', category_es.id, user) + end.to change(Article, :count).by(1) + + expect(content_service).not_to have_received(:perform) + translated = Article.last + expect(translated).to have_attributes( + title: 'Primeros pasos', + content: nil, + locale: 'es', + associated_article_id: draft_article.id + ) + end + end + + context 'when translation service fails' do + before do + allow(title_service).to receive(:perform).and_return(error: 'LLM timeout') + end + + it 'raises the error and does not create an article' do + expect do + described_class.perform_now(account, article.id, 'es', category_es.id, user) + end.to raise_error(RuntimeError, /LLM timeout/).and not_change(Article, :count) + end + end +end diff --git a/spec/enterprise/services/captain/llm/article_translation_service_spec.rb b/spec/enterprise/services/captain/llm/article_translation_service_spec.rb new file mode 100644 index 000000000..1c0d83b65 --- /dev/null +++ b/spec/enterprise/services/captain/llm/article_translation_service_spec.rb @@ -0,0 +1,67 @@ +require 'rails_helper' + +RSpec.describe Captain::Llm::ArticleTranslationService do + let(:account) { create(:account) } + let(:target_language) { 'Spanish' } + + before do + create(:installation_config, name: 'CAPTAIN_OPEN_AI_API_KEY', value: 'test-key') + allow(account).to receive(:feature_enabled?).and_call_original + allow(account).to receive(:feature_enabled?).with('captain_tasks').and_return(true) + end + + describe '#perform with type: :title' do + let(:service) do + described_class.new(account: account, text: 'Getting Started', target_language: target_language, type: :title) + end + + it 'returns the stripped translated title' do + expect(service).to receive(:make_api_call) do |args| + expect(args[:messages][0][:content]).to include('professional translator') + expect(args[:messages][0][:content]).to include(target_language) + expect(args[:messages][1][:content]).to eq('Getting Started') + { message: " Primeros pasos \n" } + end + + expect(service.perform).to include(message: 'Primeros pasos') + end + end + + describe '#perform with type: :content' do + let(:content) { "# Welcome\nSome markdown." } + let(:service) do + described_class.new(account: account, text: content, target_language: target_language, type: :content) + end + + it 'returns the stripped translated content using the markdown system prompt' do + expect(service).to receive(:make_api_call) do |args| + expect(args[:messages][0][:content]).to include('markdown') + expect(args[:messages][0][:content]).to include('Preserve ALL HTML tags') + expect(args[:messages][1][:content]).to eq(content) + { message: "# Bienvenido\nAlgo de markdown.\n" } + end + + expect(service.perform).to include(message: "# Bienvenido\nAlgo de markdown.") + end + end + + describe '#perform with an invalid type' do + it 'raises ArgumentError' do + service = described_class.new(account: account, text: 'hi', target_language: target_language, type: :invalid) + + expect { service.perform }.to raise_error(ArgumentError, /Invalid type/) + end + end + + describe '#perform when the API call fails' do + let(:service) do + described_class.new(account: account, text: 'Getting Started', target_language: target_language, type: :title) + end + + it 'returns the error hash unchanged' do + allow(service).to receive(:make_api_call).and_return(error: 'LLM timeout', error_code: 500) + + expect(service.perform).to eq(error: 'LLM timeout', error_code: 500) + end + end +end From 2ada713f29b2cf5f864cccd3f74d570e73f6b82f Mon Sep 17 00:00:00 2001 From: Pranav Date: Fri, 24 Apr 2026 09:13:43 -0700 Subject: [PATCH 07/32] feat: Add bulk actions for help center articles (translate, status change, delete) (#14137) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://linear.app/chatwoot/issue/CW-6950/support-bulk-actions-for-publish-archive-move-to-draft-delete-in How to test 1. Go to Help Center → Articles 2. Select articles using checkboxes → bulk bar appears 3. Click Publish/Draft/Archive → articles update, list refreshes 4. Click Delete → confirmation dialog → articles removed 5. Click Translate (requires Captain enabled) → select locale + category → translation starts 6. Try translating to a locale that already has translations → warning with links to existing articles → "Overwrite and translate" proceeds 8. Single article: click three-dot menu → Translate → same dialog opens for that article https://github.com/user-attachments/assets/7c76495e-f89e-4456-92bd-a6639a9992f4 --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../articles/bulk_actions_controller.rb | 24 ++ .../dashboard/api/helpCenter/articles.js | 14 ++ .../HelpCenter/ArticleCard/ArticleCard.vue | 47 +++- .../Pages/ArticlePage/ArticleList.vue | 29 ++- .../Pages/ArticlePage/ArticlesPage.vue | 231 +++++++++++++++++- .../dashboard/i18n/locale/en/helpCenter.json | 18 ++ .../pages/PortalsArticlesIndexPage.vue | 1 + config/locales/en.yml | 1 + config/routes.rb | 2 + .../articles/bulk_actions_controller_spec.rb | 141 +++++++++++ 10 files changed, 486 insertions(+), 22 deletions(-) create mode 100644 spec/controllers/api/v1/accounts/articles/bulk_actions_controller_spec.rb diff --git a/app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb b/app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb index 584e3dbf2..b45c16828 100644 --- a/app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb +++ b/app/controllers/api/v1/accounts/articles/bulk_actions_controller.rb @@ -1,11 +1,31 @@ class Api::V1::Accounts::Articles::BulkActionsController < Api::V1::Accounts::BaseController before_action :portal before_action :check_authorization + before_action :set_articles, only: [:update_status, :delete_articles] def translate head :not_implemented end + def update_status + return render_could_not_create_error(I18n.t('portals.articles.no_articles_found')) if @articles.none? + return render_could_not_create_error(I18n.t('portals.articles.invalid_status')) unless Article.statuses.key?(params[:status]) + + ActiveRecord::Base.transaction do + @articles.find_each { |article| article.update!(status: params[:status]) } + end + head :ok + rescue ActiveRecord::RecordInvalid => e + render_could_not_create_error(e.message) + end + + def delete_articles + return render_could_not_create_error(I18n.t('portals.articles.no_articles_found')) if @articles.none? + + @articles.destroy_all + head :ok + end + private def portal @@ -15,5 +35,9 @@ class Api::V1::Accounts::Articles::BulkActionsController < Api::V1::Accounts::Ba def check_authorization authorize(Article, :create?) end + + def set_articles + @articles = @portal.articles.where(id: params[:ids]) + end end Api::V1::Accounts::Articles::BulkActionsController.prepend_mod_with('Api::V1::Accounts::Articles::BulkActionsController') diff --git a/app/javascript/dashboard/api/helpCenter/articles.js b/app/javascript/dashboard/api/helpCenter/articles.js index 781570d0b..c79aa5da7 100644 --- a/app/javascript/dashboard/api/helpCenter/articles.js +++ b/app/javascript/dashboard/api/helpCenter/articles.js @@ -79,6 +79,20 @@ class ArticlesAPI extends PortalsAPI { { ids: articleIds, locale, category_id: categoryId, force } ); } + + bulkUpdateStatus({ portalSlug, articleIds, status }) { + return axios.patch( + `${this.url}/${portalSlug}/articles/bulk_actions/update_status`, + { ids: articleIds, status } + ); + } + + bulkDelete({ portalSlug, articleIds }) { + return axios.delete( + `${this.url}/${portalSlug}/articles/bulk_actions/delete_articles`, + { data: { ids: articleIds } } + ); + } } export default new ArticlesAPI(); diff --git a/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue b/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue index 25eda255a..93ff15251 100644 --- a/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue +++ b/app/javascript/dashboard/components-next/HelpCenter/ArticleCard/ArticleCard.vue @@ -17,6 +17,7 @@ import CardLayout from 'dashboard/components-next/CardLayout.vue'; import DropdownMenu from 'dashboard/components-next/dropdown-menu/DropdownMenu.vue'; import Button from 'dashboard/components-next/button/Button.vue'; import Avatar from 'dashboard/components-next/avatar/Avatar.vue'; +import Checkbox from 'dashboard/components-next/checkbox/Checkbox.vue'; const props = defineProps({ id: { @@ -47,9 +48,26 @@ const props = defineProps({ type: Number, required: true, }, + isSelected: { + type: Boolean, + default: false, + }, + selectable: { + type: Boolean, + default: false, + }, + showSelectionControl: { + type: Boolean, + default: false, + }, }); -const emit = defineEmits(['openArticle', 'articleAction']); +const emit = defineEmits([ + 'openArticle', + 'articleAction', + 'toggleSelect', + 'hover', +]); const { t } = useI18n(); @@ -143,14 +161,27 @@ const handleClick = id => { diff --git a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue index f1e177505..325f6abe6 100644 --- a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue +++ b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticlePage/ArticlesPage.vue @@ -1,9 +1,13 @@ + diff --git a/app/javascript/dashboard/i18n/locale/en/helpCenter.json b/app/javascript/dashboard/i18n/locale/en/helpCenter.json index bb9bf2e99..9ae849d25 100644 --- a/app/javascript/dashboard/i18n/locale/en/helpCenter.json +++ b/app/javascript/dashboard/i18n/locale/en/helpCenter.json @@ -590,6 +590,10 @@ "CATEGORY_PLACEHOLDER": "Select a category", "OPTIONAL": "(optional)", "CONFIRM": "Translate", + "SELECT_ALL": "Select all ({count})", + "SELECTED_COUNT": "{count} selected", + "CLEAR_SELECTION": "Clear selection", + "TRANSLATE_BUTTON": "Translate", "CONFIRM_OVERWRITE": "Overwrite and translate", "DUPLICATE_WARNING": "A translation already exists for this article in the selected language. | Translations already exist for {count} articles in the selected language.", "DUPLICATE_CONFIRM_HINT": "Click translate again to overwrite the existing translation.", @@ -597,6 +601,20 @@ "SUCCESS_MESSAGE": "Translation in progress. The article will appear as a draft once ready.", "ERROR_MESSAGE": "Failed to start translation. Please try again." } + }, + "BULK_ACTIONS": { + "PUBLISH": "Publish", + "DRAFT": "Draft", + "ARCHIVE": "Archive", + "TRANSLATE": "Translate", + "DELETE": "Delete", + "STATUS_SUCCESS": "Articles updated successfully", + "STATUS_ERROR": "Failed to update articles", + "DELETE_CONFIRM_TITLE": "Delete article | Delete {count} articles", + "DELETE_CONFIRM_DESCRIPTION": "This will permanently delete the selected article. This action cannot be undone. | This will permanently delete {count} selected articles. This action cannot be undone.", + "DELETE_CONFIRM": "Delete", + "DELETE_SUCCESS": "Articles deleted successfully", + "DELETE_ERROR": "Failed to delete articles" } }, "CATEGORY_PAGE": { diff --git a/app/javascript/dashboard/routes/dashboard/helpcenter/pages/PortalsArticlesIndexPage.vue b/app/javascript/dashboard/routes/dashboard/helpcenter/pages/PortalsArticlesIndexPage.vue index 7d636de70..c0fc800e6 100644 --- a/app/javascript/dashboard/routes/dashboard/helpcenter/pages/PortalsArticlesIndexPage.vue +++ b/app/javascript/dashboard/routes/dashboard/helpcenter/pages/PortalsArticlesIndexPage.vue @@ -119,6 +119,7 @@ watch( :is-category-articles="isCategoryArticles" @page-change="onPageChange" @fetch-portal="fetchPortalAndItsCategories" + @refresh-articles="fetchArticles" /> diff --git a/config/locales/en.yml b/config/locales/en.yml index 36f115bab..9ffd3f3d5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -493,6 +493,7 @@ en: locale_not_available: 'Locale not available in this portal' category_not_found: 'Category not found in this portal' no_articles_found: 'No articles found to process' + invalid_status: 'Invalid status value' send_instructions: email_required: 'Email is required' invalid_email_format: 'Invalid email format' diff --git a/config/routes.rb b/config/routes.rb index c6111d317..a1d3d088e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -361,6 +361,8 @@ Rails.application.routes.draw do namespace :articles do resource :bulk_actions, only: [] do post :translate + patch :update_status + delete :delete_articles end end resources :articles do diff --git a/spec/controllers/api/v1/accounts/articles/bulk_actions_controller_spec.rb b/spec/controllers/api/v1/accounts/articles/bulk_actions_controller_spec.rb new file mode 100644 index 000000000..3dab5b60f --- /dev/null +++ b/spec/controllers/api/v1/accounts/articles/bulk_actions_controller_spec.rb @@ -0,0 +1,141 @@ +require 'rails_helper' + +RSpec.describe 'Article Bulk Actions API', type: :request do + let(:account) { create(:account) } + let(:admin) { create(:user, account: account, role: :administrator) } + let(:agent) { create(:user, account: account, role: :agent) } + let!(:portal) { create(:portal, name: 'test_portal', account: account, config: { allowed_locales: %w[en es] }) } + let!(:category) { create(:category, portal: portal, account: account, locale: 'en', slug: 'getting-started') } + let!(:article_one) { create(:article, category: category, portal: portal, account: account, author: admin, status: :draft) } + let!(:article_two) { create(:article, category: category, portal: portal, account: account, author: admin, status: :draft) } + let!(:article_three) { create(:article, category: category, portal: portal, account: account, author: admin, status: :published) } + + let(:base_url) { "/api/v1/accounts/#{account.id}/portals/#{portal.slug}/articles/bulk_actions" } + + describe 'PATCH articles/bulk_actions/update_status' do + let(:update_status_url) { "#{base_url}/update_status" } + + context 'when unauthenticated' do + it 'returns unauthorized' do + patch update_status_url, params: { ids: [article_one.id], status: 'published' }, as: :json + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when authenticated as agent' do + it 'returns unauthorized' do + patch update_status_url, + headers: agent.create_new_auth_token, + params: { ids: [article_one.id], status: 'published' }, + as: :json + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when authenticated as admin' do + it 'publishes multiple articles' do + patch update_status_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id, article_two.id], status: 'published' }, + as: :json + + expect(response).to have_http_status(:ok) + expect(article_one.reload.status).to eq('published') + expect(article_two.reload.status).to eq('published') + end + + it 'archives multiple articles' do + patch update_status_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id, article_three.id], status: 'archived' }, + as: :json + + expect(response).to have_http_status(:ok) + expect(article_one.reload.status).to eq('archived') + expect(article_three.reload.status).to eq('archived') + end + + it 'sets articles to draft' do + patch update_status_url, + headers: admin.create_new_auth_token, + params: { ids: [article_three.id], status: 'draft' }, + as: :json + + expect(response).to have_http_status(:ok) + expect(article_three.reload.status).to eq('draft') + end + + it 'does not affect articles not in the list' do + patch update_status_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id], status: 'published' }, + as: :json + + expect(article_one.reload.status).to eq('published') + expect(article_three.reload.status).to eq('published') + end + + it 'returns unprocessable entity when no articles found' do + patch update_status_url, + headers: admin.create_new_auth_token, + params: { ids: [0], status: 'published' }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + describe 'DELETE articles/bulk_actions/delete_articles' do + let(:destroy_url) { "#{base_url}/delete_articles" } + + context 'when unauthenticated' do + it 'returns unauthorized' do + delete destroy_url, params: { ids: [article_one.id] }, as: :json + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when authenticated as agent' do + it 'returns unauthorized' do + delete destroy_url, + headers: agent.create_new_auth_token, + params: { ids: [article_one.id] }, + as: :json + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when authenticated as admin' do + it 'deletes multiple articles' do + expect do + delete destroy_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id, article_two.id] }, + as: :json + end.to change(Article, :count).by(-2) + + expect(response).to have_http_status(:ok) + end + + it 'does not delete articles not in the list' do + delete destroy_url, + headers: admin.create_new_auth_token, + params: { ids: [article_one.id] }, + as: :json + + expect(Article.exists?(article_one.id)).to be(false) + expect(Article.exists?(article_three.id)).to be(true) + end + + it 'returns unprocessable entity when no articles found' do + delete destroy_url, + headers: admin.create_new_auth_token, + params: { ids: [0] }, + as: :json + + expect(response).to have_http_status(:unprocessable_entity) + end + end + end +end From a651949c33383b57d08e21d2210916b1ae2e9363 Mon Sep 17 00:00:00 2001 From: Aakash Bakhle <48802744+aakashb95@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:01:44 +0530 Subject: [PATCH 08/32] fix: improve FAQ generation [AI-145] (#14062) # Pull Request Template ## Description - Fetch main content only from Firecrawl, exclude some tags to remove boilerplate - Prompt changes for FAQ generation ## Type of change Please delete options that are not relevant. - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration. tested locally ## 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 - [ ] 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 --- .../captain/documents/response_builder_job.rb | 2 +- enterprise/app/models/captain/document.rb | 4 +++ .../captain/llm/faq_generator_service.rb | 16 ++++++++---- .../llm/paginated_faq_generator_service.rb | 14 +++++------ .../captain/llm/system_prompts_service.rb | 25 ++++++++++++------- .../captain/tools/firecrawl_service.rb | 17 ++++++++----- .../documents/response_builder_job_spec.rb | 13 +++------- .../captain/llm/faq_generator_service_spec.rb | 11 ++++---- .../captain/tools/firecrawl_service_spec.rb | 4 +-- 9 files changed, 62 insertions(+), 44 deletions(-) diff --git a/enterprise/app/jobs/captain/documents/response_builder_job.rb b/enterprise/app/jobs/captain/documents/response_builder_job.rb index 60f1fa2cf..8f6643e36 100644 --- a/enterprise/app/jobs/captain/documents/response_builder_job.rb +++ b/enterprise/app/jobs/captain/documents/response_builder_job.rb @@ -26,7 +26,7 @@ class Captain::Documents::ResponseBuilderJob < ApplicationJob end def generate_standard_faqs(document) - Captain::Llm::FaqGeneratorService.new(document.content, document.account.locale_english_name, account_id: document.account_id).generate + Captain::Llm::FaqGeneratorService.new(document: document).generate end def build_paginated_service(document, options) diff --git a/enterprise/app/models/captain/document.rb b/enterprise/app/models/captain/document.rb index 2abf18437..c2b5fa214 100644 --- a/enterprise/app/models/captain/document.rb +++ b/enterprise/app/models/captain/document.rb @@ -117,6 +117,10 @@ class Captain::Document < ApplicationRecord end end + def to_llm_metadata + { document_id: id, assistant_id: assistant_id, external_link: external_link } + end + private def enqueue_crawl_job diff --git a/enterprise/app/services/captain/llm/faq_generator_service.rb b/enterprise/app/services/captain/llm/faq_generator_service.rb index 5f85ae467..b80382b3e 100644 --- a/enterprise/app/services/captain/llm/faq_generator_service.rb +++ b/enterprise/app/services/captain/llm/faq_generator_service.rb @@ -1,11 +1,12 @@ class Captain::Llm::FaqGeneratorService < Llm::BaseAiService include Integrations::LlmInstrumentation - def initialize(content, language = 'english', account_id: nil) + def initialize(document:) super() - @language = language - @content = content - @account_id = account_id + @document = document + @content = document.content + @language = document.account.locale_english_name + @account_id = document.account_id end def generate @@ -40,10 +41,15 @@ class Captain::Llm::FaqGeneratorService < Llm::BaseAiService messages: [ { role: 'system', content: system_prompt }, { role: 'user', content: @content } - ] + ], + metadata: document_metadata } end + def document_metadata + @document&.to_llm_metadata || {} + end + def parse_response(content) return [] if content.nil? diff --git a/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb b/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb index 3fe81c2ae..b567609e8 100644 --- a/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb +++ b/enterprise/app/services/captain/llm/paginated_faq_generator_service.rb @@ -51,7 +51,8 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::LegacyBaseOpenAiService account_id: @document&.account_id, feature_name: 'faq_generation', model: @model, - messages: params[:messages] + messages: params[:messages], + metadata: document_metadata } response = instrument_llm_call(instrumentation_params) do @@ -214,12 +215,11 @@ class Captain::Llm::PaginatedFaqGeneratorService < Llm::LegacyBaseOpenAiService feature_name: 'paginated_faq_generation', model: @model, messages: params[:messages], - metadata: { - document_id: @document&.id, - start_page: start_page, - end_page: end_page, - iteration: @iterations_completed + 1 - } + metadata: document_metadata.merge(start_page: start_page, end_page: end_page, iteration: @iterations_completed + 1) } end + + def document_metadata + @document&.to_llm_metadata || {} + end end diff --git a/enterprise/app/services/captain/llm/system_prompts_service.rb b/enterprise/app/services/captain/llm/system_prompts_service.rb index 9868f0360..dab147301 100644 --- a/enterprise/app/services/captain/llm/system_prompts_service.rb +++ b/enterprise/app/services/captain/llm/system_prompts_service.rb @@ -3,11 +3,15 @@ class Captain::Llm::SystemPromptsService class << self def faq_generator(language = 'english') <<~PROMPT - You are a content writer specializing in creating good FAQ sections for website help centers. Your task is to convert provided content into a structured FAQ format without losing any information. + You are a content writer specializing in creating good FAQ sections for website help centers. Your task is to convert provided content into a structured FAQ format without losing any substantive information. ## Core Requirements - **Completeness**: Extract ALL information from the source content. Every detail, example, procedure, and explanation must be captured across the FAQ set. When combined, the FAQs should reconstruct the original content entirely. + **Completeness**: Extract ALL substantive information from the source content. Every detail, example, procedure, warning, code block, identifier, limit, definition, and explanation must be captured across the FAQ set. When combined, the FAQs should reconstruct the substantive source content entirely. + + **Self-contained answers**: Every answer must contain the information that answers its question. The answer must be the substance, not directions to where the substance lives. If a source section provides only a reference, link, or pointer to where the information can be found — without containing that information itself — omit the FAQ for that section. An FAQ whose answer redirects the reader is worse than no FAQ at all. + + **Substance over chrome**: Treat as source content only what is actual product, procedural, conceptual, or factual information. Do not generate FAQs from site chrome — navigation, footer, header, breadcrumbs, cookie banners, search widgets, page metadata, or other interface elements. **Accuracy**: Base answers strictly on the provided text. Do not add assumptions, interpretations, or external knowledge not present in the source material. @@ -29,18 +33,21 @@ class Captain::Llm::SystemPromptsService ## Guidelines - **Question Creation**: Formulate questions that naturally arise from the content (What is...? How do I...? When should...? Why does...?). Do not generate questions that are not related to the content. - - **Answer Completeness**: Include all relevant details, steps, examples, and context from the original content - - **Information Preservation**: Ensure no examples, procedures, warnings, or explanatory details are omitted + - **Answer Completeness**: Include all relevant details, steps, examples, code, identifiers, limits, and definitions present in the source. + - **Information Preservation**: Never omit examples, procedures, warnings, code, IDs, limits, or definitions in the name of brevity. + - **No Deflecting FAQs**: Do not create FAQs whose answer would only tell the reader to open another link, guide, or document. If the source contains useful factual content in link text, labels, lists, or summaries (e.g., a curated list of supported integrations, plan features, resources, or article indexes), preserve that content as the answer. If it only points elsewhere without providing the answer itself, skip it. - **JSON Validity**: Always return properly formatted, valid JSON - **No Content Scenario**: If no suitable content is found, return: `{"faqs": []}` ## Process 1. Read the entire provided content carefully - 2. Identify all key information points, procedures, and examples - 3. Create questions that cover each information point - 4. Write comprehensive short answers that capture all related detail, include bullet points if needed. - 5. Verify that combined FAQs represent the complete original content. - 6. Format as valid JSON + 2. Identify all key information points: procedures, examples, code, identifiers, limits, definitions, warnings, and explanations + 3. For each candidate section, verify the source contains the substance that would answer the question. If the source only points to where the substance lives, skip the section. + 4. Disregard interface chrome (navigation, footer, header, cookie banners, breadcrumbs, page metadata). + 5. Create questions that cover each remaining substantive information point + 6. Write self-contained answers that preserve all relevant details from the source. Be concise where possible, but never trade away steps, examples, warnings, code, IDs, limits, or definitions for brevity. + 7. Verify the combined FAQs represent the complete substantive source content (excluding redirect-only sections and chrome). + 8. Format as valid JSON PROMPT end diff --git a/enterprise/app/services/captain/tools/firecrawl_service.rb b/enterprise/app/services/captain/tools/firecrawl_service.rb index fc7448593..3d1b53b7a 100644 --- a/enterprise/app/services/captain/tools/firecrawl_service.rb +++ b/enterprise/app/services/captain/tools/firecrawl_service.rb @@ -1,5 +1,6 @@ class Captain::Tools::FirecrawlService BASE_URL = 'https://api.firecrawl.dev/v1'.freeze + FIRECRAWL_EXCLUDE_TAGS = %w[iframe .sidebar .cookie-banner [role=navigation] [role=banner] [role=contentinfo]].freeze def initialize @api_key = InstallationConfig.find_by!(name: 'CAPTAIN_FIRECRAWL_API_KEY').value @@ -33,16 +34,20 @@ class Captain::Tools::FirecrawlService ignoreSitemap: false, limit: crawl_limit, webhook: webhook_url, - scrapeOptions: { - onlyMainContent: false, - formats: ['markdown'], - excludeTags: ['iframe'] - } + scrapeOptions: scrape_options }.to_json end def scrape_payload(url) - { url: url, formats: ['markdown'], excludeTags: ['iframe'] }.to_json + { url: url }.merge(scrape_options).to_json + end + + def scrape_options + { + onlyMainContent: true, + formats: ['markdown'], + excludeTags: FIRECRAWL_EXCLUDE_TAGS + } end def headers diff --git a/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb b/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb index c3e5eab1c..4d1a07aa7 100644 --- a/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb +++ b/spec/enterprise/jobs/captain/documents/response_builder_job_spec.rb @@ -12,9 +12,7 @@ RSpec.describe Captain::Documents::ResponseBuilderJob, type: :job do end before do - allow(Captain::Llm::FaqGeneratorService).to receive(:new) - .with(document.content, document.account.locale_english_name, account_id: document.account_id) - .and_return(faq_generator) + allow(Captain::Llm::FaqGeneratorService).to receive(:new).with(document: document).and_return(faq_generator) allow(faq_generator).to receive(:generate).and_return(faqs) end @@ -51,17 +49,14 @@ RSpec.describe Captain::Documents::ResponseBuilderJob, type: :job do let(:spanish_faq_generator) { instance_double(Captain::Llm::FaqGeneratorService) } before do - allow(Captain::Llm::FaqGeneratorService).to receive(:new) - .with(spanish_document.content, 'portuguese', account_id: spanish_document.account_id) - .and_return(spanish_faq_generator) + allow(Captain::Llm::FaqGeneratorService).to receive(:new).with(document: spanish_document).and_return(spanish_faq_generator) allow(spanish_faq_generator).to receive(:generate).and_return(faqs) end - it 'passes the correct locale to FAQ generator' do + it 'passes the correct document to FAQ generator' do described_class.new.perform(spanish_document) - expect(Captain::Llm::FaqGeneratorService).to have_received(:new) - .with(spanish_document.content, 'portuguese', account_id: spanish_document.account_id) + expect(Captain::Llm::FaqGeneratorService).to have_received(:new).with(document: spanish_document) end end diff --git a/spec/enterprise/services/captain/llm/faq_generator_service_spec.rb b/spec/enterprise/services/captain/llm/faq_generator_service_spec.rb index 003d5b715..ff7138c9a 100644 --- a/spec/enterprise/services/captain/llm/faq_generator_service_spec.rb +++ b/spec/enterprise/services/captain/llm/faq_generator_service_spec.rb @@ -2,8 +2,8 @@ require 'rails_helper' RSpec.describe Captain::Llm::FaqGeneratorService do let(:content) { 'Sample content for FAQ generation' } - let(:language) { 'english' } - let(:service) { described_class.new(content, language) } + let(:document) { create(:captain_document, content: content) } + let(:service) { described_class.new(document: document) } let(:mock_chat) { instance_double(RubyLLM::Chat) } let(:sample_faqs) do [ @@ -36,14 +36,15 @@ RSpec.describe Captain::Llm::FaqGeneratorService do service.generate end - it 'uses SystemPromptsService with the specified language' do - expect(Captain::Llm::SystemPromptsService).to receive(:faq_generator).with(language).at_least(:once).and_call_original + it 'uses SystemPromptsService with the account language' do + account_language = document.account.locale_english_name + expect(Captain::Llm::SystemPromptsService).to receive(:faq_generator).with(account_language).at_least(:once).and_call_original service.generate end end context 'with different language' do - let(:language) { 'spanish' } + before { allow(document.account).to receive(:locale_english_name).and_return('spanish') } it 'passes the correct language to SystemPromptsService' do expect(Captain::Llm::SystemPromptsService).to receive(:faq_generator).with('spanish').at_least(:once).and_call_original diff --git a/spec/enterprise/services/captain/tools/firecrawl_service_spec.rb b/spec/enterprise/services/captain/tools/firecrawl_service_spec.rb index d6563b163..b46633a6e 100644 --- a/spec/enterprise/services/captain/tools/firecrawl_service_spec.rb +++ b/spec/enterprise/services/captain/tools/firecrawl_service_spec.rb @@ -58,9 +58,9 @@ RSpec.describe Captain::Tools::FirecrawlService do limit: crawl_limit, webhook: webhook_url, scrapeOptions: { - onlyMainContent: false, + onlyMainContent: true, formats: ['markdown'], - excludeTags: ['iframe'] + excludeTags: Captain::Tools::FirecrawlService::FIRECRAWL_EXCLUDE_TAGS } }.to_json end From 8faa5a74b1ce27248f15aa2bce9a2c6c56d227d7 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:30:51 +0530 Subject: [PATCH 09/32] fix: prevent focus jump to title after new article auto-creates (#14145) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request Template ## Description When creating a help center article, typing a title and navigating into the content auto-creates the article and switches the route (`/articles/new` → `/articles/.../edit/:slug`). During this transition, focus was jumping back to the title, interrupting editing. This happened because `ArticleEditor` always autofocuses the title. On route change, the component remounts and re-triggers focus. Now, after auto-create, focus stays in the body as expected. Fixes https://linear.app/chatwoot/issue/CW-6951/issue-with-the-cursor-position-on-the-help-center-article-when ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? **Screencast** https://github.com/user-attachments/assets/dac3f7c6-08c4-4df2-afb0-7731ee76424b ## Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] 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 - [ ] Any dependent changes have been merged and published in downstream modules --- .../HelpCenter/Pages/ArticleEditorPage/ArticleEditor.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticleEditorPage/ArticleEditor.vue b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticleEditorPage/ArticleEditor.vue index 22cb1441a..831312e0b 100644 --- a/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticleEditorPage/ArticleEditor.vue +++ b/app/javascript/dashboard/components-next/HelpCenter/Pages/ArticleEditorPage/ArticleEditor.vue @@ -121,7 +121,7 @@ const handleCreateArticle = event => { custom-text-area-class="!text-[32px] !leading-[48px] !font-medium !tracking-[0.2px]" custom-text-area-wrapper-class="border-0 !bg-transparent dark:!bg-transparent !py-0 !px-0" placeholder="Title" - autofocus + :autofocus="isNewArticle" @blur="handleCreateArticle" /> { t('HELP_CENTER.EDIT_ARTICLE_PAGE.EDIT_ARTICLE.EDITOR_PLACEHOLDER') " :enabled-menu-options="ARTICLE_EDITOR_MENU_OPTIONS" - :autofocus="false" + :autofocus="!isNewArticle" /> From 06467057be17d900450f8eac98cb7845a65bdf29 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Mon, 27 Apr 2026 13:31:43 +0530 Subject: [PATCH 10/32] fix: oversized email signature images in Letter render (#14144) # Pull Request Template ## Description This PR fixes an issue where signature images (with `?cw_image_height=...`) render at their original large size in the email bubble. ### Cause Renderer output: ```html ``` Email UI and clients (Gmail, Outlook) apply CSS like: `img { max-width: 100%; height: auto; }` This overrides `height="24px"`. Other channels work because they use inline styles (`style="height: 24px;"`). ### Solution Use inline style instead: ```html ``` ### Why backend fix * Fixes root cause and aligns Ruby + JS renderers * Works in both Chatwoot UI and recipient inboxes * Covers all email-rendered content * Minimal change Fixes https://linear.app/chatwoot/issue/CW-6948/email-signature-image-renders-oversized-in-chatwoot-ui ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? #### Screenshots **Before** image **After** image ## 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 - [ ] 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 - [ ] Any dependent changes have been merged and published in downstream modules --- lib/base_markdown_renderer.rb | 8 ++++++-- spec/lib/base_markdown_renderer_spec.rb | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/base_markdown_renderer.rb b/lib/base_markdown_renderer.rb index df49918b3..f530e71ee 100644 --- a/lib/base_markdown_renderer.rb +++ b/lib/base_markdown_renderer.rb @@ -29,11 +29,15 @@ class BaseMarkdownRenderer < CommonMarker::HtmlRenderer def render_img_tag(src, title, height = nil) title_attribute = title.present? ? " title=\"#{title}\"" : '' - height_attribute = height ? " height=\"#{height}\" width=\"auto\"" : '' + # Use inline style instead of the HTML height attribute: email clients and + # the in-app Letter view both run images through CSS (e.g. prose / + # lettersanitizer's `img { height: auto }`) which overrides presentational + # attributes. Inline style has higher specificity and survives. + style_attribute = height ? " style=\"height: #{height};\"" : '' plain do # plain ensures that the content is not wrapped in a paragraph tag - out("") + out("") end end end diff --git a/spec/lib/base_markdown_renderer_spec.rb b/spec/lib/base_markdown_renderer_spec.rb index 262e78daf..f8bdae4be 100644 --- a/spec/lib/base_markdown_renderer_spec.rb +++ b/spec/lib/base_markdown_renderer_spec.rb @@ -12,7 +12,7 @@ describe BaseMarkdownRenderer do context 'when image has a height' do it 'renders the img tag with the correct attributes' do markdown = '![Sample Title](https://example.com/image.jpg?cw_image_height=100)' - expect(render_markdown(markdown)).to include('') + expect(render_markdown(markdown)).to include('') end end From 0920a01e662163c13714fd6bd393b292df89151d Mon Sep 17 00:00:00 2001 From: Sojan Jose Date: Mon, 27 Apr 2026 15:40:00 +0530 Subject: [PATCH 11/32] fix(i18n): align pluralization with locale rules (#14266) Loads Rails locale-specific pluralization rules so languages with an `other`-only plural model can safely use Crowdin exports without maintaining duplicate `one` keys. ## Closes None ## Why Crowdin exports Rails YAML pluralized strings using each target language's plural categories. These categories come from Unicode CLDR and represent grammatical forms, not a literal "number is 1" bucket. Some languages need separate forms such as `one` and `other`, but languages like Japanese, Korean, Indonesian, Thai, Vietnamese, and Chinese use the same form for `1`, `2`, `5`, and larger counts in these strings. For those locales, CLDR correctly models the plural category as `other` only. Before this change, Chatwoot still relied on Rails' default English-style plural behavior for these locales. That meant a valid Crowdin export containing only `other` could fail at runtime when Rails received `count: 1` and looked for a missing `one` branch. Keeping duplicate `one` keys would only fight Crowdin on every translation sync. The runtime should instead follow the locale's plural rules. ## What changed - Added `rails-i18n` and enabled only its pluralization module. - Added explicit `other`-only plural rules for Chatwoot's underscore Chinese locale aliases, `zh_CN` and `zh_TW`. - Removed redundant `one` keys from the affected Devise and `time_units` translations. ## Validation - Ran a Rails runner check across `id`, `ja`, `ko`, `ms`, `th`, `vi`, `zh_CN`, and `zh_TW` to verify `errors.messages.not_saved` and `time_units.days` resolve with only `other` for `count: 1`. - Ran YAML parse validation for all edited locale files. - Ran `bundle exec rubocop Gemfile config/application.rb config/initializers/i18n_pluralization.rb`. --- Gemfile | 1 + Gemfile.lock | 4 ++++ config/application.rb | 1 + config/initializers/i18n_pluralization.rb | 8 ++++++++ config/locales/devise.id.yml | 1 - config/locales/devise.ja.yml | 1 - config/locales/devise.ko.yml | 1 - config/locales/devise.ms.yml | 1 - config/locales/devise.th.yml | 1 - config/locales/devise.vi.yml | 1 - config/locales/devise.zh_CN.yml | 1 - config/locales/devise.zh_TW.yml | 1 - config/locales/id.yml | 4 ---- config/locales/ja.yml | 4 ---- config/locales/ko.yml | 4 ---- config/locales/ms.yml | 4 ---- config/locales/th.yml | 4 ---- config/locales/vi.yml | 4 ---- config/locales/zh_CN.yml | 4 ---- config/locales/zh_TW.yml | 4 ---- 20 files changed, 14 insertions(+), 40 deletions(-) create mode 100644 config/initializers/i18n_pluralization.rb diff --git a/Gemfile b/Gemfile index a5068e765..c4989c538 100644 --- a/Gemfile +++ b/Gemfile @@ -84,6 +84,7 @@ gem 'barnes' gem 'devise', '>= 4.9.4' gem 'devise-secure_password', git: 'https://github.com/chatwoot/devise-secure_password', branch: 'chatwoot' gem 'devise_token_auth', '>= 1.2.3' +gem 'rails-i18n', '~> 7.0' # two-factor authentication gem 'devise-two-factor', '>= 5.0.0' # authorization diff --git a/Gemfile.lock b/Gemfile.lock index b77e5880f..7d29e0b02 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -727,6 +727,9 @@ 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) + rails-i18n (7.0.10) + i18n (>= 0.7, < 2) + railties (>= 6.0.0, < 8) railties (7.1.5.2) actionpack (= 7.1.5.2) activesupport (= 7.1.5.2) @@ -1125,6 +1128,7 @@ DEPENDENCIES rack-mini-profiler (>= 3.2.0) rack-timeout rails (~> 7.1) + rails-i18n (~> 7.0) redis redis-namespace responders (>= 3.1.1) diff --git a/config/application.rb b/config/application.rb index aa150794a..08f0451c1 100644 --- a/config/application.rb +++ b/config/application.rb @@ -37,6 +37,7 @@ module Chatwoot class Application < Rails::Application # Initialize configuration defaults for originally generated Rails version. config.load_defaults 7.0 + config.rails_i18n.enabled_modules = [:pluralization] config.eager_load_paths << Rails.root.join('lib') config.eager_load_paths << Rails.root.join('enterprise/lib') diff --git a/config/initializers/i18n_pluralization.rb b/config/initializers/i18n_pluralization.rb new file mode 100644 index 000000000..c4fc3fb4b --- /dev/null +++ b/config/initializers/i18n_pluralization.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +other_plural_rule = ->(_count) { :other } + +Rails.application.config.after_initialize do + I18n.backend.store_translations(:zh_CN, i18n: { plural: { rule: other_plural_rule } }) + I18n.backend.store_translations(:zh_TW, i18n: { plural: { rule: other_plural_rule } }) +end diff --git a/config/locales/devise.id.yml b/config/locales/devise.id.yml index 71b9f46fb..fc6bfb26a 100644 --- a/config/locales/devise.id.yml +++ b/config/locales/devise.id.yml @@ -57,5 +57,4 @@ id: not_found: "tidak ditemukan" not_locked: "tidak terkunci" not_saved: - one: "%{count} kesalahan mengakibatkan %{resource} ini tidak dapat disimpan:" other: "%{count} kesalahan mengakibatkan %{resource} ini tidak dapat disimpan:" diff --git a/config/locales/devise.ja.yml b/config/locales/devise.ja.yml index 043cd8351..a5840c6fc 100644 --- a/config/locales/devise.ja.yml +++ b/config/locales/devise.ja.yml @@ -57,5 +57,4 @@ ja: not_found: "見つかりませんでした" not_locked: "はロックされていません" not_saved: - one: "%{count} 個のエラーが発生し、 %{resource} を保存できませんでした：" other: "%{count} 個のエラーが発生し、 %{resource} を保存できませんでした：" diff --git a/config/locales/devise.ko.yml b/config/locales/devise.ko.yml index 846664ec9..5afb6c1c2 100644 --- a/config/locales/devise.ko.yml +++ b/config/locales/devise.ko.yml @@ -57,5 +57,4 @@ ko: not_found: "찾을 수 없습니다" not_locked: "잠겨 있지 않습니다" not_saved: - one: "%{count}개의 오류로 인해 이 %{resource}을(를) 저장할 수 없습니다:" other: "%{count}개의 오류로 인해 이 %{resource}을(를) 저장할 수 없습니다:" diff --git a/config/locales/devise.ms.yml b/config/locales/devise.ms.yml index ebcfe89e3..cecd08588 100644 --- a/config/locales/devise.ms.yml +++ b/config/locales/devise.ms.yml @@ -57,5 +57,4 @@ ms: not_found: "not found" not_locked: "was not locked" not_saved: - one: "%{count} errors prohibited this %{resource} from being saved:" other: "%{count} errors prohibited this %{resource} from being saved:" diff --git a/config/locales/devise.th.yml b/config/locales/devise.th.yml index c9f52018d..18e1572bb 100644 --- a/config/locales/devise.th.yml +++ b/config/locales/devise.th.yml @@ -57,5 +57,4 @@ th: not_found: "not found" not_locked: "was not locked" not_saved: - one: "%{count} errors prohibited this %{resource} from being saved:" other: "%{count} errors prohibited this %{resource} from being saved:" diff --git a/config/locales/devise.vi.yml b/config/locales/devise.vi.yml index 947e756f3..15dca044a 100644 --- a/config/locales/devise.vi.yml +++ b/config/locales/devise.vi.yml @@ -57,5 +57,4 @@ vi: not_found: "không tìm thấy" not_locked: "không được khoá" not_saved: - one: "Có %{count} lỗi được tìm thấy từ %{resource}:" other: "Có %{count} lỗi được tìm thấy từ %{resource}:" diff --git a/config/locales/devise.zh_CN.yml b/config/locales/devise.zh_CN.yml index 00f239948..2bf2831a8 100644 --- a/config/locales/devise.zh_CN.yml +++ b/config/locales/devise.zh_CN.yml @@ -57,5 +57,4 @@ zh_CN: not_found: "找不到" not_locked: "未锁定" not_saved: - one: "%{count} 个错误禁止保存 %{resource}：" other: "%{count} 个错误禁止保存 %{resource}：" diff --git a/config/locales/devise.zh_TW.yml b/config/locales/devise.zh_TW.yml index c5bd49450..f892bf796 100644 --- a/config/locales/devise.zh_TW.yml +++ b/config/locales/devise.zh_TW.yml @@ -57,5 +57,4 @@ zh_TW: not_found: "找不到。" not_locked: "並未被鎖定。" not_saved: - one: "有 %{count} 個錯誤導致 %{resource} 不能被儲存：" other: "有 %{count} 個錯誤導致 %{resource} 不能被儲存：" diff --git a/config/locales/id.yml b/config/locales/id.yml index aefcff3fe..b097de83d 100644 --- a/config/locales/id.yml +++ b/config/locales/id.yml @@ -435,16 +435,12 @@ id: button: Buka percakapan time_units: days: - one: '%{count} days' other: '%{count} days' hours: - one: '%{count} hours' other: '%{count} hours' minutes: - one: '%{count} minutes' other: '%{count} minutes' seconds: - one: '%{count} seconds' other: '%{count} seconds' auto_assignment: default_policy_name: 'Default Policy' diff --git a/config/locales/ja.yml b/config/locales/ja.yml index 5e3412378..deca85ebf 100644 --- a/config/locales/ja.yml +++ b/config/locales/ja.yml @@ -435,16 +435,12 @@ ja: button: 会話を開く time_units: days: - one: '%{count} 日' other: '%{count} 日' hours: - one: '%{count} 時間' other: '%{count} 時間' minutes: - one: '%{count} 分' other: '%{count} 分' seconds: - one: '%{count} 秒' other: '%{count} 秒' auto_assignment: default_policy_name: 'Default Policy' diff --git a/config/locales/ko.yml b/config/locales/ko.yml index c010ee2ac..153f39928 100644 --- a/config/locales/ko.yml +++ b/config/locales/ko.yml @@ -435,16 +435,12 @@ ko: button: 대화 열기 time_units: days: - one: '%{count}일' other: '%{count}일' hours: - one: '%{count}시간' other: '%{count}시간' minutes: - one: '%{count}분' other: '%{count}분' seconds: - one: '%{count}초' other: '%{count}초' auto_assignment: default_policy_name: '기본 정책' diff --git a/config/locales/ms.yml b/config/locales/ms.yml index 617056d7e..e1ee39aee 100644 --- a/config/locales/ms.yml +++ b/config/locales/ms.yml @@ -435,16 +435,12 @@ ms: button: Open conversation time_units: days: - one: '%{count} days' other: '%{count} days' hours: - one: '%{count} hours' other: '%{count} hours' minutes: - one: '%{count} minutes' other: '%{count} minutes' seconds: - one: '%{count} seconds' other: '%{count} seconds' auto_assignment: default_policy_name: 'Default Policy' diff --git a/config/locales/th.yml b/config/locales/th.yml index b278ec442..aeef51e95 100644 --- a/config/locales/th.yml +++ b/config/locales/th.yml @@ -435,16 +435,12 @@ th: button: เปิดดูการสนทนา time_units: days: - one: '%{count} days' other: '%{count} days' hours: - one: '%{count} hours' other: '%{count} hours' minutes: - one: '%{count} minutes' other: '%{count} minutes' seconds: - one: '%{count} seconds' other: '%{count} seconds' auto_assignment: default_policy_name: 'Default Policy' diff --git a/config/locales/vi.yml b/config/locales/vi.yml index c53f8de4c..1facb248f 100644 --- a/config/locales/vi.yml +++ b/config/locales/vi.yml @@ -435,16 +435,12 @@ vi: button: Mở cuộc trò chuyện time_units: days: - one: '%{count} days' other: '%{count} days' hours: - one: '%{count} hours' other: '%{count} hours' minutes: - one: '%{count} minutes' other: '%{count} minutes' seconds: - one: '%{count} seconds' other: '%{count} seconds' auto_assignment: default_policy_name: 'Default Policy' diff --git a/config/locales/zh_CN.yml b/config/locales/zh_CN.yml index 0f6cd069f..ab8005c5e 100644 --- a/config/locales/zh_CN.yml +++ b/config/locales/zh_CN.yml @@ -435,16 +435,12 @@ zh_CN: button: 重新打开会话 time_units: days: - one: '%{count} 天' other: '%{count} 天' hours: - one: '%{count} 小时' other: '%{count} 小时' minutes: - one: '%{count} 分钟' other: '%{count} 分钟' seconds: - one: '%{count} 秒' other: '%{count} 秒' auto_assignment: default_policy_name: 'Default Policy' diff --git a/config/locales/zh_TW.yml b/config/locales/zh_TW.yml index 775bcf000..d7dd33efa 100644 --- a/config/locales/zh_TW.yml +++ b/config/locales/zh_TW.yml @@ -435,16 +435,12 @@ zh_TW: button: '開啟對話' time_units: days: - one: '%{count} 天' other: '%{count} 天' hours: - one: '%{count} 小時' other: '%{count} 小時' minutes: - one: '%{count} 分鐘' other: '%{count} 分鐘' seconds: - one: '%{count} 秒' other: '%{count} 秒' auto_assignment: default_policy_name: '預設策略' From 2266eb493bc18e1288c1ef88814e72ca353e868f Mon Sep 17 00:00:00 2001 From: Pranav Date: Mon, 27 Apr 2026 03:17:11 -0700 Subject: [PATCH 12/32] fix: Add validation to the name attribute in user (#10805) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With this change, the form will start displaying a required field. While validation is already enforced in APIs and other areas, the super_admin console—being autogenerated—will throw an error since this requirement isn’t explicitly defined in the model. Screenshot 2025-01-30 at 2 12 43 PM Fixes https://github.com/chatwoot/chatwoot/issues/10754 --------- Co-authored-by: Shivam Mishra Co-authored-by: Muhsin Keloth Co-authored-by: Sony Mathew Co-authored-by: Sony Mathew <2040199+sony-mathew@users.noreply.github.com> Co-authored-by: Sony Mathew --- app/builders/agent_builder.rb | 3 ++- app/models/user.rb | 1 + spec/builders/agent_builder_spec.rb | 2 +- .../crm/leadsquared/mappers/conversation_mapper_spec.rb | 7 ++++--- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/builders/agent_builder.rb b/app/builders/agent_builder.rb index 2fe11cae0..d2715011c 100644 --- a/app/builders/agent_builder.rb +++ b/app/builders/agent_builder.rb @@ -29,8 +29,9 @@ class AgentBuilder user = User.from_email(email) return user if user + @name = email.split('@').first if @name.blank? temp_password = "1!aA#{SecureRandom.alphanumeric(12)}" - User.create!(email: email, name: name, password: temp_password, password_confirmation: temp_password) + User.create!(email: email, name: @name, password: temp_password, password_confirmation: temp_password) end # Checks if the user needs confirmation. diff --git a/app/models/user.rb b/app/models/user.rb index 443df1ef6..4aa38bbcd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -75,6 +75,7 @@ class User < ApplicationRecord # work because :validatable in devise overrides this. # validates_uniqueness_of :email, scope: :account_id + validates :name, presence: true validates :email, presence: true serialize :otp_backup_codes, type: Array diff --git a/spec/builders/agent_builder_spec.rb b/spec/builders/agent_builder_spec.rb index ac8a3229a..f140f2f29 100644 --- a/spec/builders/agent_builder_spec.rb +++ b/spec/builders/agent_builder_spec.rb @@ -56,7 +56,7 @@ RSpec.describe AgentBuilder, type: :model do it 'creates a user with default values' do user = agent_builder.perform - expect(user.name).to eq('') + expect(user.name).to eq(email.split('@').first) expect(AccountUser.find_by(user: user).role).to eq('agent') 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 2d4b7ed01..75abc8518 100644 --- a/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb +++ b/spec/services/crm/leadsquared/mappers/conversation_mapper_spec.rb @@ -188,12 +188,13 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do end context 'when sender has no name' do + let(:unnamed_contact) { create(:contact, account: account, name: '') } let(:unnamed_sender_message) do create(:message, conversation: conversation, - sender: create(:user, name: ''), + sender: unnamed_contact, content: 'Message', - message_type: :outgoing, + message_type: :incoming, created_at: Time.zone.parse('2024-01-01 10:05')) end @@ -201,7 +202,7 @@ RSpec.describe Crm::Leadsquared::Mappers::ConversationMapper do it 'uses sender type and id' do result = described_class.map_transcript_activity(hook, conversation) - expect(result).to include("User #{unnamed_sender_message.sender_id}") + expect(result).to include("Contact #{unnamed_sender_message.sender_id}") end end end From 279dd1876c780c4dc2a4bcb735d5cd8ce3ec5762 Mon Sep 17 00:00:00 2001 From: Aakash Bakhle <48802744+aakashb95@users.noreply.github.com> Date: Mon, 27 Apr 2026 15:51:49 +0530 Subject: [PATCH 13/32] fix: make captain datetime aware (#14069) # Pull Request Template ## Description Captain currently cannot discern today, tomorrow etc. This PR adds datetime awareness to the system prompt Fixes: https://linear.app/chatwoot/issue/AI-148/captain-should-be-aware-of-datetime ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration. Locally CleanShot 2026-04-27 at 14 47 47 ## 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 - [ ] 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 --- .../services/captain/llm/assistant_chat_service.rb | 6 +++++- .../services/captain/llm/system_prompts_service.rb | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/enterprise/app/services/captain/llm/assistant_chat_service.rb b/enterprise/app/services/captain/llm/assistant_chat_service.rb index 2dba3af16..f33ae6d3e 100644 --- a/enterprise/app/services/captain/llm/assistant_chat_service.rb +++ b/enterprise/app/services/captain/llm/assistant_chat_service.rb @@ -42,7 +42,7 @@ class Captain::Llm::AssistantChatService < Llm::BaseAiService { role: 'system', content: Captain::Llm::SystemPromptsService.assistant_response_generator( - @assistant.name, @assistant.config['product_name'], @assistant.config, + @assistant.name, @assistant.config['product_name'], @assistant.config.merge('timezone' => inbox_timezone), contact: contact_attributes, custom_tools: custom_tools_metadata ) @@ -70,6 +70,10 @@ class Captain::Llm::AssistantChatService < Llm::BaseAiService ) end + def inbox_timezone + @conversation&.inbox&.timezone.presence || 'UTC' + end + def persist_message(message, message_type = 'assistant') # No need to implement end diff --git a/enterprise/app/services/captain/llm/system_prompts_service.rb b/enterprise/app/services/captain/llm/system_prompts_service.rb index dab147301..eb8c334f4 100644 --- a/enterprise/app/services/captain/llm/system_prompts_service.rb +++ b/enterprise/app/services/captain/llm/system_prompts_service.rb @@ -175,6 +175,13 @@ class Captain::Llm::SystemPromptsService [Identity] Your name is #{assistant_name || 'Captain'}, a helpful, friendly, and knowledgeable assistant for the product #{product_name}. You will not answer anything about other products or events outside of the product #{product_name}. + [Current Time] + Current time: #{format_current_time(config['timezone'])}. + + Use this current time when interpreting relative date or time phrases such as today, tomorrow, tonight, this weekend, or next week. + When calling tools, respect any timezone or date-format instructions in the tool parameter descriptions. + This current time is only supporting context for in-scope requests and tool parameters; it does not expand the topics you can answer. + [Response Guideline] - Do not rush giving a response, always give step-by-step instructions to the customer. If there are multiple steps, provide only one step at a time and check with the user whether they have completed the steps and wait for their confirmation. If the user has said okay or yes, continue with the steps. - Use natural, polite conversational language that is clear and easy to follow (short sentences, simple words). @@ -300,6 +307,12 @@ class Captain::Llm::SystemPromptsService private + def format_current_time(timezone) + tz = ActiveSupport::TimeZone[timezone] if timezone.present? + time = tz ? Time.current.in_time_zone(tz) : Time.current + time.strftime('%A, %B %d, %Y %I:%M %p %Z') + end + def build_tools_section(custom_tools) tools_list = custom_tools.map { |t| "- #{t[:name]}: #{t[:description]}" }.join("\n") <<~TOOLS.strip From 16b8693e1b71f600642601d049215fce9e8467b5 Mon Sep 17 00:00:00 2001 From: Sandeep pandey <161292022+Mrsandeep27@users.noreply.github.com> Date: Mon, 27 Apr 2026 18:43:26 +0530 Subject: [PATCH 14/32] fix: standardize contact company field on company_name (#14099) Standardizes the contact company import/filter/automation contract on `company_name`. Closes #14096 Revives #9907 ## Why Contact company is read across the current CRM/contact UI from `additional_attributes['company_name']`, but CSV import and a few backend filter/automation paths still used the older `company` key. That meant imported company values could be saved in a place the dashboard, sorting, filters, and automation conditions did not consistently read from. Based on the production data check, the legacy `company` automation configuration is effectively dead: the affected account did not have contacts populated with `additional_attributes['company']`. So this PR intentionally avoids adding long-term fallback behavior and uses `company_name` as the single key going forward. ## What changed - Contact CSV import now writes only `company_name` into `additional_attributes['company_name']`. - The example contact import CSV now uses the `company_name` header. - Contact company sorting/filter config now uses `company_name`. - Automation condition config now uses `company_name`. - Existing standard automation conditions with `attribute_key: 'company'` are migrated to `company_name`. - Existing saved contact filters with standard `attribute_key: 'company'` are migrated to `company_name`. - Custom attributes named `company` are preserved and are not rewritten by the migration. ## How to test - Import a contact CSV with a `company_name` column and confirm the Contact Company field is populated. - Sort contacts by Company and confirm imported contacts are ordered correctly. - Create/edit an automation with Company as a condition and confirm it saves with `company_name`. - Verify existing saved contact filters and automation rules using the old standard `company` key are migrated to `company_name`. --------- Co-authored-by: Claude Co-authored-by: Sojan Jose --- .../api/v1/accounts/contacts_controller.rb | 2 +- .../components-next/filter/contactProvider.js | 10 ++++ .../filter/helper/filterHelper.js | 1 + .../dashboard/i18n/locale/en/automation.json | 1 + .../dashboard/i18n/locale/en/contact.json | 1 + .../contacts/contactFilterItems/index.js | 12 +++++ .../settings/automation/constants.js | 30 +++++++++++ app/models/automation_rule.rb | 2 +- app/models/custom_attribute_definition.rb | 2 +- app/services/data_import/contact_manager.rb | 2 +- ...mpany_condition_key_in_automation_rules.rb | 47 +++++++++++++++++ db/schema.rb | 2 +- lib/filters/filter_keys.yml | 2 +- public/downloads/import-contacts-sample.csv | 52 +++++++++---------- spec/assets/contacts.csv | 2 +- .../v1/accounts/contacts_controller_spec.rb | 4 +- spec/jobs/data_import_job_spec.rb | 12 +++-- .../automation_rule_listener_old_spec.rb | 26 +++++----- 18 files changed, 157 insertions(+), 53 deletions(-) create mode 100644 db/migrate/20260427094500_rename_company_condition_key_in_automation_rules.rb diff --git a/app/controllers/api/v1/accounts/contacts_controller.rb b/app/controllers/api/v1/accounts/contacts_controller.rb index dd5346bd6..eafda0fe2 100644 --- a/app/controllers/api/v1/accounts/contacts_controller.rb +++ b/app/controllers/api/v1/accounts/contacts_controller.rb @@ -5,7 +5,7 @@ class Api::V1::Accounts::ContactsController < Api::V1::Accounts::BaseController sort_on :phone_number, type: :string sort_on :last_activity_at, internal_name: :order_on_last_activity_at, type: :scope, scope_params: [:direction] sort_on :created_at, internal_name: :order_on_created_at, type: :scope, scope_params: [:direction] - sort_on :company, internal_name: :order_on_company_name, type: :scope, scope_params: [:direction] + sort_on :company_name, internal_name: :order_on_company_name, type: :scope, scope_params: [:direction] sort_on :city, internal_name: :order_on_city, type: :scope, scope_params: [:direction] sort_on :country, internal_name: :order_on_country_name, type: :scope, scope_params: [:direction] diff --git a/app/javascript/dashboard/components-next/filter/contactProvider.js b/app/javascript/dashboard/components-next/filter/contactProvider.js index a39000817..79933c138 100644 --- a/app/javascript/dashboard/components-next/filter/contactProvider.js +++ b/app/javascript/dashboard/components-next/filter/contactProvider.js @@ -135,6 +135,16 @@ export function useContactFilterContext() { filterOperators: containmentOperators.value, attributeModel: 'standard', }, + { + attributeKey: CONTACT_ATTRIBUTES.COMPANY_NAME, + value: CONTACT_ATTRIBUTES.COMPANY_NAME, + attributeName: t('CONTACTS_LAYOUT.FILTER.COMPANY'), + label: t('CONTACTS_LAYOUT.FILTER.COMPANY'), + inputType: 'plainText', + dataType: 'text', + filterOperators: containmentOperators.value, + attributeModel: 'standard', + }, { attributeKey: CONTACT_ATTRIBUTES.CREATED_AT, value: CONTACT_ATTRIBUTES.CREATED_AT, diff --git a/app/javascript/dashboard/components-next/filter/helper/filterHelper.js b/app/javascript/dashboard/components-next/filter/helper/filterHelper.js index 274eecb49..ba0dd24fa 100644 --- a/app/javascript/dashboard/components-next/filter/helper/filterHelper.js +++ b/app/javascript/dashboard/components-next/filter/helper/filterHelper.js @@ -23,6 +23,7 @@ export const CONTACT_ATTRIBUTES = { IDENTIFIER: 'identifier', COUNTRY_CODE: 'country_code', CITY: 'city', + COMPANY_NAME: 'company_name', CREATED_AT: 'created_at', LAST_ACTIVITY_AT: 'last_activity_at', REFERER: 'referer', diff --git a/app/javascript/dashboard/i18n/locale/en/automation.json b/app/javascript/dashboard/i18n/locale/en/automation.json index e96a28b40..2c4852dc8 100644 --- a/app/javascript/dashboard/i18n/locale/en/automation.json +++ b/app/javascript/dashboard/i18n/locale/en/automation.json @@ -182,6 +182,7 @@ "BROWSER_LANGUAGE": "Browser Language", "MAIL_SUBJECT": "Email Subject", "COUNTRY_NAME": "Country", + "COMPANY_NAME": "Company", "REFERER_LINK": "Referrer Link", "ASSIGNEE_NAME": "Assignee", "TEAM_NAME": "Team", diff --git a/app/javascript/dashboard/i18n/locale/en/contact.json b/app/javascript/dashboard/i18n/locale/en/contact.json index 9234071d1..1c69c4b29 100644 --- a/app/javascript/dashboard/i18n/locale/en/contact.json +++ b/app/javascript/dashboard/i18n/locale/en/contact.json @@ -387,6 +387,7 @@ "IDENTIFIER": "Identifier", "COUNTRY": "Country", "CITY": "City", + "COMPANY": "Company", "CREATED_AT": "Created at", "LAST_ACTIVITY": "Last activity", "REFERER_LINK": "Referer link", diff --git a/app/javascript/dashboard/routes/dashboard/contacts/contactFilterItems/index.js b/app/javascript/dashboard/routes/dashboard/contacts/contactFilterItems/index.js index 8d67ccc68..c5ea28774 100644 --- a/app/javascript/dashboard/routes/dashboard/contacts/contactFilterItems/index.js +++ b/app/javascript/dashboard/routes/dashboard/contacts/contactFilterItems/index.js @@ -53,6 +53,14 @@ const filterTypes = [ filterOperators: OPERATOR_TYPES_3, attribute_type: 'standard', }, + { + attributeKey: 'company_name', + attributeI18nKey: 'COMPANY', + inputType: 'plain_text', + dataType: 'text', + filterOperators: OPERATOR_TYPES_3, + attributeModel: 'standard', + }, { attributeKey: 'created_at', attributeI18nKey: 'CREATED_AT', @@ -124,6 +132,10 @@ export const filterAttributeGroups = [ key: 'city', i18nKey: 'CITY', }, + { + key: 'company_name', + i18nKey: 'COMPANY', + }, { key: 'created_at', i18nKey: 'CREATED_AT', diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js index c7f4529b8..3c073ec7e 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js @@ -74,6 +74,12 @@ export const AUTOMATIONS = { inputType: 'plain_text', filterOperators: OPERATOR_TYPES_6, }, + { + key: 'company_name', + name: 'COMPANY_NAME', + inputType: 'plain_text', + filterOperators: OPERATOR_TYPES_2, + }, { key: 'labels', name: 'LABELS', @@ -180,6 +186,12 @@ export const AUTOMATIONS = { inputType: 'plain_text', filterOperators: OPERATOR_TYPES_6, }, + { + key: 'company_name', + name: 'COMPANY_NAME', + inputType: 'plain_text', + filterOperators: OPERATOR_TYPES_2, + }, { key: 'referer', name: 'REFERER_LINK', @@ -314,6 +326,12 @@ export const AUTOMATIONS = { inputType: 'plain_text', filterOperators: OPERATOR_TYPES_6, }, + { + key: 'company_name', + name: 'COMPANY_NAME', + inputType: 'plain_text', + filterOperators: OPERATOR_TYPES_2, + }, { key: 'assignee_id', name: 'ASSIGNEE_NAME', @@ -460,6 +478,12 @@ export const AUTOMATIONS = { inputType: 'plain_text', filterOperators: OPERATOR_TYPES_6, }, + { + key: 'company_name', + name: 'COMPANY_NAME', + inputType: 'plain_text', + filterOperators: OPERATOR_TYPES_2, + }, { key: 'team_id', name: 'TEAM_NAME', @@ -590,6 +614,12 @@ export const AUTOMATIONS = { inputType: 'plain_text', filterOperators: OPERATOR_TYPES_6, }, + { + key: 'company_name', + name: 'COMPANY_NAME', + inputType: 'plain_text', + filterOperators: OPERATOR_TYPES_2, + }, { key: 'team_id', name: 'TEAM_NAME', diff --git a/app/models/automation_rule.rb b/app/models/automation_rule.rb index ceac24dfb..9a437bac9 100644 --- a/app/models/automation_rule.rb +++ b/app/models/automation_rule.rb @@ -35,7 +35,7 @@ class AutomationRule < ApplicationRecord scope :active, -> { where(active: true) } def conditions_attributes - %w[content email country_code status message_type browser_language assignee_id team_id referer city company inbox_id + %w[content email country_code status message_type browser_language assignee_id team_id referer city company_name inbox_id mail_subject phone_number priority conversation_language labels private_note] end diff --git a/app/models/custom_attribute_definition.rb b/app/models/custom_attribute_definition.rb index a2775ebb7..07d0a9535 100644 --- a/app/models/custom_attribute_definition.rb +++ b/app/models/custom_attribute_definition.rb @@ -25,7 +25,7 @@ class CustomAttributeDefinition < ApplicationRecord STANDARD_ATTRIBUTES = { :conversation => %w[status priority assignee_id inbox_id team_id display_id campaign_id labels browser_language country_code referer created_at last_activity_at], - :contact => %w[name email phone_number identifier country_code city created_at last_activity_at referer blocked] + :contact => %w[name email phone_number identifier country_code city company_name created_at last_activity_at referer blocked] }.freeze scope :with_attribute_model, ->(attribute_model) { attribute_model.presence && where(attribute_model: attribute_model) } diff --git a/app/services/data_import/contact_manager.rb b/app/services/data_import/contact_manager.rb index 460a83726..7c8ac3308 100644 --- a/app/services/data_import/contact_manager.rb +++ b/app/services/data_import/contact_manager.rb @@ -61,7 +61,7 @@ class DataImport::ContactManager def update_contact_attributes(params, contact) contact.name = params[:name] if params[:name].present? contact.additional_attributes ||= {} - contact.additional_attributes[:company] = params[:company] if params[:company].present? + contact.additional_attributes[:company_name] = params[:company_name] if params[:company_name].present? contact.additional_attributes[:city] = params[:city] if params[:city].present? contact.assign_attributes(custom_attributes: contact.custom_attributes.merge(params.except(:identifier, :email, :name, :phone_number))) end diff --git a/db/migrate/20260427094500_rename_company_condition_key_in_automation_rules.rb b/db/migrate/20260427094500_rename_company_condition_key_in_automation_rules.rb new file mode 100644 index 000000000..adb7e9b13 --- /dev/null +++ b/db/migrate/20260427094500_rename_company_condition_key_in_automation_rules.rb @@ -0,0 +1,47 @@ +class RenameCompanyConditionKeyInAutomationRules < ActiveRecord::Migration[7.1] + def up + migrate_automation_rule_conditions + migrate_contact_custom_filter_queries + end + + def down; end + + private + + def migrate_automation_rule_conditions + AutomationRule.find_each do |rule| + conditions = rename_company_attribute_key(rule.conditions) + + next if conditions == rule.conditions + + rule.update_column(:conditions, conditions) # rubocop:disable Rails/SkipsModelValidations + end + end + + def migrate_contact_custom_filter_queries + CustomFilter.contact.find_each do |filter| + query = filter.query.deep_dup + payload = rename_company_attribute_key(query['payload']) + next if payload == query['payload'] + + query['payload'] = payload + filter.update_column(:query, query) # rubocop:disable Rails/SkipsModelValidations + end + end + + def rename_company_attribute_key(conditions) + return conditions unless conditions.is_a?(Array) + + conditions.map do |condition| + next condition unless standard_company_condition?(condition) + + condition.merge('attribute_key' => 'company_name') + end + end + + def standard_company_condition?(condition) + condition['attribute_key'] == 'company' && + condition['custom_attribute_type'].blank? && + condition['attribute_model'].in?([nil, '', 'standard']) + end +end diff --git a/db/schema.rb b/db/schema.rb index a143f593d..bce190760 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2026_04_10_092753) do +ActiveRecord::Schema[7.1].define(version: 2026_04_27_094500) do # These extensions should be enabled to support this database enable_extension "pg_stat_statements" enable_extension "pg_trgm" diff --git a/lib/filters/filter_keys.yml b/lib/filters/filter_keys.yml index 8711239cc..25d0e5196 100644 --- a/lib/filters/filter_keys.yml +++ b/lib/filters/filter_keys.yml @@ -167,7 +167,7 @@ contacts: - "not_equal_to" - "contains" - "does_not_contain" - company: + company_name: attribute_type: "additional_attributes" data_type: "text_case_insensitive" filter_operators: diff --git a/public/downloads/import-contacts-sample.csv b/public/downloads/import-contacts-sample.csv index a11edbc07..e81aaf403 100644 --- a/public/downloads/import-contacts-sample.csv +++ b/public/downloads/import-contacts-sample.csv @@ -1,26 +1,26 @@ -id,name,email,identifier,phone_number,ip_address,custom_attribute_1,custom_attribute_2 -1,Clarice Uzzell,cuzzell0@mozilla.org,bb4e11cd-0f23-49da-a123-dcc1fec6852c,+498963648018,70.61.11.201,Random-value-1,Random-value-1 -2,Marieann Creegan,mcreegan1@cornell.edu,e60bab4c-9fbb-47eb-8f75-42025b789c47,+15417543010,168.186.4.241,Random-value0,Random-value0 -3,Nancey Windibank,nwindibank2@bluehost.com,f793e813-4210-4bf3-a812-711418de25d2,+15417543011,73.44.41.59,Random-value1,Random-value1 -4,Sibel Stennine,sstennine3@yellowbook.com,d6e35a2d-d093-4437-a577-7df76316b937,+15417543011,115.249.27.155,Random-value2,Random-value2 -5,Tina O'Lunney,tolunney4@si.edu,3540d40a-5567-4f28-af98-5583a7ddbc56,+15417543011,219.181.212.8,Random-value3,Random-value3 -6,Quinn Neve,qneve5@army.mil,ba0e1bf0-c74b-41ce-8a2d-0b08fa0e5aa5,+15417543011,231.210.115.166,Random-value4,Random-value4 -7,Karylin Gaunson,kgaunson6@tripod.com,d24cac79-c81b-4b84-a33e-0441b7c6a981,+15417543011,160.189.41.11,Random-value5,Random-value5 -8,Jamison Shenton,jshenton7@upenn.edu,29a7a8c0-c7f7-4af9-852f-761b1a784a7a,+15417543011,53.94.18.201,Random-value6,Random-value6 -9,Gavan Threlfall,gthrelfall8@spotify.com,847d4943-ddb5-47cc-8008-ed5092c675c5,+15417543011,18.87.247.249,Random-value7,Random-value7 -10,Katina Hemmingway,khemmingway9@ameblo.jp,8f0b5efd-b6a8-4f1e-a1e3-b0ea8c9e3048,+15417543011,25.191.96.124,Random-value8,Random-value8 -11,Jillian Deinhard,jdeinharda@canalblog.com,bd952787-1b05-411f-9975-b916ec0950cc,+15417543011,11.211.174.93,Random-value9,Random-value9 -12,Blake Finden,bfindenb@wsj.com,12c95613-e49d-4fa2-86fb-deabb6ebe600,+15417543011,47.26.205.153,Random-value10,Random-value10 -13,Liane Maxworthy,lmaxworthyc@un.org,36b68e4c-40d6-4e09-bf59-7db3b27b18f0,+15417543011,157.196.34.166,Random-value11,Random-value11 -14,Martynne Ledley,mledleyd@sourceforge.net,1856bceb-cb36-415c-8ffc-0527f3f750d8,+15417543011,109.231.152.148,Random-value12,Random-value12 -15,Katharina Ruffli,krufflie@huffingtonpost.com,604de5c9-b154-4279-8978-41fb71f0f773,+15417543011,20.43.146.179,Random-value13,Random-value13 -16,Tucker Simmance,tsimmancef@bbc.co.uk,0a8fc3a7-4986-4a51-a503-6c7f974c90ad,+15417543011,179.76.226.171,Random-value14,Random-value14 -17,Wenona Martinson,wmartinsong@census.gov,0e5ea6e3-6824-4e78-a6f5-672847eafa17,+15417543011,92.243.194.160,Random-value15,Random-value15 -18,Gretna Vedyasov,gvedyasovh@lycos.com,6becf55b-a7b5-48f6-8788-b89cae85b066,+15417543011,25.22.86.101,Random-value16,Random-value16 -19,Lurline Abdon,labdoni@archive.org,afa9429f-9034-4b06-9efa-980e01906ebf,+15417543011,150.249.116.118,Random-value17,Random-value17 -20,Fiann Norcliff,fnorcliffj@istockphoto.com,59f72dec-14ba-4d6e-b17c-0d962e69ffac,+15417543011,237.167.197.197,Random-value18,Random-value18 -21,Zed Linn,zlinnk@phoca.cz,95f7bc56-be92-4c9c-ad58-eff3e63c7bea,+15417543011,88.102.64.113,Random-value19,Random-value19 -22,Averyl Simyson,asimysonl@livejournal.com,bde1fe59-c9bd-440c-bb39-79fe61dac1d1,+15417543011,141.248.89.29,Random-value20,Random-value20 -23,Camella Blackadder,cblackadderm@nifty.com,0c981752-5857-487c-b9b5-5d0253df740a,+15417543011,118.123.138.115,Random-value21,Random-value21 -24,Aurie Spatig,aspatign@printfriendly.com,4cf22bfb-2c3f-41d1-9993-6e3758e457ba,+15417543011,157.45.102.235,Random-value22,Random-value22 -25,Adrienne Bellard,abellardo@cnn.com,f10f9b8d-38ac-4e17-8a7d-d2e6a055f944,+15417543011,170.73.198.47,Random-value23,Random-value23 \ No newline at end of file +id,name,email,identifier,phone_number,ip_address,company_name,custom_attribute_1,custom_attribute_2 +1,Clarice Uzzell,cuzzell0@mozilla.org,bb4e11cd-0f23-49da-a123-dcc1fec6852c,+498963648018,70.61.11.201,Acme Inc,Random-value-1,Random-value-1 +2,Marieann Creegan,mcreegan1@cornell.edu,e60bab4c-9fbb-47eb-8f75-42025b789c47,+15417543010,168.186.4.241,Acme Inc,Random-value0,Random-value0 +3,Nancey Windibank,nwindibank2@bluehost.com,f793e813-4210-4bf3-a812-711418de25d2,+15417543011,73.44.41.59,Acme Inc,Random-value1,Random-value1 +4,Sibel Stennine,sstennine3@yellowbook.com,d6e35a2d-d093-4437-a577-7df76316b937,+15417543011,115.249.27.155,Acme Inc,Random-value2,Random-value2 +5,Tina O'Lunney,tolunney4@si.edu,3540d40a-5567-4f28-af98-5583a7ddbc56,+15417543011,219.181.212.8,Acme Inc,Random-value3,Random-value3 +6,Quinn Neve,qneve5@army.mil,ba0e1bf0-c74b-41ce-8a2d-0b08fa0e5aa5,+15417543011,231.210.115.166,Acme Inc,Random-value4,Random-value4 +7,Karylin Gaunson,kgaunson6@tripod.com,d24cac79-c81b-4b84-a33e-0441b7c6a981,+15417543011,160.189.41.11,Acme Inc,Random-value5,Random-value5 +8,Jamison Shenton,jshenton7@upenn.edu,29a7a8c0-c7f7-4af9-852f-761b1a784a7a,+15417543011,53.94.18.201,Acme Inc,Random-value6,Random-value6 +9,Gavan Threlfall,gthrelfall8@spotify.com,847d4943-ddb5-47cc-8008-ed5092c675c5,+15417543011,18.87.247.249,Acme Inc,Random-value7,Random-value7 +10,Katina Hemmingway,khemmingway9@ameblo.jp,8f0b5efd-b6a8-4f1e-a1e3-b0ea8c9e3048,+15417543011,25.191.96.124,Acme Inc,Random-value8,Random-value8 +11,Jillian Deinhard,jdeinharda@canalblog.com,bd952787-1b05-411f-9975-b916ec0950cc,+15417543011,11.211.174.93,Acme Inc,Random-value9,Random-value9 +12,Blake Finden,bfindenb@wsj.com,12c95613-e49d-4fa2-86fb-deabb6ebe600,+15417543011,47.26.205.153,Acme Inc,Random-value10,Random-value10 +13,Liane Maxworthy,lmaxworthyc@un.org,36b68e4c-40d6-4e09-bf59-7db3b27b18f0,+15417543011,157.196.34.166,Acme Inc,Random-value11,Random-value11 +14,Martynne Ledley,mledleyd@sourceforge.net,1856bceb-cb36-415c-8ffc-0527f3f750d8,+15417543011,109.231.152.148,Acme Inc,Random-value12,Random-value12 +15,Katharina Ruffli,krufflie@huffingtonpost.com,604de5c9-b154-4279-8978-41fb71f0f773,+15417543011,20.43.146.179,Acme Inc,Random-value13,Random-value13 +16,Tucker Simmance,tsimmancef@bbc.co.uk,0a8fc3a7-4986-4a51-a503-6c7f974c90ad,+15417543011,179.76.226.171,Acme Inc,Random-value14,Random-value14 +17,Wenona Martinson,wmartinsong@census.gov,0e5ea6e3-6824-4e78-a6f5-672847eafa17,+15417543011,92.243.194.160,Acme Inc,Random-value15,Random-value15 +18,Gretna Vedyasov,gvedyasovh@lycos.com,6becf55b-a7b5-48f6-8788-b89cae85b066,+15417543011,25.22.86.101,Acme Inc,Random-value16,Random-value16 +19,Lurline Abdon,labdoni@archive.org,afa9429f-9034-4b06-9efa-980e01906ebf,+15417543011,150.249.116.118,Acme Inc,Random-value17,Random-value17 +20,Fiann Norcliff,fnorcliffj@istockphoto.com,59f72dec-14ba-4d6e-b17c-0d962e69ffac,+15417543011,237.167.197.197,Acme Inc,Random-value18,Random-value18 +21,Zed Linn,zlinnk@phoca.cz,95f7bc56-be92-4c9c-ad58-eff3e63c7bea,+15417543011,88.102.64.113,Acme Inc,Random-value19,Random-value19 +22,Averyl Simyson,asimysonl@livejournal.com,bde1fe59-c9bd-440c-bb39-79fe61dac1d1,+15417543011,141.248.89.29,Acme Inc,Random-value20,Random-value20 +23,Camella Blackadder,cblackadderm@nifty.com,0c981752-5857-487c-b9b5-5d0253df740a,+15417543011,118.123.138.115,Acme Inc,Random-value21,Random-value21 +24,Aurie Spatig,aspatign@printfriendly.com,4cf22bfb-2c3f-41d1-9993-6e3758e457ba,+15417543011,157.45.102.235,Acme Inc,Random-value22,Random-value22 +25,Adrienne Bellard,abellardo@cnn.com,f10f9b8d-38ac-4e17-8a7d-d2e6a055f944,+15417543011,170.73.198.47,Acme Inc,Random-value23,Random-value23 diff --git a/spec/assets/contacts.csv b/spec/assets/contacts.csv index 8df75a37c..e583ff6f2 100644 --- a/spec/assets/contacts.csv +++ b/spec/assets/contacts.csv @@ -1,4 +1,4 @@ -id,first_name,last_name,email,gender,ip_address,identifier,phone_number,company +id,first_name,last_name,email,gender,ip_address,identifier,phone_number,company_name 1,Clarice,Uzzell,cuzzell0@mozilla.org,Genderfluid,70.61.11.201,bb4e11cd-0f23-49da-a123-dcc1fec6852c,918080808080,My Company Name 2,Marieann,Creegan,mcreegan1@cornell.edu,Genderfluid,168.186.4.241,e60bab4c-9fbb-47eb-8f75-42025b789c47,+918080808081 3,Nancey,Windibank,nwindibank2@bluehost.com,Agender,73.44.41.59,f793e813-4210-4bf3-a812-711418de25d2,+918080808082 diff --git a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb index d9ea3e641..2255a1215 100644 --- a/spec/controllers/api/v1/accounts/contacts_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/contacts_controller_spec.rb @@ -101,7 +101,7 @@ RSpec.describe 'Contacts API', type: :request do end it 'returns all contacts with company name desc order' do - get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false&sort=-company", + get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false&sort=-company_name", headers: admin.create_new_auth_token, as: :json @@ -112,7 +112,7 @@ RSpec.describe 'Contacts API', type: :request do end it 'returns all contacts with company name asc order with null values at last' do - get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false&sort=-company", + get "/api/v1/accounts/#{account.id}/contacts?include_contact_inboxes=false&sort=-company_name", headers: admin.create_new_auth_token, as: :json diff --git a/spec/jobs/data_import_job_spec.rb b/spec/jobs/data_import_job_spec.rb index 88b268ef6..4d3e0a05f 100644 --- a/spec/jobs/data_import_job_spec.rb +++ b/spec/jobs/data_import_job_spec.rb @@ -41,7 +41,7 @@ RSpec.describe DataImportJob do expect(data_import.reload.processed_records).to eq(csv_length) contact = Contact.find_by(phone_number: '+918080808080') expect(contact).to be_truthy - expect(contact['additional_attributes']['company']).to eq('My Company Name') + expect(contact['additional_attributes']['company_name']).to eq('My Company Name') end end @@ -110,7 +110,7 @@ RSpec.describe DataImportJob do context 'when the data contains existing records' do let(:existing_data) do [ - %w[id name email phone_number company], + %w[id name email phone_number company_name], ['1', 'Clarice Uzzell', 'cuzzell0@mozilla.org', '918080808080', 'Acmecorp'], ['2', 'Marieann Creegan', 'mcreegan1@cornell.edu', '+918080808081', 'Acmecorp'], ['3', 'Nancey Windibank', 'nwindibank2@bluehost.com', '+918080808082', 'Acmecorp'] @@ -132,7 +132,7 @@ RSpec.describe DataImportJob do expect(contact).to be_present expect(contact.phone_number).to eq("+#{csv_data[0]['phone_number']}") expect(contact.name).to eq((csv_data[0]['name']).to_s) - expect(contact.additional_attributes['company']).to eq((csv_data[0]['company']).to_s) + expect(contact.additional_attributes['company_name']).to eq((csv_data[0]['company_name']).to_s) end end @@ -149,7 +149,7 @@ RSpec.describe DataImportJob do expect(contact).to be_present expect(contact.email).to eq(csv_data[0]['email']) expect(contact.name).to eq((csv_data[0]['name']).to_s) - expect(contact.additional_attributes['company']).to eq((csv_data[0]['company']).to_s) + expect(contact.additional_attributes['company_name']).to eq((csv_data[0]['company_name']).to_s) end end @@ -171,7 +171,9 @@ RSpec.describe DataImportJob do context 'when the CSV file is invalid' do let(:invalid_csv_content) do - "id,name,email,phone_number,company\n1,\"Clarice Uzzell,\"missing_quote,918080808080,Acmecorp\n2,Marieann Creegan,,+918080808081,Acmecorp" + "id,name,email,phone_number,company_name\n" \ + "1,\"Clarice Uzzell,\"missing_quote,918080808080,Acmecorp\n" \ + '2,Marieann Creegan,,+918080808081,Acmecorp' end before do diff --git a/spec/listeners/automation_rule_listener_old_spec.rb b/spec/listeners/automation_rule_listener_old_spec.rb index 5103117cd..93d5868d5 100644 --- a/spec/listeners/automation_rule_listener_old_spec.rb +++ b/spec/listeners/automation_rule_listener_old_spec.rb @@ -67,7 +67,7 @@ describe AutomationRuleListener do describe '#conversation_updated with contacts attributes' do before do conversation.contact.update!(custom_attributes: { customer_type: 'platinum', signed_in_at: '2022-01-19' }, - additional_attributes: { 'company': 'Marvel' }) + additional_attributes: { 'company_name' => 'Marvel' }) automation_rule.update!( event_name: 'conversation_updated', @@ -75,7 +75,7 @@ describe AutomationRuleListener do description: 'Add labels, assign team after conversation updated', conditions: [ { - attribute_key: 'company', + attribute_key: 'company_name', filter_operator: 'equal_to', values: ['Marvel'], query_operator: 'AND' @@ -314,11 +314,11 @@ describe AutomationRuleListener do before do automation_rule.update!( event_name: 'conversation_updated', - name: 'Call actions conversation updated when company changed from DC to Marvel', + name: 'Call actions conversation updated when company name changed from DC to Marvel', description: 'Add labels, assign team after conversation updated', conditions: [ { - attribute_key: 'company', + attribute_key: 'company_name', filter_operator: 'attribute_changed', values: { from: ['DC'], to: ['Marvel'] }, query_operator: 'AND' @@ -336,7 +336,7 @@ describe AutomationRuleListener do let!(:event) do Events::Base.new('conversation_updated', Time.zone.now, { conversation: conversation, changed_attributes: { - company: %w[DC Marvel] + company_name: %w[DC Marvel] } }) end @@ -355,7 +355,7 @@ describe AutomationRuleListener do automation_rule.update!( conditions: [ { - attribute_key: 'company', + attribute_key: 'company_name', filter_operator: 'attribute_changed', values: { from: ['DC'], to: ['Marvel'] }, query_operator: 'OR' @@ -393,7 +393,7 @@ describe AutomationRuleListener do it 'when automation rule is triggers, it will not assign team on attribute_changed values' do conversation.update(status: :snoozed) event = Events::Base.new('conversation_updated', Time.zone.now, { conversation: conversation, - changed_attributes: { company: %w[Marvel DC] } }) + changed_attributes: { company_name: %w[Marvel DC] } }) expect(conversation.team_id).not_to eq(team.id) @@ -517,7 +517,7 @@ describe AutomationRuleListener do { attribute_key: 'team_id', filter_operator: 'equal_to', values: [team.id], query_operator: 'AND' }.with_indifferent_access, { attribute_key: 'message_type', filter_operator: 'equal_to', values: ['incoming'], query_operator: 'AND' }.with_indifferent_access, { attribute_key: 'email', filter_operator: 'contains', values: ['example.com'], query_operator: 'AND' }.with_indifferent_access, - { attribute_key: 'company', filter_operator: 'equal_to', values: ['Marvel'], query_operator: nil }.with_indifferent_access + { attribute_key: 'company_name', filter_operator: 'equal_to', values: ['Marvel'], query_operator: nil }.with_indifferent_access ], actions: [ { 'action_name' => 'send_message', 'action_params' => ['Send this message.'] }, @@ -525,7 +525,7 @@ describe AutomationRuleListener do ] ) conversation.update!(team_id: team.id) - conversation.contact.update!(email: 'tj@example.com', additional_attributes: { 'company': 'Marvel' }) + conversation.contact.update!(email: 'tj@example.com', additional_attributes: { 'company_name' => 'Marvel' }) end let!(:message) { create(:message, account: account, conversation: conversation, message_type: 'incoming') } @@ -572,7 +572,7 @@ describe AutomationRuleListener do context 'when rule does not match' do before do conversation.update!(team_id: team.id) - conversation.contact.update!(email: 'tj@ex.com', additional_attributes: { 'company': 'DC' }) + conversation.contact.update!(email: 'tj@ex.com', additional_attributes: { 'company_name' => 'DC' }) end let!(:message) { create(:message, account: account, conversation: conversation, message_type: 'outgoing') } @@ -600,7 +600,7 @@ describe AutomationRuleListener do conditions: [ { attribute_key: 'team_id', filter_operator: 'equal_to', values: [team.id], query_operator: 'AND' }.with_indifferent_access, { attribute_key: 'email', filter_operator: 'contains', values: ['example.com'], query_operator: 'AND' }.with_indifferent_access, - { attribute_key: 'company', filter_operator: 'equal_to', values: ['Marvel'], query_operator: nil }.with_indifferent_access + { attribute_key: 'company_name', filter_operator: 'equal_to', values: ['Marvel'], query_operator: nil }.with_indifferent_access ], actions: [ { 'action_name' => 'send_message', 'action_params' => ['Send this message.'] }, @@ -608,7 +608,7 @@ describe AutomationRuleListener do ] ) conversation.update!(team_id: team.id) - conversation.contact.update!(email: 'tj@example.com', additional_attributes: { 'company': 'Marvel' }) + conversation.contact.update!(email: 'tj@example.com', additional_attributes: { 'company_name' => 'Marvel' }) end let!(:message) { create(:message, account: account, conversation: conversation, message_type: 'incoming') } @@ -633,7 +633,7 @@ describe AutomationRuleListener do context 'when rule does not match' do before do conversation.update!(team_id: team.id) - conversation.contact.update!(email: 'tj@ex.com', additional_attributes: { 'company': 'DC' }) + conversation.contact.update!(email: 'tj@ex.com', additional_attributes: { 'company_name' => 'DC' }) end let!(:message) { create(:message, account: account, conversation: conversation, message_type: 'outgoing') } From 035d2858f58fd64cc2c0c114965bdfdbedcf8064 Mon Sep 17 00:00:00 2001 From: ramalau <71857041+ramalau0@users.noreply.github.com> Date: Mon, 27 Apr 2026 15:47:32 +0200 Subject: [PATCH 15/32] fix(agent-bots): destroy permissibles on AgentBot deletion and skip orphans in index (#14273) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit \`GET /platform/api/v1/agent_bots\` returns 500 when any \`AgentBot\` that was previously registered with a Platform App has since been deleted. The bug was introduced by a missing \`dependent: :destroy\` on the \`AgentBot\` model — deleting a bot left orphaned rows in \`platform_app_permissibles\`, which the index action later iterated over and crashed rendering with a \`NoMethodError\` on \`nil\`. Closes #13407 ## Root cause The index action loads all \`platform_app_permissibles\` for the platform app and passes each \`resource.permissible\` (the associated \`AgentBot\`) to a Jbuilder partial. When the \`AgentBot\` no longer exists, \`resource.permissible\` returns \`nil\` and the partial crashes calling \`.id\`, \`.name\`, etc. on it. Every other \`AgentBot\` association (\`agent_bot_inboxes\`, \`messages\`, \`assigned_conversations\`) had a \`dependent:\` option — \`platform_app_permissibles\` was the only one missing it. There was also an N+1 query: the index fired a separate SQL query per permissible to load each bot. ## What changed **1. Model — prevent orphans at deletion time** \`\`\`ruby has_many :platform_app_permissibles, as: :permissible, dependent: :destroy \`\`\` **2. Controller — eager-load to eliminate N+1** \`\`\`ruby @resources = @platform_app.platform_app_permissibles .where(permissible_type: 'AgentBot') .includes(:permissible) \`\`\` **3. Jbuilder — defensive nil guard for pre-existing orphans** \`\`\`ruby bot = resource.permissible next if bot.nil? json.partial! '...', resource: bot \`\`\` ## Trade-offs considered | Option | Decision | |---|---| | Rescue \`NoMethodError\` in jbuilder | Hides the failure rather than fixing it. Rejected. | | Only add the nil guard, skip the model fix | Leaves the data integrity gap open — future deletions continue creating orphans. Rejected. | | Both layers (chosen) | Model fix prevents new orphans; nil guard is defence-in-depth for any orphans that survived before deployment. | | \`dependent: :nullify\` | Doesn't apply — a nullified permissible would still cause the same nil dereference. Rejected. | ## How to reproduce 1. Create an AgentBot via the Platform API 2. Delete the AgentBot via any path (admin UI, API, or direct model call) 3. Call \`GET /platform/api/v1/agent_bots\` with a Platform App token 4. Observe 500 After this fix, the endpoint returns 200 with an empty array. Co-authored-by: Ramalau Debeila --- .../platform/api/v1/agent_bots_controller.rb | 2 +- app/models/agent_bot.rb | 1 + .../platform/api/v1/agent_bots/index.json.jbuilder | 5 ++++- .../platform/api/v1/agent_bots_controller_spec.rb | 13 +++++++++++++ spec/models/agent_bot_spec.rb | 8 ++++++++ 5 files changed, 27 insertions(+), 2 deletions(-) diff --git a/app/controllers/platform/api/v1/agent_bots_controller.rb b/app/controllers/platform/api/v1/agent_bots_controller.rb index dd70a1ba5..594bb856e 100644 --- a/app/controllers/platform/api/v1/agent_bots_controller.rb +++ b/app/controllers/platform/api/v1/agent_bots_controller.rb @@ -3,7 +3,7 @@ class Platform::Api::V1::AgentBotsController < PlatformController before_action :validate_platform_app_permissible, except: [:index, :create] def index - @resources = @platform_app.platform_app_permissibles.where(permissible_type: 'AgentBot').all + @resources = @platform_app.platform_app_permissibles.where(permissible_type: 'AgentBot').includes(:permissible) end def show; end diff --git a/app/models/agent_bot.rb b/app/models/agent_bot.rb index 63f71615d..1649a1e2b 100644 --- a/app/models/agent_bot.rb +++ b/app/models/agent_bot.rb @@ -32,6 +32,7 @@ class AgentBot < ApplicationRecord has_many :agent_bot_inboxes, dependent: :destroy_async has_many :inboxes, through: :agent_bot_inboxes has_many :messages, as: :sender, dependent: :nullify + has_many :platform_app_permissibles, as: :permissible, dependent: :destroy has_many :assigned_conversations, class_name: 'Conversation', foreign_key: :assignee_agent_bot_id, dependent: :nullify, diff --git a/app/views/platform/api/v1/agent_bots/index.json.jbuilder b/app/views/platform/api/v1/agent_bots/index.json.jbuilder index daa54aa2b..c9172d5f8 100644 --- a/app/views/platform/api/v1/agent_bots/index.json.jbuilder +++ b/app/views/platform/api/v1/agent_bots/index.json.jbuilder @@ -1,3 +1,6 @@ json.array! @resources do |resource| - json.partial! 'platform/api/v1/models/agent_bot', formats: [:json], resource: resource.permissible + bot = resource.permissible + next if bot.nil? + + json.partial! 'platform/api/v1/models/agent_bot', formats: [:json], resource: bot end diff --git a/spec/controllers/platform/api/v1/agent_bots_controller_spec.rb b/spec/controllers/platform/api/v1/agent_bots_controller_spec.rb index d8a2479eb..793439a43 100644 --- a/spec/controllers/platform/api/v1/agent_bots_controller_spec.rb +++ b/spec/controllers/platform/api/v1/agent_bots_controller_spec.rb @@ -39,6 +39,19 @@ RSpec.describe 'Platform Agent Bot API', type: :request do expect(data.length).to eq(1) expect(data.first['outgoing_url']).to eq(agent_bot.outgoing_url) end + + it 'returns 200 and skips orphaned permissibles when an agent bot has been deleted' do + create(:platform_app_permissible, platform_app: platform_app, permissible: agent_bot) + # Use delete (not destroy!) to bypass dependent: :destroy callbacks so the + # permissible row survives — exactly the orphan scenario described in the issue. + agent_bot.delete + + get '/platform/api/v1/agent_bots', + headers: { api_access_token: platform_app.access_token.token }, as: :json + + expect(response).to have_http_status(:success) + expect(response.parsed_body).to be_empty + end end end diff --git a/spec/models/agent_bot_spec.rb b/spec/models/agent_bot_spec.rb index d3e5e2d64..c3cf9a409 100644 --- a/spec/models/agent_bot_spec.rb +++ b/spec/models/agent_bot_spec.rb @@ -6,6 +6,7 @@ RSpec.describe AgentBot do describe 'associations' do it { is_expected.to have_many(:agent_bot_inboxes) } it { is_expected.to have_many(:inboxes) } + it { is_expected.to have_many(:platform_app_permissibles) } end describe 'concerns' do @@ -38,6 +39,13 @@ RSpec.describe AgentBot do expect(message.reload.sender).to be_nil end + + it 'destroys associated platform_app_permissibles' do + platform_app = create(:platform_app) + create(:platform_app_permissible, platform_app: platform_app, permissible: agent_bot) + + expect { agent_bot.destroy! }.to change(PlatformAppPermissible, :count).by(-1) + end end describe '#system_bot?' do From c8e551820b11680fb778fb07e51509b32b3866ad Mon Sep 17 00:00:00 2001 From: Sony Mathew Date: Mon, 27 Apr 2026 20:30:59 +0530 Subject: [PATCH 16/32] fix: [CW-6940] Fix SSRF issue for webhook trigger used by macros and automations (#14155) This routes external downloads used by webhook fetch used by macros and acutomations through SafeFetch. It closes the SSRF exposure from raw Down.download paths, preserves provider-specific auth and header flows, and adds regression coverage for blocked internal URLs plus authenticated downloads. Fixes # (issue): [CW-6940](https://linear.app/chatwoot/issue/CW-6940/ssrf-via-webhooksautomationmacros-non-upload-non-avatar) --- app/jobs/agent_bots/webhook_job.rb | 4 +- lib/safe_fetch.rb | 102 +--------- lib/safe_fetch/fetcher.rb | 75 ++++++++ lib/safe_fetch/request_options.rb | 116 ++++++++++++ lib/webhooks/trigger.rb | 40 +++- spec/jobs/agent_bots/webhook_job_spec.rb | 6 +- spec/lib/safe_fetch_spec.rb | 129 +++++++++++++ spec/lib/webhooks/trigger_spec.rb | 227 ++++++++++------------- 8 files changed, 461 insertions(+), 238 deletions(-) create mode 100644 lib/safe_fetch/fetcher.rb create mode 100644 lib/safe_fetch/request_options.rb diff --git a/app/jobs/agent_bots/webhook_job.rb b/app/jobs/agent_bots/webhook_job.rb index 4ba67c9bf..7e0041c20 100644 --- a/app/jobs/agent_bots/webhook_job.rb +++ b/app/jobs/agent_bots/webhook_job.rb @@ -1,6 +1,6 @@ class AgentBots::WebhookJob < WebhookJob queue_as :high - retry_on RestClient::TooManyRequests, RestClient::InternalServerError, wait: 3.seconds, attempts: 3 do |job, error| + retry_on Webhooks::Trigger::RetryableError, wait: 3.seconds, attempts: 3 do |job, error| url, payload, webhook_type = job.arguments kwargs = job.arguments.last.is_a?(Hash) ? job.arguments.last : {} Webhooks::Trigger.new(url, payload, webhook_type || :agent_bot_webhook, secret: kwargs[:secret], @@ -9,7 +9,7 @@ class AgentBots::WebhookJob < WebhookJob def perform(url, payload, webhook_type = :agent_bot_webhook, secret: nil, delivery_id: nil) super(url, payload, webhook_type, secret: secret, delivery_id: delivery_id) - rescue RestClient::TooManyRequests, RestClient::InternalServerError => e + rescue Webhooks::Trigger::RetryableError => e Rails.logger.warn("[AgentBots::WebhookJob] attempt #{executions} failed #{e.class.name} payload=#{payload.to_json}") raise end diff --git a/lib/safe_fetch.rb b/lib/safe_fetch.rb index 2264b2850..d664dcf6a 100644 --- a/lib/safe_fetch.rb +++ b/lib/safe_fetch.rb @@ -2,6 +2,8 @@ require 'ssrf_filter' module SafeFetch DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES = %w[image/ video/].freeze + DEFAULT_ALLOWED_CONTENT_TYPES = [].freeze + DEFAULT_SENSITIVE_HEADERS = %w[authorization cookie proxy-authorization].freeze DEFAULT_OPEN_TIMEOUT = 2 DEFAULT_READ_TIMEOUT = 20 DEFAULT_MAX_BYTES_FALLBACK_MB = 40 @@ -19,106 +21,22 @@ module SafeFetch class HttpError < Error; end class FileTooLargeError < Error; end class UnsupportedContentTypeError < Error; end + class UnsupportedMethodError < Error; end +end - def self.fetch(url, - max_bytes: nil, - allowed_content_type_prefixes: DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES, - allowed_content_types: []) +require_relative 'safe_fetch/request_options' +require_relative 'safe_fetch/fetcher' + +module SafeFetch + def self.fetch(url, **, &) raise ArgumentError, 'block required' unless block_given? - effective_max_bytes = max_bytes || default_max_bytes - filename = filename_for(parse_and_validate_url!(url)) - tempfile = Tempfile.new('chatwoot-safe-fetch', binmode: true) - response = fetch_response(url, tempfile, effective_max_bytes, allowed_content_type_prefixes, allowed_content_types) - yield build_result(tempfile, filename, response) + Fetcher.new(RequestOptions.new(url: url, **)).fetch(&) rescue SsrfFilter::InvalidUriScheme, URI::InvalidURIError => e raise InvalidUrlError, e.message rescue SsrfFilter::Error, Resolv::ResolvError => e raise UnsafeUrlError, e.message rescue Net::OpenTimeout, Net::ReadTimeout, SocketError, OpenSSL::SSL::SSLError => e raise FetchError, e.message - ensure - tempfile&.close! - end - - class << self - private - - def fetch_response(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types) - stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types) - end - - def stream_to_tempfile(url, tempfile, max_bytes, allowed_content_type_prefixes, allowed_content_types) - response = nil - bytes_written = 0 - - SsrfFilter.get( - url, - request_proc: ->(request) { apply_url_basic_auth(request) }, - http_options: { open_timeout: DEFAULT_OPEN_TIMEOUT, read_timeout: DEFAULT_READ_TIMEOUT } - ) do |res| - response = res - next unless res.is_a?(Net::HTTPSuccess) - - unless allowed_content_type?(res['content-type'], allowed_content_type_prefixes, allowed_content_types) - raise UnsupportedContentTypeError, "content-type not allowed: #{res['content-type']}" - end - - res.read_body do |chunk| - bytes_written += chunk.bytesize - raise FileTooLargeError, "exceeded #{max_bytes} bytes" if bytes_written > max_bytes - - tempfile.write(chunk) - end - end - - response - end - - def filename_for(uri) - File.basename(uri.path).presence || "download-#{Time.current.to_i}-#{SecureRandom.hex(4)}" - end - - def build_result(tempfile, filename, response) - raise HttpError, "#{response.code} #{response.message}" unless response.is_a?(Net::HTTPSuccess) - - tempfile.rewind - content_type = normalized_content_type(response['content-type']) - Result.new(tempfile: tempfile, filename: filename, content_type: content_type) - end - - def default_max_bytes - limit_mb = GlobalConfigService.load('MAXIMUM_FILE_UPLOAD_SIZE', DEFAULT_MAX_BYTES_FALLBACK_MB).to_i - limit_mb = DEFAULT_MAX_BYTES_FALLBACK_MB if limit_mb <= 0 - limit_mb.megabytes - end - - def parse_and_validate_url!(url) - uri = URI.parse(url) - raise InvalidUrlError, 'scheme must be http or https' unless uri.is_a?(URI::HTTP) || uri.is_a?(URI::HTTPS) - raise InvalidUrlError, 'missing host' if uri.host.blank? - - uri - end - - def allowed_content_type?(value, prefixes, content_types) - mime = normalized_content_type(value) - return false if mime.blank? - - prefixes.any? { |prefix| mime.start_with?(prefix) } || content_types.include?(mime) - end - - def normalized_content_type(value) - value.to_s.split(';').first&.strip&.downcase - end - - def apply_url_basic_auth(request) - uri = request.uri - return if uri.user.blank? - - username = URI.decode_uri_component(uri.user) - password = URI.decode_uri_component(uri.password.to_s) - request.basic_auth(username, password) - end end end diff --git a/lib/safe_fetch/fetcher.rb b/lib/safe_fetch/fetcher.rb new file mode 100644 index 000000000..fa3c01f55 --- /dev/null +++ b/lib/safe_fetch/fetcher.rb @@ -0,0 +1,75 @@ +class SafeFetch::Fetcher + def initialize(options) + @options = options + end + + def fetch + with_tempfile do |tempfile| + response = stream_response(tempfile) + raise SafeFetch::HttpError, "#{response.code} #{response.message}" unless response.is_a?(Net::HTTPSuccess) + + tempfile.rewind + yield SafeFetch::Result.new( + tempfile: tempfile, + filename: options.filename, + content_type: normalized_content_type(response['content-type']) + ) + end + end + + private + + attr_reader :options + + def with_tempfile + tempfile = Tempfile.new('chatwoot-safe-fetch', binmode: true) + yield tempfile + ensure + tempfile&.close! + end + + def stream_response(tempfile) + response = nil + bytes_written = 0 + + SsrfFilter.public_send(options.method, options.url, **options.request_options) do |res| + response = res + next unless res.is_a?(Net::HTTPSuccess) + + validate_content_type!(res['content-type']) + bytes_written = write_response_body(res, tempfile, bytes_written) + end + + response + end + + def validate_content_type!(content_type) + return unless options.validate_content_type? + return if allowed_content_type?(content_type) + + raise SafeFetch::UnsupportedContentTypeError, "content-type not allowed: #{content_type}" + end + + def write_response_body(response, tempfile, bytes_written) + response.read_body do |chunk| + bytes_written += chunk.bytesize + raise SafeFetch::FileTooLargeError, "exceeded #{options.effective_max_bytes} bytes" if bytes_written > options.effective_max_bytes + + tempfile.write(chunk) + end + + bytes_written + end + + def allowed_content_type?(value) + mime = normalized_content_type(value) + return false if mime.blank? + + options.allowed_content_type_prefixes.any? { |prefix| mime.start_with?(prefix) } || + options.allowed_content_types.include?(mime) + end + + def normalized_content_type(value) + value.to_s.split(';').first&.strip&.downcase + end +end diff --git a/lib/safe_fetch/request_options.rb b/lib/safe_fetch/request_options.rb new file mode 100644 index 000000000..72969a76d --- /dev/null +++ b/lib/safe_fetch/request_options.rb @@ -0,0 +1,116 @@ +class SafeFetch::RequestOptions + DEFAULTS = { + method: :get, + body: nil, + max_bytes: nil, + open_timeout: SafeFetch::DEFAULT_OPEN_TIMEOUT, + read_timeout: SafeFetch::DEFAULT_READ_TIMEOUT, + headers: nil, + http_basic_authentication: nil, + allowed_content_type_prefixes: SafeFetch::DEFAULT_ALLOWED_CONTENT_TYPE_PREFIXES, + allowed_content_types: SafeFetch::DEFAULT_ALLOWED_CONTENT_TYPES, + validate_content_type: true + }.freeze + + attr_reader :allowed_content_type_prefixes, :allowed_content_types, :body, :headers, + :http_basic_authentication, :method, :open_timeout, :read_timeout, :uri, :url + + def initialize(url:, **options) + config = DEFAULTS.merge(options) + @url = url + @uri = parse_and_validate_url!(url) + @method = normalize_method(config[:method]) + @body = config[:body] + @max_bytes = config[:max_bytes] + @open_timeout = config[:open_timeout] + @read_timeout = config[:read_timeout] + @headers = normalize_headers(config[:headers]) + @http_basic_authentication = config[:http_basic_authentication] + @allowed_content_type_prefixes = Array(config[:allowed_content_type_prefixes]) + @allowed_content_types = Array(config[:allowed_content_types]) + @validate_content_type = config[:validate_content_type] + end + + def effective_max_bytes + @effective_max_bytes ||= @max_bytes || default_max_bytes + end + + def filename + @filename ||= File.basename(uri.path).presence || "download-#{Time.current.to_i}-#{SecureRandom.hex(4)}" + end + + def request_options + { + headers: headers, + body: body, + request_proc: request_proc, + sensitive_headers: sensitive_headers, + http_options: { open_timeout: open_timeout, read_timeout: read_timeout } + } + end + + def validate_content_type? + @validate_content_type + end + + private + + def default_max_bytes + limit_mb = GlobalConfigService.load('MAXIMUM_FILE_UPLOAD_SIZE', SafeFetch::DEFAULT_MAX_BYTES_FALLBACK_MB).to_i + limit_mb = SafeFetch::DEFAULT_MAX_BYTES_FALLBACK_MB if limit_mb <= 0 + limit_mb.megabytes + end + + def parse_and_validate_url!(value) + parsed_uri = URI.parse(value) + raise SafeFetch::InvalidUrlError, 'scheme must be http or https' unless parsed_uri.is_a?(URI::HTTP) || parsed_uri.is_a?(URI::HTTPS) + raise SafeFetch::InvalidUrlError, 'missing host' if parsed_uri.host.blank? + + parsed_uri + end + + def normalize_method(value) + http_method = value.to_s.downcase.to_sym + return http_method if SsrfFilter::VERB_MAP.key?(http_method) + + raise SafeFetch::UnsupportedMethodError, "unsupported method: #{value}" + end + + def normalize_headers(value) + value&.to_h + end + + def request_proc + proc do |request| + credentials = http_basic_authentication.presence || basic_authentication_for(request.uri) + request.basic_auth(*credentials) if credentials.present? + end + end + + def sensitive_headers + SafeFetch::DEFAULT_SENSITIVE_HEADERS + end + + def basic_authentication_for(request_uri) + uri_basic_authentication(request_uri) || original_uri_basic_authentication(request_uri) + end + + def original_uri_basic_authentication(request_uri) + return unless same_origin?(request_uri, uri) + + uri_basic_authentication(uri) + end + + def same_origin?(request_uri, other_uri) + request_uri.scheme == other_uri.scheme && request_uri.hostname == other_uri.hostname && request_uri.port == other_uri.port + end + + def uri_basic_authentication(value) + return if value.user.blank? + + [ + URI.decode_uri_component(value.user), + URI.decode_uri_component(value.password.to_s) + ] + end +end diff --git a/lib/webhooks/trigger.rb b/lib/webhooks/trigger.rb index 7cb15c836..57d96f51d 100644 --- a/lib/webhooks/trigger.rb +++ b/lib/webhooks/trigger.rb @@ -1,5 +1,15 @@ class Webhooks::Trigger SUPPORTED_ERROR_HANDLE_EVENTS = %w[message_created message_updated].freeze + RETRYABLE_AGENT_BOT_STATUSES = [429, 500].freeze + + class RetryableError < StandardError + attr_reader :status + + def initialize(status:, message:) + @status = status + super(message) + end + end def initialize(url, payload, webhook_type, secret: nil, delivery_id: nil) @url = url @@ -15,11 +25,9 @@ class Webhooks::Trigger def execute perform_request - rescue RestClient::TooManyRequests, RestClient::InternalServerError => e - raise if @webhook_type == :agent_bot_webhook - - handle_failure(e) rescue StandardError => e + raise RetryableError.new(status: http_status(e), message: e.message) if retryable_agent_bot_error?(e) + handle_failure(e) end @@ -32,17 +40,19 @@ class Webhooks::Trigger def perform_request body = @payload.to_json - RestClient::Request.execute( + SafeFetch.fetch( + @url, method: :post, - url: @url, - payload: body, + body: body, headers: request_headers(body), - timeout: webhook_timeout - ) + open_timeout: webhook_timeout, + read_timeout: webhook_timeout, + validate_content_type: false + ) { |_response| nil } end def request_headers(body) - headers = { content_type: :json, accept: :json } + headers = { 'Content-Type' => 'application/json', 'Accept' => 'application/json' } headers['X-Chatwoot-Delivery'] = @delivery_id if @delivery_id.present? if @secret.present? ts = Time.now.to_i.to_s @@ -111,4 +121,14 @@ class Webhooks::Trigger timeout&.positive? ? timeout : 5 end + + def retryable_agent_bot_error?(error) + @webhook_type == :agent_bot_webhook && RETRYABLE_AGENT_BOT_STATUSES.include?(http_status(error)) + end + + def http_status(error) + return unless error.is_a?(SafeFetch::HttpError) + + error.message.to_s[/\A(\d{3})\b/, 1]&.to_i + end end diff --git a/spec/jobs/agent_bots/webhook_job_spec.rb b/spec/jobs/agent_bots/webhook_job_spec.rb index c14c46cb3..a8d026cd3 100644 --- a/spec/jobs/agent_bots/webhook_job_spec.rb +++ b/spec/jobs/agent_bots/webhook_job_spec.rb @@ -8,7 +8,7 @@ RSpec.describe AgentBots::WebhookJob do let(:url) { 'https://test.com' } let(:payload) { { name: 'test' } } let(:webhook_type) { :agent_bot_webhook } - let(:retryable_error) { RestClient::InternalServerError.new(nil, 500) } + let(:retryable_error) { Webhooks::Trigger::RetryableError.new(status: 500, message: '500 Internal Server Error') } before do ActiveJob::Base.queue_adapter = :test @@ -33,7 +33,7 @@ RSpec.describe AgentBots::WebhookJob do it 'configures retry handlers for 429 and 500 errors' do handlers = described_class.rescue_handlers.map(&:first) - expect(handlers).to include('RestClient::TooManyRequests', 'RestClient::InternalServerError') + expect(handlers).to include('Webhooks::Trigger::RetryableError') end it 'retries 3 times and handles failure after retries are exhausted' do @@ -43,7 +43,7 @@ RSpec.describe AgentBots::WebhookJob do allow(Rails.logger).to receive(:warn) expect(Webhooks::Trigger).to receive(:execute).exactly(3).times - expect(trigger_instance).to receive(:handle_failure).with(instance_of(RestClient::InternalServerError)).once + expect(trigger_instance).to receive(:handle_failure).with(instance_of(Webhooks::Trigger::RetryableError)).once expect(Rails.logger).to receive(:warn).with(/AgentBots::WebhookJob/).exactly(3).times perform_enqueued_jobs { job } diff --git a/spec/lib/safe_fetch_spec.rb b/spec/lib/safe_fetch_spec.rb index e2c513587..83f2bdde5 100644 --- a/spec/lib/safe_fetch_spec.rb +++ b/spec/lib/safe_fetch_spec.rb @@ -80,6 +80,66 @@ RSpec.describe SafeFetch do expect(result.content_type).to eq('image/png') end end + + it 'preserves embedded credentials after a same-origin redirect removes userinfo' do + authenticated_url = 'http://user:pass@example.com/protected.png' + initial_url = 'http://example.com/protected.png' + redirect_url = 'http://example.com/public.png' + redirected_headers = nil + + stub_request(:get, initial_url) + .with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNz' }) + .to_return( + status: 302, + headers: { 'Location' => '/public.png' } + ) + stub_request(:get, redirect_url) + .with do |request| + redirected_headers = request.headers.transform_keys(&:downcase) + true + end + .to_return( + status: 200, + body: File.new(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + + described_class.fetch(authenticated_url) do |result| + expect(result.content_type).to eq('image/png') + end + + expect(redirected_headers).to include('authorization' => 'Basic dXNlcjpwYXNz') + end + + it 'strips embedded credentials on cross-origin redirects' do + authenticated_url = 'http://user:pass@example.com/protected.png' + initial_url = 'http://example.com/protected.png' + redirect_url = 'https://example.com/public.png' + redirected_headers = nil + + stub_request(:get, initial_url) + .with(headers: { 'Authorization' => 'Basic dXNlcjpwYXNz' }) + .to_return( + status: 302, + headers: { 'Location' => redirect_url } + ) + stub_request(:get, redirect_url) + .with do |request| + redirected_headers = request.headers.transform_keys(&:downcase) + true + end + .to_return( + status: 200, + body: File.new(Rails.root.join('spec/assets/avatar.png')), + headers: { 'Content-Type' => 'image/png' } + ) + + described_class.fetch(authenticated_url) do |result| + expect(result.content_type).to eq('image/png') + end + + expect(redirected_headers).not_to include('authorization') + end end context 'with URL validation' do @@ -223,6 +283,75 @@ RSpec.describe SafeFetch do end end + context 'with custom request options' do + let(:post_body) { { hello: 'world' }.to_json } + let(:headers) do + { + 'Authorization' => 'Bearer test-token', + 'Content-Type' => 'application/json' + } + end + + it 'supports POST requests with custom headers when content-type validation is disabled' do + stub_request(:post, url) + .with(body: post_body, headers: headers) + .to_return(status: 200, body: '', headers: {}) + + expect do + described_class.fetch( + url, + method: :post, + body: post_body, + headers: headers, + validate_content_type: false + ) { nil } + end.not_to raise_error + end + + it 'preserves non-credential headers on cross-origin redirects' do + redirect_url = 'https://example.com/image.png' + redirected_headers = nil + headers = { + 'Authorization' => 'Bearer test-token', + 'Cookie' => 'session=test', + 'Content-Type' => 'application/json', + 'X-Chatwoot-Delivery' => 'test-uuid', + 'X-Chatwoot-Signature' => 'sha256=test-signature' + } + + stub_request(:post, url).to_return( + status: 307, + headers: { 'Location' => redirect_url } + ) + stub_request(:post, redirect_url) + .with do |request| + redirected_headers = request.headers.transform_keys(&:downcase) + true + end + .to_return(status: 200, body: '', headers: {}) + + described_class.fetch( + url, + method: :post, + body: post_body, + headers: headers, + validate_content_type: false + ) { nil } + + expect(redirected_headers).to include( + 'content-type' => 'application/json', + 'x-chatwoot-delivery' => 'test-uuid', + 'x-chatwoot-signature' => 'sha256=test-signature' + ) + expect(redirected_headers).not_to include('authorization', 'cookie') + end + + it 'raises UnsupportedMethodError for unsupported HTTP methods' do + expect { described_class.fetch(url, method: :options) { nil } } + .to raise_error(described_class::UnsupportedMethodError) + end + end + context 'with body size cap' do it 'honours a custom max_bytes argument' do stub_request(:get, url).to_return( diff --git a/spec/lib/webhooks/trigger_spec.rb b/spec/lib/webhooks/trigger_spec.rb index 90d1ce7f8..1c5ebc16a 100644 --- a/spec/lib/webhooks/trigger_spec.rb +++ b/spec/lib/webhooks/trigger_spec.rb @@ -11,10 +11,13 @@ describe Webhooks::Trigger do let!(:message) { create(:message, account: account, inbox: inbox, conversation: conversation) } let(:webhook_type) { :api_inbox_webhook } - let!(:url) { 'https://test.com' } + let(:url) { 'https://test.com' } + let(:payload) { { hello: :hello } } + let(:fetch_result) { instance_double(SafeFetch::Result) } let(:agent_bot_error_content) { I18n.t('conversations.activity.agent_bot.error_moved_to_open') } let(:default_timeout) { 5 } let(:webhook_timeout) { default_timeout } + let(:base_headers) { { 'Content-Type' => 'application/json', 'Accept' => 'application/json' } } before do ActiveJob::Base.queue_adapter = :test @@ -30,30 +33,23 @@ describe Webhooks::Trigger do describe '#execute' do it 'triggers webhook' do - payload = { hello: :hello } + expect(SafeFetch).to receive(:fetch).with( + url, + method: :post, + body: payload.to_json, + headers: base_headers, + open_timeout: webhook_timeout, + read_timeout: webhook_timeout, + validate_content_type: false + ).and_yield(fetch_result) - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).once trigger.execute(url, payload, webhook_type) end it 'updates message status if webhook fails for message-created event' do payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error')) expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed') end @@ -61,17 +57,18 @@ describe Webhooks::Trigger do it 'updates message status if webhook fails for message-updated event' do payload = { event: 'message_updated', conversation: { id: conversation.id }, id: message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error')) + expect { trigger.execute(url, payload, webhook_type) }.to change { message.reload.status }.from('sent').to('failed') end + it 'treats blocked private webhook URLs as failures' do + payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id } + + expect { trigger.execute('http://127.0.0.1/webhook', payload, webhook_type) } + .to change { message.reload.status }.from('sent').to('failed') + end + context 'when webhook type is agent bot' do let(:webhook_type) { :agent_bot_webhook } let!(:pending_conversation) { create(:conversation, inbox: inbox, status: :pending, account: account) } @@ -80,16 +77,13 @@ describe Webhooks::Trigger do it 'raises 500 errors for retry and does not reopen conversation immediately' do payload = { event: 'message_created', id: pending_message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::InternalServerError.new(nil, 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error')) - expect { trigger.execute(url, payload, webhook_type) }.to raise_error(RestClient::InternalServerError) + expect { trigger.execute(url, payload, webhook_type) } + .to(raise_error do |error| + expect(error.class.name).to eq('Webhooks::Trigger::RetryableError') + expect(error.status).to eq(500) + end) expect(pending_conversation.reload.status).to eq('pending') expect(Conversations::ActivityMessageJob).not_to have_been_enqueued end @@ -97,16 +91,13 @@ describe Webhooks::Trigger do it 'raises 429 errors for retry and does not reopen conversation immediately' do payload = { event: 'message_created', id: pending_message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::TooManyRequests.new(nil, 429)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('429 Too Many Requests')) - expect { trigger.execute(url, payload, webhook_type) }.to raise_error(RestClient::TooManyRequests) + expect { trigger.execute(url, payload, webhook_type) } + .to(raise_error do |error| + expect(error.class.name).to eq('Webhooks::Trigger::RetryableError') + expect(error.status).to eq(429) + end) expect(pending_conversation.reload.status).to eq('pending') expect(Conversations::ActivityMessageJob).not_to have_been_enqueued end @@ -114,14 +105,7 @@ describe Webhooks::Trigger do it 'reopens conversation and enqueues activity message if pending' do payload = { event: 'message_created', id: pending_message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found')) expect do perform_enqueued_jobs do @@ -139,14 +123,7 @@ describe Webhooks::Trigger do it 'does not change message status or enqueue activity when conversation is not pending' do payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found')) expect do trigger.execute(url, payload, webhook_type) @@ -160,14 +137,7 @@ describe Webhooks::Trigger do account.update(keep_pending_on_bot_failure: true) payload = { event: 'message_created', id: pending_message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found')) trigger.execute(url, payload, webhook_type) @@ -179,14 +149,8 @@ describe Webhooks::Trigger do account.update(keep_pending_on_bot_failure: false) payload = { event: 'message_created', id: pending_message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('404 Not Found')) + expect do perform_enqueued_jobs do trigger.execute(url, payload, webhook_type) @@ -204,14 +168,7 @@ describe Webhooks::Trigger do it 'handles 500 without raising for non-agent webhooks' do payload = { event: 'message_created', conversation: { id: conversation.id }, id: message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::InternalServerError.new(nil, 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error')) expect { trigger.execute(url, payload, webhook_type) }.not_to raise_error expect(message.reload.status).to eq('failed') @@ -224,20 +181,30 @@ describe Webhooks::Trigger do context 'without secret or delivery_id' do it 'sends only content-type and accept headers' do - expect(RestClient::Request).to receive(:execute).with( - hash_including(headers: { content_type: :json, accept: :json }) - ) + expect(SafeFetch).to receive(:fetch).with( + url, + method: :post, + body: body, + headers: base_headers, + open_timeout: webhook_timeout, + read_timeout: webhook_timeout, + validate_content_type: false + ).and_yield(fetch_result) + trigger.execute(url, payload, webhook_type) end end context 'with delivery_id' do it 'adds X-Chatwoot-Delivery header' do - expect(RestClient::Request).to receive(:execute) do |args| - expect(args[:headers]['X-Chatwoot-Delivery']).to eq('test-uuid') - expect(args[:headers]).not_to have_key('X-Chatwoot-Signature') - expect(args[:headers]).not_to have_key('X-Chatwoot-Timestamp') + expect(SafeFetch).to receive(:fetch) do |received_url, **options, &block| + expect(received_url).to eq(url) + expect(options[:headers]['X-Chatwoot-Delivery']).to eq('test-uuid') + expect(options[:headers]).not_to have_key('X-Chatwoot-Signature') + expect(options[:headers]).not_to have_key('X-Chatwoot-Timestamp') + block.call(fetch_result) end + trigger.execute(url, payload, webhook_type, delivery_id: 'test-uuid') end end @@ -246,38 +213,45 @@ describe Webhooks::Trigger do let(:secret) { 'test-secret' } it 'adds X-Chatwoot-Timestamp header' do - expect(RestClient::Request).to receive(:execute) do |args| - expect(args[:headers]['X-Chatwoot-Timestamp']).to match(/\A\d+\z/) + expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block| + expect(options[:headers]['X-Chatwoot-Timestamp']).to match(/\A\d+\z/) + block.call(fetch_result) end + trigger.execute(url, payload, webhook_type, secret: secret) end it 'adds X-Chatwoot-Signature header with correct HMAC' do - expect(RestClient::Request).to receive(:execute) do |args| - ts = args[:headers]['X-Chatwoot-Timestamp'] + expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block| + ts = options[:headers]['X-Chatwoot-Timestamp'] expected_sig = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, "#{ts}.#{body}")}" - expect(args[:headers]['X-Chatwoot-Signature']).to eq(expected_sig) + expect(options[:headers]['X-Chatwoot-Signature']).to eq(expected_sig) + block.call(fetch_result) end + trigger.execute(url, payload, webhook_type, secret: secret) end it 'signs timestamp.body not just body' do - expect(RestClient::Request).to receive(:execute) do |args| - args[:headers]['X-Chatwoot-Timestamp'] + expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block| wrong_sig = "sha256=#{OpenSSL::HMAC.hexdigest('SHA256', secret, body)}" - expect(args[:headers]['X-Chatwoot-Signature']).not_to eq(wrong_sig) + expect(options[:headers]['X-Chatwoot-Signature']).not_to eq(wrong_sig) + block.call(fetch_result) end + trigger.execute(url, payload, webhook_type, secret: secret) end end context 'with both secret and delivery_id' do it 'includes all three security headers' do - expect(RestClient::Request).to receive(:execute) do |args| - expect(args[:headers]['X-Chatwoot-Delivery']).to eq('abc-123') - expect(args[:headers]['X-Chatwoot-Timestamp']).to be_present - expect(args[:headers]['X-Chatwoot-Signature']).to start_with('sha256=') + expect(SafeFetch).to receive(:fetch) do |_received_url, **options, &block| + expect(options[:headers]['X-Chatwoot-Delivery']).to eq('abc-123') + expect(options[:headers]['X-Chatwoot-Timestamp']).to be_present + expect(options[:headers]['X-Chatwoot-Signature']).to start_with('sha256=') + block.call(fetch_result) end + trigger.execute(url, payload, webhook_type, secret: 'mysecret', delivery_id: 'abc-123') end end @@ -286,14 +260,7 @@ describe Webhooks::Trigger do it 'does not update message status if webhook fails for other events' do payload = { event: 'conversation_created', conversation: { id: conversation.id }, id: message.id } - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: webhook_timeout - ).and_raise(RestClient::ExceptionWithResponse.new('error', 500)).once + expect(SafeFetch).to receive(:fetch).and_raise(SafeFetch::HttpError.new('500 Internal Server Error')) expect { trigger.execute(url, payload, webhook_type) }.not_to(change { message.reload.status }) end @@ -302,16 +269,15 @@ describe Webhooks::Trigger do let(:webhook_timeout) { nil } it 'falls back to default timeout' do - payload = { hello: :hello } - - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: default_timeout - ).once + expect(SafeFetch).to receive(:fetch).with( + url, + method: :post, + body: payload.to_json, + headers: base_headers, + open_timeout: default_timeout, + read_timeout: default_timeout, + validate_content_type: false + ).and_yield(fetch_result) trigger.execute(url, payload, webhook_type) end @@ -321,16 +287,15 @@ describe Webhooks::Trigger do let(:webhook_timeout) { -1 } it 'falls back to default timeout' do - payload = { hello: :hello } - - expect(RestClient::Request).to receive(:execute) - .with( - method: :post, - url: url, - payload: payload.to_json, - headers: { content_type: :json, accept: :json }, - timeout: default_timeout - ).once + expect(SafeFetch).to receive(:fetch).with( + url, + method: :post, + body: payload.to_json, + headers: base_headers, + open_timeout: default_timeout, + read_timeout: default_timeout, + validate_content_type: false + ).and_yield(fetch_result) trigger.execute(url, payload, webhook_type) end From b0aa844a3266604c589dff0943399a8ad31bf656 Mon Sep 17 00:00:00 2001 From: ramalau <71857041+ramalau0@users.noreply.github.com> Date: Mon, 27 Apr 2026 21:44:51 +0200 Subject: [PATCH 17/32] fix(portals): handle integer blob_id in process_attached_logo without 500 (#14274) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updating portal settings (name, header text, page title, homepage link) on a portal that already has a logo attached returns 500. The error is \`NoMethodError: undefined method 'valid_encoding?' for an instance of Integer\`. The fix is a two-character change in \`process_attached_logo\`. Closes #13300 ## Root cause \`ActiveStorage::Blob.find_signed\` expects a signed ID string (e.g. \`"eyJfcmFpbH..."\`). Internally it calls \`valid_encoding?\` on the argument to validate the signature payload — a method that exists on \`String\` but not \`Integer\`. When a portal already has a logo, the frontend includes the blob's raw database integer ID (e.g. \`blob_id: 170\`) in the update request payload. The controller passes this integer directly to \`find_signed\`, which immediately raises \`NoMethodError\` before any database query is made. \`\`\`ruby # before, crashes when blob_id is an Integer blob_id = params[:blob_id] blob = ActiveStorage::Blob.find_signed(blob_id) # NoMethodError here @portal.logo.attach(blob) \`\`\` ## What changed \`\`\`ruby # after, safe for any input type blob = ActiveStorage::Blob.find_signed(params[:blob_id].to_s) @portal.logo.attach(blob) if blob \`\`\` \`.to_s\` on an Integer produces a plain decimal string (\`"170"\`), which is not a valid signed ID. \`find_signed\` returns \`nil\` for any invalid signature rather than raising, so the nil guard prevents a broken \`attach\` call. The existing logo remains attached and the settings update succeeds. ## Trade-offs considered | Option | Decision | |---|---| | \`find(blob_id)\` when input is an Integer | Bypasses signature verification — any authenticated user knowing a blob ID could attach arbitrary files to a portal. Security risk. Rejected. | | Raise a 422 for non-string blob_id | Overly strict — the frontend sending an integer is pre-existing behaviour this PR shouldn't break. | | Silently no-op for invalid blob_id (chosen) | Correct product behaviour: if no valid signed upload is provided, leave the logo unchanged. The settings update still succeeds. | ## Known limitation The correct long-term fix is also on the frontend: it should only send \`blob_id\` when attaching a **new** upload (using the signed ID from the direct-upload flow), not when re-submitting the existing logo's raw database integer ID. This PR makes the server robust against the current frontend behaviour without requiring a coordinated frontend change. ## How to reproduce 1. Create a Help Center portal and upload a logo 2. Update any text field via \`PUT /api/v1/accounts/:id/portals/:slug\` while including \`blob_id: \` in the payload 3. Observe 500 with \`NoMethodError: undefined method 'valid_encoding?' for an instance of Integer\` After this fix, the request returns 200, settings are updated, and the existing logo is preserved. Co-authored-by: Ramalau Debeila --- .../api/v1/accounts/portals_controller.rb | 5 ++--- .../api/v1/accounts/portals_controller_spec.rb | 12 ++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v1/accounts/portals_controller.rb b/app/controllers/api/v1/accounts/portals_controller.rb index 972b244fa..ade83d8ec 100644 --- a/app/controllers/api/v1/accounts/portals_controller.rb +++ b/app/controllers/api/v1/accounts/portals_controller.rb @@ -61,9 +61,8 @@ class Api::V1::Accounts::PortalsController < Api::V1::Accounts::BaseController end def process_attached_logo - blob_id = params[:blob_id] - blob = ActiveStorage::Blob.find_signed(blob_id) - @portal.logo.attach(blob) + blob = ActiveStorage::Blob.find_signed(params[:blob_id].to_s) + @portal.logo.attach(blob) if blob end private diff --git a/spec/controllers/api/v1/accounts/portals_controller_spec.rb b/spec/controllers/api/v1/accounts/portals_controller_spec.rb index 19bc795c3..9c780c00a 100644 --- a/spec/controllers/api/v1/accounts/portals_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/portals_controller_spec.rb @@ -180,6 +180,18 @@ RSpec.describe 'Api::V1::Accounts::Portals', type: :request do expect(portal.archived).to be_truthy end + it 'does not raise when blob_id is an integer (existing logo re-sent by frontend)' do + portal.logo.attach(io: Rails.root.join('spec/assets/avatar.png').open, filename: 'avatar.png', content_type: 'image/png') + + put "/api/v1/accounts/#{account.id}/portals/#{portal.slug}", + params: { portal: { name: 'updated_name' }, blob_id: portal.logo.blob.id }, + headers: admin.create_new_auth_token + + expect(response).to have_http_status(:success) + expect(response.parsed_body['name']).to eq('updated_name') + expect(portal.reload.logo).to be_attached + end + it 'clears associated web widget when inbox selection is blank' do web_widget_inbox = create(:inbox, account: account) portal.update!(channel_web_widget: web_widget_inbox.channel) From 51eb626b889e49a5210735ce5f771c804081ce99 Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma <32020192+tds-1@users.noreply.github.com> Date: Tue, 28 Apr 2026 10:09:41 +0700 Subject: [PATCH 18/32] feat: allow disabling 2FA with a backup code (#14102) ## Linear Ticket - https://linear.app/chatwoot/issue/CW-6883/allow-disabling-2fa-using-a-backup-code ## Description When a user loses access to their authenticator app, they can now disable 2FA using one of their saved backup codes (in addition to their password), so they can re-enroll a new authenticator. The disable dialog includes a toggle to switch between entering a verification code and a backup code. ## Type of change - [x] Bug fix (non-breaking change which fixes an issue) ## How Has This Been Tested? - Via UI flows Screenshot 2026-04-20 at 2 17 21 PM Screenshot 2026-04-20 at 2 17 36 PM ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented on my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --- .../api/v1/profile/mfa_controller.rb | 7 ++-- app/javascript/dashboard/api/mfa.js | 4 +-- .../dashboard/i18n/locale/en/mfa.json | 6 +++- .../settings/profile/MfaManagementActions.vue | 33 ++++++++++++++++++- .../settings/profile/MfaSettings.vue | 4 +-- .../api/v1/profile/mfa_controller_spec.rb | 17 ++++++++++ 6 files changed, 62 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/v1/profile/mfa_controller.rb b/app/controllers/api/v1/profile/mfa_controller.rb index dd874f222..8480b64fb 100644 --- a/app/controllers/api/v1/profile/mfa_controller.rb +++ b/app/controllers/api/v1/profile/mfa_controller.rb @@ -2,8 +2,8 @@ class Api::V1::Profile::MfaController < Api::BaseController before_action :check_mfa_feature_available before_action :check_mfa_enabled, only: [:destroy, :backup_codes] before_action :check_mfa_disabled, only: [:create, :verify] - before_action :validate_otp, only: [:verify, :backup_codes, :destroy] before_action :validate_password, only: [:destroy] + before_action :validate_otp, only: [:verify, :backup_codes, :destroy] def show; end @@ -48,7 +48,8 @@ class Api::V1::Profile::MfaController < Api::BaseController def validate_otp authenticated = Mfa::AuthenticationService.new( user: current_user, - otp_code: mfa_params[:otp_code] + otp_code: mfa_params[:otp_code], + backup_code: mfa_params[:backup_code] ).authenticate return if authenticated @@ -63,6 +64,6 @@ class Api::V1::Profile::MfaController < Api::BaseController end def mfa_params - params.permit(:otp_code, :password) + params.permit(:otp_code, :backup_code, :password) end end diff --git a/app/javascript/dashboard/api/mfa.js b/app/javascript/dashboard/api/mfa.js index c18bea3e9..38cb93810 100644 --- a/app/javascript/dashboard/api/mfa.js +++ b/app/javascript/dashboard/api/mfa.js @@ -14,9 +14,9 @@ class MfaAPI extends ApiClient { return axios.post(`${this.url}/verify`, { otp_code: otpCode }); } - disable(password, otpCode) { + disable(password, { otpCode, backupCode } = {}) { return axios.delete(this.url, { - data: { password, otp_code: otpCode }, + data: { password, otp_code: otpCode, backup_code: backupCode }, }); } diff --git a/app/javascript/dashboard/i18n/locale/en/mfa.json b/app/javascript/dashboard/i18n/locale/en/mfa.json index b03917bcd..8e356aad4 100644 --- a/app/javascript/dashboard/i18n/locale/en/mfa.json +++ b/app/javascript/dashboard/i18n/locale/en/mfa.json @@ -51,10 +51,14 @@ }, "DISABLE": { "TITLE": "Disable Two-Factor Authentication", - "DESCRIPTION": "You'll need to enter your password and a verification code to disable two-factor authentication.", + "DESCRIPTION": "You'll need to enter your password and either a verification code from your authenticator app or a backup code to disable two-factor authentication.", "PASSWORD": "Password", "OTP_CODE": "Verification Code", "OTP_CODE_PLACEHOLDER": "000000", + "BACKUP_CODE": "Backup Code", + "BACKUP_CODE_PLACEHOLDER": "Enter one of your backup codes", + "USE_BACKUP_CODE": "Lost access to your authenticator? Use a backup code instead", + "USE_OTP_CODE": "Use a verification code from your authenticator app", "CONFIRM": "Disable 2FA", "CANCEL": "Cancel", "SUCCESS": "Two-factor authentication has been disabled", diff --git a/app/javascript/dashboard/routes/dashboard/settings/profile/MfaManagementActions.vue b/app/javascript/dashboard/routes/dashboard/settings/profile/MfaManagementActions.vue index caf9e2a6a..b49bae086 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/profile/MfaManagementActions.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/profile/MfaManagementActions.vue @@ -31,6 +31,8 @@ const backupCodesDialogRef = ref(null); // Form values const disablePassword = ref(''); const disableOtpCode = ref(''); +const disableBackupCode = ref(''); +const useBackupCodeToDisable = ref(false); const regenerateOtpCode = ref(''); // Utility functions @@ -54,10 +56,17 @@ const downloadBackupCodes = () => { const handleDisableMfa = async () => { emit('disableMfa', { password: disablePassword.value, - otpCode: disableOtpCode.value, + otpCode: useBackupCodeToDisable.value ? '' : disableOtpCode.value, + backupCode: useBackupCodeToDisable.value ? disableBackupCode.value : '', }); }; +const toggleDisableMethod = () => { + useBackupCodeToDisable.value = !useBackupCodeToDisable.value; + disableOtpCode.value = ''; + disableBackupCode.value = ''; +}; + const handleRegenerateBackupCodes = async () => { emit('regenerateBackupCodes', { otpCode: regenerateOtpCode.value, @@ -68,6 +77,8 @@ const handleRegenerateBackupCodes = async () => { const resetDisableForm = () => { disablePassword.value = ''; disableOtpCode.value = ''; + disableBackupCode.value = ''; + useBackupCodeToDisable.value = false; disableDialogRef.value?.close(); }; @@ -157,12 +168,32 @@ defineExpose({ :label="$t('MFA_SETTINGS.DISABLE.PASSWORD')" /> + + diff --git a/app/javascript/dashboard/routes/dashboard/settings/profile/MfaSettings.vue b/app/javascript/dashboard/routes/dashboard/settings/profile/MfaSettings.vue index 80dce179b..bd21dfafb 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/profile/MfaSettings.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/profile/MfaSettings.vue @@ -104,9 +104,9 @@ const cancelSetup = () => { }; // Disable MFA -const disableMfa = async ({ password, otpCode }) => { +const disableMfa = async ({ password, otpCode, backupCode }) => { try { - await mfaAPI.disable(password, otpCode); + await mfaAPI.disable(password, { otpCode, backupCode }); mfaEnabled.value = false; backupCodesGenerated.value = false; managementActionsRef.value?.resetDisableForm(); diff --git a/spec/requests/api/v1/profile/mfa_controller_spec.rb b/spec/requests/api/v1/profile/mfa_controller_spec.rb index 97a2e206f..eebd997ee 100644 --- a/spec/requests/api/v1/profile/mfa_controller_spec.rb +++ b/spec/requests/api/v1/profile/mfa_controller_spec.rb @@ -205,6 +205,23 @@ RSpec.describe 'MFA API', type: :request do expect(json_response['error']).to include('Invalid') end end + + context 'with valid password and backup code' do + it 'disables 2FA successfully' do + backup_code = user.otp_backup_codes.first + + delete '/api/v1/profile/mfa', + params: { password: 'Test@123456', backup_code: backup_code }, + headers: user.create_new_auth_token, + as: :json + + expect(response).to have_http_status(:success) + user.reload + expect(user.otp_required_for_login).to be_falsey + expect(user.otp_secret).to be_nil + expect(user.otp_backup_codes).to be_blank + end + end end context 'when 2FA is not enabled' do From 224556fd1b330f3467c2ca79f5c70056f958edb0 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Tue, 28 Apr 2026 10:35:51 +0530 Subject: [PATCH 19/32] feat: onboarding account details with enriched data [UPM-17][UPM-18] (#13979) Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> --- app/builders/account_builder.rb | 6 +- app/controllers/api/v1/accounts_controller.rb | 8 +- app/javascript/dashboard/App.vue | 6 +- .../inline-input/InlineInput.vue | 6 + .../helper/AnalyticsHelper/events.js | 5 + .../dashboard/helper/actionCable.js | 5 + .../dashboard/i18n/locale/en/index.js | 2 + .../dashboard/i18n/locale/en/onboarding.json | 34 ++ .../routes/dashboard/dashboard.routes.js | 9 + .../routes/dashboard/onboarding/Index.vue | 417 ++++++++++++++++++ .../onboarding/OnboardingFormRow.vue | 18 + .../onboarding/OnboardingFormSelect.vue | 37 ++ .../dashboard/onboarding/OnboardingLayout.vue | 120 +++++ .../onboarding/OnboardingSection.vue | 49 ++ .../routes/dashboard/onboarding/constants.js | 68 +++ app/javascript/dashboard/routes/index.js | 30 +- app/javascript/dashboard/routes/index.spec.js | 10 +- .../dashboard/store/modules/accounts.js | 19 +- .../dashboard/store/modules/auth.js | 14 + .../modules/specs/auth/mutations.spec.js | 42 ++ .../dashboard/store/mutation-types.js | 1 + app/models/account.rb | 8 + .../api/v1/models/_account.json.jbuilder | 4 +- app/views/api/v1/models/_user.json.jbuilder | 1 + lib/tasks/onboarding.rake | 14 + spec/builders/account_builder_spec.rb | 5 + .../api/v1/accounts_controller_spec.rb | 10 + theme/icons.js | 5 + 28 files changed, 926 insertions(+), 27 deletions(-) create mode 100644 app/javascript/dashboard/i18n/locale/en/onboarding.json create mode 100644 app/javascript/dashboard/routes/dashboard/onboarding/Index.vue create mode 100644 app/javascript/dashboard/routes/dashboard/onboarding/OnboardingFormRow.vue create mode 100644 app/javascript/dashboard/routes/dashboard/onboarding/OnboardingFormSelect.vue create mode 100644 app/javascript/dashboard/routes/dashboard/onboarding/OnboardingLayout.vue create mode 100644 app/javascript/dashboard/routes/dashboard/onboarding/OnboardingSection.vue create mode 100644 app/javascript/dashboard/routes/dashboard/onboarding/constants.js create mode 100644 lib/tasks/onboarding.rake diff --git a/app/builders/account_builder.rb b/app/builders/account_builder.rb index 532487a1b..5127ea612 100644 --- a/app/builders/account_builder.rb +++ b/app/builders/account_builder.rb @@ -44,7 +44,11 @@ class AccountBuilder end def create_account - @account = Account.create!(name: account_name, locale: I18n.locale) + @account = Account.create!( + name: account_name, + locale: I18n.locale, + custom_attributes: { 'onboarding_step' => 'account_details' } + ) Current.account = @account end diff --git a/app/controllers/api/v1/accounts_controller.rb b/app/controllers/api/v1/accounts_controller.rb index 7176d6e1b..dddf3dd09 100644 --- a/app/controllers/api/v1/accounts_controller.rb +++ b/app/controllers/api/v1/accounts_controller.rb @@ -58,6 +58,7 @@ class Api::V1::AccountsController < Api::BaseController @account.assign_attributes(account_params.slice(:name, :locale, :domain, :support_email)) @account.custom_attributes.merge!(custom_attributes_params) @account.settings.merge!(settings_params) + @account.custom_attributes.delete('onboarding_step') if @account.custom_attributes['onboarding_step'] == 'account_details' @account.custom_attributes['onboarding_step'] = 'invite_team' if @account.custom_attributes['onboarding_step'] == 'account_update' @account.save! end @@ -71,9 +72,10 @@ class Api::V1::AccountsController < Api::BaseController private def enqueue_branding_enrichment - return if account_params[:email].blank? + email = account_params[:email].presence || @user&.email + return if email.blank? - Account::BrandingEnrichmentJob.perform_later(@account.id, account_params[:email]) + Account::BrandingEnrichmentJob.perform_later(@account.id, email) Redis::Alfred.set(format(Redis::Alfred::ACCOUNT_ONBOARDING_ENRICHMENT, account_id: @account.id), '1', ex: 30) rescue StandardError => e # Enrichment is optional — never let queue/Redis failures abort signup @@ -109,7 +111,7 @@ class Api::V1::AccountsController < Api::BaseController end def custom_attributes_params - params.permit(:industry, :company_size, :timezone) + params.permit(:industry, :company_size, :timezone, :referral_source, :user_role) end def settings_params diff --git a/app/javascript/dashboard/App.vue b/app/javascript/dashboard/App.vue index a706e2df5..988829407 100644 --- a/app/javascript/dashboard/App.vue +++ b/app/javascript/dashboard/App.vue @@ -59,7 +59,6 @@ export default { isRTL: 'accounts/isRTL', currentUser: 'getCurrentUser', authUIFlags: 'getAuthUIFlags', - accountUIFlags: 'accounts/getUIFlags', }), hideOnOnboardingView() { return !isOnOnboardingView(this.$route); @@ -107,8 +106,9 @@ export default { this.$store.dispatch('setActiveAccount', { accountId: this.currentAccountId, }); + const account = this.getAccount(this.currentAccountId); const { locale, latest_chatwoot_version: latestChatwootVersion } = - this.getAccount(this.currentAccountId); + account; const { pubsub_token: pubsubToken } = this.currentUser || {}; // If user locale is set, use it; otherwise use account locale this.setLocale(this.uiSettings?.locale || locale); @@ -131,7 +131,7 @@ export default { -
-

+

+

{{ $t('TEAMS_SETTINGS.AGENTS.SELECTED_COUNT', { selected: selectedAgents.length, diff --git a/app/javascript/dashboard/routes/dashboard/settings/teams/Create/AddAgents.vue b/app/javascript/dashboard/routes/dashboard/settings/teams/Create/AddAgents.vue index ef1e14663..19a7a325d 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/teams/Create/AddAgents.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/teams/Create/AddAgents.vue @@ -88,7 +88,7 @@ export default {