Refactoring the code for pub sub (#155)

- We were using the attribute name 'channel' to store pubsub tokens, which was confusing.
- switched to faker from ffaker
- spec for contact.rb
This commit is contained in:
Sojan Jose 2019-10-17 03:18:07 +05:30 committed by GitHub
parent ad5fb525f5
commit 3988777718
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 89 additions and 52 deletions

View file

@ -63,7 +63,7 @@ end
group :development, :test do group :development, :test do
gem 'byebug', platform: :mri gem 'byebug', platform: :mri
gem 'factory_bot_rails' gem 'factory_bot_rails'
gem 'ffaker' gem 'faker'
gem 'listen' gem 'listen'
gem 'pry-rails' gem 'pry-rails'
gem 'rspec-rails', '~> 3.8' gem 'rspec-rails', '~> 3.8'

View file

@ -186,9 +186,10 @@ GEM
factory_bot_rails (5.1.1) factory_bot_rails (5.1.1)
factory_bot (~> 5.1.0) factory_bot (~> 5.1.0)
railties (>= 4.2.0) railties (>= 4.2.0)
faker (2.1.2)
i18n (>= 0.8)
faraday (0.16.2) faraday (0.16.2)
multipart-post (>= 1.2, < 3) multipart-post (>= 1.2, < 3)
ffaker (2.13.0)
ffi (1.11.1) ffi (1.11.1)
figaro (1.1.1) figaro (1.1.1)
thor (~> 0.14) thor (~> 0.14)
@ -435,7 +436,7 @@ DEPENDENCIES
devise_token_auth! devise_token_auth!
facebook-messenger (~> 0.11.1) facebook-messenger (~> 0.11.1)
factory_bot_rails factory_bot_rails
ffaker faker
figaro figaro
foreman foreman
hashie hashie

View file

@ -108,9 +108,9 @@ export default {
} }
return false; return false;
}, },
getChannel() { getPubSubToken() {
if (this.isLoggedIn()) { if (this.isLoggedIn()) {
return Cookies.getJSON('user').channel; return Cookies.getJSON('user').pubsub_token;
} }
return null; return null;
}, },

View file

