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

Set koala's base url based on current environment #820

Merged
merged 1 commit into from Aug 28, 2019

Conversation

dusi
Copy link
Contributor

@dusi dusi commented Aug 28, 2019

πŸ“² What

Tracking events are send to the right Koala endpoint based on current environment.

πŸ€” Why

We ran into an issue where all tracking events were being sent to production environment. We would like to set the Koala endpoint based on current environment.

πŸ›  How

See the code change

βœ… Acceptance criteria

Release build

  1. Proxy your device's network communication through your machine either by using Charlesproxy or mitmproxy (there are instructions in Guru how to do this)
  2. Build and run in Release mode by following these instructions

Screen Shot 2019-08-27 at 4 55 27 PM

  • Tracking events are being sent to production endpoint (this value is accessible in Secrets.KoalaEndpoint.production
  • Terminating the app and restarting it again does send Koala events to the right endpoint based on Environment

Debug build (Alpha, Beta, Debug)

  1. Proxy your device's network communication through your machine either by using Charlesproxy or mitmproxy (there are instructions in Guru how to do this)
  2. Build and run in Debug mode
  3. Make sure KOALA_TRACKING environment variable is turned ON by following these instructions

Screen Shot 2019-08-27 at 4 52 26 PM

  • By changing the Environment to Staging (in Beta tools) tracking events are being sent to
    staging endpoint (this value is accessible in Secrets.KkoalaEndpoint.staging)
  • By changing the Environment to Production (in Beta tools) tracking events are being sent to production endpoint (this value is accessible in Secrets.KkoalaEndpoint.production)
  • Logging in / out does send Koala events to the right endpoint based on Environment

Please note that if you're immediately not seeing any events you can background the app which should flush the queue.

case .production:
return Secrets.KoalaEndpoint.production
}
fileprivate var base: String {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since our base url is being affected by environmentType we no longer require a separate Endpoint enum.

fileprivate var base: String {
switch AppEnvironment.current.environmentType {
case .production:
return Secrets.KoalaEndpoint.production
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only production will go to production ... anything else (development or local) should default to staging

@@ -130,7 +123,7 @@ public final class KoalaTrackingClient: TrackingClientType {
if dataString.count >= 10_000 {
print("🐨 [Koala Error]: Base64 payload is longer than 10,000 characters.")
}
return URL(string: "\(self.endpoint.base)?data=\(dataString)")
return URL(string: "\(self.base)?data=\(dataString)")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately KoalaClient isn't very testable (most if not all functions are fileprivate) so I'd suggest we proceed without tests for now (as I wasn't able to figure out how to test)...open to any suggestions!!!

Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

All scenarios worked. (The Release AC was tested on build 15670112). 🏌

@dusi dusi merged commit 388942f into master Aug 28, 2019
@dusi dusi deleted the set-koala-base-url-based-on-current-environment branch August 28, 2019 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants