pulledover
pulledover copied to clipboard
mattstauffer
Add code to handle Twilio error `21610` (customer has replied with `STOP`)
21610 - SMS cannot be sent to the 'To' number because the customer has replied with STOP
How should the exception be handled?
- Should blacklisted numbers and friends be removed completely or just flagged as blacklisted and appear that way on the dashboard?
- If either of the above happen do you want to notify the user via email?
Started working on this here https://github.com/RDelorier/pulledover/tree/handle-twilio-error-21610
Merged those jobs and got it down to 3 spots that the blacklisted exception can happen, see the latest commit to find those 3 spots.
@RDelorier I like some of the directions you're going there--merging and catching exceptions--and don't like some others--interfaces that we don't need, tons of docBlocks, etc.
However, it's not really up to you to read my brain on that :D In general, you're making some really great progress over there. I'll happily give a lot more feedback once you PR.
In general.. I think if someone STOPs, we need to keep that number, but we need to add a "deactivated" flag to it. Then we need to show that flag in the UI, and MAYBE we need to notify their friend? If the owner's the one that deactivated their own number.. I don't even know.
Additionally, if it says STOP we probably should prohibit that number from being added again.. but i don't know if I care to do all that work. Because my bet is, if someone adds their number later, it'll try to send them the verification text message, and then it'll error out, and our code will handle that.
Side note: If you're not in your fork, we'll want to handle this error on EVERY text we send, not just on verification--they might STOP and then later be sent a legit "your friend was pulled over" text, and we need to handle this exception on that too.
Thanks for your work on this @RDelorier; as always, do what you can/will and pass whatever is left, if anything to me. Thanks!!
@mattstauffer thanks for the feedback. The interface I'll go ahead and take out, I was thinking it would be helpful when merging those jobs but ended up not using it.
Just added the following stuff
- display numbers as blacklisted in ui
- blacklisted error messages on number and friend store
- flagging number/friend as blacklisted if exception is raised in send recording job
I think the error messages on create are going to work not work on production since they are queued... I'm guessing you will want to just do some notifications instead.
I'm going to hand over the reins to you because there are some exec decisions to make on how to handle the exception. It also looks like you're going to want in incoming text route so that you can un-flag an number that removes themselves from the blacklist by responding "START".
Do you want me to pr what I have or do you just want to branch off mine.
Links for quick reference Twilio FAQ - Handling "STOP"
@RDelorier Why don't you PR it to the handle-customer-STOP branch and I'll work from there? Thanks man!
https://github.com/mattstauffer/pulledover/tree/handle-customer-STOP
@mattstauffer I'll put it together now, I've never written a pr for an work in progress so here goes nothing!
in regards to episode 77, there is a reason I chose this particular project its just not as noble as your own
@mattstauffer Can you fast-forward that branch so the pr is smaller. I can rebase if you prefer but there will be some conflicts to deal with later.
@RDelorier uhhh so I JUST got an email about fast-forwarding a branch.. i have no idea whether we resolved this or not. No idea what happened here. Do you still want me to do something?
@mattstauffer Ha I think your notifications are on the fritz. Can you fast forward handle-customer-STOP so I can switch #30 to it. There are still a few things that need to be done before it goes to master.
@RDelorier yep. definitely on the fritz. Just got this.
By fast-forward it, do you mean rebase it onto master so it has the latest commits? Sorry, I'm not familiar with using the term that way. Thanks for your patience!
Ha yea rebase onto master, I've been pretty swamped with our latest deployment so patience is all I have 😉 On Jun 3, 2016 3:33 PM, "Matt Stauffer" [email protected] wrote:
@RDelorier https://github.com/RDelorier yep. definitely on the fritz. Just got this.
By fast-forward it, do you mean rebase it onto master so it has the latest commits? Sorry, I'm not familiar with using the term that way. Thanks for your patience!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mattstauffer/pulledover/issues/26#issuecomment-223672967, or mute the thread https://github.com/notifications/unsubscribe/ACMHe0HX7fjooONciU5o1SpqWPQlZuCbks5qIIGHgaJpZM4Hvk5h .
W00t thanks On Jun 3, 2016 3:45 PM, "Matt Stauffer" [email protected] wrote:
@RDelorier https://github.com/RDelorier done!
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mattstauffer/pulledover/issues/26#issuecomment-223675900, or mute the thread https://github.com/notifications/unsubscribe/ACMHe4VcfTUP3BUcxnmNBxDtXExQWBGeks5qIIQ9gaJpZM4Hvk5h .