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

[MOB-10094] merging bug fix #827

Merged
merged 6 commits into from
Nov 6, 2024
Merged

Conversation

evantk91
Copy link
Contributor

@evantk91 evantk91 commented Nov 1, 2024

🔹 Jira Ticket(s) if any

✏️ Description

Moving the clearing of the anonymous user id within the attemptMergeAndEventReplay function after the merge function is called allows the merge function to be called with the proper parameters

@evantk91 evantk91 requested a review from Ayyanchira November 1, 2024 19:38
Copy link

codecov bot commented Nov 4, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 60.10%. Comparing base (527acc4) to head (9bd4618).
Report is 8 commits behind head on feature/itbl-track-anon-user.

Files with missing lines Patch % Lines
...ain/java/com/iterable/iterableapi/IterableApi.java 88.88% 1 Missing ⚠️
Additional details and impacted files
@@                       Coverage Diff                        @@
##           feature/itbl-track-anon-user     #827      +/-   ##
================================================================
- Coverage                         60.17%   60.10%   -0.08%     
================================================================
  Files                                86       86              
  Lines                              5961     5960       -1     
  Branches                            756      756              
================================================================
- Hits                               3587     3582       -5     
- Misses                             2001     2003       +2     
- Partials                            373      375       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@Ayyanchira Ayyanchira left a comment

Choose a reason for hiding this comment

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

LGTM. JWT flow needs to be tested properly though

} else {
getAuthManager().requestNewAuthToken(false, data -> attemptMergeAndEventReplay(userIdOrEmail, isEmail, merge, replay, failureHandler));
getAuthManager().requestNewAuthToken(false, data -> attemptMergeAndEventReplay(userIdOrEmail, isEmail, merge, replay, isAnon, failureHandler));
Copy link
Member

Choose a reason for hiding this comment

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

From a JWT perspective, we will have to check how it plays with timings of JWT RetryPolicy. It might have to go through scheduling a new request instead of requesting right away.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I will make a ticket for tracking.

@evantk91 evantk91 merged commit dd21046 into feature/itbl-track-anon-user Nov 6, 2024
4 checks passed
@evantk91 evantk91 deleted the evan/MOB-10094 branch November 6, 2024 15:18
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.

2 participants