From 2b1d445003bc723d1fc4f29093cf78b5df5b9eaa Mon Sep 17 00:00:00 2001
From: Pranav Raj S <pranav@thoughtwoot.com>
Date: Mon, 27 Jul 2020 22:19:26 +0530
Subject: [PATCH] feat: Use display_name instead of name of the agent (#1097)

* feat: Use display_name instead of name of the agent
---
 app/controllers/api/v1/profiles_controller.rb     | 10 +++++++++-
 app/javascript/dashboard/api/auth.js              |  8 +++++++-
 .../dashboard/components/layout/Sidebar.vue       |  8 ++++++--
 .../widgets/conversation/ConversationHeader.vue   |  4 ++--
 .../components/widgets/conversation/Message.vue   | 11 +++++++----
 .../dashboard/i18n/locale/en/settings.json        | 11 ++++++++---
 .../routes/dashboard/settings/inbox/Settings.vue  | 11 ++---------
 .../routes/dashboard/settings/profile/Index.vue   | 15 +++++++++++++++
 .../store/modules/conversations/getters.js        |  5 ++---
 app/mailers/conversation_reply_mailer.rb          |  6 +++---
 app/models/conversation.rb                        |  4 ++--
 app/models/user.rb                                |  3 ++-
 .../conversations/event_data_presenter.rb         |  2 +-
 .../v1/accounts/inbox_members/show.json.jbuilder  |  3 +--
 .../partials/_conversation.json.jbuilder          |  6 +++++-
 app/views/api/v1/models/_agent.json.jbuilder      |  1 +
 app/views/api/v1/models/_user.json.jbuilder       |  1 +
 .../v1/widget/inbox_members/index.json.jbuilder   |  2 +-
 .../reply_with_summary.html.erb                   |  2 +-
 spec/mailers/conversation_reply_mailer_spec.rb    |  4 ++--
 spec/models/conversation_spec.rb                  |  4 ++--
 21 files changed, 80 insertions(+), 41 deletions(-)

diff --git a/app/controllers/api/v1/profiles_controller.rb b/app/controllers/api/v1/profiles_controller.rb
index 38f1a9b3b..bda3e8c90 100644
--- a/app/controllers/api/v1/profiles_controller.rb
+++ b/app/controllers/api/v1/profiles_controller.rb
@@ -16,6 +16,14 @@ class Api::V1::ProfilesController < Api::BaseController
   end
 
   def profile_params
-    params.require(:profile).permit(:email, :name, :password, :password_confirmation, :avatar, :availability)
+    params.require(:profile).permit(
+      :email,
+      :name,
+      :display_name,
+      :password,
+      :password_confirmation,
+      :avatar,
+      :availability
+    )
   end
 end
diff --git a/app/javascript/dashboard/api/auth.js b/app/javascript/dashboard/api/auth.js
index 42d1fbc41..ae5031cd4 100644
--- a/app/javascript/dashboard/api/auth.js
+++ b/app/javascript/dashboard/api/auth.js
@@ -118,7 +118,12 @@ export default {
     return axios.post(urlData.url, { email });
   },
 
-  profileUpdate({ password, password_confirmation, ...profileAttributes }) {
+  profileUpdate({
+    password,
+    password_confirmation,
+    displayName,
+    ...profileAttributes
+  }) {
     const formData = new FormData();
     Object.keys(profileAttributes).forEach(key => {
       const value = profileAttributes[key];
@@ -126,6 +131,7 @@ export default {
         formData.append(`profile[${key}]`, value);
       }
     });
+    formData.append('profile[display_name]', displayName || '');
     if (password && password_confirmation) {
       formData.append('profile[password]', password);
       formData.append('profile[password_confirmation]', password_confirmation);
diff --git a/app/javascript/dashboard/components/layout/Sidebar.vue b/app/javascript/dashboard/components/layout/Sidebar.vue
index fbd10920c..f13fbee92 100644
--- a/app/javascript/dashboard/components/layout/Sidebar.vue
+++ b/app/javascript/dashboard/components/layout/Sidebar.vue
@@ -58,12 +58,12 @@
       <div class="current-user" @click.prevent="showOptions()">
         <thumbnail
           :src="currentUser.avatar_url"
-          :username="currentUser.name"
+          :username="currentUserAvailableName"
           :status="currentUser.availability_status"
         />
         <div class="current-user--data">
           <h3 class="current-user--name">
-            {{ currentUser.name }}
+            {{ currentUserAvailableName }}
           </h3>
           <h5 class="current-user--role">
             {{ currentRole }}
@@ -202,6 +202,10 @@ export default {
       uiFlags: 'agents/getUIFlags',
       accountLabels: 'labels/getLabelsOnSidebar',
     }),
+    currentUserAvailableName() {
+      const { available_name: availableName } = this.currentUser;
+      return availableName;
+    },
     showChangeAccountOption() {
       if (this.globalConfig.createNewAccountFromDashboard) {
         return true;
diff --git a/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue b/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue
index 7783e8c94..c5ce3d0be 100644
--- a/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue
+++ b/app/javascript/dashboard/components/widgets/conversation/ConversationHeader.vue
@@ -25,7 +25,7 @@
         <multiselect
           v-model="currentChat.meta.assignee"
           :options="agentList"
-          label="name"
+          label="available_name"
           :allow-empty="true"
           deselect-label="Remove"
           placeholder="Select Agent"
@@ -95,7 +95,7 @@ export default {
       return [
         {
           confirmed: true,
-          name: 'None',
+          available_name: 'None',
           id: 0,
           role: 'agent',
           account_id: 0,
diff --git a/app/javascript/dashboard/components/widgets/conversation/Message.vue b/app/javascript/dashboard/components/widgets/conversation/Message.vue
index 4ef1ddb4c..8d7a69700 100644
--- a/app/javascript/dashboard/components/widgets/conversation/Message.vue
+++ b/app/javascript/dashboard/components/widgets/conversation/Message.vue
@@ -95,10 +95,13 @@ export default {
         : false;
     },
     sentByMessage() {
-      return this.data.message_type === 1 &&
-        !this.isHovered &&
-        this.data.sender !== undefined
-        ? { content: `Sent by: ${this.data.sender.name}`, classes: 'top' }
+      const { sender } = this.data;
+
+      return this.data.message_type === 1 && !this.isHovered && sender
+        ? {
+            content: `Sent by: ${sender.available_name || sender.name}`,
+            classes: 'top',
+          }
         : false;
     },
     wrapClass() {
diff --git a/app/javascript/dashboard/i18n/locale/en/settings.json b/app/javascript/dashboard/i18n/locale/en/settings.json
index 7ab8d34d0..f39997b6e 100644
--- a/app/javascript/dashboard/i18n/locale/en/settings.json
+++ b/app/javascript/dashboard/i18n/locale/en/settings.json
@@ -44,9 +44,14 @@
         "LABEL": "Profile Image"
       },
       "NAME": {
-        "LABEL": "Your name",
-        "ERROR": "Please enter a valid name",
-        "PLACEHOLDER": "Please enter your name, this would be displayed in conversations"
+        "LABEL": "Your full name",
+        "ERROR": "Please enter a valid full name",
+        "PLACEHOLDER": "Please enter your full name"
+      },
+      "DISPLAY_NAME": {
+        "LABEL": "Display name",
+        "ERROR": "Please enter a valid display name",
+        "PLACEHOLDER": "Please enter a display name, this would be displayed in conversations"
       },
       "AVAILABILITY": {
         "LABEL": "Availability",
diff --git a/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue b/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue
index 8a30a4dce..10f5ff5a0 100644
--- a/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue
+++ b/app/javascript/dashboard/routes/dashboard/settings/inbox/Settings.vue
@@ -291,16 +291,9 @@ export default {
           inboxId: this.currentInboxId,
         });
         const {
-          data: { payload },
+          data: { payload: inboxMembers },
         } = response;
-        payload.forEach(el => {
-          const [item] = this.agentList.filter(
-            agent => agent.id === el.user_id
-          );
-          if (item) {
-            this.selectedAgents.push(item);
-          }
-        });
+        this.selectedAgents = inboxMembers;
       } catch (error) {
         console.log(error);
       }
diff --git a/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue b/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue
index 00d8b9a35..e0575ccaa 100644
--- a/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue
+++ b/app/javascript/dashboard/routes/dashboard/settings/profile/Index.vue
@@ -26,6 +26,17 @@
               {{ $t('PROFILE_SETTINGS.FORM.NAME.ERROR') }}
             </span>
           </label>
+          <label :class="{ error: $v.displayName.$error }">
+            {{ $t('PROFILE_SETTINGS.FORM.DISPLAY_NAME.LABEL') }}
+            <input
+              v-model="displayName"
+              type="text"
+              :placeholder="
+                $t('PROFILE_SETTINGS.FORM.DISPLAY_NAME.PLACEHOLDER')
+              "
+              @input="$v.displayName.$touch"
+            />
+          </label>
           <label :class="{ error: $v.email.$error }">
             {{ $t('PROFILE_SETTINGS.FORM.EMAIL.LABEL') }}
             <input
@@ -130,6 +141,7 @@ export default {
       avatarFile: '',
       avatarUrl: '',
       name: '',
+      displayName: '',
       email: '',
       password: '',
       passwordConfirmation: '',
@@ -141,6 +153,7 @@ export default {
     name: {
       required,
     },
+    displayName: {},
     email: {
       required,
       email,
@@ -188,6 +201,7 @@ export default {
       this.email = this.currentUser.email;
       this.avatarUrl = this.currentUser.avatar_url;
       this.availability = this.currentUser.availability_status;
+      this.displayName = this.currentUser.display_name;
     },
     async updateUser() {
       this.$v.$touch();
@@ -203,6 +217,7 @@ export default {
           email: this.email,
           avatar: this.avatarFile,
           password: this.password,
+          displayName: this.displayName,
           availability: this.availability,
           password_confirmation: this.passwordConfirmation,
         });
diff --git a/app/javascript/dashboard/store/modules/conversations/getters.js b/app/javascript/dashboard/store/modules/conversations/getters.js
index c1a42c3e4..04d7f58c3 100644
--- a/app/javascript/dashboard/store/modules/conversations/getters.js
+++ b/app/javascript/dashboard/store/modules/conversations/getters.js
@@ -21,7 +21,7 @@ const getters = {
   getMineChats(_state) {
     const currentUserID = authAPI.getCurrentUser().id;
     return _state.allConversations.filter(chat =>
-      chat.meta.assignee === null
+      !chat.meta.assignee
         ? false
         : chat.status === _state.chatStatusFilter &&
           chat.meta.assignee.id === currentUserID
@@ -29,8 +29,7 @@ const getters = {
   },
   getUnAssignedChats(_state) {
     return _state.allConversations.filter(
-      chat =>
-        chat.meta.assignee === null && chat.status === _state.chatStatusFilter
+      chat => !chat.meta.assignee && chat.status === _state.chatStatusFilter
     );
   },
   getAllStatusChats(_state) {
diff --git a/app/mailers/conversation_reply_mailer.rb b/app/mailers/conversation_reply_mailer.rb
index eed4a4c43..33e93d1a6 100644
--- a/app/mailers/conversation_reply_mailer.rb
+++ b/app/mailers/conversation_reply_mailer.rb
@@ -56,7 +56,7 @@ class ConversationReplyMailer < ApplicationMailer
 
   def reply_email
     if inbound_email_enabled?
-      "#{@agent.name} <reply+#{@conversation.uuid}@#{current_domain}>"
+      "#{@agent.available_name} <reply+#{@conversation.uuid}@#{current_domain}>"
     else
       @agent&.email
     end
@@ -64,9 +64,9 @@ class ConversationReplyMailer < ApplicationMailer
 
   def from_email
     if inbound_email_enabled?
-      "#{@agent.name} <#{account_support_email}>"
+      "#{@agent.available_name} <#{account_support_email}>"
     else
-      "#{@agent.name} <#{ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')}>"
+      "#{@agent.available_name} <#{ENV.fetch('MAILER_SENDER_EMAIL', 'accounts@chatwoot.com')}>"
     end
   end
 
diff --git a/app/models/conversation.rb b/app/models/conversation.rb
index ca1ad7f58..aad39a5ee 100644
--- a/app/models/conversation.rb
+++ b/app/models/conversation.rb
@@ -152,7 +152,7 @@ class Conversation < ApplicationRecord
   def create_activity
     return unless Current.user
 
-    user_name = Current.user&.name
+    user_name = Current.user&.available_name
 
     create_status_change_message(user_name) if saved_change_to_status?
     create_assignee_change(user_name) if saved_change_to_assignee_id?
@@ -208,7 +208,7 @@ class Conversation < ApplicationRecord
   end
 
   def create_assignee_change(user_name)
-    params = { assignee_name: assignee&.name, user_name: user_name }.compact
+    params = { assignee_name: assignee&.available_name, user_name: user_name }.compact
     key = assignee_id ? 'assigned' : 'removed'
     content = I18n.t("conversations.activity.assignee.#{key}", **params)
 
diff --git a/app/models/user.rb b/app/models/user.rb
index e311dda95..34e4799ce 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -97,7 +97,7 @@ class User < ApplicationRecord
     account_users.find_by(account_id: Current.account.id) if Current.account
   end
 
-  def display_name
+  def available_name
     self[:display_name].presence || name
   end
 
@@ -134,6 +134,7 @@ class User < ApplicationRecord
     {
       id: id,
       name: name,
+      available_name: available_name,
       avatar_url: avatar_url,
       type: 'user',
       availability_status: availability_status
diff --git a/app/presenters/conversations/event_data_presenter.rb b/app/presenters/conversations/event_data_presenter.rb
index b98e126a0..0c1660fb0 100644
--- a/app/presenters/conversations/event_data_presenter.rb
+++ b/app/presenters/conversations/event_data_presenter.rb
@@ -25,7 +25,7 @@ class Conversations::EventDataPresenter < SimpleDelegator
   end
 
   def push_meta
-    { sender: contact.push_event_data, assignee: assignee }
+    { sender: contact.push_event_data, assignee: assignee&.push_event_data }
   end
 
   def push_timestamps
diff --git a/app/views/api/v1/accounts/inbox_members/show.json.jbuilder b/app/views/api/v1/accounts/inbox_members/show.json.jbuilder
index 493a47035..f30b5e9fe 100644
--- a/app/views/api/v1/accounts/inbox_members/show.json.jbuilder
+++ b/app/views/api/v1/accounts/inbox_members/show.json.jbuilder
@@ -1,6 +1,5 @@
 json.payload do
   json.array! @agents do |agent|
-    json.user_id agent.id
-    json.name agent.name
+    json.partial! 'api/v1/models/user.json.jbuilder', resource: agent
   end
 end
diff --git a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder
index 6c2861292..7b79ba077 100644
--- a/app/views/api/v1/conversations/partials/_conversation.json.jbuilder
+++ b/app/views/api/v1/conversations/partials/_conversation.json.jbuilder
@@ -3,7 +3,11 @@ json.meta do
     json.partial! 'api/v1/models/contact.json.jbuilder', resource: conversation.contact
   end
   json.channel conversation.inbox.try(:channel_type)
-  json.assignee conversation.assignee
+  if conversation.assignee
+    json.assignee do
+      json.partial! 'api/v1/models/user.json.jbuilder', resource: conversation.assignee
+    end
+  end
 end
 
 json.id conversation.display_id
diff --git a/app/views/api/v1/models/_agent.json.jbuilder b/app/views/api/v1/models/_agent.json.jbuilder
index a53b1f2cc..6a0932d7a 100644
--- a/app/views/api/v1/models/_agent.json.jbuilder
+++ b/app/views/api/v1/models/_agent.json.jbuilder
@@ -2,6 +2,7 @@ json.account_id resource.account.id
 json.availability_status resource.availability_status
 json.confirmed resource.confirmed?
 json.email resource.email
+json.available_name resource.available_name
 json.id resource.id
 json.name resource.name
 json.role resource.role
diff --git a/app/views/api/v1/models/_user.json.jbuilder b/app/views/api/v1/models/_user.json.jbuilder
index 6caf4d4bb..bbe1414ac 100644
--- a/app/views/api/v1/models/_user.json.jbuilder
+++ b/app/views/api/v1/models/_user.json.jbuilder
@@ -3,6 +3,7 @@ json.provider resource.provider
 json.uid resource.uid
 json.name resource.name
 json.display_name resource.display_name
+json.available_name resource.available_name
 json.email resource.email
 json.account_id resource.active_account_user&.account_id
 json.pubsub_token resource.pubsub_token
diff --git a/app/views/api/v1/widget/inbox_members/index.json.jbuilder b/app/views/api/v1/widget/inbox_members/index.json.jbuilder
index 1ab4c57a5..af38fc1ee 100644
--- a/app/views/api/v1/widget/inbox_members/index.json.jbuilder
+++ b/app/views/api/v1/widget/inbox_members/index.json.jbuilder
@@ -1,7 +1,7 @@
 json.payload do
   json.array! @inbox_members do |inbox_member|
     json.id inbox_member.user.id
-    json.name inbox_member.user.name
+    json.name inbox_member.user.available_name
     json.avatar_url inbox_member.user.avatar_url
     json.availability_status inbox_member.user.availability_status
   end
diff --git a/app/views/mailers/conversation_reply_mailer/reply_with_summary.html.erb b/app/views/mailers/conversation_reply_mailer/reply_with_summary.html.erb
index 67de4919e..7d8b6b680 100644
--- a/app/views/mailers/conversation_reply_mailer/reply_with_summary.html.erb
+++ b/app/views/mailers/conversation_reply_mailer/reply_with_summary.html.erb
@@ -5,7 +5,7 @@
 <% @messages.each do |message| %>
   <tr>
     <td>
-      <b><%= message.incoming? ? 'You' : message.sender.name %></b>
+      <b><%= message.incoming? ? 'You' : message.sender.available_name %></b>
     </td>
   </tr>
   <tr>
diff --git a/spec/mailers/conversation_reply_mailer_spec.rb b/spec/mailers/conversation_reply_mailer_spec.rb
index 915232231..64e08136f 100644
--- a/spec/mailers/conversation_reply_mailer_spec.rb
+++ b/spec/mailers/conversation_reply_mailer_spec.rb
@@ -108,13 +108,13 @@ RSpec.describe ConversationReplyMailer, type: :mailer do
 
       it 'sets reply to email to be based on the domain' do
         reply_to_email = "reply+#{message.conversation.uuid}@#{conversation.account.domain}"
-        reply_to = "#{agent.name} <#{reply_to_email}>"
+        reply_to = "#{agent.available_name} <#{reply_to_email}>"
         expect(mail['REPLY-TO'].value).to eq(reply_to)
         expect(mail.reply_to).to eq([reply_to_email])
       end
 
       it 'sets the from email to be the support email' do
-        expect(mail['FROM'].value).to eq("#{agent.name} <#{conversation.account.support_email}>")
+        expect(mail['FROM'].value).to eq("#{agent.available_name} <#{conversation.account.support_email}>")
         expect(mail.from).to eq([conversation.account.support_email])
       end
 
diff --git a/spec/models/conversation_spec.rb b/spec/models/conversation_spec.rb
index 66d8077ca..b1e298b3d 100644
--- a/spec/models/conversation_spec.rb
+++ b/spec/models/conversation_spec.rb
@@ -88,8 +88,8 @@ RSpec.describe Conversation, type: :model do
 
     it 'creates conversation activities' do
       # create_activity
-      expect(conversation.messages.pluck(:content)).to include("Conversation was marked resolved by #{old_assignee.name}")
-      expect(conversation.messages.pluck(:content)).to include("Assigned to #{new_assignee.name} by #{old_assignee.name}")
+      expect(conversation.messages.pluck(:content)).to include("Conversation was marked resolved by #{old_assignee.available_name}")
+      expect(conversation.messages.pluck(:content)).to include("Assigned to #{new_assignee.available_name} by #{old_assignee.available_name}")
     end
   end