Merge branch 'staging/deploy-22aug24' of github.com:chatwoot/chatwoot into staging/deploy-22aug24

This reverts commit c26490e9c1.

app/controllers/api/v1/accounts/integrations/hooks_controller.rb

@@ -12,7 +12,12 @@ class Api::V1::Accounts::Integrations::HooksController < Api::V1::Accounts::Base
 
   def process_event
     response = @hook.process_event(params[:event])
-    if response[:error]
+
+    # for cases like an invalid event, or when conversation does not have enough messages
+    # for a label suggestion, the response is nil
+    if response.nil?
+      render json: { message: nil }
+    elsif response[:error]
       render json: { error: response[:error] }, status: :unprocessable_entity
     else
       render json: { message: response[:message] }

app/javascript/dashboard/components/widgets/conversation/conversation/LabelSuggestion.vue

@@ -172,7 +172,7 @@ export default {
               delay: { show: 600, hide: 0 },
               hideOnClick: true,
             }"
-            class="label-suggestion--option"
+            class="label-suggestion--option !px-0"
             @click="pushOrAddLabel(label.title)"
           >
             <woot-label

app/javascript/widget/components/AgentMessage.vue

@@ -9,7 +9,7 @@ import FileBubble from 'widget/components/FileBubble.vue';
 import Thumbnail from 'dashboard/components/widgets/Thumbnail.vue';
 import { MESSAGE_TYPE } from 'widget/helpers/constants';
 import configMixin from '../mixins/configMixin';
-import { useMessage } from '../composables/useMessage';
+import messageMixin from '../mixins/messageMixin';
 import { isASubmittedFormMessage } from 'shared/helpers/MessageTypeHelper';
 import darkModeMixin from 'widget/mixins/darkModeMixin.js';
 import ReplyToChip from 'widget/components/ReplyToChip.vue';
@@ -28,7 +28,7 @@ export default {
     MessageReplyButton,
     ReplyToChip,
   },
-  mixins: [configMixin, darkModeMixin],
+  mixins: [configMixin, messageMixin, darkModeMixin],
   props: {
     message: {
       type: Object,
@@ -39,15 +39,6 @@ export default {
       default: () => {},
     },
   },
-  setup(props) {
-    const { messageContentAttributes, hasAttachments } = useMessage(
-      props.message
-    );
-    return {
-      messageContentAttributes,
-      hasAttachments,
-    };
-  },
   data() {
     return {
       hasImageError: false,

app/javascript/widget/components/UserMessage.vue

@@ -6,7 +6,7 @@ import VideoBubble from 'widget/components/VideoBubble.vue';
 import FluentIcon from 'shared/components/FluentIcon/Index.vue';
 import FileBubble from 'widget/components/FileBubble.vue';
 import { messageStamp } from 'shared/helpers/timeHelper';
-import { useMessage } from '../composables/useMessage';
+import messageMixin from '../mixins/messageMixin';
 import ReplyToChip from 'widget/components/ReplyToChip.vue';
 import DragWrapper from 'widget/components/DragWrapper.vue';
 import { BUS_EVENTS } from 'shared/constants/busEvents';
@@ -25,6 +25,7 @@ export default {
     ReplyToChip,
     DragWrapper,
   },
+  mixins: [messageMixin],
   props: {
     message: {
       type: Object,
@@ -35,12 +36,6 @@ export default {
       default: () => {},
     },
   },
-  setup(props) {
-    const { hasAttachments } = useMessage(props.message);
-    return {
-      hasAttachments,
-    };
-  },
   data() {
     return {
       hasImageError: false,

app/javascript/widget/composables/specs/useMessage.spec.js

@@ -1,71 +0,0 @@
-import { describe, it, expect } from 'vitest';
-import { reactive, nextTick } from 'vue';
-import { useMessage } from '../useMessage';
-
-describe('useMessage', () => {
-  it('should handle deleted messages', () => {
-    const message = reactive({
-      content_attributes: { deleted: true },
-      attachments: [],
-    });
-
-    const { messageContentAttributes, hasAttachments } = useMessage(message);
-
-    expect(messageContentAttributes.value).toEqual({ deleted: true });
-    expect(hasAttachments.value).toBe(false);
-  });
-
-  it('should handle non-deleted messages with attachments', () => {
-    const message = reactive({
-      content_attributes: {},
-      attachments: ['attachment1', 'attachment2'],
-    });
-
-    const { messageContentAttributes, hasAttachments } = useMessage(message);
-
-    expect(messageContentAttributes.value).toEqual({});
-    expect(hasAttachments.value).toBe(true);
-  });
-
-  it('should handle messages without content_attributes', () => {
-    const message = reactive({
-      attachments: [],
-    });
-
-    const { messageContentAttributes, hasAttachments } = useMessage(message);
-
-    expect(messageContentAttributes.value).toEqual({});
-    expect(hasAttachments.value).toBe(false);
-  });
-
-  it('should handle messages with empty attachments array', () => {
-    const message = reactive({
-      content_attributes: { someAttribute: 'value' },
-      attachments: [],
-    });
-
-    const { messageContentAttributes, hasAttachments } = useMessage(message);
-
-    expect(messageContentAttributes.value).toEqual({ someAttribute: 'value' });
-    expect(hasAttachments.value).toBe(false);
-  });
-
-  it('should update reactive properties when message changes', async () => {
-    const message = reactive({
-      content_attributes: {},
-      attachments: [],
-    });
-
-    const { messageContentAttributes, hasAttachments } = useMessage(message);
-
-    expect(messageContentAttributes.value).toEqual({});
-    expect(hasAttachments.value).toBe(false);
-
-    message.content_attributes = { updated: true };
-    message.attachments.push('newAttachment');
-    await nextTick();
-
-    expect(messageContentAttributes.value).toEqual({ updated: true });
-    expect(hasAttachments.value).toBe(true);
-  });
-});

app/javascript/widget/composables/useMessage.js

@@ -1,22 +0,0 @@
-import { computed } from 'vue';
-
-/**
- * Composable for handling message-related computations.
- * @param {Object} message - The message object to be processed.
- * @returns {Object} An object containing computed properties for message content and attachments.
- */
-export function useMessage(message) {
-  const messageContentAttributes = computed(() => {
-    const { content_attributes: attribute = {} } = message;
-    return attribute;
-  });
-
-  const hasAttachments = computed(() => {
-    return !!(message.attachments && message.attachments.length > 0);
-  });
-
-  return {
-    messageContentAttributes,
-    hasAttachments,
-  };
-}

app/javascript/widget/mixins/messageMixin.js

@@ -0,0 +1,13 @@
+export default {
+  computed: {
+    messageContentAttributes() {
+      const { content_attributes: attribute = {} } = this.message;
+      return attribute;
+    },
+    hasAttachments() {
+      return !!(
+        this.message.attachments && this.message.attachments.length > 0
+      );
+    },
+  },
+};

app/javascript/widget/mixins/specs/messageMixin.spec.js

@@ -0,0 +1,37 @@
+import { shallowMount } from '@vue/test-utils';
+import messageMixin from '../messageMixin';
+import messages from './messageFixture';
+
+describe('messageMixin', () => {
+  let Component = {
+    render() {},
+    title: 'TestComponent',
+    mixins: [messageMixin],
+  };
+
+  it('deleted messages', async () => {
+    const wrapper = shallowMount(Component, {
+      data() {
+        return {
+          message: messages.deletedMessage,
+        };
+      },
+    });
+    expect(wrapper.vm.messageContentAttributes).toEqual({
+      deleted: true,
+    });
+    expect(wrapper.vm.hasAttachments).toBe(false);
+  });
+  it('non-deleted messages', async () => {
+    const wrapper = shallowMount(Component, {
+      data() {
+        return {
+          message: messages.nonDeletedMessage,
+        };
+      },
+    });
+    expect(wrapper.vm.deletedMessage).toBe(undefined);
+    expect(wrapper.vm.messageContentAttributes).toEqual({});
+    expect(wrapper.vm.hasAttachments).toBe(true);
+  });
+});

app/listeners/notification_listener.rb

@@ -33,19 +33,12 @@ class NotificationListener < BaseListener
     return if event.data[:notifiable_assignee_change].blank?
     return if conversation.pending?
 
-    Rails.logger.info "[NotificationListener] Assignee changed for conversation: #{conversation.id} to #{assignee&.id} [#{assignee&.email}]"
-
     NotificationBuilder.new(
       notification_type: 'conversation_assignment',
       user: assignee,
       account: account,
       primary_actor: conversation
     ).perform
-  rescue NoMethodError => e
-    Rails.logger.error "[NotificationListener] Error in assignee_changed: #{e.message}"
-    exception_tracker = ChatwootExceptionTracker.new(e, account: account,
-                                                        additional_context: { 'conversation': conversation, 'assignee': assignee })
-    exception_tracker.capture_exception
   end
 
   def message_created(event)

lib/chatwoot_exception_tracker.rb

@@ -5,11 +5,10 @@
 ############
 
 class ChatwootExceptionTracker
-  def initialize(exception, user: nil, account: nil, additional_context: {})
+  def initialize(exception, user: nil, account: nil)
     @exception = exception
     @user = user
     @account = account
-    @additional_context = additional_context
   end
 
   def capture_exception
@@ -21,29 +20,13 @@ class ChatwootExceptionTracker
 
   def capture_exception_with_sentry
     Sentry.with_scope do |scope|
-      append_account_context(scope)
-      append_additional_context(scope)
-      append_user_context(scope)
+      if @account.present?
+        scope.set_context('account', { id: @account.id, name: @account.name })
+        scope.set_tags(account_id: @account.id)
+      end
+
+      scope.set_user(id: @user.id, email: @user.email) if @user.is_a?(User)
       Sentry.capture_exception(@exception)
     end
   end
-
-  def append_account_context(scope)
-    return if @account.blank?
-
-    scope.set_context('account', { id: @account.id, name: @account.name })
-    scope.set_tags(account_id: @account.id)
-  end
-
-  def append_additional_context(scope)
-    @additional_context.each do |key, value|
-      scope.set_context(key.to_s, value)
-    end
-  end
-
-  def append_user_context(scope)
-    return unless @user.is_a?(User)
-
-    scope.set_user(id: @user.id, email: @user.email)
-  end
 end

lib/integrations/openai_base_service.rb

@@ -42,13 +42,26 @@ class Integrations::OpenaiBaseService
     return nil unless event_is_cacheable?
     return nil if cache_key.blank?
 
-    Redis::Alfred.get(cache_key)
+    deserialize_cached_value(Redis::Alfred.get(cache_key))
+  end
+
+  def deserialize_cached_value(value)
+    return nil if value.blank?
+
+    JSON.parse(value, symbolize_names: true)
+  rescue JSON::ParserError
+    # If json parse failed, returning the value as is will fail too
+    # since we access the keys as symbols down the line
+    # So it's best to return nil
+    nil
   end
 
   def save_to_cache(response)
     return nil unless event_is_cacheable?
 
-    Redis::Alfred.setex(cache_key, response)
+    # Serialize to JSON
+    # This makes parsing easy when response is a hash
+    Redis::Alfred.setex(cache_key, response.to_json)
   end
 
   def conversation

lib/redis/redis_keys.rb

@@ -33,7 +33,7 @@ module Redis::RedisKeys
   LATEST_CHATWOOT_VERSION = 'LATEST_CHATWOOT_VERSION'.freeze
   # Check if a message create with same source-id is in progress?
   MESSAGE_SOURCE_KEY = 'MESSAGE_SOURCE_KEY::%<id>s'.freeze
-  OPENAI_CONVERSATION_KEY = 'OPEN_AI_CONVERSATION_KEY::v1::%<event_name>s::%<conversation_id>d::%<updated_at>d'.freeze
+  OPENAI_CONVERSATION_KEY = 'OPEN_AI_CONVERSATION_KEY::V1::%<event_name>s::%<conversation_id>d::%<updated_at>d'.freeze
 
   ## Sempahores / Locks
   # We don't want to process messages from the same sender concurrently to prevent creating double conversations

spec/lib/chatwoot_exception_tracker_spec.rb

@@ -22,20 +22,5 @@ describe ChatwootExceptionTracker do
         described_class.new('random').capture_exception
       end
     end
-
-    it 'sets additional context when provided' do
-      additional_context = { key1: 'value1', key2: 'value2' }
-
-      with_modified_env SENTRY_DSN: 'random dsn' do
-        scope = instance_double(Sentry::Scope)
-        allow(Sentry).to receive(:with_scope).and_yield(scope)
-
-        expect(scope).to receive(:set_context).with('key1', 'value1')
-        expect(scope).to receive(:set_context).with('key2', 'value2')
-        expect(Sentry).to receive(:capture_exception).with('random')
-
-        described_class.new('random', additional_context: additional_context).capture_exception
-      end
-    end
   end
 end