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

[FIX] Missing User when forwarding Omnichannel conversations via Apps-Engine #17918

Merged
merged 3 commits into from
Jun 19, 2020
Merged

[FIX] Missing User when forwarding Omnichannel conversations via Apps-Engine #17918

merged 3 commits into from
Jun 19, 2020

Conversation

murtaza98
Copy link
Contributor

Description

This PR attempts to close Issue #17911
Additionally, it adds a missing property transferredBy to the transferData object begin passed on to transfer method 👇

async transfer(room, guest, transferData) {

This is property is required for the following check in saveTransferHistory method.

check(transferredBy, Match.ObjectIncluding({

I'm assigning the app-User to this missing property since this method will always be triggered by an app.

Issue(s)

Closes #17911

Screenshots

Handover Action is being processed properly
image

return Livechat.transfer(
this.orch.getConverters().get('rooms').convertAppRoom(currentRoom),
this.orch.getConverters().get('visitors').convertAppVisitor(visitor),
{ userId: targetAgent.id, departmentId },
{ userId: targetAgent ? targetAgent.id : undefined, departmentId, transferredBy},
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
{ userId: targetAgent ? targetAgent.id : undefined, departmentId, transferredBy},
{ userId: targetAgent ? targetAgent.id : undefined, departmentId, transferredBy },

@renatobecker renatobecker added this to the 3.4.0 milestone Jun 19, 2020
- add transferredBy property to transferData object in transferVisitor()
@@ -134,10 +134,18 @@ export class AppLivechatBridge {
currentRoom,
} = transferData;

const appUser = Users.findOneByAppId(appId);
const transferredBy = {
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
const transferredBy = {
if (!appUser) {
throw new Error('Invalid user, cannot transfer');
}
const { _id, username, name, type } = appUser;
const transferredBy = {
_id,
username,
name,
type,
}

Copy link
Contributor Author

@murtaza98 murtaza98 Jun 19, 2020

Choose a reason for hiding this comment

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

Incorporated this change in the latest commit. REF 997469f
Thanks for the review 😄

@renatobecker renatobecker changed the title [FIX] Apps-Engine transferVisitor() issue (#17911) + add an missing property to TransferData Object [FIX] Missing User when requesting the transfer of omnichannel conversations via Apps-Engine Jun 19, 2020
@renatobecker renatobecker changed the title [FIX] Missing User when requesting the transfer of omnichannel conversations via Apps-Engine [FIX] Missing User when forwarding Omnichannel conversations via Apps-Engine Jun 19, 2020
@rodrigok rodrigok merged commit 567b265 into RocketChat:develop Jun 19, 2020
@sampaiodiego sampaiodiego mentioned this pull request Jun 30, 2020
@murtaza98 murtaza98 deleted the apps-transferVisitor-method branch August 31, 2020 16:28
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.

transferVisitor() method of App.Engine not working without optional param - TargetUserAgent
3 participants