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

Splitting responsibilities of analytics provider #1884

Merged
merged 7 commits into from
Nov 8, 2024

Conversation

erenbesel
Copy link
Contributor

@erenbesel erenbesel commented Oct 28, 2024

Summary

Splits analytics provider responsibilities into two by moving the conditional follow up events on its own class EventAnalyticsProvider with the timer/batching logic.
This object is only created by the context if the configuration is turned on.
The original analytics provider is now only responsible for the initial call, is not aware of any of the events provider's lifetime and only passes the calls to it.

Ticket

COIOS-815

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['new', 'changed', 'fixed', 'removed', 'deprecated', 'chore', 'improvement']

atmamont
atmamont previously approved these changes Oct 28, 2024
Adyen/Core/AdyenContext/AdyenContext.swift Show resolved Hide resolved
@erenbesel erenbesel added the chore a pull request that has chore changes that shouldn't be in the release notes label Oct 30, 2024
github-actions[bot]
github-actions bot previously approved these changes Oct 30, 2024
goergisn
goergisn previously approved these changes Oct 30, 2024
github-actions[bot]
github-actions bot previously approved these changes Nov 6, 2024
@erenbesel erenbesel marked this pull request as ready for review November 6, 2024 15:54
@erenbesel erenbesel changed the title Testing splitting responsibilities of analytics provider Splitting responsibilities of analytics provider Nov 6, 2024
atmamont
atmamont previously approved these changes Nov 7, 2024
Copy link
Contributor

@atmamont atmamont left a comment

Choose a reason for hiding this comment

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

Nice job Eren! 🥂
Some comments regarding tests readability.

goergisn
goergisn previously approved these changes Nov 7, 2024
Copy link
Contributor

@goergisn goergisn left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Contributor

github-actions bot commented Nov 8, 2024

✅ No changes detected

Comparing https://github.com/Adyen/adyen-ios.git @ analytics_provider_refactor to https://github.com/Adyen/adyen-ios.git @ develop


Analyzed targets: Adyen, AdyenActions, AdyenCard, AdyenCashAppPay, AdyenComponents, AdyenDelegatedAuthentication, AdyenDropIn, AdyenEncryption, AdyenSession, AdyenSwiftUI, AdyenTwint, AdyenWeChatPay

Copy link

sonarcloud bot commented Nov 8, 2024

@erenbesel erenbesel merged commit 785fd56 into develop Nov 8, 2024
13 checks passed
@erenbesel erenbesel deleted the analytics_provider_refactor branch November 8, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore a pull request that has chore changes that shouldn't be in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants