From 429d281501be22d45ac431f929a70f9629eb1830 Mon Sep 17 00:00:00 2001 From: Muhsin Keloth Date: Thu, 22 Aug 2024 10:24:13 +0530 Subject: [PATCH 1/8] fix: Handle OpenAI API errors (#9560) --- .../accounts/integrations/hooks_controller.rb | 7 ++++++- app/javascript/dashboard/mixins/aiMixin.js | 6 +++++- app/models/integrations/hook.rb | 2 +- .../integrations/openai_processor_service.rb | 2 +- lib/integrations/openai_base_service.rb | 6 +++++- .../integrations/hooks_controller_spec.rb | 5 ++--- .../openai/processor_service_spec.rb | 18 +++++++++--------- spec/models/integrations/hook_spec.rb | 2 +- 8 files changed, 30 insertions(+), 18 deletions(-) diff --git a/app/controllers/api/v1/accounts/integrations/hooks_controller.rb b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb index d09ea2f00..13bb2738b 100644 --- a/app/controllers/api/v1/accounts/integrations/hooks_controller.rb +++ b/app/controllers/api/v1/accounts/integrations/hooks_controller.rb @@ -11,7 +11,12 @@ class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::Base end def process_event - render json: { message: @hook.process_event(params[:event]) } + response = @hook.process_event(params[:event]) + if response[:error] + render json: { error: response[:error] }, status: :unprocessable_entity + else + render json: { message: response[:message] } + end end def destroy diff --git a/app/javascript/dashboard/mixins/aiMixin.js b/app/javascript/dashboard/mixins/aiMixin.js index a775b2ecd..8263ca8c2 100644 --- a/app/javascript/dashboard/mixins/aiMixin.js +++ b/app/javascript/dashboard/mixins/aiMixin.js @@ -100,7 +100,11 @@ export default { } = result; return generatedMessage; } catch (error) { - useAlert(this.$t('INTEGRATION_SETTINGS.OPEN_AI.GENERATE_ERROR')); + const errorData = error.response.data.error; + const errorMessage = + errorData?.error?.message || + this.$t('INTEGRATION_SETTINGS.OPEN_AI.GENERATE_ERROR'); + useAlert(errorMessage); return ''; } }, diff --git a/app/models/integrations/hook.rb b/app/models/integrations/hook.rb index 076a5fce9..f523af4a5 100644 --- a/app/models/integrations/hook.rb +++ b/app/models/integrations/hook.rb @@ -56,7 +56,7 @@ class Integrations::Hook < ApplicationRecord when 'openai' Integrations::Openai::ProcessorService.new(hook: self, event: event).perform if app_id == 'openai' else - 'No processor found' + { error: 'No processor found' } end end diff --git a/enterprise/lib/enterprise/integrations/openai_processor_service.rb b/enterprise/lib/enterprise/integrations/openai_processor_service.rb index b3cf9aeee..55b7d33cd 100644 --- a/enterprise/lib/enterprise/integrations/openai_processor_service.rb +++ b/enterprise/lib/enterprise/integrations/openai_processor_service.rb @@ -31,7 +31,7 @@ module Enterprise::Integrations::OpenaiProcessorService # To what you ask? Sometimes, the response includes # "Labels:" in it's response in some format. This is a hacky way to remove it # TODO: Fix with with a better prompt - response.present? ? response.gsub(/^(label|labels):/i, '') : '' + response[:message] ? response[:message].gsub(/^(label|labels):/i, '') : '' end private diff --git a/lib/integrations/openai_base_service.rb b/lib/integrations/openai_base_service.rb index da2878aab..056ec998a 100644 --- a/lib/integrations/openai_base_service.rb +++ b/lib/integrations/openai_base_service.rb @@ -77,8 +77,12 @@ class Integrations::OpenaiBaseService response = HTTParty.post(API_URL, headers: headers, body: body) Rails.logger.info("OpenAI API response: #{response.body}") + return { error: response.parsed_response, error_code: response.code } unless response.success? + choices = JSON.parse(response.body)['choices'] - choices.present? ? choices.first['message']['content'] : nil + return { message: choices.first['message']['content'] } if choices.present? + + { message: nil } end end diff --git a/spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb b/spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb index 562272f14..23c1045c5 100644 --- a/spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb +++ b/spec/controllers/api/v1/accounts/integrations/hooks_controller_spec.rb @@ -97,9 +97,8 @@ RSpec.describe 'Integration Hooks API', type: :request do params: params, headers: agent.create_new_auth_token, as: :json - - expect(response).to have_http_status(:success) - expect(response.parsed_body['message']).to eq('No processor found') + expect(response).to have_http_status(:unprocessable_entity) + expect(response.parsed_body['error']).to eq 'No processor found' end end end diff --git a/spec/lib/integrations/openai/processor_service_spec.rb b/spec/lib/integrations/openai/processor_service_spec.rb index 4d8443ea1..ec0a7bc91 100644 --- a/spec/lib/integrations/openai/processor_service_spec.rb +++ b/spec/lib/integrations/openai/processor_service_spec.rb @@ -52,7 +52,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -76,7 +76,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -101,7 +101,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -140,7 +140,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -162,7 +162,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -184,7 +184,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -206,7 +206,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -228,7 +228,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end @@ -250,7 +250,7 @@ RSpec.describe Integrations::Openai::ProcessorService do .to_return(status: 200, body: openai_response, headers: {}) result = subject.perform - expect(result).to eq('This is a reply from openai.') + expect(result).to eq({ :message => 'This is a reply from openai.' }) end end end diff --git a/spec/models/integrations/hook_spec.rb b/spec/models/integrations/hook_spec.rb index fbad9192e..aecb2981c 100644 --- a/spec/models/integrations/hook_spec.rb +++ b/spec/models/integrations/hook_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Integrations::Hook do it 'returns no processor found for hooks with out processor defined' do hook = create(:integrations_hook, account: account) - expect(hook.process_event(params)).to eq('No processor found') + expect(hook.process_event(params)).to eq({ :error => 'No processor found' }) end it 'returns results from procesor for openai hook' do From c63a6ed8ec040b779bef5bcfe21fe812bcfa5315 Mon Sep 17 00:00:00 2001 From: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> Date: Thu, 22 Aug 2024 13:02:11 +0530 Subject: [PATCH 2/8] feat: Rewrite `agentMixin` to a helper (#9940) # Pull Request Template ## Description This PR will replace the usage of `agentMixin`with the utility helpers functions. **Files updated** 1. dashboard/components/widgets/conversation/contextMenu/Index.vue 2. dashboard/components/widgets/conversation/ConversationHeader.vue **(Not used)** 3. dashboard/routes/dashboard/commands/commandbar.vue 4. dashboard/routes/dashboard/conversation/ConversationAction.vue 5. dashboard/routes/dashboard/conversation/ConversationParticipant.vue Fixes https://linear.app/chatwoot/issue/CW-3442/rewrite-agentmixin-mixin-to-a-composable ## Type of change - [x] New feature (non-breaking change which adds functionality) ## How Has This Been Tested? **Test cases** 1. See agent list sorting based on availability, if agents are on the same status, then sorted by name. 2. Test actions like assigning/unassigning agent from conversation sidebar, CMD bar, Context menu. 3. Test actions like adding/removing participants from conversation sidebar. 4. See agent list is generated properly, none value. ## 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 --- .../conversation/ConversationHeader.vue | 3 +- .../conversation/contextMenu/Index.vue | 18 +- .../spec/fixtures/agentFixtures.js | 63 +++++ .../composables/spec/useAgentsList.spec.js | 91 +++++++ .../dashboard/composables/useAgentsList.js | 57 ++++ .../dashboard/helper/agentHelper.js | 83 ++++++ .../helper/specs/agentHelper.spec.js | 131 ++++++++++ .../helper/specs/fixtures/agentFixtures.js | 184 +++++++++++++ app/javascript/dashboard/mixins/agentMixin.js | 69 ----- .../dashboard/mixins/specs/agentFixtures.js | 246 ------------------ .../dashboard/mixins/specs/agentMixin.spec.js | 140 ---------- .../routes/dashboard/commands/commandbar.vue | 7 +- .../conversation/ConversationAction.vue | 16 +- .../conversation/ConversationParticipant.vue | 17 +- 14 files changed, 643 insertions(+), 482 deletions(-) create mode 100644 app/javascript/dashboard/composables/spec/fixtures/agentFixtures.js create mode 100644 app/javascript/dashboard/composables/spec/useAgentsList.spec.js create mode 100644 app/javascript/dashboard/composables/useAgentsList.js create mode 100644 app/javascript/dashboard/helper/agentHelper.js create mode 100644 app/javascript/dashboard/helper/specs/agentHelper.spec.js create mode 100644 app/javascript/dashboard/helper/specs/fixtures/agentFixtures.js delete mode 100644 app/javascript/dashboard/mixins/agentMixin.js delete mode 100644 app/javascript/dashboard/mixins/specs/agentFixtures.js delete mode 100644 app/javascript/dashboard/mixins/specs/agentMixin.spec.js diff --git a/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue b/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue index 428e5549f..8bfc1d314 100644 --- a/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue +++ b/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue @@ -2,7 +2,6 @@ import { ref } from 'vue'; import { mapGetters } from 'vuex'; import { useKeyboardEvents } from 'dashboard/composables/useKeyboardEvents'; -import agentMixin from '../../../mixins/agentMixin.js'; import BackButton from '../BackButton.vue'; import inboxMixin from 'shared/mixins/inboxMixin'; import InboxName from '../InboxName.vue'; @@ -24,7 +23,7 @@ export default { SLACardLabel, Linear, }, - mixins: [inboxMixin, agentMixin], + mixins: [inboxMixin], props: { chat: { type: Object, diff --git a/app/javascript/dashboard/components/widgets/conversation/contextMenu/Index.vue b/app/javascript/dashboard/components/widgets/conversation/contextMenu/Index.vue index fc9ec29fe..90c14e450 100644 --- a/app/javascript/dashboard/components/widgets/conversation/contextMenu/Index.vue +++ b/app/javascript/dashboard/components/widgets/conversation/contextMenu/Index.vue @@ -1,9 +1,12 @@ + + diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/Index.vue b/app/javascript/dashboard/routes/dashboard/settings/automation/Index.vue index cbd200f09..f637b5c30 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/Index.vue +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/Index.vue @@ -1,246 +1,219 @@ - diff --git a/app/javascript/dashboard/routes/dashboard/settings/automation/automation.routes.js b/app/javascript/dashboard/routes/dashboard/settings/automation/automation.routes.js index 14b03a45a..39e13bd94 100644 --- a/app/javascript/dashboard/routes/dashboard/settings/automation/automation.routes.js +++ b/app/javascript/dashboard/routes/dashboard/settings/automation/automation.routes.js @@ -1,17 +1,12 @@ import { frontendURL } from '../../../../helper/URLHelper'; -const SettingsContent = () => import('../Wrapper.vue'); +const SettingsWrapper = () => import('../SettingsWrapper.vue'); const Automation = () => import('./Index.vue'); export default { routes: [ { path: frontendURL('accounts/:accountId/settings/automation'), - component: SettingsContent, - props: { - headerTitle: 'AUTOMATION.HEADER', - icon: 'automation', - showNewButton: false, - }, + component: SettingsWrapper, children: [ { path: '', diff --git a/app/javascript/dashboard/store/modules/automations.js b/app/javascript/dashboard/store/modules/automations.js index df3bf6f6a..e36a2f4a4 100644 --- a/app/javascript/dashboard/store/modules/automations.js +++ b/app/javascript/dashboard/store/modules/automations.js @@ -15,7 +15,7 @@ export const state = { export const getters = { getAutomations(_state) { - return _state.records; + return _state.records.sort((a1, a2) => a1.id - a2.id); }, getUIFlags(_state) { return _state.uiFlags; From 776579ba5bbee3755764e9fd53ca6af55b76a4bc Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 22 Aug 2024 16:40:27 +0530 Subject: [PATCH 6/8] feat: enable disposable email check for account creation (#9989) This PR disallows usage of disposable emails when creating an account --------- Co-authored-by: Pranav --- Gemfile.lock | 6 +++--- app/builders/account_builder.rb | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index de348a2b5..1bd5f4f05 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -467,7 +467,7 @@ GEM mini_magick (4.12.0) mini_mime (1.1.5) mini_portile2 (2.8.7) - minitest (5.24.1) + minitest (5.25.1) mock_redis (0.36.0) ruby2_keywords msgpack (1.7.0) @@ -480,7 +480,7 @@ GEM uri net-http-persistent (4.0.2) connection_pool (~> 2.2) - net-imap (0.4.12) + net-imap (0.4.14) date net-protocol net-pop (0.1.2) @@ -796,7 +796,7 @@ GEM uniform_notifier (1.16.0) uri (0.13.0) uri_template (0.7.0) - valid_email2 (4.0.6) + valid_email2 (5.2.6) activemodel (>= 3.2) mail (~> 2.5) version_gem (1.1.4) diff --git a/app/builders/account_builder.rb b/app/builders/account_builder.rb index f179c6405..a3a90451d 100644 --- a/app/builders/account_builder.rb +++ b/app/builders/account_builder.rb @@ -33,10 +33,10 @@ class AccountBuilder def validate_email address = ValidEmail2::Address.new(@email) - if address.valid? # && !address.disposable? + if address.valid? && !address.disposable? true else - raise InvalidEmail.new(valid: address.valid?) + raise InvalidEmail.new({ valid: address.valid?, disposable: address.disposable? }) end end From dadd572f9d5fdf47162c8b04c141adfac256dd30 Mon Sep 17 00:00:00 2001 From: Shivam Mishra Date: Thu, 22 Aug 2024 16:40:55 +0530 Subject: [PATCH 7/8] refactor: `useKeyboardEvents` composable (#9959) This PR has the following changes 1. Fix tab styles issue caused by adding an additional wrapper for getting an element ref on `ChatTypeTabs.vue` 2. Refactor `useKeyboardEvents` composable to not require an element ref. It will use a local abort controller to abort any listener --------- Co-authored-by: Sivin Varghese <64252451+iamsivin@users.noreply.github.com> --- .../dashboard/components/ChatList.vue | 2 +- .../components/buttons/ResolveAction.vue | 8 +-- .../dashboard/components/layout/Sidebar.vue | 7 +-- .../components/widgets/AIAssistanceButton.vue | 6 +-- .../components/widgets/ChatTypeTabs.vue | 33 ++++++------ .../components/widgets/LabelSelector.vue | 10 +--- .../widgets/WootWriter/ReplyBottomPanel.vue | 17 ++---- .../widgets/WootWriter/ReplyTopPanel.vue | 11 +--- .../conversation/ConversationHeader.vue | 10 +--- .../widgets/conversation/MessagesView.vue | 5 +- .../widgets/conversation/TagAgents.vue | 1 - .../conversation/components/GalleryView.vue | 4 +- .../widgets/mentions/MentionBox.vue | 1 - .../spec/useKeyboardEvents.spec.js | 5 +- .../spec/useKeyboardNavigableList.spec.js | 22 +------- .../composables/useKeyboardEvents.js | 53 +++++-------------- .../composables/useKeyboardNavigableList.js | 3 +- .../modules/notes/components/AddNote.vue | 4 +- .../conversation/labels/LabelBox.vue | 6 +-- .../components/ArticleSearch/Header.vue | 5 +- .../portal/components/SearchSuggestions.vue | 1 - .../components/ui/dropdown/DropdownMenu.vue | 2 +- 22 files changed, 56 insertions(+), 160 deletions(-) diff --git a/app/javascript/dashboard/components/ChatList.vue b/app/javascript/dashboard/components/ChatList.vue index fa95c5024..476fc7438 100644 --- a/app/javascript/dashboard/components/ChatList.vue +++ b/app/javascript/dashboard/components/ChatList.vue @@ -141,7 +141,7 @@ export default { allowOnFocusedInput: true, }, }; - useKeyboardEvents(keyboardEvents, conversationListRef); + useKeyboardEvents(keyboardEvents); return { uiSettings, diff --git a/app/javascript/dashboard/components/buttons/ResolveAction.vue b/app/javascript/dashboard/components/buttons/ResolveAction.vue index 7c4c4daaa..0626ced59 100644 --- a/app/javascript/dashboard/components/buttons/ResolveAction.vue +++ b/app/javascript/dashboard/components/buttons/ResolveAction.vue @@ -19,7 +19,6 @@ const store = useStore(); const getters = useStoreGetters(); const { t } = useI18n(); -const resolveActionsRef = ref(null); const arrowDownButtonRef = ref(null); const isLoading = ref(false); @@ -131,17 +130,14 @@ const keyboardEvents = { }, }; -useKeyboardEvents(keyboardEvents, resolveActionsRef); +useKeyboardEvents(keyboardEvents); useEmitter(CMD_REOPEN_CONVERSATION, onCmdOpenConversation); useEmitter(CMD_RESOLVE_CONVERSATION, onCmdResolveConversation);