diff --git a/app/builders/email/base_builder.rb b/app/builders/email/base_builder.rb index 731b1b0f5..6f79d6018 100644 --- a/app/builders/email/base_builder.rb +++ b/app/builders/email/base_builder.rb @@ -1,4 +1,6 @@ class Email::BaseBuilder + include EmailAddressParseable + pattr_initialize [:inbox!] private @@ -47,8 +49,4 @@ class Email::BaseBuilder # can save it in the format "Name " parse_email(account.support_email) end - - def parse_email(email_string) - Mail::Address.new(email_string).address - end end diff --git a/app/javascript/dashboard/composables/spec/useAutomation.spec.js b/app/javascript/dashboard/composables/spec/useAutomation.spec.js index 6cce15996..2e07f1671 100644 --- a/app/javascript/dashboard/composables/spec/useAutomation.spec.js +++ b/app/javascript/dashboard/composables/spec/useAutomation.spec.js @@ -8,6 +8,7 @@ import { agents, teams, labels, + booleanFilterOptions, statusFilterOptions, messageTypeOptions, priorityOptions, @@ -73,6 +74,8 @@ describe('useAutomation', () => { return countries; case 'message_type': return messageTypeOptions; + case 'private_note': + return booleanFilterOptions; case 'priority': return priorityOptions; default: @@ -226,6 +229,9 @@ describe('useAutomation', () => { expect(getConditionDropdownValues('message_type')).toEqual( messageTypeOptions ); + expect(getConditionDropdownValues('private_note')).toEqual( + booleanFilterOptions + ); expect(getConditionDropdownValues('priority')).toEqual(priorityOptions); }); diff --git a/app/javascript/dashboard/composables/spec/useEditableAutomation.spec.js b/app/javascript/dashboard/composables/spec/useEditableAutomation.spec.js new file mode 100644 index 000000000..c6177e9a2 --- /dev/null +++ b/app/javascript/dashboard/composables/spec/useEditableAutomation.spec.js @@ -0,0 +1,54 @@ +import { useEditableAutomation } from '../useEditableAutomation'; +import useAutomationValues from '../useAutomationValues'; + +vi.mock('../useAutomationValues'); + +describe('useEditableAutomation', () => { + beforeEach(() => { + useAutomationValues.mockReturnValue({ + getConditionDropdownValues: vi.fn(attributeKey => { + if (attributeKey === 'private_note') { + return [ + { id: true, name: 'True' }, + { id: false, name: 'False' }, + ]; + } + + return []; + }), + getActionDropdownValues: vi.fn(), + }); + }); + + it('rehydrates boolean conditions as a single selected option', () => { + const automation = { + event_name: 'message_created', + conditions: [ + { + attribute_key: 'private_note', + filter_operator: 'equal_to', + values: [false], + query_operator: null, + }, + ], + actions: [], + }; + const automationTypes = { + message_created: { + conditions: [{ key: 'private_note', inputType: 'search_select' }], + }, + }; + + const { formatAutomation } = useEditableAutomation(); + const result = formatAutomation(automation, [], automationTypes, []); + + expect(result.conditions).toEqual([ + { + attribute_key: 'private_note', + filter_operator: 'equal_to', + values: { id: false, name: 'False' }, + query_operator: 'and', + }, + ]); + }); +}); diff --git a/app/javascript/dashboard/composables/useEditableAutomation.js b/app/javascript/dashboard/composables/useEditableAutomation.js index 8b9041a8f..3f4e65b3c 100644 --- a/app/javascript/dashboard/composables/useEditableAutomation.js +++ b/app/javascript/dashboard/composables/useEditableAutomation.js @@ -46,11 +46,26 @@ export function useEditableAutomation() { if (inputType === 'comma_separated_plain_text') { return { ...condition, values: condition.values.join(',') }; } + const dropdownValues = getConditionDropdownValues( + condition.attribute_key + ); + const hasBooleanOptions = + inputType === 'search_select' && + dropdownValues.length && + dropdownValues.every(item => typeof item.id === 'boolean'); + + if (hasBooleanOptions) { + return { + ...condition, + query_operator: condition.query_operator || 'and', + values: dropdownValues.find(item => item.id === condition.values[0]), + }; + } return { ...condition, query_operator: condition.query_operator || 'and', - values: [...getConditionDropdownValues(condition.attribute_key)].filter( - item => [...condition.values].includes(item.id) + values: [...dropdownValues].filter(item => + [...condition.values].includes(item.id) ), }; }); diff --git a/app/javascript/dashboard/helper/automationHelper.js b/app/javascript/dashboard/helper/automationHelper.js index fa6120c16..8aed8dcda 100644 --- a/app/javascript/dashboard/helper/automationHelper.js +++ b/app/javascript/dashboard/helper/automationHelper.js @@ -150,6 +150,7 @@ export const getConditionOptions = ({ conversation_language: languages, country_code: countries, message_type: messageTypeOptions, + private_note: booleanFilterOptions, priority: priorityOptions, labels: generateConditionOptions(labels, 'title'), }; diff --git a/app/javascript/dashboard/helper/specs/automationHelper.spec.js b/app/javascript/dashboard/helper/specs/automationHelper.spec.js index 10088963a..033481a0b 100644 --- a/app/javascript/dashboard/helper/specs/automationHelper.spec.js +++ b/app/javascript/dashboard/helper/specs/automationHelper.spec.js @@ -178,6 +178,21 @@ describe('getConditionOptions', () => { }) ).toEqual(testOptions); }); + + it('returns boolean options for private_note', () => { + const booleanOptions = [ + { id: true, name: 'True' }, + { id: false, name: 'False' }, + ]; + + expect( + helpers.getConditionOptions({ + booleanFilterOptions: booleanOptions, + customAttributes, + type: 'private_note', + }) + ).toEqual(booleanOptions); + }); }); describe('getFileName', () => { diff --git a/app/javascript/dashboard/i18n/locale/en/automation.json b/app/javascript/dashboard/i18n/locale/en/automation.json index 22a9735f4..d338fa9a2 100644 --- a/app/javascript/dashboard/i18n/locale/en/automation.json +++ b/app/javascript/dashboard/i18n/locale/en/automation.json @@ -169,6 +169,7 @@ }, "ATTRIBUTES": { "MESSAGE_TYPE": "Message Type", + "PRIVATE_NOTE": "Private Note", "MESSAGE_CONTAINS": "Message Contains", "EMAIL": "Email", "INBOX": "Inbox", diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js index 3acca3e2e..24947c63b 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/constants.js @@ -14,6 +14,12 @@ export const AUTOMATIONS = { inputType: 'search_select', filterOperators: OPERATOR_TYPES_1, }, + { + key: 'private_note', + name: 'PRIVATE_NOTE', + inputType: 'search_select', + filterOperators: OPERATOR_TYPES_1, + }, { key: 'content', name: 'MESSAGE_CONTAINS', diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb index 220531221..d9e6ec8e0 100644 --- a/app/mailers/conversation_reply_mailer.rb +++ b/app/mailers/conversation_reply_mailer.rb @@ -5,6 +5,7 @@ class ConversationReplyMailer < ApplicationMailer include ConversationReplyMailerHelper include ReferencesHeaderBuilder + include EmailAddressParseable default from: ENV.fetch('MAILER_SENDER_EMAIL', 'Chatwoot ') layout :choose_layout @@ -139,10 +140,6 @@ class ConversationReplyMailer < ApplicationMailer sender_name(@channel.email) end - def parse_email(email_string) - Mail::Address.new(email_string).address - end - def inbox_from_email_address return @inbox.email_address if @inbox.email_address diff --git a/app/models/account.rb b/app/models/account.rb index 06f47636e..181081aad 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -30,50 +30,7 @@ class Account < ApplicationRecord include CacheKeys include CaptainFeaturable include AccountEmailRateLimitable - - SETTINGS_PARAMS_SCHEMA = { - 'type': 'object', - 'properties': - { - 'auto_resolve_after': { 'type': %w[integer null], 'minimum': 10, 'maximum': 1_439_856 }, - 'auto_resolve_message': { 'type': %w[string null] }, - 'auto_resolve_ignore_waiting': { 'type': %w[boolean null] }, - 'audio_transcriptions': { 'type': %w[boolean null] }, - 'auto_resolve_label': { 'type': %w[string null] }, - 'keep_pending_on_bot_failure': { 'type': %w[boolean null] }, - 'captain_auto_resolve_mode': { 'type': %w[string null], 'enum': ['evaluated', 'legacy', 'disabled', nil] }, - 'conversation_required_attributes': { - 'type': %w[array null], - 'items': { 'type': 'string' } - }, - 'captain_models': { - 'type': %w[object null], - 'properties': { - 'editor': { 'type': %w[string null] }, - 'assistant': { 'type': %w[string null] }, - 'copilot': { 'type': %w[string null] }, - 'label_suggestion': { 'type': %w[string null] }, - 'audio_transcription': { 'type': %w[string null] }, - 'help_center_search': { 'type': %w[string null] } - }, - 'additionalProperties': false - }, - 'captain_features': { - 'type': %w[object null], - 'properties': { - 'editor': { 'type': %w[boolean null] }, - 'assistant': { 'type': %w[boolean null] }, - 'copilot': { 'type': %w[boolean null] }, - 'label_suggestion': { 'type': %w[boolean null] }, - 'audio_transcription': { 'type': %w[boolean null] }, - 'help_center_search': { 'type': %w[boolean null] } - }, - 'additionalProperties': false - } - }, - 'required': [], - 'additionalProperties': true - }.to_json.freeze + include AccountSettingsSchema DEFAULT_QUERY_SETTING = { flag_query_mode: :bit_operator, @@ -86,6 +43,7 @@ class Account < ApplicationRecord schema: SETTINGS_PARAMS_SCHEMA, attribute_resolver: ->(record) { record.settings } validate :validate_reporting_timezone + validate :validate_support_email_format, if: :will_save_change_to_support_email? store_accessor :settings, :auto_resolve_after, :auto_resolve_message, :auto_resolve_ignore_waiting @@ -223,6 +181,16 @@ class Account < ApplicationRecord errors.add(:reporting_timezone, I18n.t('errors.account.reporting_timezone.invalid')) end + def validate_support_email_format + value = attributes['support_email'] + return if value.blank? + + parsed = Mail::Address.new(value).address + errors.add(:support_email, I18n.t('errors.account.support_email.invalid')) if parsed.blank? + rescue Mail::Field::ParseError, Mail::Field::IncompleteParseError + errors.add(:support_email, I18n.t('errors.account.support_email.invalid')) + end + def remove_account_sequences ActiveRecord::Base.connection.exec_query("drop sequence IF EXISTS camp_dpid_seq_#{id}") ActiveRecord::Base.connection.exec_query("drop sequence IF EXISTS conv_dpid_seq_#{id}") diff --git a/app/models/automation_rule.rb b/app/models/automation_rule.rb index 8162abb91..3ab23530d 100644 --- a/app/models/automation_rule.rb +++ b/app/models/automation_rule.rb @@ -36,7 +36,7 @@ class AutomationRule < ApplicationRecord def conditions_attributes %w[content email country_code status message_type browser_language assignee_id team_id referer city company inbox_id - mail_subject phone_number priority conversation_language labels] + mail_subject phone_number priority conversation_language labels private_note] end def actions_attributes diff --git a/app/models/concerns/account_settings_schema.rb b/app/models/concerns/account_settings_schema.rb new file mode 100644 index 000000000..52e1c2811 --- /dev/null +++ b/app/models/concerns/account_settings_schema.rb @@ -0,0 +1,47 @@ +module AccountSettingsSchema + extend ActiveSupport::Concern + + SETTINGS_PARAMS_SCHEMA = { + 'type': 'object', + 'properties': + { + 'auto_resolve_after': { 'type': %w[integer null], 'minimum': 10, 'maximum': 1_439_856 }, + 'auto_resolve_message': { 'type': %w[string null] }, + 'auto_resolve_ignore_waiting': { 'type': %w[boolean null] }, + 'audio_transcriptions': { 'type': %w[boolean null] }, + 'auto_resolve_label': { 'type': %w[string null] }, + 'keep_pending_on_bot_failure': { 'type': %w[boolean null] }, + 'captain_auto_resolve_mode': { 'type': %w[string null], 'enum': ['evaluated', 'legacy', 'disabled', nil] }, + 'conversation_required_attributes': { + 'type': %w[array null], + 'items': { 'type': 'string' } + }, + 'captain_models': { + 'type': %w[object null], + 'properties': { + 'editor': { 'type': %w[string null] }, + 'assistant': { 'type': %w[string null] }, + 'copilot': { 'type': %w[string null] }, + 'label_suggestion': { 'type': %w[string null] }, + 'audio_transcription': { 'type': %w[string null] }, + 'help_center_search': { 'type': %w[string null] } + }, + 'additionalProperties': false + }, + 'captain_features': { + 'type': %w[object null], + 'properties': { + 'editor': { 'type': %w[boolean null] }, + 'assistant': { 'type': %w[boolean null] }, + 'copilot': { 'type': %w[boolean null] }, + 'label_suggestion': { 'type': %w[boolean null] }, + 'audio_transcription': { 'type': %w[boolean null] }, + 'help_center_search': { 'type': %w[boolean null] } + }, + 'additionalProperties': false + } + }, + 'required': [], + 'additionalProperties': true + }.to_json.freeze +end diff --git a/app/models/concerns/email_address_parseable.rb b/app/models/concerns/email_address_parseable.rb new file mode 100644 index 000000000..7cca4a577 --- /dev/null +++ b/app/models/concerns/email_address_parseable.rb @@ -0,0 +1,15 @@ +module EmailAddressParseable + extend ActiveSupport::Concern + + private + + def parse_email(email_string) + Mail::Address.new(email_string).address.presence || default_sender_email_address + rescue Mail::Field::ParseError, Mail::Field::IncompleteParseError + default_sender_email_address + end + + def default_sender_email_address + Mail::Address.new(ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')).address + end +end diff --git a/app/services/automation_rules/conditions_filter_service.rb b/app/services/automation_rules/conditions_filter_service.rb index 993ed21c9..862faceac 100644 --- a/app/services/automation_rules/conditions_filter_service.rb +++ b/app/services/automation_rules/conditions_filter_service.rb @@ -113,6 +113,7 @@ class AutomationRules::ConditionsFilterService < FilterService query_operator = query_hash['query_operator'] attribute_key = 'processed_message_content' if attribute_key == 'content' + attribute_key = 'private' if attribute_key == 'private_note' filter_operator_value = filter_operation(query_hash, current_index) diff --git a/config/locales/en.yml b/config/locales/en.yml index 1cb3c4d12..057f41b81 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -51,6 +51,8 @@ en: account: reporting_timezone: invalid: is not a valid timezone + support_email: + invalid: is not a valid email address validations: presence: must not be blank webhook: diff --git a/enterprise/app/models/enterprise/account/plan_usage_and_limits.rb b/enterprise/app/models/enterprise/account/plan_usage_and_limits.rb index 705097030..0937c7b82 100644 --- a/enterprise/app/models/enterprise/account/plan_usage_and_limits.rb +++ b/enterprise/app/models/enterprise/account/plan_usage_and_limits.rb @@ -16,20 +16,15 @@ module Enterprise::Account::PlanUsageAndLimits # rubocop:disable Metrics/ModuleL end def increment_response_usage - current_usage = custom_attributes[CAPTAIN_RESPONSES_USAGE].to_i || 0 - custom_attributes[CAPTAIN_RESPONSES_USAGE] = current_usage + 1 - save + increment_custom_attribute(CAPTAIN_RESPONSES_USAGE) end def reset_response_usage - custom_attributes[CAPTAIN_RESPONSES_USAGE] = 0 - save + update_custom_attribute(CAPTAIN_RESPONSES_USAGE, 0) end def update_document_usage - # this will ensure that the document count is always accurate - custom_attributes[CAPTAIN_DOCUMENTS_USAGE] = captain_documents.count - save + update_custom_attribute(CAPTAIN_DOCUMENTS_USAGE, captain_documents.count) end def email_transcript_enabled? @@ -130,6 +125,27 @@ module Enterprise::Account::PlanUsageAndLimits # rubocop:disable Metrics/ModuleL ChatwootApp.max_limit end + # Atomic jsonb_set to avoid clobbering concurrent writes to other custom_attributes keys. + # Goes through Account relation (rather than raw connection) so shard routing is respected. + # rubocop:disable Rails/SkipsModelValidations + def update_custom_attribute(key, value) + Account.where(id: id).update_all([ + "custom_attributes = jsonb_set(COALESCE(custom_attributes, '{}'), ARRAY[:key], :value::jsonb)", + { key: key, value: value.to_json } + ]) + custom_attributes[key] = value + end + + def increment_custom_attribute(key) + Account.where(id: id).update_all([ + "custom_attributes = jsonb_set(COALESCE(custom_attributes, '{}'), ARRAY[:key], " \ + '(COALESCE((custom_attributes ->> :key)::int, 0) + 1)::text::jsonb)', + { key: key } + ]) + custom_attributes[key] = custom_attributes[key].to_i + 1 + end + # rubocop:enable Rails/SkipsModelValidations + def validate_limit_keys errors.add(:limits, ': Invalid data') unless self[:limits].is_a? Hash self[:limits] = {} if self[:limits].blank? diff --git a/lib/filters/filter_keys.yml b/lib/filters/filter_keys.yml index bfaf39325..8711239cc 100644 --- a/lib/filters/filter_keys.yml +++ b/lib/filters/filter_keys.yml @@ -214,6 +214,12 @@ messages: filter_operators: - "equal_to" - "not_equal_to" + private_note: + attribute_type: "standard" + data_type: "boolean" + filter_operators: + - "equal_to" + - "not_equal_to" content: attribute_type: "standard" data_type: "text" diff --git a/spec/enterprise/services/enterprise/billing/handle_stripe_event_service_spec.rb b/spec/enterprise/services/enterprise/billing/handle_stripe_event_service_spec.rb index 0f8ce1494..f9b550ef8 100644 --- a/spec/enterprise/services/enterprise/billing/handle_stripe_event_service_spec.rb +++ b/spec/enterprise/services/enterprise/billing/handle_stripe_event_service_spec.rb @@ -83,6 +83,37 @@ describe Enterprise::Billing::HandleStripeEventService do end end + describe 'subscription quantity update' do + before do + allow(subscription).to receive(:[]).with('plan') + .and_return({ 'id' => 'price_startups', 'product' => 'plan_id_startups', 'name' => 'Startups' }) + end + + it 'updates subscribed_quantity' do + allow(subscription).to receive(:[]).with('quantity').and_return(6) + + stripe_event_service.new.perform(event: event) + + expect(account.reload.custom_attributes['subscribed_quantity']).to eq(6) + end + + it 'persists quantity even when increment_response_usage runs concurrently' do + allow(subscription).to receive(:[]).with('quantity').and_return(6) + account.update!(custom_attributes: account.custom_attributes.merge('captain_responses_usage' => 100)) + + # Simulate: webhook updates quantity, then a concurrent increment_response_usage writes usage + stripe_event_service.new.perform(event: event) + account.reload + + # Simulate concurrent increment_response_usage (atomic jsonb_set, not full hash overwrite) + account.increment_response_usage + + # Quantity must survive the concurrent usage update + expect(account.reload.custom_attributes['subscribed_quantity']).to eq(6) + expect(account.reload.custom_attributes['captain_responses_usage']).to eq(101) + end + end + describe 'subscription deletion handling' do it 'calls CreateStripeCustomerService on subscription deletion' do allow(event).to receive(:type).and_return('customer.subscription.deleted') diff --git a/spec/listeners/automation_rule_listener_spec.rb b/spec/listeners/automation_rule_listener_spec.rb index 57a096a10..08085da7a 100644 --- a/spec/listeners/automation_rule_listener_spec.rb +++ b/spec/listeners/automation_rule_listener_spec.rb @@ -220,6 +220,15 @@ describe AutomationRuleListener do expect(AutomationRules::ActionService).not_to have_received(:new) end + it 'calls AutomationRules::ActionService if message is a private note' do + message.update!(private: true) + allow(condition_match).to receive(:present?).and_return(true) + + listener.message_created(event) + + expect(AutomationRules::ActionService).to have_received(:new).with(automation_rule, account, conversation) + end + it 'does not call AutomationRules::ActionService if conditions do not match based on content' do message.update!(processed_message_content: 'hi', content: "hi\n\nhello") allow(condition_match).to receive(:present?).and_return(false) diff --git a/spec/models/account_spec.rb b/spec/models/account_spec.rb index e010a3123..76dbbcba2 100644 --- a/spec/models/account_spec.rb +++ b/spec/models/account_spec.rb @@ -256,6 +256,29 @@ RSpec.describe Account do end end + context 'when support_email is set' do + it 'allows a plain email address' do + account.support_email = 'support@example.com' + expect(account).to be_valid + end + + it 'allows display-name format' do + account.support_email = 'Support Team ' + expect(account).to be_valid + end + + it 'allows blank values' do + account.support_email = '' + expect(account).to be_valid + end + + it 'rejects malformed strings with no email part' do + account.support_email = 'Smith Smith' + expect(account).not_to be_valid + expect(account.errors[:support_email]).to include(I18n.t('errors.account.support_email.invalid')) + end + end + context 'when reporting_timezone is set' do it 'allows valid timezone names' do account.reporting_timezone = 'America/New_York' diff --git a/spec/models/automation_rule_spec.rb b/spec/models/automation_rule_spec.rb index 91452b8a4..cd6297713 100644 --- a/spec/models/automation_rule_spec.rb +++ b/spec/models/automation_rule_spec.rb @@ -86,6 +86,19 @@ RSpec.describe AutomationRule do rule = FactoryBot.build(:automation_rule, params) expect(rule.valid?).to be true end + + it 'allows private_note as a valid condition attribute' do + params[:conditions] = [ + { + attribute_key: 'private_note', + filter_operator: 'equal_to', + values: [true], + query_operator: nil + } + ] + rule = FactoryBot.build(:automation_rule, params) + expect(rule.valid?).to be true + end end describe 'reauthorizable' do diff --git a/spec/services/automation_rules/condition_validation_service_spec.rb b/spec/services/automation_rules/condition_validation_service_spec.rb index 36387754a..1f65fb475 100644 --- a/spec/services/automation_rules/condition_validation_service_spec.rb +++ b/spec/services/automation_rules/condition_validation_service_spec.rb @@ -10,7 +10,8 @@ RSpec.describe AutomationRules::ConditionValidationService do rule.conditions = [ { 'values': ['open'], 'attribute_key': 'status', 'query_operator': nil, 'filter_operator': 'equal_to' }, { 'values': ['+918484'], 'attribute_key': 'phone_number', 'query_operator': 'OR', 'filter_operator': 'contains' }, - { 'values': ['test'], 'attribute_key': 'email', 'query_operator': nil, 'filter_operator': 'contains' } + { 'values': ['test'], 'attribute_key': 'email', 'query_operator': 'OR', 'filter_operator': 'contains' }, + { 'values': [true], 'attribute_key': 'private_note', 'query_operator': nil, 'filter_operator': 'equal_to' } ] rule.save end diff --git a/spec/services/automation_rules/conditions_filter_service_spec.rb b/spec/services/automation_rules/conditions_filter_service_spec.rb index 426cb533e..c4ff81275 100644 --- a/spec/services/automation_rules/conditions_filter_service_spec.rb +++ b/spec/services/automation_rules/conditions_filter_service_spec.rb @@ -83,6 +83,27 @@ RSpec.describe AutomationRules::ConditionsFilterService do end end + context 'when filtering private notes' do + before do + rule.conditions = [ + { 'values': [true], 'attribute_key': 'private_note', 'query_operator': nil, 'filter_operator': 'equal_to' } + ] + rule.save + end + + it 'will return true when the message is a private note' do + message.update!(private: true) + + expect(described_class.new(rule, conversation, { message: message, changed_attributes: {} }).perform).to be(true) + end + + it 'will return false when the message is not a private note' do + message.update!(private: false) + + expect(described_class.new(rule, conversation, { message: message, changed_attributes: {} }).perform).to be(false) + end + end + context 'when filter_operator is on processed_message_content' do before do rule.conditions = [