Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Messaging abstraction updates #134
base: master
Are you sure you want to change the base?
Messaging abstraction updates #134
Changes from 3 commits
1558c89
c3f33a1
fedd8c2
39eafdc
e502b2b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 might need to shake up the whole flow to make this happen.
However, you receive bonus points for the use of "OG".
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.
Started implementing this and am getting a bit lost in the separation of everything. It seems to me like there is nothing super Twilio-specific besides how the response is being generated. Do we just need something like
textingService.buildResponse()
for now? Could be misunderstandingThere 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 assume we will have more complex needs as we get deeper into MessageBird flows though
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.
A
buildResponse()
would work for the default "yup I got it" responses, but we might need something else more specific for certain circumstances.In Twilio land, webhooks out to external services are expected to respond back with TwiML, which is just an XML schema used to tell Twilio what to do next. So in some situations, we might return a
reply with message "foobar"
. It's more useful for things like decision trees being handled by external services. But in our cases, often we're simply returning back empty TwiML since there's nothing extra to do.That part is definitely Twilio-specific -- not sure if we'll need something similar in MB land.
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.
Same as above
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.
Same as above
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.
Not sold on TextingService as the name, since it's not really different than MessagingService, but I don't have any better ideas either. Let's circle back on it...
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.
Yeah wasn't sold on it myself either... words are hard
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.
What if we simply used MessagingLogicService for logic and MessagingSegmentService for segment?