Updates Voice Client Quickstart to support incoming number #196
Conversation
| }; | ||
| } | ||
|
|
||
| async clearCallerId() { |
There was a problem hiding this comment.
why is this function setCallerId, updateIncomingNumber, and useExistingTwimlApp all async? None of them seem to be making any network/async requests
There was a problem hiding this comment.
When I call these from the UI, it is done dynamically. As I am a async/await n00b, do you know what happens if I await a non-async function? I was considering asynchronous behavior as part of the "loose Interface" of an Action (a function that does something on your behalf and returns a dictionary of environment variable name to value).
There was a problem hiding this comment.
since these aren't truly async functions, if you rewrote the method to be a standard function, then you can also remove the need to call it with await.
The pattern makes sense for chooseLogicalCallerId but just because one of the functions uses the asyc/await behavior, that doesn't mean everything needs to have the same interface
There was a problem hiding this comment.
Oh sorry I should've been more clear, I meant it's called generically here:
I can just see what happens if I pop a regular function through...I'm assuming I got burned originally.
| async function getCallerIdStatus(context) { | ||
| const client = context.getTwilioClient(); | ||
| const callerId = process.env.CALLER_ID; | ||
| const callerId = context.CALLER_ID; |
There was a problem hiding this comment.
Nit: most times you are still using context.CALLER_ID below, and only once do you use this callerId. I'd either stick with one or other
There was a problem hiding this comment.
since most look ups are using context.CALLER_ID i would just remove this line and change ln 212 to be context.CALLER_ID
| }; | ||
| } | ||
|
|
||
| async clearCallerId() { |
There was a problem hiding this comment.
since these aren't truly async functions, if you rewrote the method to be a standard function, then you can also remove the need to call it with await.
The pattern makes sense for chooseLogicalCallerId but just because one of the functions uses the asyc/await behavior, that doesn't mean everything needs to have the same interface
| @@ -39,31 +37,31 @@ async function checkEnvironmentInitialization(context) { | |||
| ]; | |||
| } else { | |||
| status.valid = true; | |||
| status.description = `Your application is initialized! View your [running application](../index.html)`; | |||
| status.description = `Your application is initialized! View your [running application](../browser.html)`; | |||
There was a problem hiding this comment.
we have a helper utility you can use to generate the deployed url here https://github.com/twilio-labs/function-templates/blob/main/docs/static/v1/ce-helpers.js
| async function getCallerIdStatus(context) { | ||
| const client = context.getTwilioClient(); | ||
| const callerId = process.env.CALLER_ID; | ||
| const callerId = context.CALLER_ID; |
There was a problem hiding this comment.
since most look ups are using context.CALLER_ID i would just remove this line and change ln 212 to be context.CALLER_ID
| status.description = stripIndents` | ||
| Please take a moment to change your admin password from the provided default password. | ||
|
|
||
| You can do this by editing the \`ADMIN_PASSWORD\` value in the \`.env\` in the root of this project. | ||
| You can do this by editing the \`ADMIN_PASSWORD\` value in your environment. |
There was a problem hiding this comment.
I'm sorry i should have been more clear before. We can't assume a quick deploy user is going to know how to update this and redeploy. Also this is not going to mean anything to the user in the quick deploy context.
\`\`\`bash
twilio serverless:deploy
\`\`\`
Is there any way we can help streamline/simplify this process?
| @@ -0,0 +1,41 @@ | |||
| <!DOCTYPE html> | |||
| <html> | |||
There was a problem hiding this comment.
would it be possible to adapt this page to use the new index.html template design? Possibly the admin page as well?
| </p> | ||
| <ol class="steps"> | ||
| <li>Log in to the <a href="./admin/index.html">admin page</a> and initialize your environment</li> |
There was a problem hiding this comment.
can you make sure to add target="_blank" for all the links on this page
Description
Updates the Voice Client Quickstart by wiring up an incoming number. This also allows for unsetting environment values.
Some tidying up of tests.
Checklist
npm testlocally and it passed without errors.Related issues