@ -63,7 +63,7 @@ export default {
const pusher = new VuePusher(CONSTANTS.PUSHER.token, options); const pusher = new VuePusher(CONSTANTS.PUSHER.token, options);
// Add to global Obj // Add to global Obj
if (AuthAPI.isLoggedIn()) { if (AuthAPI.isLoggedIn()) {
pusher.subscribe(AuthAPI.getChannel()); pusher.subscribe(AuthAPI.getPubSubToken());
return pusher.pusher; return pusher.pusher;
} }
return null; return null;

View file

@ -3,45 +3,45 @@ class PusherListener < BaseListener
def conversation_created(event) def conversation_created(event)
conversation, account, timestamp = extract_conversation_and_account(event) conversation, account, timestamp = extract_conversation_and_account(event)
members = conversation.inbox.members.pluck(:channel) members = conversation.inbox.members.pluck(:pubsub_token)
Pusher.trigger(members, CONVERSATION_CREATED , conversation.push_event_data) if members.present? Pusher.trigger(members, CONVERSATION_CREATED , conversation.push_event_data) if members.present?
end end
def conversation_read(event) def conversation_read(event)
conversation, account, timestamp = extract_conversation_and_account(event) conversation, account, timestamp = extract_conversation_and_account(event)
members = conversation.inbox.members.pluck(:channel) members = conversation.inbox.members.pluck(:pubsub_token)
Pusher.trigger(members, CONVERSATION_READ , conversation.push_event_data) if members.present? Pusher.trigger(members, CONVERSATION_READ , conversation.push_event_data) if members.present?
end end
def message_created(event) def message_created(event)
message, account, timestamp = extract_message_and_account(event) message, account, timestamp = extract_message_and_account(event)
conversation = message.conversation conversation = message.conversation
members = conversation.inbox.members.pluck(:channel) members = conversation.inbox.members.pluck(:pubsub_token)
Pusher.trigger(members, MESSAGE_CREATED , message.push_event_data) if members.present? Pusher.trigger(members, MESSAGE_CREATED , message.push_event_data) if members.present?
end end
def conversation_reopened(event) def conversation_reopened(event)
conversation, account, timestamp = extract_conversation_and_account(event) conversation, account, timestamp = extract_conversation_and_account(event)
members = conversation.inbox.members.pluck(:channel) members = conversation.inbox.members.pluck(:pubsub_token)
Pusher.trigger(members, CONVERSATION_REOPENED, conversation.push_event_data) if members.present? Pusher.trigger(members, CONVERSATION_REOPENED, conversation.push_event_data) if members.present?
end end
def conversation_lock_toggle(event) def conversation_lock_toggle(event)
conversation, account, timestamp = extract_conversation_and_account(event) conversation, account, timestamp = extract_conversation_and_account(event)
members = conversation.inbox.members.pluck(:channel) members = conversation.inbox.members.pluck(:pubsub_token)
Pusher.trigger(members, CONVERSATION_LOCK_TOGGLE, conversation.lock_event_data) if members.present? Pusher.trigger(members, CONVERSATION_LOCK_TOGGLE, conversation.lock_event_data) if members.present?
end end
def assignee_changed(event) def assignee_changed(event)
conversation, account, timestamp = extract_conversation_and_account(event) conversation, account, timestamp = extract_conversation_and_account(event)
members = conversation.inbox.members.pluck(:channel) members = conversation.inbox.members.pluck(:pubsub_token)
Pusher.trigger(members, ASSIGNEE_CHANGED, conversation.push_event_data) if members.present? Pusher.trigger(members, ASSIGNEE_CHANGED, conversation.push_event_data) if members.present?
end end
private private
def push(channel, data) def push(pubsub_token, data)
# Enqueue sidekiq job to push event to corresponding channel # Enqueue sidekiq job to push event to corresponding channel
end end
end end

View file

@ -1,5 +1,7 @@
class Contact < ApplicationRecord class Contact < ApplicationRecord
#Used by the pusher/PubSub Service we use for real time communications
has_secure_token :pubsub_token
validates :account_id, presence: true validates :account_id, presence: true
validates :inbox_id, presence: true validates :inbox_id, presence: true
@ -8,21 +10,13 @@ class Contact < ApplicationRecord
has_many :conversations, dependent: :destroy, foreign_key: :sender_id has_many :conversations, dependent: :destroy, foreign_key: :sender_id
mount_uploader :avatar, AvatarUploader mount_uploader :avatar, AvatarUploader
before_create :set_channel
def push_event_data def push_event_data
{ {
id: id, id: id,
name: name, name: name,
thumbnail: avatar.thumb.url, thumbnail: avatar.thumb.url,
channel: inbox.try(:channel).try(:name), channel: inbox.try(:channel).try(:name),
chat_channel: chat_channel pubsub_token: pubsub_token
} }
end end
def set_channel
begin
self.chat_channel = SecureRandom.hex
end while self.class.exists?(chat_channel: chat_channel)
end
end end

View file

@ -11,6 +11,9 @@ class User < ApplicationRecord
:validatable, :validatable,
:confirmable :confirmable
#Used by the pusher/PubSub Service we use for real time communications
has_secure_token :pubsub_token
validates_uniqueness_of :email, scope: :account_id validates_uniqueness_of :email, scope: :account_id
validates :email, presence: true validates :email, presence: true
validates :name, presence: true validates :name, presence: true
@ -26,7 +29,6 @@ class User < ApplicationRecord
has_many :assigned_inboxes, through: :inbox_members, source: :inbox has_many :assigned_inboxes, through: :inbox_members, source: :inbox
has_many :messages has_many :messages
before_create :set_channel
before_validation :set_password_and_uid, on: :create before_validation :set_password_and_uid, on: :create
accepts_nested_attributes_for :account accepts_nested_attributes_for :account
@ -42,12 +44,6 @@ class User < ApplicationRecord
super(options).merge(confirmed: confirmed?, subscription: account.try(:subscription).try(:summary) ) super(options).merge(confirmed: confirmed?, subscription: account.try(:subscription).try(:summary) )
end end
def set_channel
begin
self.channel = SecureRandom.hex
end while self.class.exists?(channel: channel)
end
def notify_creation def notify_creation
Rails.configuration.dispatcher.dispatch(AGENT_ADDED, Time.zone.now, account: self.account) Rails.configuration.dispatcher.dispatch(AGENT_ADDED, Time.zone.now, account: self.account)
end end

View file

@ -1,19 +1,8 @@
# frozen_string_literal: true
require_relative 'boot' require_relative 'boot'
require "rails" require 'rails/all'
# Pick the frameworks you want:
require "active_model/railtie"
require "active_job/railtie"
require "active_record/railtie"
require "active_storage/engine"
require "action_controller/railtie"
require "action_mailer/railtie"
require "action_mailbox/engine"
require "action_text/engine"
require "action_view/railtie"
# require "action_cable/engine"
require "sprockets/railtie"
require "rails/test_unit/railtie"
# Require the gems listed in Gemfile, including any gems # Require the gems listed in Gemfile, including any gems
# you've limited to :test, :development, or :production. # you've limited to :test, :development, or :production.
@ -23,8 +12,7 @@ module Chatwoot
class Application < Rails::Application class Application < Rails::Application
# Initialize configuration defaults for originally generated Rails version. # Initialize configuration defaults for originally generated Rails version.
config.load_defaults 5.0 config.load_defaults 5.0
config.paths.add File.join('app', 'bot'), glob: File.join('**', '*.rb')
config.autoload_paths += Dir[Rails.root.join('app', 'bot', '*')]
config.autoload_paths << Rails.root.join('lib') config.autoload_paths << Rails.root.join('lib')
config.eager_load_paths << Rails.root.join('lib') config.eager_load_paths << Rails.root.join('lib')

View file

@ -0,0 +1,9 @@
class RenameChannelAttributeNameInModels < ActiveRecord::Migration[6.1]
def change
rename_column :users, :channel, :pubsub_token
rename_column :contacts, :chat_channel, :pubsub_token
add_index :users, :pubsub_token, unique: true
add_index :contacts, :pubsub_token, unique: true
end
end

View file

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2019_10_14_051743) do ActiveRecord::Schema.define(version: 2019_10_16_211109) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -67,8 +67,9 @@ ActiveRecord::Schema.define(version: 2019_10_14_051743) do
t.integer "inbox_id", null: false t.integer "inbox_id", null: false
t.bigserial "source_id", null: false t.bigserial "source_id", null: false
t.string "avatar" t.string "avatar"
t.string "chat_channel" t.string "pubsub_token"
t.index ["account_id"], name: "index_contacts_on_account_id" t.index ["account_id"], name: "index_contacts_on_account_id"
t.index ["pubsub_token"], name: "index_contacts_on_pubsub_token", unique: true
end end
create_table "conversations", id: :serial, force: :cascade do |t| create_table "conversations", id: :serial, force: :cascade do |t|
@ -203,11 +204,12 @@ ActiveRecord::Schema.define(version: 2019_10_14_051743) do
t.integer "account_id", null: false t.integer "account_id", null: false
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.string "channel" t.string "pubsub_token"
t.integer "role", default: 0 t.integer "role", default: 0
t.bigint "inviter_id" t.bigint "inviter_id"
t.index ["email"], name: "index_users_on_email" t.index ["email"], name: "index_users_on_email"
t.index ["inviter_id"], name: "index_users_on_inviter_id" t.index ["inviter_id"], name: "index_users_on_inviter_id"
t.index ["pubsub_token"], name: "index_users_on_pubsub_token", unique: true
t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true
t.index ["uid", "provider"], name: "index_users_on_uid_and_provider", unique: true t.index ["uid", "provider"], name: "index_users_on_uid_and_provider", unique: true
end end

