-
Notifications
You must be signed in to change notification settings - Fork 71
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
Added Chat Barge to Supervisor Barge Coach Feature #521
base: main
Are you sure you want to change the base?
Conversation
Thanks for working on this! It's looking pretty good! Here's some feedback from my testing out the feature. I'll let others do the code review before I look to add any input there 😃
|
…when a supervisor attempts to barge in (logic around if they are already a member of the conversation or not)
Great catches @dremin . I've updated the code based on your feedback/suggestions. A few additions:
|
0 ESLint error(s) and 2 ESLint warning(s) found in pull request changed files. |
Nice updates @aestellwag!
This brings a new problem--if I am an interaction participant for this conversation (i.e. I have a task for it), I should not have access to the Leave button. Clicking it keeps the task but then you can't see the conversation from the agent view. Also, I noticed this part uses a serverless function, but doesn't Flex already have a full participant map in redux once the view loads? Or was it missing something needed for this? |
Enhancements added in the latest release:
|
I've enhanced the Conversation Transfer participant tab/list to include the barged user. Prior to this enhancement it would only show Flex Interaction Participants (which the barged participant is not). IE it's showed there were 3 participants, but only list 2 in the list of participant details (as it was only checking the Interaction Flex Participants). |
Sorry, found another edge case to handle! If I am barged, and the agent tries to transfer the task to me, I cannot accept the task because I am already a participant. A few thoughts on how we might be able to handle this situation:
|
…ow all participants
…that is barged + fixed a render bug around participant count
Great catches @dremin , here are the changes made on this commit:
Please let me know if you see anything else, it's been fun squashing these bugs/adding safeguards for these edge cases (hence why I'm up at 10pm writing code, haha) |
Some more issues I noticed testing the latest changes:
|
…ssue after transfer was a timing issue and requires you "refresh" or relaunch the task. I was able to reproduce and fix each of the issues you identified.
Great catch, took me some time to get back to patching this but with the latest merge I was no longer able to reproduce the bugs you surfaced (I was able to originally reproduce them). I found another bug in there as I was testing around the Leave/Join redux state not cleaning up properly when a supervisor was added. Give this a test and let me know if any of you see any other bugs. |
Yay!!
|
const replaceEndTaskButton = (task: ITask) => { | ||
if (TaskHelper.isCBMTask(task) && task.taskStatus === 'assigned') { | ||
const interactionParticipants = | ||
props?.store?.getState()?.[reduxNamespace]?.supervisorBargeCoach?.interactionParticipants; | ||
// more than two participants or are there any active invites? | ||
const conversationState = StateHelper.getConversationStateForTask(task); | ||
if ( | ||
conversationState && | ||
(conversationState.participants.size > 2 || countOfOutstandingInvitesForConversation(conversationState)) | ||
(interactionParticipants > 2 || countOfOutstandingInvitesForConversation(conversationState)) |
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 would break functionality if the supervisor-barge-coach feature is removed, but not the conversation-transfer feature. This will need to be implemented a different way to not introduce a co-dependency of features.
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 could maybe approach it with the following
-
Add a config value in conversation-transfer that checks whether supervisor barge coach is enabled or not (similar example is in custom-transfer-directory with its dependency on conversation-transfer)
-
Use that config value here to determine whether to extract the participants count from redux or use the old method
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.
It looks like this piece of state is only set from the conversation-transfer feature, and only read by the conversation-transfer feature, so we shouldn't need multiple implementations. This piece of state can probably just live within the conversation-transfer feature, but after I've looked into things some more, I don't think we need to keep this state at all!
I think we should remove useParticipantCountEffect
and instead fetch the participant list using existing data in Redux, which seems to be up-to-date and includes supervisor participants. Its location in Redux is flex.chat.conversations[conversationSid].participants
. Using this should be a lot more efficient, as the current code makes 8 or so network requests when selecting the task.
Summary
I've added a chat barge feature to the Supervisor Barge Coach feature. This simply adds a "Join" button to the Monitor Task canvas when monitoring an active conversation. I've added enhancements to state management as I could imagine supervisors may join multiple chats at the same time using this new feature.
The button will also remove them from there, it changes to "Leave", simply click this to leave the conversation when done.
Open to feedback on the UI experience and if you catch any bugs just holler at me.
https://github.com/twilio-professional-services/flex-project-template/blob/chatbarge/docs/static/img/features/supervisor-barge-coach/Supervisor-Barge-Coach-Plugin-10.gif
Checklist