Skip to content

Conversation

@MarcosSpessatto
Copy link
Contributor

No description provided.

@MarcosSpessatto MarcosSpessatto marked this pull request as ready for review August 27, 2019 16:03
@sampaiodiego sampaiodiego added this to the 2.1.0 milestone Sep 13, 2019
},

saveTransferHistory(room, transferData) {
const transferedToDepartment = Boolean(transferData.departmentId);

Choose a reason for hiding this comment

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

I would use destructuring assignment here:

Suggested change
const transferedToDepartment = Boolean(transferData.departmentId);
const { departmentId: previousDepartment, servedBy: { _id: previousAgent } = {} } = room;
const { departmentId: nextDepartment, userId: nextAgent, transferedBy } = transferData;
const transferedToDepartment = Boolean(transferData.departmentId);

@engelgabriel engelgabriel modified the milestones: 2.1.0, 2.2.0 Oct 13, 2019
@rodrigok rodrigok modified the milestones: 2.2.0, 2.3.0 Oct 24, 2019
Copy link

@renatobecker-zz renatobecker-zz left a comment

Choose a reason for hiding this comment

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

Just a few typo fixes.

const transferData = {
roomId,
departmentId,
transferedBy: visitor._id,

Choose a reason for hiding this comment

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

Suggested change
transferedBy: visitor._id,
transferredBy: visitor._id,

scope: nextDepartment ? 'department' : 'agent',
...previousDepartment && { previousDepartment },
...nextDepartment && { nextDepartment },
...transferedTo && { transferedTo },

Choose a reason for hiding this comment

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

Suggested change
...transferedTo && { transferedTo },
...transferredTo && { transferredTo },


const guest = LivechatVisitors.findOneById(room.v && room.v._id);
const user = Meteor.user();
transferData.transferedBy = { _id: user._id, username: user.username };
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
transferData.transferedBy = { _id: user._id, username: user.username };
const { _id, username } = user || {};
transferData.transferredBy = { _id, username };


saveTransferHistory(room, transferData) {
const { departmentId: previousDepartment } = room;
const { departmentId: nextDepartment, transferedBy, transferedTo } = transferData;

Choose a reason for hiding this comment

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

Suggested change
const { departmentId: nextDepartment, transferedBy, transferedTo } = transferData;
const { departmentId: nextDepartment, transferredBy, transferedTo } = transferData;

Messages.keepHistoryForToken(token);

if (!Promise.await(Livechat.transfer(room, guest, { roomId: rid, departmentId: department }))) {
const transferedBy = { _id: room.servedBy._id, username: room.servedBy.username };

Choose a reason for hiding this comment

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

Through this endpoint, the transfer process is requested from the visitor, not the current room agent.

const transferData = {
roomId,
departmentId,
transferedBy: visitor._id,

Choose a reason for hiding this comment

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

the transferedBy prop is an object, right?

const update = {
$inc: { availableTime },
$set: {
lastStopedAt,

Choose a reason for hiding this comment

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

Suggested change
lastStopedAt,
lastStoppedAt,

return this.update(query, update);
}

updateServiceHistory({ agentId, date, historyObject }) {

Choose a reason for hiding this comment

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

Suggested change
updateServiceHistory({ agentId, date, historyObject }) {
updateServiceHistory({ agentId, date, serviceHistory }) {

};
const update = {
$addToSet: {
serviceHistory: historyObject,

Choose a reason for hiding this comment

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

Suggested change
serviceHistory: historyObject,
serviceHistory,

return this.remove(query);
}

updateTransferHistoryByRoomId(roomId, transfer) {

Choose a reason for hiding this comment

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

Suggested change
updateTransferHistoryByRoomId(roomId, transfer) {
updateTransferHistoryByRoomId(roomId, transferHistory) {

};
const update = {
$addToSet: {
transferHistory: transfer,

Choose a reason for hiding this comment

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

Suggested change
transferHistory: transfer,
transferHistory,

Copy link

@renatobecker-zz renatobecker-zz left a comment

Choose a reason for hiding this comment

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

We need to define when the user who is transferring is the visitor.

import { API } from '../../../../api';
import { findGuest, findRoom, getRoom, settings, findAgent } from '../lib/livechat';
import { Livechat } from '../../lib/Livechat';
import { getTransferredData } from '../../lib/Helper';
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
import { getTransferredData } from '../../lib/Helper';
import { normalizeTransferredByData } from '../../lib/Helper';

Messages.keepHistoryForToken(token);

if (!Promise.await(Livechat.transfer(room, guest, { roomId: rid, departmentId: department }))) {
const transferredBy = getTransferredData(guest, room);
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
const transferredBy = getTransferredData(guest, room);
const data = { _id, username, type: 'visitor' } = guest;
const transferredBy = normalizeTransferredByData(data, room);

return true;
};

export const getTransferredData = (transferredBy, room) => {
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
export const getTransferredData = (transferredBy, room) => {
export const normalizeTransferredByData = (transferredBy, room) => {

if (!transferredBy || !room) {
throw new Error('You must provide "transferredBy" and "room" params to "getTransferredByData"');
}
return {

Choose a reason for hiding this comment

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

Suggested change
return {
const { servedBy: { _id: agentId } = {} } = room;
const { _id, username, type: transferType } = transferredBy;
const type = transferType || (_id === agentId ? 'agent' : 'user');
return {
_id,
username,
type,


import { LivechatRooms, Messages, LivechatVisitors } from '../../../models';
import { Livechat } from '../lib/Livechat';
import { getTransferredData } from '../lib/Helper';
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
import { getTransferredData } from '../lib/Helper';
import { normalizeTransferredByData } from '../lib/Helper';

const transferData = {
roomId,
departmentId,
transferredBy: getTransferredData(visitor, room),
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
transferredBy: getTransferredData(visitor, room),
transferredBy: normalizeTransferredByData(data, room),

import { LivechatRooms, Subscriptions, LivechatVisitors } from '../../../models';
import { LivechatRooms, Subscriptions, LivechatVisitors, Users } from '../../../models';
import { Livechat } from '../lib/Livechat';
import { getTransferredData } from '../lib/Helper';
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
import { getTransferredData } from '../lib/Helper';
import { normalizeTransferredByData } from '../lib/Helper';

}

const guest = LivechatVisitors.findOneById(room.v && room.v._id);
transferData.transferredBy = getTransferredData(Meteor.user() || {}, room);
Copy link

@renatobecker-zz renatobecker-zz Nov 7, 2019

Choose a reason for hiding this comment

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

Suggested change
transferData.transferredBy = getTransferredData(Meteor.user() || {}, room);
transferData.transferredBy = normalizeTransferredByData(Meteor.user() || {}, room);

status,
});
});
callbacks.runAsync('livechat.agentStatusChanged', { userId, status });
Copy link

@renatobecker-zz renatobecker-zz Nov 8, 2019

Choose a reason for hiding this comment

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

This callback doesn't depend on the 'Livechat_show_agent_info setting, so I suggest to run the callback at the beginning of the method, before testing the Livechat_show_agent_info setting.

if (!result) {
return false;
}
this.saveTransferHistory(room, transferData);

Choose a reason for hiding this comment

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

Suggested change
this.saveTransferHistory(room, transferData);
return this.saveTransferHistory(room, transferData);

...nextDepartment && { nextDepartment },
...transferredTo && { transferredTo },
};
LivechatRooms.updateTransferHistoryByRoomId(room._id, transfer);

Choose a reason for hiding this comment

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

Suggested change
LivechatRooms.updateTransferHistoryByRoomId(room._id, transfer);
return LivechatRooms.updateTransferHistoryByRoomId(room._id, transfer);

}

start() {
this._setupListeners();

Choose a reason for hiding this comment

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

Would not it be good to have a property to store when the monitor starts? Otherwise, in case the start() is called again, new callbacks will be assigned and duplicated, right?

@lgtm-com
Copy link

lgtm-com bot commented Nov 11, 2019

This pull request introduces 4 alerts when merging 0aa93f0 into a8ad42b - view on LGTM.com

new alerts:

  • 4 for Identical operands

@lgtm-com
Copy link

lgtm-com bot commented Nov 11, 2019

This pull request introduces 2 alerts when merging 2421b94 into a8ad42b - view on LGTM.com

new alerts:

  • 2 for Identical operands

this.orch.getConverters().get('rooms').convertAppRoom(currentRoom),
this.orch.getConverters().get('visitors').convertAppVisitor(visitor),
{ userId: targetAgent.id, departmentId }
{ userId: targetAgent.id, department: LivechatDepartment.findOneById(departmentId) },

Choose a reason for hiding this comment

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

You don't need to read the model here, you can do it inside the Livechat.transfer.

if (!Promise.await(Livechat.transfer(room, guest, { roomId: rid, departmentId: department }))) {
const { _id, username, name } = guest;
const transferredBy = normalizeTransferredByData({ _id, username, name, userType: 'visitor' }, room);
const department = LivechatDepartment.findOneById(departmentId);

Choose a reason for hiding this comment

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

You don't need to read the model here, you can do it inside the Livechat.transfer.

const user = Users.findOneById(userId);
const { _id, username, name } = user;
const transferredBy = normalizeTransferredByData({ _id, username, name }, room);
this.transfer(room, guest, { roomId: room._id, transferredBy, department: LivechatDepartment.findOneById(guest.department) });

Choose a reason for hiding this comment

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

You don't need to read the model here, you can do it inside the Livechat.transfer.


const guest = LivechatVisitors.findOneById(room.v && room.v._id);
transferData.transferredBy = normalizeTransferredByData(Meteor.user() || {}, room);
transferData.department = LivechatDepartment.findOneById(transferData.departmentId);

Choose a reason for hiding this comment

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

You don't need to read the model here, you can do it inside the Livechat.transfer.

if (!room.open) {
throw new Meteor.Error('room-closed', 'Room closed', { method: 'livechat:returnAsInquiry' });
}
const transferredBy = normalizeTransferredByData(Meteor.user() || {}, room);

Choose a reason for hiding this comment

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

Since you already pass all the information you need through the Livechat.returnRoomAsInquiry, why not processing the transferData inside the Livechat.returnRoomAsInquiry?
IMO it would reduce the complexity of the algorithm.

return this.remove(query);
}

updateTransferHistoryByRoomId(roomId, transferHistory) {

Choose a reason for hiding this comment

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

We don't need this method anymore.

Copy link

@renatobecker-zz renatobecker-zz left a comment

Choose a reason for hiding this comment

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

This changes may fixe the problem related to the order of the events when forwarding and returning the chat to the queue, what do you think?

}

this.saveTransferHistory(room, transferData);
return RoutingManager.unassignAgent(inquiry, departmentId);

Choose a reason for hiding this comment

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

Suggested change
return RoutingManager.unassignAgent(inquiry, departmentId);
return RoutingManager.unassignAgent(inquiry, departmentId) && this.saveTransferHistory(room, transferData);


async transfer(room, guest, transferData) {
return RoutingManager.transferRoom(room, guest, transferData);
const result = await RoutingManager.transferRoom(room, guest, transferData);

Choose a reason for hiding this comment

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

Suggested change
const result = await RoutingManager.transferRoom(room, guest, transferData);
const result = await RoutingManager.transferRoom(room, guest, transferData) && this.saveTransferHistory(room, transferData);

@renatobecker-zz renatobecker-zz merged commit c68f7b4 into develop Nov 13, 2019
@renatobecker-zz renatobecker-zz deleted the livechat-analytics branch November 13, 2019 01:41
@sampaiodiego sampaiodiego mentioned this pull request Nov 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants