twilio-ivr
twilio-ivr copied to clipboard
add query to twimlFor, backgroundTrigger, and transitionOut
eyyy if this is something you'd like in twilio-ivr, lmk, and i can update your docs.
otherwise, feel free to close
I'm gonna leave this open because I'm not totally opposed to it, at least as a stopgap, but I think you might run into some issues with this approach. I'd say try it out on the hotline repo and update the PR as needed; once you get the hotline migrated to this successfully and work out any kinks, then we can talk about merging it.
Among the current issues I see:
- How can
transitionOut
(andbackgroundTrigger
) add to the query params/session? Does it (they) need to? - This structure seems likely to lead to bugs: if any normal state's
twimlFor
forgets to add the query params to theprocessTransitionUrl
, the chain will break. Maybe at least a helper can be written to aid with that? - The query params need to be passed along here too.
@data-doge fyi, i moved some files around today—looking at the code through your eyes made me realize how badly defined the boundaries were between various subcomponents—so this pr will need some updating. almost none of the logic has changed, though, just which files are where has changed, so the updates should be very easy/mechanical.
@data-doge just saw that the latest tweaks here add a trustProxy
config setting to twilio-ivr. For the sake of merging this PR eventually, I thought I'd say now that I'm not sure that's the best approach.
Part of me is hesistant to add express-configuration logic to twilio-ivr at all; it seems out of scope. I think maybe a preferred solution would be to configure an external app, and then have that app use twilio-ivr? I.e.:
// in user-land code, configure app
const app = express();
app.set('trust-proxy', true);
// add twilio-ivr
app.use(twilioIvr(states, config));
Does that feel too clunky to you? If so, I could also maybe see accepting an serverConfig
setting in twilio-ivr that gets passed along to express, like we used to do here. Adding one-off setting flags doesn't seem scalable though, if we have to add more and more over time or if we want to generalize twilio-ivr to work with different server frameworks (like hapi).
@ethanresnick
yeah, external app sounds like a great idea. feels less prescriptive.