Chore: Convert Message Sender to polymorphic (#740)

Fixes #680
This commit is contained in:
Sojan Jose 2020-06-27 21:34:53 +05:30 committed by GitHub
parent 4f83d5451e
commit cc02611007
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
27 changed files with 71 additions and 49 deletions

View file

@ -29,7 +29,7 @@ class ContactMergeAction
end
def merge_messages
Message.where(contact_id: @mergee_contact.id).update(contact_id: @base_contact.id)
Message.where(sender: @mergee_contact).update(sender: @base_contact)
end
def merge_contact_inboxes

View file

@ -117,7 +117,8 @@ class Messages::MessageBuilder
inbox_id: conversation.inbox_id,
message_type: @message_type,
content: response.content,
source_id: response.identifier
source_id: response.identifier,
sender: contact
}
end

View file

@ -37,7 +37,7 @@ class Messages::Outgoing::NormalBuilder
message_type: :outgoing,
content: @content,
private: @private,
user_id: @user&.id,
sender: @user,
source_id: @fb_id,
content_type: @content_type,
items: @items

View file

@ -4,7 +4,8 @@ class Api::V1::Accounts::Conversations::MessagesController < Api::V1::Accounts::
end
def create
mb = Messages::Outgoing::NormalBuilder.new(current_user, @conversation, params)
user = current_user || @resource
mb = Messages::Outgoing::NormalBuilder.new(user, @conversation, params)
@message = mb.perform
end

View file

@ -45,7 +45,7 @@ class Api::V1::Widget::MessagesController < Api::V1::Widget::BaseController
def message_params
{
account_id: conversation.account_id,
contact_id: @contact.id,
sender: @contact,
content: permitted_params[:message][:content],
inbox_id: conversation.inbox_id,
message_type: :incoming

View file

@ -20,7 +20,7 @@ class ConversationMailbox < ApplicationMailbox
def create_message
@message = @conversation.messages.create(
account_id: @conversation.account_id,
contact_id: @conversation.contact_id,
sender: @conversation.contact,
content: processed_mail.text_content[:reply],
inbox_id: @conversation.inbox_id,
message_type: 'incoming',

View file

@ -17,12 +17,13 @@ class AgentBot < ApplicationRecord
has_many :agent_bot_inboxes, dependent: :destroy
has_many :inboxes, through: :agent_bot_inboxes
has_many :messages, as: :sender, dependent: :restrict_with_exception
def push_event_data
def push_event_data(inbox = nil)
{
id: id,
name: name,
avatar_url: avatar_url,
avatar_url: avatar_url || inbox&.avatar_url,
type: 'agent_bot'
}
end

View file

@ -35,7 +35,7 @@ class Contact < ApplicationRecord
has_many :conversations, dependent: :destroy
has_many :contact_inboxes, dependent: :destroy
has_many :inboxes, through: :contact_inboxes
has_many :messages, dependent: :destroy
has_many :messages, as: :sender, dependent: :destroy
before_validation :downcase_email
after_create_commit :dispatch_create_event

View file

@ -8,28 +8,23 @@
# content_type :integer default("text")
# message_type :integer not null
# private :boolean default(FALSE)
# sender_type :string
# status :integer default("sent")
# created_at :datetime not null
# updated_at :datetime not null
# account_id :integer not null
# contact_id :bigint
# conversation_id :integer not null
# inbox_id :integer not null
# sender_id :bigint
# source_id :string
# user_id :integer
#
# Indexes
#
# index_messages_on_account_id (account_id)
# index_messages_on_contact_id (contact_id)
# index_messages_on_conversation_id (conversation_id)
# index_messages_on_inbox_id (inbox_id)
# index_messages_on_source_id (source_id)
# index_messages_on_user_id (user_id)
#
# Foreign Keys
#
# fk_rails_... (contact_id => contacts.id)
# index_messages_on_account_id (account_id)
# index_messages_on_conversation_id (conversation_id)
# index_messages_on_inbox_id (inbox_id)
# index_messages_on_sender_type_and_sender_id (sender_type,sender_id)
# index_messages_on_source_id (source_id)
#
class Message < ApplicationRecord
@ -65,8 +60,11 @@ class Message < ApplicationRecord
belongs_to :account
belongs_to :inbox
belongs_to :conversation, touch: true
# FIXME: phase out user and contact after 1.4 since the info is there in sender
belongs_to :user, required: false
belongs_to :contact, required: false
belongs_to :sender, polymorphic: true, required: false
has_many :attachments, dependent: :destroy, autosave: true, before_add: :validate_attachments_limit
@ -88,7 +86,8 @@ class Message < ApplicationRecord
conversation_id: conversation.display_id
)
data.merge!(attachments: attachments.map(&:push_event_data)) if attachments.present?
data.merge!(sender: user.push_event_data) if user
data.merge!(sender: sender.push_event_data) if sender && !sender.is_a?(AgentBot)
data.merge!(sender: sender.push_event_data(inbox)) if sender&.is_a?(AgentBot)
data
end
@ -105,8 +104,7 @@ class Message < ApplicationRecord
content_type: content_type,
content_attributes: content_attributes,
source_id: source_id,
sender: user.try(:webhook_data),
contact: contact.try(:webhook_data),
sender: sender.try(:webhook_data),
inbox: inbox.webhook_data,
conversation: conversation.webhook_data,
account: account.webhook_data

View file

@ -65,7 +65,7 @@ class User < ApplicationRecord
has_many :assigned_conversations, foreign_key: 'assignee_id', class_name: 'Conversation', dependent: :nullify
has_many :inbox_members, dependent: :destroy
has_many :inboxes, through: :inbox_members, source: :inbox
has_many :messages
has_many :messages, as: :sender
has_many :invitees, through: :account_users, class_name: 'User', foreign_key: 'inviter_id', dependent: :nullify
has_many :notifications, dependent: :destroy

View file

@ -11,7 +11,7 @@ class Twilio::IncomingMessageService
account_id: @inbox.account_id,
inbox_id: @inbox.id,
message_type: :incoming,
contact_id: @contact.id,
sender: @contact,
source_id: params[:SmsSid]
)
attach_files

View file

@ -12,7 +12,7 @@ class Twitter::DirectMessageParserService < Twitter::WebhooksBaseService
account_id: @inbox.account_id,
inbox_id: @inbox.id,
message_type: outgoing_message? ? :outgoing : :incoming,
contact_id: @contact.id,
sender: @contact,
source_id: direct_message_data['id']
)
end

View file

@ -73,7 +73,7 @@ class Twitter::TweetParserService < Twitter::WebhooksBaseService
set_conversation
@conversation.messages.create(
account_id: @inbox.account_id,
contact_id: @contact.id,
sender: @contact,
content: tweet_text,
inbox_id: @inbox.id,
message_type: message_type,

View file

@ -7,4 +7,5 @@ json.content_type @message.content_type
json.content_attributes @message.content_attributes
json.created_at @message.created_at.to_i
json.private @message.private
json.sender @message.sender.push_event_data
json.attachments @message.attachments.map(&:push_event_data) if @message.attachments.present?

View file

@ -17,6 +17,6 @@ json.payload do
json.private message.private
json.source_id message.source_id
json.attachments message.attachments.map(&:push_event_data) if message.attachments.present?
json.sender message.user.push_event_data if message.user
json.sender message.sender.push_event_data if message.sender
end
end

View file

@ -7,4 +7,4 @@ json.created_at @message.created_at.to_i
json.private @message.private
json.source_id @message.source_id
json.attachments @message.attachments.map(&:push_event_data) if @message.attachments.present?
json.sender @message.user.push_event_data if @message.user
json.sender @message.sender.push_event_data if @message.sender

View file

@ -7,5 +7,5 @@ json.array! @messages do |message|
json.created_at message.created_at.to_i
json.conversation_id message.conversation.display_id
json.attachments message.attachments.map(&:push_event_data) if message.attachments.present?
json.sender message.user.push_event_data if message.user
json.sender message.sender.push_event_data if message.sender
end

View file

@ -0,0 +1,25 @@
class AddSenderToMessages < ActiveRecord::Migration[6.0]
def change
add_reference :messages, :sender, polymorphic: true, index: true
add_sender_from_message
remove_index :messages, name: 'index_messages_on_contact_id', column: 'contact_id'
remove_index :messages, name: 'index_messages_on_user_id', column: 'user_id'
remove_column :messages, :user_id, :integer # rubocop:disable Rails/BulkChangeTable
remove_column :messages, :contact_id, :integer
end
def add_sender_from_message
::Message.find_in_batches do |messages_batch|
Rails.logger.info "migrated till #{messages_batch.first.id}\n"
messages_batch.each do |message|
# rubocop:disable Rails/SkipsModelValidations
message.update_columns(sender_id: message.user.id, sender_type: 'User') if message.user.present?
message.update_columns(sender_id: message.contact.id, sender_type: 'Contact') if message.contact.present?
if message.sender.nil?
message.update_columns(sender_id: message.conversation.contact.id, sender_type: 'Contact') if message.incoming?
end
# rubocop:enable Rails/SkipsModelValidations
end
end
end
end

View file

@ -292,18 +292,17 @@ ActiveRecord::Schema.define(version: 2020_06_25_154254) do
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.boolean "private", default: false
t.integer "user_id"
t.integer "status", default: 0
t.string "source_id"
t.integer "content_type", default: 0
t.json "content_attributes", default: {}
t.bigint "contact_id"
t.string "sender_type"
t.bigint "sender_id"
t.index ["account_id"], name: "index_messages_on_account_id"
t.index ["contact_id"], name: "index_messages_on_contact_id"
t.index ["conversation_id"], name: "index_messages_on_conversation_id"
t.index ["inbox_id"], name: "index_messages_on_inbox_id"
t.index ["sender_type", "sender_id"], name: "index_messages_on_sender_type_and_sender_id"
t.index ["source_id"], name: "index_messages_on_source_id"
t.index ["user_id"], name: "index_messages_on_user_id"
end
create_table "notification_settings", force: :cascade do |t|
@ -436,5 +435,4 @@ ActiveRecord::Schema.define(version: 2020_06_25_154254) do
add_foreign_key "contact_inboxes", "contacts"
add_foreign_key "contact_inboxes", "inboxes"
add_foreign_key "conversations", "contact_inboxes"
add_foreign_key "messages", "contacts"
end

View file

@ -82,7 +82,7 @@ class Integrations::Slack::IncomingMessageBuilder
content: params[:event][:text],
source_id: "slack_#{params[:event][:ts]}",
private: private_note?,
user: sender
sender: sender
)
{ status: 'success' }

View file

@ -20,10 +20,6 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService
update_reference_id
end
def agent
@agent ||= message.user
end
def message_content
private_indicator = message.private? ? 'private: ' : ''
if conversation.identifier.present?
@ -38,7 +34,7 @@ class Integrations::Slack::SendOnSlackService < Base::SendOnChannelService
end
def send_message
sender = message.outgoing? ? agent : contact
sender = message.sender
sender_type = sender.class == Contact ? 'Contact' : 'Agent'
@slack_message = slack_client.chat_postMessage(

View file

@ -51,7 +51,8 @@ class Integrations::Widget::IncomingMessageBuilder
account_id: conversation.account_id,
inbox_id: conversation.inbox_id,
message_type: 0,
content: options[:content]
content: options[:content],
sender: contact
}
end
end

View file

@ -46,7 +46,7 @@ class Integrations::Widget::OutgoingMessageBuilder
inbox_id: @conversation.inbox_id,
message_type: 1,
content: options[:content],
user_id: user.id
sender: user
}
end
end

View file

@ -10,7 +10,7 @@ describe ::ContactMergeAction do
before do
2.times.each { create(:conversation, contact: base_contact) }
2.times.each { create(:conversation, contact: mergee_contact) }
2.times.each { create(:message, contact: mergee_contact) }
2.times.each { create(:message, sender: mergee_contact) }
end
describe '#perform' do

View file

@ -9,7 +9,7 @@ FactoryBot.define do
account { create(:account) }
after(:build) do |message|
message.user ||= create(:user, account: message.account)
message.sender ||= create(:user, account: message.account)
message.conversation ||= create(:conversation, account: message.account)
message.inbox ||= create(:inbox, account: message.account)
end

View file

@ -20,7 +20,7 @@ describe Integrations::Slack::SendOnSlackService do
expect(slack_client).to receive(:chat_postMessage).with(
channel: hook.reference_id,
text: message.content,
username: "Contact: #{contact.name}",
username: "Agent: #{message.sender.name}",
thread_ts: conversation.identifier,
icon_url: anything
)

View file

@ -254,7 +254,7 @@ RSpec.describe Conversation, type: :model do
conversation: conversation,
account: conversation.account,
inbox: conversation.inbox,
user: conversation.assignee
sender: conversation.assignee
}
end
let!(:message) do
@ -279,7 +279,7 @@ RSpec.describe Conversation, type: :model do
conversation: conversation,
account: conversation.account,
inbox: conversation.inbox,
user: conversation.assignee,
sender: conversation.assignee,
created_at: 1.minute.ago
}
end