Chore: Routine Bugfixes and enhancements (#979)

- Fix slack scopes
- Docs for authentication
Fixes: #704 , #973

.circleci/config.yml

@@ -98,7 +98,7 @@ jobs:
       - run:
           name: Run backend tests
           command: |
-            bundle exec rspec $(circleci tests glob "spec/**/*_spec.rb" | circleci tests split --split-by=timings)
+            bundle exec rspec $(circleci tests glob "spec/**/*_spec.rb" | circleci tests split --split-by=timings) --profile=10
             ~/tmp/cc-test-reporter format-coverage -t simplecov -o ~/tmp/codeclimate.backend.json coverage/backend/.resultset.json
       - persist_to_workspace:
           root: ~/tmp

Gemfile.lock

@@ -93,10 +93,10 @@ GEM
       rake (>= 10.4, < 14.0)
     ast (2.4.1)
     attr_extras (6.2.4)
-    autoprefixer-rails (9.7.6)
+    autoprefixer-rails (9.8.2)
       execjs
     aws-eventstream (1.1.0)
-    aws-partitions (1.329.0)
+    aws-partitions (1.332.0)
     aws-sdk-core (3.100.0)
       aws-eventstream (~> 1, >= 1.0.2)
       aws-partitions (~> 1, >= 1.239.0)
@@ -105,12 +105,12 @@ GEM
     aws-sdk-kms (1.34.1)
       aws-sdk-core (~> 3, >= 3.99.0)
       aws-sigv4 (~> 1.1)
-    aws-sdk-s3 (1.68.1)
+    aws-sdk-s3 (1.69.1)
       aws-sdk-core (~> 3, >= 3.99.0)
       aws-sdk-kms (~> 1)
       aws-sigv4 (~> 1.1)
-    aws-sigv4 (1.1.4)
-      aws-eventstream (~> 1.0, >= 1.0.2)
+    aws-sigv4 (1.2.0)
+      aws-eventstream (~> 1, >= 1.0.2)
     axiom-types (0.1.1)
       descendants_tracker (~> 0.0.4)
       ice_nine (~> 0.11.0)
@@ -163,7 +163,7 @@ GEM
       devise (> 3.5.2, < 5)
       rails (>= 4.2.0, < 6.1)
       sprockets (= 3.7.2)
-    diff-lcs (1.3)
+    diff-lcs (1.4)
     digest-crc (0.5.1)
     docile (1.3.2)
     domain_name (0.5.20190701)
@@ -178,11 +178,11 @@ GEM
     facebook-messenger (1.5.0)
       httparty (~> 0.13, >= 0.13.7)
       rack (>= 1.4.5)
-    factory_bot (5.2.0)
-      activesupport (>= 4.2.0)
-    factory_bot_rails (5.2.0)
-      factory_bot (~> 5.2.0)
-      railties (>= 4.2.0)
+    factory_bot (6.0.2)
+      activesupport (>= 5.0.0)
+    factory_bot_rails (6.0.0)
+      factory_bot (~> 6.0.0)
+      railties (>= 5.0.0)
     faker (2.12.0)
       i18n (>= 1.6, < 2)
     faraday (1.0.1)
@@ -197,7 +197,7 @@ GEM
     gli (2.19.1)
     globalid (0.4.2)
       activesupport (>= 4.2.0)
-    google-api-client (0.40.2)
+    google-api-client (0.41.0)
       addressable (~> 2.5, >= 2.5.1)
       googleauth (~> 0.9)
       httpclient (>= 2.8.1, < 3.0)
@@ -218,7 +218,7 @@ GEM
       google-cloud-core (~> 1.2)
       googleauth (~> 0.9)
       mini_mime (~> 1.0)
-    googleauth (0.12.0)
+    googleauth (0.13.0)
       faraday (>= 0.17.3, < 2.0)
       jwt (>= 1.4, < 3.0)
       memoist (~> 0.16)
@@ -306,8 +306,8 @@ GEM
     orm_adapter (0.5.0)
     os (1.1.0)
     parallel (1.19.2)
-    parser (2.7.1.3)
-      ast (~> 2.4.0)
+    parser (2.7.1.4)
+      ast (~> 2.4.1)
     pg (1.2.3)
     pry (0.13.1)
       coderay (~> 1.1)
@@ -367,7 +367,7 @@ GEM
     redis-rack-cache (2.2.1)
       rack-cache (>= 1.10, < 2)
       redis-store (>= 1.6, < 2)
-    redis-store (1.8.2)
+    redis-store (1.9.0)
       redis (>= 4, < 5)
     regexp_parser (1.7.1)
     representable (3.0.4)
@@ -401,13 +401,13 @@ GEM
       rspec-mocks (~> 3.9)
       rspec-support (~> 3.9)
     rspec-support (3.9.3)
-    rubocop (0.85.1)
+    rubocop (0.86.0)
       parallel (~> 1.10)
       parser (>= 2.7.0.1)
       rainbow (>= 2.2.2, < 4.0)
       regexp_parser (>= 1.7)
       rexml
-      rubocop-ast (>= 0.0.3)
+      rubocop-ast (>= 0.0.3, < 1.0)
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 1.4.0, < 2.0)
     rubocop-ast (0.0.3)

app/controllers/api/v1/accounts/labels_controller.rb

@@ -14,7 +14,7 @@ def create
   end
 
   def update
-    @label.update(permitted_params)
+    @label.update!(permitted_params)
   end
 
   def destroy

app/controllers/application_controller.rb

@@ -59,11 +59,19 @@ def render_error_response(exception)
     render json: exception.to_hash, status: exception.http_status
   end
 
+  def locale_from_params
+    I18n.available_locales.map(&:to_s).include?(params[:locale]) ? params[:locale] : nil
+  end
+
+  def locale_from_account(account)
+    I18n.available_locales.map(&:to_s).include?(account.locale) ? account.locale : nil
+  end
+
   def switch_locale(account)
     # priority is for locale set in query string (mostly for widget/from js sdk)
-    locale ||= (I18n.available_locales.map(&:to_s).include?(params[:locale]) ? params[:locale] : nil)
+    locale ||= locale_from_params
     # if local is not set in param, lets try account
-    locale ||= (I18n.available_locales.map(&:to_s).include?(account.locale) ? account.locale : nil)
+    locale ||= locale_from_account(account)
     I18n.locale = locale || I18n.default_locale
   end
 

app/controllers/widgets_controller.rb

@@ -4,6 +4,7 @@ class WidgetsController < ActionController::Base
   before_action :set_token
   before_action :set_contact
   before_action :build_contact
+  after_action :allow_iframe_requests
 
   def index; end
 
@@ -50,4 +51,8 @@ def build_contact
   def permitted_params
     params.permit(:website_token, :cw_conversation)
   end
+
+  def allow_iframe_requests
+    response.headers.delete('X-Frame-Options')
+  end
 end

app/finders/conversation_finder.rb

@@ -62,7 +62,7 @@ def set_assignee_type
 
   def find_all_conversations
     @conversations = current_account.conversations.includes(
-      :assignee, :contact, :inbox
+      :assignee, :inbox, contact: [:avatar_attachment]
     ).where(inbox_id: @inbox_ids)
   end
 

app/javascript/dashboard/routes/dashboard/settings/labels/validations.js

@@ -1,6 +1,6 @@
 import { required, minLength } from 'vuelidate/lib/validators';
 
