Change sender_id to contact_id in conversations (#167)

* change sender_id to contact_id in conversations

* Fix failing tests

* Fix seeds

* fix specs

* Fix issues in facebook messenger

app/builders/messages/message_builder.rb

@@ -1,11 +1,13 @@
 require 'open-uri'
-class Messages::MessageBuilder
-  # This class creates both outgoing messages from chatwoot and echo outgoing messages based on the flag `outgoing_echo`
-  # Assumptions
-  # 1. Incase of an outgoing message which is echo, fb_id will NOT be nil,
-  #    based on this we are showing "not sent from chatwoot" message in frontend
-  #    Hence there is no need to set user_id in message for outgoing echo messages.
 
+# This class creates both outgoing messages from chatwoot and echo outgoing messages based on the flag `outgoing_echo`
+# Assumptions
+# 1. Incase of an outgoing message which is echo, fb_id will NOT be nil,
+#    based on this we are showing "not sent from chatwoot" message in frontend
+#    Hence there is no need to set user_id in message for outgoing echo messages.
+
+module Messages
+  class MessageBuilder
     attr_reader :response
 
     def initialize(response, inbox, outgoing_echo = false)
@@ -15,23 +17,18 @@ class Messages::MessageBuilder
       @message_type = (outgoing_echo ? :outgoing : :incoming)
     end
 
-  def perform # for incoming
+    def perform
       ActiveRecord::Base.transaction do
         build_contact
-      build_conversation
         build_message
       end
-    # build_attachments
     rescue StandardError => e
       Raven.capture_exception(e)
-    # change this asap
       true
     end
 
     private
 
-  def build_attachments; end
-
     def contact
       @contact ||= @inbox.contacts.find_by(source_id: @sender_id)
     end
@@ -41,20 +38,17 @@ class Messages::MessageBuilder
     end
 
     def build_message
-    @message = @conversation.messages.new(message_params)
+      @message = conversation.messages.new(message_params)
       (response.attachments || []).each do |attachment|
         @message.build_attachment(attachment_params(attachment))
       end
       @message.save!
     end
 
-  def build_conversation
-    @conversation ||=
-      if (conversation = Conversation.find_by(conversation_params))
-        conversation
-      else
-        Conversation.create!(conversation_params)
-      end
+    def build_attachment; end
+
+    def conversation
+      @conversation ||= Conversation.find_by(conversation_params) || Conversation.create!(conversation_params)
     end
 
     def attachment_params(attachment)
@@ -101,14 +95,14 @@ class Messages::MessageBuilder
       {
         account_id: @inbox.account_id,
         inbox_id: @inbox.id,
-      sender_id: contact.id
+        contact_id: contact.id
       }
     end
 
     def message_params
       {
-      account_id: @conversation.account_id,
-      inbox_id: @conversation.inbox_id,
+        account_id: conversation.account_id,
+        inbox_id: conversation.inbox_id,
         message_type: @message_type,
         content: response.content,
         fb_id: response.identifier
@@ -123,11 +117,12 @@ class Messages::MessageBuilder
         result = {}
         Raven.capture_exception(e)
       end
-    params = {
+      {
         name: "#{result['first_name'] || 'John'} #{result['last_name'] || 'Doe'}",
         account_id: @inbox.account_id,
         source_id: @sender_id,
         remote_avatar_url: result['profile_pic'] || nil
       }
     end
+  end
 end

app/models/account.rb

@@ -5,7 +5,7 @@ class Account < ApplicationRecord
   has_many :inboxes, dependent: :destroy
   has_many :conversations, dependent: :destroy
   has_many :contacts, dependent: :destroy
-  has_many :facebook_pages, dependent: :destroy
+  has_many :facebook_pages, dependent: :destroy, class_name: '::Channel::FacebookPage'
   has_many :telegram_bots, dependent: :destroy
   has_many :canned_responses, dependent: :destroy
   has_one :subscription, dependent: :destroy

app/models/channel/facebook_page.rb

@@ -12,7 +12,7 @@ module Channel
     before_destroy :unsubscribe
 
     def name
-      `Facebook`
+      'Facebook'
     end
 
     private

app/models/contact.rb

@@ -6,7 +6,7 @@ class Contact < ApplicationRecord
 
   belongs_to :account
   belongs_to :inbox
-  has_many :conversations, dependent: :destroy, foreign_key: :sender_id
+  has_many :conversations, dependent: :destroy
   mount_uploader :avatar, AvatarUploader
 
   def push_event_data

app/models/conversation.rb

@@ -13,7 +13,7 @@ class Conversation < ApplicationRecord
   belongs_to :account
   belongs_to :inbox
   belongs_to :assignee, class_name: 'User', optional: true
-  belongs_to :sender, class_name: 'Contact'
+  belongs_to :contact
 
   has_many :messages, dependent: :destroy, autosave: true
 

app/presenters/conversations/event_data_presenter.rb

@@ -23,7 +23,7 @@ module Conversations
     end
 
     def push_meta
-      { sender: sender.push_event_data, assignee: assignee }
+      { sender: contact.push_event_data, assignee: assignee }
     end
 
     def push_timestamps

app/services/facebook/send_reply_service.rb

@@ -12,6 +12,8 @@ module Facebook
 
     private
 
+    delegate :contact, to: :conversation
+
     def inbox
       @inbox ||= message.inbox
     end
@@ -20,10 +22,6 @@ module Facebook
       @conversation ||= message.conversation
     end
 
-    def sender
-      conversation.sender
-    end
-
     def outgoing_message_from_chatwoot?
       # messages sent directly from chatwoot won't have fb_id.
       message.outgoing? && !message.fb_id
@@ -37,7 +35,7 @@ module Facebook
 
     def fb_message_params
       {
-        recipient: { id: sender.source_id },
+        recipient: { id: contact.source_id },
         message: { text: message.content }
       }
     end

app/views/api/v1/conversations/index.json.jbuilder

@@ -9,16 +9,16 @@ json.data do
     json.array! @conversations do |conversation|
       json.meta do
         json.sender do
-          json.id conversation.sender_id
-          json.name conversation.sender.name
-          json.thumbnail conversation.sender.avatar.thumb.url
+          json.id conversation.contact.source_id
+          json.name conversation.contact.name
+          json.thumbnail conversation.contact.avatar.thumb.url
           json.channel conversation.inbox.try(:channel).try(:name)
         end
         json.assignee conversation.assignee
       end
 
       json.id conversation.display_id
-      if conversation.unread_incoming_messages.count == 0
+      if conversation.unread_incoming_messages.count.zero?
         json.messages [conversation.messages.last.try(:push_event_data)]
       else
         json.messages conversation.unread_messages.map(&:push_event_data)

app/views/api/v1/inboxes/index.json.jbuilder

@@ -8,8 +8,8 @@ json.data do
       json.channel_id inbox.channel_id
       json.name inbox.name
       json.channel_type inbox.channel_type
-      json.avatar_url inbox.channel.avatar.url
-      json.page_id inbox.channel.page_id
+      json.avatar_url inbox.channel.try(:avatar).try(:url)
+      json.page_id inbox.channel.try(:page_id)
     end
   end
 end

db/migrate/20191020173522_rename_sender_id_to_contact_in_conversation.rb

@@ -0,0 +1,5 @@
+class RenameSenderIdToContactInConversation < ActiveRecord::Migration[6.1]
+  def change
+    rename_column :conversations, :sender_id, :contact_id
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_10_20_085608) do
+ActiveRecord::Schema.define(version: 2019_10_20_173522) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -85,7 +85,7 @@ ActiveRecord::Schema.define(version: 2019_10_20_085608) do
     t.integer "assignee_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.bigint "sender_id"
+    t.bigint "contact_id"
     t.integer "display_id", null: false
     t.datetime "user_last_seen_at"
     t.datetime "agent_last_seen_at"

db/seeds.rb

@@ -10,5 +10,5 @@ inbox = Inbox.create!(channel: web_widget, account: account, name: 'Acme Support
 InboxMember.create!(user: user, inbox: inbox)
 
 contact = Contact.create!(name: 'jane', email: 'jane@example.com', phone_number: '0000', inbox: inbox, account: account)
-Conversation.create!(account: account, inbox: inbox, status: :open, assignee_id: 1, sender: contact)
-Message.create!(content: 'Hello', account_id: 1, inbox_id: 1, conversation_id: 1, message_type: :incoming)
+conversation = Conversation.create!(account: account, inbox: inbox, status: :open, assignee: user, contact: contact)
+Message.create!(content: 'Hello', account: account, inbox: inbox, conversation: conversation, message_type: :incoming)

lib/integrations/facebook/delivery_status.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
-class Integrations::Facebook::DeliveryStatus
+module Integrations
+  module Facebook
+    class DeliveryStatus
       def initialize(params)
         @params = params
       end
@@ -15,16 +17,18 @@ class Integrations::Facebook::DeliveryStatus
         @params.sender['id']
       end
 
-  def contact_id
-    @contact ||= ::Contact.find_by(source_id: sender_id)
+      def contact
+        Contact.find_by(source_id: sender_id)
       end
 
       def conversation
-    @conversation ||= ::Conversation.find_by(sender_id: contact_id)
+        @conversation ||= ::Conversation.find_by(contact_id: contact.id)
       end
 
       def update_message_status
         conversation.user_last_seen_at = @params.at
         conversation.save!
       end
+    end
+  end
 end

lib/integrations/facebook/message_creator.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
-class Integrations::Facebook::MessageCreator
+module Integrations
+  module Facebook
+    class MessageCreator
       attr_reader :response
 
       def initialize(response)
@@ -40,4 +42,6 @@ class Integrations::Facebook::MessageCreator
           mb.perform
         end
       end
+    end
+  end
 end

lib/integrations/facebook/message_parser.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
-class Integrations::Facebook::MessageParser
+module Integrations
+  module Facebook
+    class MessageParser
       def initialize(response_json)
         @response = response_json
       end
@@ -44,6 +46,8 @@ class Integrations::Facebook::MessageParser
       def sent_from_chatwoot_app?
         app_id && app_id == ENV['fb_app_id'].to_i
       end
+    end
+  end
 end
 
 # Sample Reponse

lib/integrations/widget/incoming_message_builder.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
-class Integrations::Widget::IncomingMessageBuilder
+module Integrations
+  module Widget
+    class Integrations::Widget::IncomingMessageBuilder
       #  params = {
       #   contact_id: 1,
       #   inbox_id: 1,
@@ -15,7 +17,6 @@ class Integrations::Widget::IncomingMessageBuilder
 
       def perform
         ActiveRecord::Base.transaction do
-      build_conversation
           build_message
         end
       end
@@ -30,17 +31,12 @@ class Integrations::Widget::IncomingMessageBuilder
         @contact ||= Contact.find(options[:contact_id])
       end
 
-  def build_conversation
-    @conversation ||=
-      if (conversation = Conversation.find_by(conversation_params))
-        conversation
-      else
-        Conversation.create!(conversation_params)
-      end
+      def conversation
+        @conversation ||= Conversation.find_by(conversation_params) || Conversation.create!(conversation_params)
       end
 
       def build_message
-    @message = @conversation.messages.new(message_params)
+        @message = conversation.messages.new(message_params)
         @message.save!
       end
 
@@ -48,16 +44,18 @@ class Integrations::Widget::IncomingMessageBuilder
         {
           account_id: inbox.account_id,
           inbox_id: inbox.id,
-      sender_id: options[:contact_id]
+          contact_id: options[:contact_id]
         }
       end
 
       def message_params
         {
-      account_id: @conversation.account_id,
-      inbox_id: @conversation.inbox_id,
+          account_id: conversation.account_id,
+          inbox_id: conversation.inbox_id,
           message_type: 0,
           content: options[:content]
         }
       end
+    end
+  end
 end

spec/factories/conversations.rb

@@ -16,7 +16,7 @@ FactoryBot.define do
           account: conversation.account,
           channel: create(:channel_widget, account: conversation.account)
         )
-        conversation.sender ||= create(:contact, account: conversation.account)
+        conversation.contact ||= create(:contact, account: conversation.account)
         conversation.assignee ||= create(:user)
       end
     end

spec/fixtures/messages.yml

@@ -1,23 +0,0 @@
-# Read about fixtures at http://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
-
-one:
-  content: MyText
-  account_id: 1
-  channel_id: 1
-  inbox_id: 1
-  conversation_id: 1
-  sender_id: 1
-  sender_type: MyString
-  reciever_id: 1
-  reciever_type: MyString
-
-two:
-  content: MyText
-  account_id: 1
-  channel_id: 1
-  inbox_id: 1
-  conversation_id: 1
-  sender_id: 1
-  sender_type: MyString
-  reciever_id: 1
-  reciever_type: MyString

spec/models/conversation_spec.rb

@@ -79,7 +79,7 @@ RSpec.describe Conversation, type: :model do
       create(
         :conversation,
         account: account,
-        sender: create(:contact, account: account),
+        contact: create(:contact, account: account),
         inbox: inbox,
         assignee: nil
       )
@@ -205,7 +205,7 @@ RSpec.describe Conversation, type: :model do
     let(:expected_data) do
       {
         meta: {
-          sender: conversation.sender.push_event_data,
+          sender: conversation.contact.push_event_data,
           assignee: conversation.assignee
         },
         id: conversation.display_id,

spec/presenters/conversations/event_data_presenter_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe Conversations::EventDataPresenter do
     let(:expected_data) do
       {
         meta: {
-          sender: conversation.sender.push_event_data,
+          sender: conversation.contact.push_event_data,
           assignee: conversation.assignee
         },
         id: conversation.display_id,