Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMPROVE][Omnichannel] More info and better design of Past Chats List #17346

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
7ac2a47
Add all the features
nitinkumartiwari Apr 17, 2020
428c047
css improved
nitinkumartiwari Apr 17, 2020
cf71487
changed some css
nitinkumartiwari Apr 17, 2020
759eaff
Merge branch 'develop' into refactor-past-chat-history
renatobecker Apr 18, 2020
faaa0e8
Merge branch 'develop' into refactor-past-chat-history
renatobecker Apr 21, 2020
1dd9fae
Code and CSS Improvement
nitinkumartiwari Apr 22, 2020
19d6184
Code and CSS Improvement
nitinkumartiwari Apr 22, 2020
2650e56
Merge branch 'refactor-past-chat-history' of https://github.com/nitin…
nitinkumartiwari Apr 22, 2020
8388d6e
some changes
nitinkumartiwari Apr 22, 2020
6ec867f
changes made
nitinkumartiwari Apr 22, 2020
65d03ae
Api modified
nitinkumartiwari Apr 27, 2020
45ee0dd
css changes with Api modification
nitinkumartiwari Apr 27, 2020
4a7e476
chnages made on 28April
nitinkumartiwari Apr 28, 2020
2996e42
changes made on 1may
nitinkumartiwari May 1, 2020
63ee375
changes made on 6th may
nitinkumartiwari May 6, 2020
1450021
css chang
nitinkumartiwari May 6, 2020
866130c
css chang
nitinkumartiwari May 6, 2020
ff89afc
css chang
nitinkumartiwari May 6, 2020
74e51a6
css chang
nitinkumartiwari May 6, 2020
efee120
css chang
nitinkumartiwari May 6, 2020
3cf7fa9
css chang
nitinkumartiwari May 6, 2020
845b7e9
css chang
nitinkumartiwari May 6, 2020
36e5ad7
changes made on 7th may
nitinkumartiwari May 7, 2020
57d6457
Merge branch 'develop' into refactor-past-chat-history
renatobecker Jun 1, 2020
587cd3d
Update visitors.js
renatobecker Jun 1, 2020
048c1d4
Fix indentations.
renatobecker Jun 1, 2020
6e14f32
Fix css/style issues.
renatobecker Jun 2, 2020
7db6ce0
Merge branch 'develop' into refactor-past-chat-history
renatobecker Jun 2, 2020
685a770
Merge branch 'develop' into refactor-past-chat-history
renatobecker Jun 2, 2020
7317eac
Merge branch 'develop' into refactor-past-chat-history
renatobecker Jun 15, 2020
12111e1
Refactored the codebase.
renatobecker Jun 16, 2020
9536f15
Fix unused imports.
renatobecker Jun 16, 2020
c0990b2
Fix CSS file.
renatobecker Jun 16, 2020
9607da0
Merge branch 'develop' into refactor-past-chat-history
renatobecker Jun 16, 2020
b5ed51a
Add new query to fetch chat history.
renatobecker Jun 17, 2020
1e98343
Remove unnecessary console log.
renatobecker Jun 17, 2020
0e4bc40
UI-UX improvements.
renatobecker Jun 18, 2020
127132e
Fix Css stuff.
renatobecker Jun 18, 2020
bba74e3
Fix fetch data.
renatobecker Jun 18, 2020
0dacdf9
Merge branch 'develop' into refactor-past-chat-history
renatobecker Jun 18, 2020
9d2e42e
Remove unnecessary helper method.
renatobecker Jun 18, 2020
a6d8ef4
Final version.
renatobecker Jun 19, 2020
5d28d35
Fix empty-line missing.
renatobecker Jun 19, 2020
26a5c98
Additional improvements.
renatobecker Jun 19, 2020
0632ee2
Merge branch 'develop' into refactor-past-chat-history
renatobecker Jun 19, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/livechat/client/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ TabBar.addButton({
id: 'visitor-history',
i18nTitle: 'Past_Chats',
icon: 'clock',
template: 'visitorHistory',
template: 'customerChatHistory',
order: 11,
});

Expand Down
89 changes: 89 additions & 0 deletions app/livechat/client/views/app/tabbar/customerChatHistory.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<template name='customerChatHistory'>
<div class="content">
<div class="list-view">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's check if the customer already has chat history, if not, let's display a message.

Suggested change
<div class="list-view">
{{#if chatHistoryEmpty}}
{{_ "Customer_has_no_chat_history"}}
{{else}}
<div class="list-view">

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added a check if there is chat history available or not

<div>
<div class="active-pink-4 mb-4 inner-addon right-addon">
<i class="glyphicon glyphicon-search icon-search"></i>
<input type="text" id="searchInput" class="" placeholder="Search" aria-label="Search">
</div>
</div>
Copy link
Contributor

@renatobecker renatobecker Apr 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the indentation.
Your indentation has both spaces and tabs. We use tabs for the indentation code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

{{#if isAllChat}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this {{#if isAllChat}}?

<div class="vistior-history-block">

<ul id='allist'>
{{#each previousChats}}
<li class='list-chat' id='{{_id}}' aria='{{v.token}}'>

<div class="history-div">
<div class="visitor-img">
<img src="/avatar/{{responseBy.username}}" alt="">
</div>
<div class="visitor-history-details">
<div class="visitor-details">
<h1>{{responseBy.username}}</h1>
{{#if open}}
<p class='open'>Open</p>
{{else}}
<p>Close at time {{time}}</p>
{{/if}}
</div>
<div class="total-messages">
<p>{{count}} messages</p>
</div>
<div class="agent-comment">
<p class="comment-heading">Agent comment:</p>
<p class="comment-text">"{{lastMessage.msg}}"</p>
</div>

</div>
</div>

</li>
{{/each}}
</ul>
</div>
{{else}}
{{#if isChatClicked}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can move this to the top of the template, don't need any if/else statement.

{{> customerChatHistoryMessages}}
{{else}}
<div class='visitor-history-block'>
{{#if isfound }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this {#if isfound }}?

<ul class='searchresults' id='searchresul'>

{{#each searchResults}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whi do you need another list to display the search results? You can use only one list and apply the filters through the same endpoint you fetch the data from the server.


<li class='search-li'>
<div class="history-main">
<div class="history-img">
<img src="/avatar/{{u.username}}" >
</div>
<div class="history-detail-main">
<div class="detail-main-upper">
<h1>{{u.name}}</h1>
<p>{{time}}</p>
</div>
<div class="detail-main-lower">
<p>{{msg}}</p>
</div>
</div>
</div>

</li>
{{/each}}

</ul>
{{else}}
<h1 class='noresult'>No result found</h1>
{{/if}}
</div>

{{/if}}

{{/if}}

</div>
</div>



</template>
230 changes: 230 additions & 0 deletions app/livechat/client/views/app/tabbar/customerChatHistory.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
import { ReactiveVar } from 'meteor/reactive-var';
import { Template } from 'meteor/templating';
import moment from 'moment';
import _ from 'underscore';
import './customerChatHistory.html';
import { APIClient, t } from '../../../../../utils/client';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

't' is defined but never used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

import { AccountBox, TabBar, MessageTypes } from '../../../../../ui-utils';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot of unused imports, please remove all of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only required packags now imported others removed

import {Mongo ,MongoInternals} from 'meteor/mongo';
import { Tracker } from 'meteor/tracker';
import { ReactiveDict } from 'meteor/reactive-dict';
import { Session } from 'meteor/session';

const ITEMS_COUNT = 50;
let historyResult
let len, msgs
let allHistoryChat ;

Template.customerChatHistory.helpers({

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove all of the blank lines like this.

isAllChat:function(){
// will return is have to load all chat
return Template.instance().isAllChat.get();
},
isChatClicked:function(){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this 'function' here? You don't need it.

Suggested change
isChatClicked:function(){
isChatClicked() {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the function

// will return that if you have clicked in a single chatHistory
return Template.instance().isChatClicked.get();
},
isfound:function() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i need it to load the template chatHistoryMessages when user will click on a room

// will return if find any search result
var isfound= Session.get('found');

return isfound;
},
isLoading() {
return Template.instance().isLoading.get();
},
searchResults(){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have removed it

// will return search result
var r = Session.get('searchResult');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this?
Also, you don't use var, we use let. In addition, when you assign a value to a variable that will not be reassigned, you can use const.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

return Template.instance().searchResult.get();
},

previousChats() {
// will return pervious chats list
let history = Template.instance().history.get();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let history = Template.instance().history.get();
return Template.instance().history.get();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

let newHisTory = []
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need any of this code between the lines 46 ~ 58.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i need this because of i am adding a time property inside the message object then pushing into a new array

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can create a helper method to return the history item, as I mentioned above(customerChatHistoryItem) and then you can have a method like:

    getRoom(item) {
          // do what you need here and return...
    }

for(i=1; i<history.length; i++){
var d = Date.parse(history[i].ts);
var D = new Date();
D.setTime(d);
var newTs = D.toTimeString();
var time = getTime(newTs);
history[i].time = time;
history[i].count = history[i].msgs-3;
newHisTory.push(history[i]);
}
Template.instance().modifiedHistory.set(newHisTory);
return Template.instance().modifiedHistory.get();
},
len(){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this.

// will return length of total messages in room
len = Template.instance().historyResu.get();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this, the room object provides the total messages count.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this

return len = len.length-2;
},
title() {

let title = moment(this.ts).format('L LTS');

if (this.label) {
title += ` - ${ this.label }`;
}
return title;
},
});

Template.customerChatHistory.onCreated(function() {
const currentData = Template.currentData();
this.visitorId = new ReactiveVar();
this.isLoading = new ReactiveVar(false);
this.modifiedHistory = new ReactiveVar([])
this.history = new ReactiveVar([]);
this.offset = new ReactiveVar(0);
this.total = new ReactiveVar(0);
this.isAllChat = new ReactiveVar(true);
this.isChatClicked = new ReactiveVar(true);
this.autorun(async () => {
const { room } = await APIClient.v1.get(`rooms.info?roomId=${ currentData.rid }`);
if (room && room.v) {
this.visitorId.set(room.v._id);
}
});

this.autorun(async () => {

if (!this.visitorId.get() || !currentData || !currentData.rid) {
return;
}

const offset = this.offset.get();

this.isLoading.set(true);
const { history, total } = await APIClient.v1.get(`livechat/visitors.chatHistory/room/${ currentData.rid }/visitor/${ this.visitorId.get() }?count=${ ITEMS_COUNT }&offset=${ offset }`);

this.isLoading.set(false);
this.total.set(total);
this.history.set(this.history.get().concat(history));
// this will load all the chat history chat messages
allHistoryChat = [];
Copy link
Contributor

@renatobecker renatobecker Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this code? You don't need to fetch all of the chat histories once, you can rely on the pagination and you'll fetch more rooms on the scroll event until you identify that you fetched all of the chat histories.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

var limit = 100;
let count = this.history.get();
if(allHistoryChat.length>0){
return
}
for(i=0;i<count.length;i++){
let id = count[i]._id;
let token = count[i].v.token;
setTimeout(async function(){
const historyResult = await APIClient.v1.get(`livechat/messages.history/${ id }?token=${token}&limit=${limit}`);
let count2 = historyResult.messages;

for(j=0; j<count2.length; j++){

allHistoryChat.push(count2[j]);

}

},1000)

}


});
});

Template.customerChatHistory.events({
'scroll .visitor-scroll': _.throttle(function(e, instance) {
if (e.target.scrollTop >= (e.target.scrollHeight - e.target.clientHeight)) {
const history = instance.history.get();
if (instance.total.get() <= history.length) {
return;
}
return instance.offset.set(instance.offset.get() + ITEMS_COUNT);
}
}, 200),
'keyup #searchInput': async function(event,template){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to filter the rooms on the server-side which means you need to have a specific method to load the customer history chats and here you will use the same method, but you will pass the search term to the server and get the results.

Also, you need to use debounce to filter the chat history. We have a very similar use case here:
https://github.com/RocketChat/Rocket.Chat/blob/develop/app/livechat/client/views/app/livechatDepartments.js#L81

Please, take a look at that code and use it as inspiration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now i am using the search api to search the message inside a room

Copy link
Contributor

@renatobecker renatobecker Apr 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are performing a loop of each customer history room, this is not a good practice.
As I mentioned before, you need to call the same endpoint -> livechat/visitors.chatHistory when your searching or not.
There is a lot of mess here, this implementation needs to be simpler, not so complex.

It's simple:

  • You'll always use the same endpoint: livechat/visitors.chatHistory/. What you need to do is to make the endpoint supports a searchTerm parameter, like chat.search does. So, when you are searching, you will pass this parameter to the endpoint, when you're NOT searching, you'll call the same method but without the searchTerm.
    Did you get it?


searchResults = [];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of the code below is not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have done some changes please check

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned, all of this code below is unnecessary.
You need to create a loadRooms method that supports an optional parameter: searchTerm.
The method will call the livechat/visitors.chatHistory/ endpoint. When you are searching, you will call loadRooms(searchTerm).

Is not much more simple?
I shared with you an example on how to implement this, why not just follow this instruction?

There is no reason for this loop, well you call the endpoint, searching or not, then you'll get the rooms you need to render.
If you need to change the object before rendering, you can create a helper method to do it, the this will be the object you pass to the chatRoomHistoryItem template.

let text = event.target.value;
template.isChatClicked.set(false);

Session.set('found',false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this? I didn't get yet why you're using sessions inside the templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now i am not using session anywhere

if(event.target.value == ''){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this if/else statement.
Improve the loadRooms method following my previous suggestions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have performed check on loadRoom method and removed it from here

template.isAllChat.set(true);
}else{
template.isAllChat.set(false);
Template.instance().searchResult = new ReactiveVar([]);

for(var i=0; i<allHistoryChat.length;i++){
if(allHistoryChat[i].msg == text){ // check search input matches with any msg
Session.set('found',true);
var d = Date.parse(allHistoryChat[i].ts);
var D = new Date();
D.setTime(d);
var newTs = D.toTimeString();

var time = getTime(newTs);
allHistoryChat[i].time = time;
searchResults.push(allHistoryChat[i]);
Session.set('searchResult',searchResults);
template.searchResult.set(searchResults)

}

}

if(searchResults.length > 0){
Session.set('found',true);

}

}},
'click .list-chat': async function(event,template){
Copy link
Contributor

@renatobecker renatobecker Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you just need to get the roomId will be used to fetch the customer history messages.
My suggestion is to have a property to define when you click on a room in the list, very similar to what you do with the isChatClicked, but you can use a better name.
Also, you can store the roomId of the clicked room in another property and pass this value to the Customer Chat History Messages template.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am getting the Room Id and token and storing into the session and getting from the customer chat history messages

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use ReactiveVars to store this information, not sessions.

event.preventDefault();
template.isAllChat.set(false);
template.isChatClicked.set(true);
let id = event.currentTarget.id
let token = event.currentTarget.attributes.aria.value;
Session.set('FetchID',id);
Session.set('FetchToken',token)
}

});

Template.customerChatHistory.onDestroyed(function(){
var header = document.getElementsByClassName('Contextualheading');
if(header[0]){
header[0].innerText = ''
header[0].className = 'contextual-bar__header-title';
}
})
function getTime(newTs){
Copy link
Contributor

@renatobecker renatobecker Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this method, you can use moment, the lib you imported in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have removed this

var hr = newTs.slice(0,2);
var min = newTs.slice(3,5);
if(hr > 12 && hr < 24){
hr = hr-12;
return time = `${hr}:${min} PM`;
}else if(hr == 24){
hr = 00;
return time = `${hr}:${min} AM`;
}else if(hr < 12 ){
return time = `${hr}:${min} AM`;
}else if(hr == 12){
return time = `${hr}:${min} PM`;
}
}
function getDay(obj){
Copy link
Contributor

@renatobecker renatobecker Apr 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need this method, you can use moment, the lib you imported in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this and using moment now

var d = Date.parse(obj.ts);
var D = new Date();
D.setTime(d);
var day = D.getDay();
switch(day){
case 1 : return 'Monday';
case 2 : return 'Tuesday';
case 3 : return 'Wednesday';
case 4 : return 'Thursday';
case 5 : return 'Friday';
case 6 : return 'Saturday';
case 7 : return 'Sunday';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<template name='customerChatHistoryMessages'>
{{#if isLoading}}
<h2 class='loding'>
{{_ "Loading..."}}
</h2>
{{else}}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove all of the blank lines and fix the indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

<div class="chatHistory">
<div class='msgcount'>

<p><span>{{len}} messages</span></p>

</div>
<ul class='chatHistoryul'>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chatHistoryul ? This is not a good name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed the class i was not using it before


{{#each historyResult}}
{{#unless t}}

<li>
<div class="history-main">
<div class="history-img">
<img src="/avatar/{{u.username}}" >
</div>
<div class="history-detail-main">
<div class="detail-main-upper">
<h1>{{u.name}}</h1>
<p>{{time}}</p>
</div>
<div class="detail-main-lower">
<p>{{msg}}</p>
</div>
</div>
</div>
</li>
{{else}}
{{#if u.name}}
<div class="closed-div">
<p class='closed-div-p'><span>Conversation closed</span></p>
<div class="history-main">
<div class="history-img">
<div class='belldiv'>
<i class="icon-bell"></i>
</div>
</div>
<div class="history-detail-main">
<div class="detail-main-upper closed-h1">
<h1>Agent comment:</h1>
</div>
<div class="detail-main-lower closed-p">
<p>"{{msg}}".</p>
</div>
</div>
</div>
</div>
{{/if}}
{{/unless}}
{{/each}}
</ul>
</div>

{{/if}}
</template>
Loading