View file

@ -6,7 +6,6 @@ FactoryBot.define do
sequence(:email) { |n| "widget-#{n}@example.com" } sequence(:email) { |n| "widget-#{n}@example.com" }
phone_number { "+123456789011" } phone_number { "+123456789011" }
source_id { rand(100) } source_id { rand(100) }
chat_channel { "chat_channel" }
account account
inbox inbox
end end

View file

@ -8,8 +8,8 @@ FactoryBot.define do
provider { 'email' } provider { 'email' }
uid { SecureRandom.uuid } uid { SecureRandom.uuid }
name { FFaker::Name.name } name { Faker::Name.name }
nickname { FFaker::InternetSE.user_name_from_name(name) } nickname { Faker::Name.first_name }
email { nickname + '@example.com' } email { nickname + '@example.com' }
role { 'agent' } role { 'agent' }
password { "password" } password { "password" }

View file

@ -0,0 +1,32 @@
# frozen_string_literal: true
require 'rails_helper'
RSpec.describe Contact do
context 'validations' do
it { is_expected.to validate_presence_of(:account_id) }
it { is_expected.to validate_presence_of(:inbox_id) }
end
context 'associations' do
it { is_expected.to belong_to(:account) }
it { is_expected.to belong_to(:inbox) }
it { is_expected.to have_many(:conversations).dependent(:destroy) }
end
describe 'pubsub_token' do
let(:user) { create(:user) }
it 'gets created on object create' do
obj = user
expect(obj.pubsub_token).not_to eq(nil)
end
it 'does not get updated on object update' do
obj = user
old_token = obj.pubsub_token
obj.update(name: 'test')
expect(obj.pubsub_token).to eq(old_token)
end
end
end

View file

@ -21,4 +21,20 @@ RSpec.describe User do
it { is_expected.to have_many(:assigned_inboxes).through(:inbox_members) } it { is_expected.to have_many(:assigned_inboxes).through(:inbox_members) }
it { is_expected.to have_many(:messages) } it { is_expected.to have_many(:messages) }
end end
describe 'pubsub_token' do
let(:user) { create(:user) }
it 'gets created on object create' do
obj = user
expect(obj.pubsub_token).not_to eq(nil)
end
it 'does not get updated on object update' do
obj = user
old_token = obj.pubsub_token
obj.update(name: 'test')
expect(obj.pubsub_token).to eq(old_token)
end
end
end end