Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add support for custom domain proxy #28

Conversation

ngarg-mparticle
Copy link
Contributor

Summary

  • Read the connection setting "customDomainProxyUrl" and set it

Testing Plan

  • Was this tested locally? If not, explain why.
  • Tested without setting customDomainProxyUrl in mP UI first. The integration worked normally and events were forwarded to Airship.
  • After setting a dummy value "https://analytics.example.com" for customDomainProxyUrl in mP UI, the integration stopped working and I started getting "A server with the specified hostname could not be found." in the Xcode console.
  • Further testing needs to be done with a working custom domain proxy.

mParticle-UrbanAirship/MPKitUrbanAirship.m Outdated Show resolved Hide resolved
Co-authored-by: Ben Baron <[email protected]>
@BrandonStalnaker
Copy link
Contributor

@ngarg-mparticle any update on testing this?

// Enable custom domain proxy if provided
if (self.configuration[UAConfigCustomDomainProxyUrl]) {
config.initialConfigURL = self.configuration[UAConfigCustomDomainProxyUrl];
config.URLAllowList = @[self.configuration[UAConfigCustomDomainProxyUrl]];
Copy link
Contributor

@ShaneQi ShaneQi Mar 1, 2024

Choose a reason for hiding this comment

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

Why are we setting URLAllowList to UAConfigCustomDomainProxyUrl? I assume this would overwrite the URLAllowList populated from the default config, which is probably not what we want to do.

Context: I'm from the customer that requested this feature. I will be testing this and the above doesn't seems to be right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@ngarg-mparticle ngarg-mparticle Mar 26, 2024

Choose a reason for hiding this comment

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

@ShaneQi perhaps a better approach would be to append if URLAllowList already exists? I can look into making that change soon.

Copy link
Contributor

@BrandonStalnaker BrandonStalnaker left a comment

Choose a reason for hiding this comment

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

LGTM

@BrandonStalnaker BrandonStalnaker merged commit f288079 into mparticle-integrations:development Apr 3, 2024
2 of 3 checks passed
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.

4 participants