-export const validLabelCharacters = (str = '') => /^[\w-_]+$/g.test(str);
+export const validLabelCharacters = (str = '') => !!str && !str.includes(' ');
 
 export default {
   title: {

app/jobs/hook_job.rb

@@ -4,7 +4,7 @@ class HookJob < ApplicationJob
   def perform(hook, message)
     return unless hook.slack?
 
-    Integrations::Slack::OutgoingMessageBuilder.perform(hook, message)
+    Integrations::Slack::SendOnSlackService.new(message: message, hook: hook).perform
   rescue StandardError => e
     Raven.capture_exception(e)
   end

app/models/message.rb

@@ -138,11 +138,11 @@ def dispatch_update_event
   def send_reply
     channel_name = conversation.inbox.channel.class.to_s
     if channel_name == 'Channel::FacebookPage'
-      ::Facebook::SendReplyService.new(message: self).perform
+      ::Facebook::SendOnFacebookService.new(message: self).perform
     elsif channel_name == 'Channel::TwitterProfile'
-      ::Twitter::SendReplyService.new(message: self).perform
+      ::Twitter::SendOnTwitterService.new(message: self).perform
     elsif channel_name == 'Channel::TwilioSms'
-      ::Twilio::OutgoingMessageService.new(message: self).perform
+      ::Twilio::SendOnTwilioService.new(message: self).perform
     end
   end
 

app/services/base/send_on_channel_service.rb

@@ -0,0 +1,55 @@
+#######################################
+# To create an external channel reply service
+# - Inherit this as the base class.
+# - Implement `channel_class` method in your child class.
+# - Implement `perform_reply` method in your child class.
+# - Implement additional custom logic for your `perform_reply` method.
+# - When required override the validation_methods.
+# - Use Childclass.new.perform.
+######################################
+class Base::SendOnChannelService
+  pattr_initialize [:message!]
+
+  def perform
+    validate_target_channel
+    return unless outgoing_message?
+    return if invalid_message?
+
+    perform_reply
+  end
+
+  private
+
+  delegate :conversation, to: :message
+  delegate :contact, :contact_inbox, :inbox, to: :conversation
+  delegate :channel, to: :inbox
+
+  def channel_class
+    raise 'Overwrite this method in child class'
+  end
+
+  def perform_reply
+    raise 'Overwrite this method in child class'
+  end
+
+  def outgoing_message_originated_from_channel?
+    # TODO: we need to refactor this logic as more integrations comes by
+    # chatwoot messages won't have source id at the moment
+    # outgoing messages may be created in slack which should be send to the channel
+    message.source_id.present? && !message.source_id.starts_with?('slack_')
+  end
+
+  def outgoing_message?
+    message.outgoing? || message.template?
+  end
+
+  def invalid_message?
+    # private notes aren't send to the channels
+    # we should also avoid the case of message loops, when outgoing messages are created from channel
+    message.private? || outgoing_message_originated_from_channel?
+  end
+
+  def validate_target_channel
+    raise 'Invalid channel service was called' if inbox.channel.class != channel_class
+  end
+end

app/services/facebook/send_reply_service.rb → app/services/facebook/send_on_facebook_service.rb

@@ -1,37 +1,14 @@
-class Facebook::SendReplyService
-  pattr_initialize [:message!]
-
-  def perform
-    return if message.private
-    return if inbox.channel.class.to_s != 'Channel::FacebookPage'
-    return unless outgoing_message_from_chatwoot?
-
-    FacebookBot::Bot.deliver(delivery_params, access_token: message.channel_token)
-  end
-
+class Facebook::SendOnFacebookService < Base::SendOnChannelService
   private
 
-  delegate :contact, to: :conversation
-
-  def inbox
-    @inbox ||= message.inbox
-  end
-
-  def conversation
-    @conversation ||= message.conversation
+  def channel_class
+    Channel::FacebookPage
   end
 
-  def outgoing_message_from_chatwoot?
-    # messages sent directly from chatwoot won't have source_id.
-    (message.outgoing? || message.template?) && !message.source_id
+  def perform_reply
+    FacebookBot::Bot.deliver(delivery_params, access_token: message.channel_token)
   end
 
-  # def reopen_lock
-  #   if message.incoming? && conversation.locked?
-  #     conversation.unlock!
-  #   end
-  # end
-
   def fb_text_message_params
     {
       recipient: { id: contact.get_source_id(inbox.id) },

app/services/twilio/outgoing_message_service.rb → app/services/twilio/send_on_twilio_service.rb

@@ -1,22 +1,15 @@
-class Twilio::OutgoingMessageService
-  pattr_initialize [:message!]
+class Twilio::SendOnTwilioService < Base::SendOnChannelService
+  private
 
-  def perform
-    return if message.private
-    return if message.source_id
-    return if inbox.channel.class.to_s != 'Channel::TwilioSms'
-    return unless outgoing_message?
+  def channel_class
+    Channel::TwilioSms
+  end
 
+  def perform_reply
     twilio_message = client.messages.create(message_params)
     message.update!(source_id: twilio_message.sid)
   end
 
-  private
-
-  delegate :conversation, to: :message
-  delegate :contact, to: :conversation
-  delegate :contact_inbox, to: :conversation
-
   def message_params
     params = {
       body: message.content,

app/services/twitter/send_reply_service.rb → app/services/twitter/send_on_twitter_service.rb

@@ -1,16 +1,17 @@
-class Twitter::SendReplyService
+class Twitter::SendOnTwitterService < Base::SendOnChannelService
   pattr_initialize [:message!]
 
-  def perform
-    return if message.private
-    return if message.source_id
-    return if inbox.channel.class.to_s != 'Channel::TwitterProfile'
-    return unless outgoing_message_from_chatwoot?
+  private
+
+  delegate :additional_attributes, to: :contact
 
-    send_reply
+  def channel_class
+    Channel::TwitterProfile
   end
 
-  private
+  def perform_reply
+    conversation_type == 'tweet' ? send_tweet_reply : send_direct_message
+  end
 
   def twitter_client
     Twitty::Facade.new do |config|
@@ -50,19 +51,4 @@ def send_tweet_reply
       Rails.logger.info 'TWITTER_TWEET_REPLY_ERROR' + response.body
     end
   end
-
-  def send_reply
-    conversation_type == 'tweet' ? send_tweet_reply : send_direct_message
-  end
-
-  def outgoing_message_from_chatwoot?
-    (message.outgoing? || message.template?)
-  end
-
-  delegate :additional_attributes, to: :contact
-  delegate :contact, to: :conversation
-  delegate :contact_inbox, to: :conversation
-  delegate :conversation, to: :message
-  delegate :inbox, to: :conversation
-  delegate :channel, to: :inbox
 end

app/views/api/v1/conversations/partials/_conversation.json.jbuilder

@@ -12,7 +12,7 @@ json.id conversation.display_id
 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)
+  json.messages conversation.unread_messages.includes([:user, :attachments]).map(&:push_event_data)
 end
 
 json.inbox_id conversation.inbox_id