feat: support appcues integration custom domain option#811
Open
zhukevgeniy wants to merge 1 commit intosegmentio:masterfrom
Open
feat: support appcues integration custom domain option#811zhukevgeniy wants to merge 1 commit intosegmentio:masterfrom
zhukevgeniy wants to merge 1 commit intosegmentio:masterfrom
Conversation
philwarner-pd
approved these changes
Feb 7, 2025
Contributor
There was a problem hiding this comment.
Pull request overview
Adds support for hosting the Appcues SDK under a custom domain (CNAME) while still using the Segment Appcues integration, by allowing the script host to be configured via an integration option.
Changes:
- Add a new
domainintegration option for Appcues. - Update the Appcues loader to build the script URL from
domain + appcuesId. - Extend the Appcues integration test settings to include the new
domainoption.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| integrations/appcues/lib/index.js | Introduces domain option and uses it to construct the Appcues SDK script URL. |
| integrations/appcues/test/index.test.js | Updates settings expectations and test options to include domain. |
| .gitignore | Ignores JetBrains .idea/ directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Appcues.prototype.load = function(callback) { | ||
| var id = this.options.appcuesId || 'appcues'; | ||
| load('//fast.appcues.com/' + id + '.js', callback); | ||
| var domain = this.options.domain || '//fast.appcues.com/'; |
Comment on lines
+51
to
+52
| var domain = this.options.domain || '//fast.appcues.com/'; | ||
| load(domain + id + '.js', callback); |
| var options = { | ||
| appcuesId: '1663' | ||
| appcuesId: '1663', | ||
| domain: '//fast.appcues.net/' |
Comment on lines
49
to
+52
| Appcues.prototype.load = function(callback) { | ||
| var id = this.options.appcuesId || 'appcues'; | ||
| load('//fast.appcues.com/' + id + '.js', callback); | ||
| var domain = this.options.domain || '//fast.appcues.com/'; | ||
| load(domain + id + '.js', callback); |
|
Please review the comments from Copilot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This PR adds a custom domain (also known as CNAME) option for appcues integration.
Currently, the only way to use a custom domain for appcues is to integrate directly with their SDK,
So by providing this custom option, a user will still be able to use appcues as a segment integration.
Link: https://docs.appcues.com/dev-installing-appcues/host-appcues-sdk-under-your-own-domain
Are there breaking changes in this PR?
NO
Testing
No special tests are needed
Any background context you want to provide?
Is there parity with the server-side/android/iOS integration components (if applicable)?
Does this require a new integration setting? If so, please explain how the new setting works
Links to helpful docs and other external resources