Fix bug where AnalyticsServices fires same event multiple times #1210
Conversation
This file contains 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
… AnalyticsService.sendAnalyticEvent() method call Signed-off-by: Jax DesMarais-Leder <[email protected]>
Signed-off-by: Jax DesMarais-Leder <[email protected]>
scannillo
commented
Mar 7, 2024
| apiClient.fetchOrReturnRemoteConfiguration { configuration, error in | ||
| guard let configuration, error == nil else { | ||
| if let error { | ||
| completion(error) |
There was a problem hiding this comment.
This completion was always ignored, so we removed it entirely
There was a problem hiding this comment.
Curious why this is happening, thoughts?
There was a problem hiding this comment.
BTAPIClient was always no-oping the result of this completion. I don't think we would ever want to return an error to the merchant and stop the flow if there is an issue with analytics since that's just for us, so this is safe for us to remove since we just discarded the result.
jaxdesmarais
approved these changes
Mar 8, 2024
Co-authored-by: Jax DesMarais-Leder <[email protected]>
# Conflicts: # CHANGELOG.md # Sources/BraintreeCore/Analytics/BTAnalyticsService.swift # Sources/BraintreeCore/BTAPIClient.swift # UnitTests/BraintreeCoreTests/Analytics/FakeAnalyticsService.swift
agedd
approved these changes
Mar 11, 2024
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.
Summary
sendAnalyticsEvent()was called, but with this change, the payload won't contain duplicate eventsChanges
flushThreshold& flush logic (this was never called within the SDK)DispatchQueuefor holding events. This was never getting cleared, thus sending every event multiple times. We can re-add this when we have more time to properly implement the queue's maintenance and clearance.BTAnalyticsService.sendAnalyticsEvent()methodChecklist
Authors
@scannillo @jaxdesmarais