diff --git a/app/actions/contact_identify_action.rb b/app/actions/contact_identify_action.rb index ff8efd3b5..91060f215 100644 --- a/app/actions/contact_identify_action.rb +++ b/app/actions/contact_identify_action.rb @@ -34,7 +34,9 @@ class ContactIdentifyAction def update_contact custom_attributes = params[:custom_attributes] ? @contact.custom_attributes.merge(params[:custom_attributes]) : @contact.custom_attributes - @contact.update!(params.slice(:name, :email, :identifier).merge({ custom_attributes: custom_attributes })) + # blank identifier or email will throw unique index error + # TODO: replace reject { |_k, v| v.blank? } with compact_blank when rails is upgraded + @contact.update!(params.slice(:name, :email, :identifier).reject { |_k, v| v.blank? }.merge({ custom_attributes: custom_attributes })) ContactAvatarJob.perform_later(@contact, params[:avatar_url]) if params[:avatar_url].present? end diff --git a/app/controllers/api/v1/widget/base_controller.rb b/app/controllers/api/v1/widget/base_controller.rb index 52f19595e..4c7dd4220 100644 --- a/app/controllers/api/v1/widget/base_controller.rb +++ b/app/controllers/api/v1/widget/base_controller.rb @@ -32,7 +32,8 @@ class Api::V1::Widget::BaseController < ApplicationController @contact_inbox = @web_widget.inbox.contact_inboxes.find_by( source_id: auth_token_params[:source_id] ) - @contact = @contact_inbox.contact + @contact = @contact_inbox&.contact + raise ActiveRecord::RecordNotFound unless @contact end def create_conversation diff --git a/app/mailboxes/application_mailbox.rb b/app/mailboxes/application_mailbox.rb index a13110dfe..fb3b451b0 100644 --- a/app/mailboxes/application_mailbox.rb +++ b/app/mailboxes/application_mailbox.rb @@ -6,7 +6,7 @@ class ApplicationMailbox < ActionMailbox::Base def self.reply_mail? proc do |inbound_mail_obj| is_a_reply_email = false - inbound_mail_obj.mail.to.each do |email| + inbound_mail_obj.mail.to&.each do |email| username = email.split('@')[0] match_result = username.match(REPLY_EMAIL_USERNAME_PATTERN) if match_result @@ -21,7 +21,7 @@ class ApplicationMailbox < ActionMailbox::Base def self.support_mail? proc do |inbound_mail_obj| is_a_support_email = false - inbound_mail_obj.mail.to.each do |email| + inbound_mail_obj.mail.to&.each do |email| channel = Channel::Email.find_by('email = ? OR forward_to_email = ?', email, email) if channel.present? is_a_support_email = true diff --git a/app/models/notification.rb b/app/models/notification.rb index 5d559befe..a92faf67e 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -70,7 +70,7 @@ class Notification < ApplicationRecord I18n.t( 'notifications.notification_title.assigned_conversation_new_message', display_id: conversation.display_id, - content: primary_actor.content.truncate_words(10) + content: primary_actor.content&.truncate_words(10) ) when 'conversation_mention' I18n.t('notifications.notification_title.conversation_mention', display_id: conversation.display_id, name: secondary_actor.name) diff --git a/lib/exception_list.rb b/lib/exception_list.rb index edb5899ab..6ea285bd0 100644 --- a/lib/exception_list.rb +++ b/lib/exception_list.rb @@ -1,4 +1,5 @@ module ExceptionList URI_EXCEPTIONS = [Errno::ETIMEDOUT, Errno::ECONNREFUSED, URI::InvalidURIError, Net::OpenTimeout, SocketError].freeze - REST_CLIENT_EXCEPTIONS = [RestClient::NotFound, RestClient::GatewayTimeout, RestClient::BadRequest, RestClient::MethodNotAllowed].freeze + REST_CLIENT_EXCEPTIONS = [RestClient::NotFound, RestClient::GatewayTimeout, RestClient::BadRequest, + RestClient::MethodNotAllowed, RestClient::Forbidden].freeze end diff --git a/spec/actions/contact_identify_action_spec.rb b/spec/actions/contact_identify_action_spec.rb index dd09f7760..be139a22e 100644 --- a/spec/actions/contact_identify_action_spec.rb +++ b/spec/actions/contact_identify_action_spec.rb @@ -44,5 +44,15 @@ describe ::ContactIdentifyAction do expect { contact.reload }.to raise_error(ActiveRecord::RecordNotFound) end end + + context 'when contacts with blank identifiers exist and identify action is called with blank identifier' do + it 'updates the attributes of contact passed in to identify action' do + create(:contact, account: account, identifier: '') + params = { identifier: '', name: 'new name' } + result = described_class.new(contact: contact, params: params).perform + expect(result.id).to eq contact.id + expect(result.name).to eq 'new name' + end + end end end diff --git a/spec/controllers/api/v1/widget/conversations_controller_spec.rb b/spec/controllers/api/v1/widget/conversations_controller_spec.rb index 43d60db69..9ba2d066a 100644 --- a/spec/controllers/api/v1/widget/conversations_controller_spec.rb +++ b/spec/controllers/api/v1/widget/conversations_controller_spec.rb @@ -25,6 +25,21 @@ RSpec.describe '/api/v1/widget/conversations/toggle_typing', type: :request do expect(json_response['status']).to eq(conversation.status) end end + + context 'with a conversation but invalid source id' do + it 'returns the correct conversation params' do + allow(Rails.configuration.dispatcher).to receive(:dispatch) + + payload = { source_id: 'invalid source id', inbox_id: web_widget.inbox.id } + token = ::Widget::TokenService.new(payload: payload).generate_token + get '/api/v1/widget/conversations', + headers: { 'X-Auth-Token' => token }, + params: { website_token: web_widget.website_token }, + as: :json + + expect(response).to have_http_status(:not_found) + end + end end describe 'POST /api/v1/widget/conversations' do diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 0978bab4f..be10fc411 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -41,6 +41,14 @@ RSpec.describe Notification do #{message.content.truncate_words(10)}" end + it 'returns appropriate title suited for the notification type assigned_conversation_new_message when attachment message' do + # checking content nil should be suffice for attachments + message = create(:message, sender: create(:user), content: nil) + notification = create(:notification, notification_type: 'assigned_conversation_new_message', primary_actor: message) + + expect(notification.push_message_title).to eq "[New message] - ##{notification.conversation.display_id} " + end + it 'returns appropriate title suited for the notification type conversation_mention' do message = create(:message, sender: create(:user)) notification = create(:notification, notification_type: 'conversation_mention', primary_actor: message, secondary_actor: message.sender)