-
Notifications
You must be signed in to change notification settings - Fork 10.9k
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
[NEW] Send LiveChat visitor navigation history as messages #10091
[NEW] Send LiveChat visitor navigation history as messages #10091
Conversation
…navigaton-history-as-a-message
- Created a setting to manage it (enabled by default) - Created a migration to move navigation historys to messages collection
msgStream.on(roomId, { token: this.getToken() }, (msg) => { | ||
if (msg.t === 'command') { | ||
Commands[msg.msg] && Commands[msg.msg](); | ||
} else if (msg.t !== 'livechat_video_call') { | ||
} else if (!_.contains(msgTypesNotDisplayed, msg.t)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to use underscore
here, you can use includes
@@ -311,9 +311,31 @@ RocketChat.Livechat = { | |||
}); | |||
}, | |||
|
|||
savePageHistory(token, pageInfo) { | |||
savePageHistory(token, roomId, pageInfo) { |
There was a problem hiding this comment.
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 receive roomId
here.. we can rely on RocketChat.models.Rooms.findOneOpenByVisitorToken
to get visitor's current room.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sampaiodiego, the RocketChat.models.Rooms.findOneOpenByVisitorToken
has a roomId
parameter, due to this I changed the savePageHistory
to get this parameter.
How can I call the RocketChat.models.Rooms.findOneOpenByVisitorToken
method if I don't pass the roomId
parameter?
I'm sorry if I'm making a mistake but I think this parameter is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're correct @renatobecker .. I didn't notice RocketChat.models.Rooms.findOneOpenByVisitorToken
has a roomId
parameter as well. 😉
I'll give this PR a whole review to see how we could still save visitor's navigation history without a room.
@@ -7,9 +7,6 @@ Meteor.methods({ | |||
department | |||
}); | |||
|
|||
// update visited page history to not expire | |||
RocketChat.models.LivechatPageVisited.keepHistoryForToken(token); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should keep this behavior. the idea is that page history of unregistered visitors can be deleted (expired), because we track visitor's navigation from the very first interaction.. but if that visitor sends a message then we want to keep his page history "forever"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a problem here.. I tried to keep this behavior, but currently we are sending and storing the navigation history as a message and we have a relation between message and room.
The point is that the method RocketChat.models.Messages.createWithTypeRoomIdMessageAndUser
calls another method RocketChat.models.Rooms.incMsgCountById(room._id, 1)
and a valid room is required.
So, because of this we can't store messages without a room using this method.
If you could give me an orientation about this it would be nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll give this another look, but once we merge #9903 we will not call incMsgCountById
anymore.. so we might be able to keep this behavior..
edit: this is not true as well 😞 I've made a confusion with incMsgCountById
method. it needs to be there.. what I think we need to do is to create a new method to insert navigation histories and do not call incMsgCountById
there..
if (room && room.v && room.v.token) { | ||
return LivechatPageVisited.find({ token: room.v.token }, { sort: { ts: -1 } }); | ||
if (room) { | ||
return LivechatMessage.find({rid: room._id, t: 'livechat_navigation_history', $or: [{ _shared: { $exists: false }}, { _shared: true }]}, { sort: { ts: -1 } }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you use this _shared
filter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OMG! It's a good question...
As you could see, I made a mistake.. Actually, I was trying to deal with the _hidden
property and not the _shared
property, but I just see that it is not necessary because I'm filtering the messages type(t: 'livechat_navigation_history'
) on the subscribe.
I made this mess because we have a setting to activate this feature and when the feature is not enabled, the messages are still sending, but with the _hidden
property defined. So, I thought that if I didn't force the filter, the messages would not be received on the client side.
Anyway, I just pushed a new commit fixing this mistake.
Thanks!
// It's necessary to check the room due to the incMsgCountById method inside RocketChat.models.Rooms.findOneOpenByVisitorToken | ||
const room = RocketChat.models.Rooms.findOneOpenByVisitorToken(token, roomId); | ||
if (!room) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to keep saving navigation history even if there is no room
yet, so we cannot abort here
extraData._hidden = true; | ||
} | ||
|
||
return RocketChat.models.Messages.createWithTypeRoomIdMessageAndUser('livechat_navigation_history', room._id, `${ TAPi18n.__('New_visitor_navigation') }: ${ pageTitle } - ${ pageUrl }`, user, extraData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since this method requires a room, we'll need to create another one that still saves the message even with no room.
@@ -7,9 +7,6 @@ Meteor.methods({ | |||
department | |||
}); | |||
|
|||
// update visited page history to not expire | |||
RocketChat.models.LivechatPageVisited.keepHistoryForToken(token); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to keep this behavior, as stated before, we'll have to use a new method to store navigations histories as messages. this new method will use RocketChat.models.Rooms.incMsgCountById
only if there is a room opened. please take a special look at this, we'll need an index like that on the messages
collection as well.
@@ -5,9 +5,6 @@ Meteor.methods({ | |||
department | |||
}); | |||
|
|||
// update visited page history to not expire | |||
RocketChat.models.LivechatPageVisited.keepHistoryForToken(token); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remember to add this back too =)
*/ | ||
class LivechatMessage extends RocketChat.models._Base { | ||
constructor() { | ||
super('rocketchat_message'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is working as expected, it is creating a collection called rocketchat_rocketchat_message
. Also looks like RocketChat.models.LivechatMessage
is not used anywhere.
server/startup/migrations/v108.js
Outdated
@@ -0,0 +1,45 @@ | |||
RocketChat.Migrations.add({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this migration can be huge, so I think we need to make it asynchronous migration 106
6fdebef
to
b2535ea
Compare
@RocketChat/core
Closes #9853
The purpose of this PR is to send the navigation history of the Livechat visitor as a message.
Using this feature, the agent that is attending the chat will be notified each time the visitor's navigation is changed.
So, the informations about the navigation will be displayed as a message into the LiveChat room, but the visitor will not be notified.
The default value of the setting is
false
.Closes #9874
New feature added to send messages related to Livechat visitor navigation history in CRM integrations.
When this feature is enabled, the stored messages containing the visitor navigation history will be
sent to the CRM along with other requests.
The default value of the setting is
false
.