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

Flex Insights friendly changes #32

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

fplacido
Copy link

Hi team,
I am proposing these two changes to link the callback and outbound tasks to the original conversation.
This will help creating custom metrics in Flex Insights.

  • [ x ] I acknowledge that all my contributions will be made under the project's license.

Insights friendly:
Added line 39 to link the new task to the original, setting the conversation_id
Insights friendly: added lines 53 to link the callback task to the original task
@vernig
Copy link
Contributor

vernig commented Aug 24, 2021

Thank you so much @fplacido!! That's a great addition.

Just to bring everybody up to speed on our conversation, this will allow the callback / voicemail task to be associated to the same conversation as the original task in Flex Insight:

image

I'll do a proper review and some more tests in the next few days and let you know. @bermudezmt @ktalebian @rnairtwilio fell free to pitch in on this.

@vernig vernig self-requested a review August 24, 2021 10:49
@vernig vernig self-assigned this Aug 24, 2021
@vernig
Copy link
Contributor

vernig commented Aug 26, 2021

Hi @fplacido the PR looks good to me. Just one question: I can see you are adding conversation_sid only to callback and not to voice mail. Is that a design choice?

Also, the feature doesn't seem to be a breaking change, but I can see a scenario where the customer may not want to link the two events through the same conversations id. So I was thinking we can add it as an option in options.private.js. What do you think?

Copy link
Contributor

@vernig vernig left a comment

Choose a reason for hiding this comment

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

@fplacido as discussed, let work on:

  • Adding conversation_id to voice mail scenario
  • Add an option to enable this feature. The option will be disabled by default to avoid breaking changes for customers that already deployed this solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants