Skip to content

Conversation

@xTEddie
Copy link
Contributor

@xTEddie xTEddie commented May 21, 2025

  • Attachment client improvements on widgets hosted on whitelisted URLs
  • Attachment client multi-region support on non-whitelisted URLs

@xTEddie xTEddie marked this pull request as ready for review May 28, 2025 16:49
@elopezanaya elopezanaya requested a review from Copilot June 2, 2025 18:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the AMS attachment client with support for whitelisted and region-based URLs, configurable via new internal flags, and refactors widget snippet URL parsing.

  • Introduced AMSClientUtils with whitelistedUrls, shouldUseFramedMode, and retrieveRegionBasedUrl
  • Added disableAMSWhitelistedUrls & disableAMSRegionBasedUrl config flags and wired them into createAMSClient
  • Extracted widget snippet base URL parsing into setWidgetSnippetBaseUrl and updated loading logic
  • Simplified Jest node setup and added tests for new utilities

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/utils/AMSClientUtils.ts New utility functions and URL whitelist array
src/core/ChatSDKConfig.ts Added internal config flags; minor doc comment update
src/OmnichannelChatSDK.ts Integrated new AMSClientUtils, added widgetSnippetBaseUrl handling, and updated AMS client calls
jest.setup.node.js Removed legacy JSDOM mocks, retained global.self mock
tests/utils/AMSClientUtils.web.spec.ts Tests for new utils in browser environment
tests/utils/AMSClientUtils.node.spec.ts Tests for new utils in Node environment
tests/external/ACSAdapter/createFileScanIngressMiddleware.spec.ts Minor window mock addition
CHANGELOG.md Updated changelog entry for attachment client improvements
Comments suppressed due to low confidence (1)

src/core/ChatSDKConfig.ts:36

  • [nitpick] The flag name disableAMSRegionBasedUrl is inconsistent with disableAMSWhitelistedUrls (singular vs plural). Consider renaming for consistency, e.g., disableAMSRegionBasedUrls.
disableAMSRegionBasedUrl?: boolean;

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
xTEddie and others added 3 commits June 4, 2025 10:31
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@xTEddie xTEddie merged commit e08c1d6 into microsoft:main Jun 5, 2025
2 checks passed
@xTEddie xTEddie deleted the attachment-client-improvements branch June 5, 2025 18:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants