feat(samples): create calling lab #4435
Conversation
📝 WalkthroughWalkthroughAdds a new “Calling Labs” entry to docs/index.html and introduces a complete Webex Calling WebRTC SDK lab under docs/labs/calling/. The lab includes an index.html UI, index.js controller, supporting modules for auth, registration, and call controls, and styles.css. It implements OAuth and PAT initialization, registration/deregistration, line registration, microphone/media wiring, outgoing/incoming calls, mute/hold, transfer (blind/consult), and DTMF (per-key and batch). A README documents lab goals, flow, OAuth setup, scopes, and usage details. No existing exported/public APIs in the main codebase are changed. Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (13)
docs/labs/calling/styles.css (1)
24-30: Consider extracting magic numbers to CSS custom properties.The padding, border radius, and color values are repeated throughout the stylesheet. Consider defining CSS custom properties for consistency and maintainability.
+:root { + --border-radius: 4px; + --border-radius-lg: 8px; + --padding-sm: 8px; + --padding-md: 12px; + --color-border: #ddd; + --color-primary: #00a6d6; + --color-accent: #1a73e8; +} + fieldset { border: 1px solid #ddd; border-radius: 6px; margin: 12px 0; padding: 12px; } -legend { color: #1a73e8; font-weight: 600; } +legend { color: var(--color-accent); font-weight: 600; } .s-row { display: flex; gap: 8px; align-items: center; flex-wrap: wrap; } -button { background:#00a6d6; color:#fff; border:none; border-radius:4px; padding:8px 12px; cursor:pointer; } +button { background: var(--color-primary); color:#fff; border:none; border-radius: var(--border-radius); padding: var(--padding-sm) var(--padding-md); cursor:pointer; }docs/labs/calling/modules/registration.js (1)
11-18: Remove debug console.log statements.Console logging in production code can expose sensitive information and clutters the console output.
export function getPrimaryLine(callingClient) { if (!callingClient) return undefined; - console.log('callingClient', callingClient); const lines = callingClient.getLines(); const first = Object.values(lines)[0]; - console.log('first', first); return first; }docs/labs/calling/README.md (3)
14-23: Fix markdownlint MD040: add a language to the fenced code block.Static analysis flagged MD040. Add a language (e.g., text) to the directory tree block.
Apply this diff:
-``` +```text docs/labs/calling/ ├── index.html # Lab UI ├── index.js # Orchestration ├── styles.css # Styles └── modules/ ├── auth.js # Initialization ├── registration.js # Register/deregister/line helpers - └── call-controls.js# Call control helpers (placeholder) + └── call-controls.js # Call control helpers--- `20-22`: **Update comment: module is no longer a placeholder.** The comment says “placeholder,” but call-controls.js implements dialpad, mute, and hold. Adjust the comment for accuracy (see diff in previous comment). --- `50-86`: **Prefer Authorization Code + PKCE over Implicit for production OAuth.** For production apps, Authorization Code with PKCE is recommended over the Implicit flow. Consider adding a short note here that the Implicit sample is for labs only, and point to the Code+PKCE guidance. Optionally insert a note right after Line 52: ```diff For production, use OAuth instead of a PAT. Create an Integration at the Webex Developer Portal and configure: +Note: For production SPAs, prefer the Authorization Code + PKCE flow over the Implicit flow for improved security (token handling, refresh, reduced exposure). The Implicit flow shown below is intended for this lab only.docs/labs/calling/index.html (4)
9-11: Unify highlight.js versions and defer its loading.CSS is 11.11.1 while JS is 11.7.0. Use the same version to avoid quirks. Also add defer to avoid blocking render.
Apply this diff:
- <script src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.7.0/highlight.min.js"></script> + <script defer src="https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.11.1/highlight.min.js"></script>Also applies to: 20-20
14-19: Consider pinning Webex UMD to a stable version.Using webex@next is non-deterministic and may break the lab unexpectedly. Pin to a known-good version and update intentionally.
Example:
- <script src="https://unpkg.com/webex@next/umd/webex.min.js"></script> + <script src="https://unpkg.com/webex@<pinned-version>/umd/webex.min.js"></script>
176-176: Improve keyboard/mobile UX for destination field.Use a telephone keypad for number/SIP entry and disable auto-features that can alter input.
Apply this diff:
- <input id="destination" type="text" placeholder="Destination (SIP/phone)" /> + <input + id="destination" + type="text" + placeholder="Destination (SIP/phone)" + inputmode="tel" + autocomplete="off" + autocapitalize="off" + autocorrect="off" + spellcheck="false" + />
227-227: Dialer UX: use tel input mode and limit allowed characters.Helps ensure only valid DTMF characters are entered.
Apply this diff:
- <input id="dtmf-display" type="text" placeholder="Digits" /> + <input + id="dtmf-display" + type="text" + placeholder="Digits" + inputmode="tel" + pattern="[0-9*#]*" + autocomplete="off" + autocapitalize="off" + autocorrect="off" + spellcheck="false" + />docs/labs/calling/index.js (4)
164-169: Cleanup remote media and incoming UI on disconnect.Helps prevent stale audio elements and UI text after a call ends.
Apply this diff:
call.on('disconnect', () => { els.callStatus.textContent = 'Call Disconnected'; setCallUI(false); activeCall = undefined; resetTransferUI(); + if (els.remoteAudio) els.remoteAudio.srcObject = null; + if (els.incomingInfo) els.incomingInfo.textContent = 'No incoming calls'; });
72-91: Early validation for empty token on PAT init.Short-circuit with a helpful status instead of relying on thrown errors.
Apply this diff:
async function handleInit() { els.init.disabled = true; els.authStatus.textContent = 'Initializing...'; const token = els.token.value.trim(); + if (!token) { + els.authStatus.textContent = 'Please paste an access token or use OAuth.'; + els.init.disabled = false; + return; + } try { const { callingInstance, client } = await initCalling({ token, fedramp: !!els.fedramp.checked, useIntegration: !!els.intEnv.checked });
330-330: Drop stray console log.Avoids noisy console output in the lab.
Apply this diff:
- console.log('bindUI');
96-103: Hardcoded OAuth clientId is acceptable for labs; consider environment override.For a lab, a placeholder is fine (per prior team learning), but optionally read from a query param or env to avoid editing source when testing different integrations.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/index.html(1 hunks)docs/labs/calling/README.md(1 hunks)docs/labs/calling/index.html(1 hunks)docs/labs/calling/index.js(1 hunks)docs/labs/calling/modules/auth.js(1 hunks)docs/labs/calling/modules/call-controls.js(1 hunks)docs/labs/calling/modules/registration.js(1 hunks)docs/labs/calling/styles.css(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-12-06T12:48:03.643Z
Learnt from: bhabalan
PR: webex/webex-js-sdk#4022
File: docs/samples/contact-center/app.js:116-118
Timestamp: 2024-12-06T12:48:03.643Z
Learning: In `docs/samples/contact-center/app.js`, hardcoding the OAuth client ID and secret is acceptable for testing purposes in an isolated sandbox environment.
Applied to files:
docs/labs/calling/README.md
🧬 Code Graph Analysis (4)
docs/labs/calling/modules/call-controls.js (1)
docs/labs/calling/index.js (2)
els(12-43)call(220-220)
docs/labs/calling/modules/registration.js (1)
docs/labs/calling/index.js (2)
calling(5-5)callingClient(6-6)
docs/labs/calling/modules/auth.js (1)
docs/labs/calling/index.js (3)
initCalling(77-81)token(75-75)token(322-322)
docs/labs/calling/index.js (3)
docs/labs/calling/modules/auth.js (3)
initCalling(1-41)callingInstance(34-34)initOauth(59-72)docs/labs/calling/modules/registration.js (3)
register(1-4)getPrimaryLine(11-18)deregister(6-9)docs/labs/calling/modules/call-controls.js (1)
setupCallControls(9-63)
🪛 markdownlint-cli2 (0.17.2)
docs/labs/calling/README.md
14-14: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: AWS Amplify Console Web Preview
🔇 Additional comments (17)
docs/index.html (1)
88-88: LGTM! Navigation link properly added.The new "Calling Labs" navigation item is correctly positioned alongside the existing "Contact Center Labs" entry and follows the established HTML structure and naming conventions.
docs/labs/calling/styles.css (3)
1-8: LGTM! Well-structured base styling.The typography and layout foundation is solid with proper font stack, line height, and centered container with reasonable max-width.
10-14: LGTM! Consistent branding colors.The blue color (#0052bf) appears to align with Cisco/Webex branding and the visual styling is appropriate for headers.
32-36: LGTM! Responsive grid layouts are well-implemented.Both the media grid and dialpad use appropriate CSS Grid with good responsive behavior and consistent spacing.
docs/labs/calling/modules/call-controls.js (5)
1-9: LGTM! Clear function documentation and parameter validation.The JSDoc is comprehensive and the function signature is well-defined with proper destructuring.
12-20: Review mute behavior - potential UX confusion.The mute implementation calls
call.mute(stream, 'user_mute')but doesn't track or toggle mute state. Users may not have visual feedback about whether they're muted or unmuted.Consider implementing a toggle pattern or updating UI state to reflect mute status:
els.mute.addEventListener('click', () => { const call = getActiveCall?.(); const stream = getLocalStream?.(); if (!call || !stream) return; + // Consider tracking mute state and toggling + // const isMuted = call.isMuted(stream); + // if (isMuted) { + // call.unmute(stream, 'user_unmute'); + // } else { call.mute(stream, 'user_mute'); + // } + // Update button text/state to reflect current mute status });
22-29: Hold/Resume implementation looks correct.The
doHoldResume()method name suggests it handles both hold and resume operations internally, which is appropriate for a toggle button.
46-55: LGTM! Batch DTMF sending with proper validation.The implementation correctly validates both the call existence and input content before sending digits.
57-62: LGTM! Simple and effective clear functionality.The clear operation is straightforward with appropriate null checking.
docs/labs/calling/modules/auth.js (5)
1-8: LGTM! Proper input validation and configuration structure.The function correctly validates the required token and builds the webex configuration appropriately.
19-32: Review feature toggles logic for FedRAMP compliance.The feature flags disable several capabilities (
calling,callSettings,contact) whenfedrampis true. Ensure this aligns with FedRAMP compliance requirements and doesn't break essential functionality.The current logic disables these features in FedRAMP mode:
calling: !fedramp(disables calling when FedRAMP is true)callSettings: !fedrampcontact: !fedrampPlease verify this matches the intended FedRAMP compliance requirements, as disabling core calling functionality in FedRAMP mode seems counterintuitive for a calling lab.
34-41: LGTM! Proper async initialization with event handling.The initialization correctly waits for the 'ready' event before resolving and returns both the calling instance and client.
43-50: LGTM! Comprehensive calling scopes defined.The OAuth scopes cover the necessary permissions for WebRTC calling functionality.
10-17: Integration service URLs are reachable (HTTP 401 as expected)
- https://u2c-intb.ciscospark.com/u2c/api/v1 → HTTP/2 401
- https://hydra-intb.ciscospark.com/v1/ → HTTP/2 401
Both endpoints are live and returning the expected “authorization required” response. No updates to these URLs are needed at this time.
docs/labs/calling/modules/registration.js (2)
1-4: LGTM! Proper error handling for required parameter.The function correctly validates the calling parameter and provides a clear error message.
6-9: LGTM! Graceful handling of missing calling instance.The early return pattern appropriately handles the case where calling might not be initialized.
docs/labs/calling/index.js (1)
264-269: completeTransfer usage for CONSULT transfer is correct as-isThe Webex Calling WebRTC SDK’s completeTransfer signature is completeTransfer(transferType, transferCallId?, transferTarget?) and the Call object exposes getCallId() to retrieve its ID. Passing consultCall.getCallId() for a CONSULT transfer is exactly what the API expects, so no changes are needed.
| </p> | ||
| <fieldset> | ||
| <legend>Credentials</legend> | ||
| <input id="access-token" type="text" placeholder="Paste access token" /> |
There was a problem hiding this comment.
Hide sensitive PAT input and disable auto-features that can leak it.
PATs are sensitive. Use a password field and disable autocorrect/capitalize/spellcheck and autocomplete.
Apply this diff:
- <input id="access-token" type="text" placeholder="Paste access token" />
+ <input
+ id="access-token"
+ type="password"
+ placeholder="Paste access token"
+ autocomplete="off"
+ autocapitalize="off"
+ autocorrect="off"
+ spellcheck="false"
+ aria-label="Access token"
+ />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <input id="access-token" type="text" placeholder="Paste access token" /> | |
| <input | |
| id="access-token" | |
| type="password" | |
| placeholder="Paste access token" | |
| autocomplete="off" | |
| autocapitalize="off" | |
| autocorrect="off" | |
| spellcheck="false" | |
| aria-label="Access token" | |
| /> |
🤖 Prompt for AI Agents
In docs/labs/calling/index.html around line 43, the access token input is
currently a plain text field which can expose PATs; change the input to
type="password" and add attributes to prevent browser and OS features from
leaking the token: set autocomplete="off" (or autocomplete="new-password"),
autocorrect="off", autocapitalize="off", and spellcheck="false" so the token is
hidden and auto-features are disabled.
| <p><strong>Using OAuth (for production):</strong></p> | ||
| <p> | ||
| To use OAuth, first create an Integration in the | ||
| <a href="https://developer.webex.com/my-apps" target="_blank">Webex Developer Portal</a>. |
There was a problem hiding this comment.
Add rel="noopener noreferrer" to external link with target="_blank".
Prevents reverse tabnabbing and improves security.
Apply this diff:
- <a href="https://developer.webex.com/my-apps" target="_blank">Webex Developer Portal</a>.
+ <a href="https://developer.webex.com/my-apps" target="_blank" rel="noopener noreferrer">Webex Developer Portal</a>.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href="https://developer.webex.com/my-apps" target="_blank">Webex Developer Portal</a>. | |
| <a href="https://developer.webex.com/my-apps" target="_blank" rel="noopener noreferrer">Webex Developer Portal</a>. |
🤖 Prompt for AI Agents
In docs/labs/calling/index.html around line 78, the external anchor uses
target="_blank" but is missing rel="noopener noreferrer"; update the <a> tag to
include rel="noopener noreferrer" alongside target="_blank" to prevent reverse
tabnabbing and improve security.
| async function handleDeregister() { | ||
| try { | ||
| await deregister(calling); | ||
| setRegistrationUI(false); | ||
| setCallUI(false); | ||
| activeCall = undefined; | ||
| } catch (e) { | ||
| console.error('Deregister failed', e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
End active/consult calls before deregistering and reset media UI.
Avoid dangling calls and stale UI when deregistering.
Apply this diff:
async function handleDeregister() {
try {
+ if (consultCall) {
+ try { consultCall.end(); } catch {}
+ consultCall = undefined;
+ }
+ if (activeCall) {
+ try { activeCall.end(); } catch {}
+ activeCall = undefined;
+ }
await deregister(calling);
setRegistrationUI(false);
setCallUI(false);
- activeCall = undefined;
+ resetTransferUI();
+ if (els.remoteAudio) els.remoteAudio.srcObject = null;
} catch (e) {
console.error('Deregister failed', e);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function handleDeregister() { | |
| try { | |
| await deregister(calling); | |
| setRegistrationUI(false); | |
| setCallUI(false); | |
| activeCall = undefined; | |
| } catch (e) { | |
| console.error('Deregister failed', e); | |
| } | |
| } | |
| async function handleDeregister() { | |
| try { | |
| if (consultCall) { | |
| try { consultCall.end(); } catch {} | |
| consultCall = undefined; | |
| } | |
| if (activeCall) { | |
| try { activeCall.end(); } catch {} | |
| activeCall = undefined; | |
| } | |
| await deregister(calling); | |
| setRegistrationUI(false); | |
| setCallUI(false); | |
| resetTransferUI(); | |
| if (els.remoteAudio) els.remoteAudio.srcObject = null; | |
| } catch (e) { | |
| console.error('Deregister failed', e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In docs/labs/calling/index.js around lines 133 to 142, the deregister handler
currently calls deregister and then hides UI but can leave active or consult
calls and media streams dangling; before calling deregister, if there is an
activeCall or consultCall, await their proper termination (call their
hangup/terminate/end method or send a BYE and await confirmation), stop and
release any local media tracks and clear audio/video element srcObject, then
call deregister, and only after successful deregistration
setRegistrationUI(false), setCallUI(false) and clear activeCall/consultCall
variables.
| function handleAnswer() { | ||
| if (!activeCall || !localAudioStream) return; | ||
| wireActiveCall(activeCall); | ||
| activeCall.answer(localAudioStream); | ||
| els.answer.disabled = true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Answer incoming calls even if the mic stream hasn’t been acquired yet.
Users may click Answer before Step 4. Make handleAnswer async and fetch the mic if needed.
Apply this diff:
-function handleAnswer() {
- if (!activeCall || !localAudioStream) return;
- wireActiveCall(activeCall);
- activeCall.answer(localAudioStream);
- els.answer.disabled = true;
-}
+async function handleAnswer() {
+ if (!activeCall) return;
+ if (!localAudioStream) {
+ await handleGetMedia();
+ if (!localAudioStream) {
+ els.callStatus.textContent = 'Microphone unavailable.';
+ return;
+ }
+ }
+ wireActiveCall(activeCall);
+ activeCall.answer(localAudioStream);
+ els.answer.disabled = true;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function handleAnswer() { | |
| if (!activeCall || !localAudioStream) return; | |
| wireActiveCall(activeCall); | |
| activeCall.answer(localAudioStream); | |
| els.answer.disabled = true; | |
| } | |
| async function handleAnswer() { | |
| if (!activeCall) return; | |
| if (!localAudioStream) { | |
| await handleGetMedia(); | |
| if (!localAudioStream) { | |
| els.callStatus.textContent = 'Microphone unavailable.'; | |
| return; | |
| } | |
| } | |
| wireActiveCall(activeCall); | |
| activeCall.answer(localAudioStream); | |
| els.answer.disabled = true; | |
| } |
🤖 Prompt for AI Agents
In docs/labs/calling/index.js around lines 312 to 317, handleAnswer currently
returns if localAudioStream is missing, preventing answering when the user
clicks before the mic is acquired; make handleAnswer an async function, check
for localAudioStream and if absent call the existing mic acquisition routine
(await getLocalAudioStream or equivalent) inside a try/catch, assign the result
to localAudioStream, then proceed to wireActiveCall(activeCall), call
activeCall.answer(localAudioStream), and disable els.answer; ensure errors
during mic acquisition are caught and logged and that the function still exits
gracefully if acquisition fails.
| function autoInitFromHash() { | ||
| const hash = window.location.hash?.substring(1) || ''; | ||
| const params = new URLSearchParams(hash); | ||
| const token = params.get('access_token'); | ||
| if (token) { | ||
| els.token.value = token; | ||
| handleInit(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Clear the access token from the URL after auto-init.
After extracting the access_token from the fragment, remove it to avoid leaving tokens in browser history or user-shared URLs.
Apply this diff:
function autoInitFromHash() {
const hash = window.location.hash?.substring(1) || '';
const params = new URLSearchParams(hash);
const token = params.get('access_token');
if (token) {
- els.token.value = token;
- handleInit();
+ els.token.value = token;
+ // Remove the token fragment from the URL
+ window.history.replaceState(null, '', window.location.pathname + window.location.search);
+ handleInit();
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function autoInitFromHash() { | |
| const hash = window.location.hash?.substring(1) || ''; | |
| const params = new URLSearchParams(hash); | |
| const token = params.get('access_token'); | |
| if (token) { | |
| els.token.value = token; | |
| handleInit(); | |
| } | |
| } | |
| function autoInitFromHash() { | |
| const hash = window.location.hash?.substring(1) || ''; | |
| const params = new URLSearchParams(hash); | |
| const token = params.get('access_token'); | |
| if (token) { | |
| els.token.value = token; | |
| // Remove the token fragment from the URL | |
| window.history.replaceState(null, '', window.location.pathname + window.location.search); | |
| handleInit(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In docs/labs/calling/index.js around lines 319 to 327, after extracting
access_token from the URL fragment and calling handleInit(), remove the token
from the URL so it isn't left in browser history or shared links: clear or
replace the location hash and use history.replaceState (or set location.hash =
'' and call history.replaceState with the current URL sans fragment) to avoid
navigating or adding a new history entry while preserving the current page
state.
| export async function initOauth({ clientId, redirectUri, scope } = {}) { | ||
| const webex = Webex.init({ | ||
| config: { | ||
| credentials: { | ||
| client_id: clientId || 'YOUR_PUBLIC_CLIENT_ID', | ||
| redirect_uri: redirectUri || (window.location.origin + window.location.pathname), | ||
| scope: scope || callingScopes | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| await webex.authorization.initiateLogin({ state: {} }); | ||
| return webex; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve OAuth configuration validation.
The function uses a placeholder client ID that could lead to runtime failures if not properly configured.
export async function initOauth({ clientId, redirectUri, scope } = {}) {
+ if (!clientId || clientId === 'YOUR_PUBLIC_CLIENT_ID') {
+ throw new Error('Valid clientId is required for OAuth initialization');
+ }
+
const webex = Webex.init({
config: {
credentials: {
- client_id: clientId || 'YOUR_PUBLIC_CLIENT_ID',
+ client_id: clientId,
redirect_uri: redirectUri || (window.location.origin + window.location.pathname),
scope: scope || callingScopes
}
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function initOauth({ clientId, redirectUri, scope } = {}) { | |
| const webex = Webex.init({ | |
| config: { | |
| credentials: { | |
| client_id: clientId || 'YOUR_PUBLIC_CLIENT_ID', | |
| redirect_uri: redirectUri || (window.location.origin + window.location.pathname), | |
| scope: scope || callingScopes | |
| } | |
| } | |
| }); | |
| await webex.authorization.initiateLogin({ state: {} }); | |
| return webex; | |
| } | |
| export async function initOauth({ clientId, redirectUri, scope } = {}) { | |
| if (!clientId || clientId === 'YOUR_PUBLIC_CLIENT_ID') { | |
| throw new Error('Valid clientId is required for OAuth initialization'); | |
| } | |
| const webex = Webex.init({ | |
| config: { | |
| credentials: { | |
| client_id: clientId, | |
| redirect_uri: redirectUri || (window.location.origin + window.location.pathname), | |
| scope: scope || callingScopes | |
| } | |
| } | |
| }); | |
| await webex.authorization.initiateLogin({ state: {} }); | |
| return webex; | |
| } |
🤖 Prompt for AI Agents
In docs/labs/calling/modules/auth.js around lines 59 to 72, the initOauth
function currently falls back to a placeholder client ID which can produce
runtime failures; validate the provided clientId (and optionally
redirectUri/scope) at the start of the function and fail fast: if clientId is
missing or equals the placeholder string, throw a clear Error (or return a
rejected Promise) instructing the caller to provide a valid clientId; also
validate redirectUri and scope and either supply safe defaults or throw with
actionable messages if they are invalid, so the function never initializes Webex
with a known-bad configuration.
| // Dialpad per-key sending | ||
| document.querySelectorAll('.dialpad button[data-tone]') | ||
| .forEach((btn) => { | ||
| btn.addEventListener('click', () => { | ||
| const tone = btn.getAttribute('data-tone'); | ||
| if (els.dtmfDisplay) { | ||
| els.dtmfDisplay.value += tone; | ||
| } | ||
| const call = getActiveCall?.(); | ||
| if (call) { | ||
| try { call.sendDigit(tone); } catch (e) { /* ignore */ } | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for DTMF operations.
The try/catch block silently ignores errors, which could hide important issues like invalid tones or network problems.
const call = getActiveCall?.();
if (call) {
- try { call.sendDigit(tone); } catch (e) { /* ignore */ }
+ try {
+ call.sendDigit(tone);
+ } catch (e) {
+ console.warn(`Failed to send DTMF tone "${tone}":`, e);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Dialpad per-key sending | |
| document.querySelectorAll('.dialpad button[data-tone]') | |
| .forEach((btn) => { | |
| btn.addEventListener('click', () => { | |
| const tone = btn.getAttribute('data-tone'); | |
| if (els.dtmfDisplay) { | |
| els.dtmfDisplay.value += tone; | |
| } | |
| const call = getActiveCall?.(); | |
| if (call) { | |
| try { call.sendDigit(tone); } catch (e) { /* ignore */ } | |
| } | |
| }); | |
| }); | |
| // Dialpad per-key sending | |
| document.querySelectorAll('.dialpad button[data-tone]') | |
| .forEach((btn) => { | |
| btn.addEventListener('click', () => { | |
| const tone = btn.getAttribute('data-tone'); | |
| if (els.dtmfDisplay) { | |
| els.dtmfDisplay.value += tone; | |
| } | |
| const call = getActiveCall?.(); | |
| if (call) { | |
| try { | |
| call.sendDigit(tone); | |
| } catch (e) { | |
| console.warn(`Failed to send DTMF tone "${tone}":`, e); | |
| } | |
| } | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In docs/labs/calling/modules/call-controls.js around lines 31 to 44, the current
try/catch for call.sendDigit(tone) swallows all errors; validate the tone before
sending and replace the empty catch with proper error handling: check tone
against the allowed DTMF characters (0-9, *, #, A-D) and if invalid skip and
show a user-visible message; wrap call.sendDigit in a try/catch that logs the
error (include tone and call id/context) to console or the app logger and
surface a brief inline error/toast instead of ignoring, or rethrow for
higher-level handling if appropriate.
| const lines = callingClient.getLines(); | ||
| const first = Object.values(lines)[0]; | ||
| console.log('first', first); | ||
| return first; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for empty lines collection.
The function assumes getLines() returns an object with values, but it could be empty or null.
export function getPrimaryLine(callingClient) {
if (!callingClient) return undefined;
const lines = callingClient.getLines();
+ if (!lines || typeof lines !== 'object') return undefined;
const first = Object.values(lines)[0];
+ if (!first) return undefined;
return first;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In docs/labs/calling/modules/registration.js around lines 14 to 17, the code
assumes callingClient.getLines() returns a non-empty object and immediately
accesses Object.values(lines)[0]; add a null/empty check before accessing
values: verify lines is truthy and has at least one key (e.g.,
Object.keys(lines).length > 0), handle the empty/null case by returning null (or
throwing a clear error) and optionally logging a warning, otherwise compute and
return the first value; ensure no runtime error occurs when getLines() returns
null or an empty object.
| // After redirect back, the access_token is in the URL hash. The lab auto-detects | ||
| // it and initializes Calling using that token. | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Document removing the access token from the URL fragment after parsing.
To reduce accidental token exposure (browser history, on-screen sharing), mention clearing the hash after extraction.
You can augment the sample with this line:
// After redirect back, the access_token is in the URL hash. The lab auto-detects
-// it and initializes Calling using that token.
+// it and initializes Calling using that token, then removes it from the URL:
+window.history.replaceState(null, '', window.location.pathname + window.location.search);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // After redirect back, the access_token is in the URL hash. The lab auto-detects | |
| // it and initializes Calling using that token. | |
| ``` | |
| // After redirect back, the access_token is in the URL hash. The lab auto-detects | |
| // it and initializes Calling using that token, then removes it from the URL: | |
| window.history.replaceState(null, '', window.location.pathname + window.location.search); |
🤖 Prompt for AI Agents
In docs/labs/calling/README.md around lines 84–86, after describing extraction
of the access_token from the URL hash, add a step to remove the fragment from
the browser address bar to avoid accidental token exposure: immediately after
parsing the token, call the browser history API to replace the current URL with
the same path+query but without the hash (e.g., use history.replaceState to
clear the fragment) so the token is not stored in history or visible on-screen.
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
COMPLETES #< INSERT LINK TO ISSUE >
This pull request addresses
< DESCRIBE THE CONTEXT OF THE ISSUE >
by making the following changes
< DESCRIBE YOUR CHANGES >
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.