Comments and changes to fix the double callback problem #978
Conversation
| void tryMergeUser(IterableApiClient apiClient, String unknownUserId, String destinationUser, boolean isEmail, boolean merge, MergeResultCallback callback) { | ||
| IterableLogger.v(TAG, "tryMergeUser"); | ||
| if (unknownUserId != null && merge) { | ||
| if (unknownUserId != null && merge) { //todo: why can we try to merge and have merge false? |
There was a problem hiding this comment.
Can you elaborate the question here?
There was a problem hiding this comment.
Yes, i feel like it can be more of a semantical question in the regard of the method beingCalled tryMergeUser and then we get a flag that can say not to merge
Ayyanchira
left a comment
Ayyanchira
left a comment
There was a problem hiding this comment.
Added some comments @franco-zalamena-iterable
| if (config.enableUnknownUserActivation && getVisitorUsageTracked()) { | ||
|
|
||
| if (emailOrUserId != null && !emailOrUserId.equals(_userIdUnknown)) { | ||
| if (emailOrUserId != null && !emailOrUserId.equals(_userIdUnknown)) { //todo: when would the userIdUnknown be the same? |
There was a problem hiding this comment.
This is a part of feature called unknownUser. Will have to audit the algorithm here to sit and verify if its as expected
iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java
Outdated
Show resolved
Hide resolved
|
The new commit has a new way of handling the callbacks, this will be enough for the client that currently receives 2 callbacks since they can choose to only listen to the last one. Also, not sure if making the actual changes to make the callback null make sense now, since there are places that this can be used. I will be reverting the callback to null fix so we can keep the behaviour as is today and make a different PR for better handling the callback lifecycle |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * remote and local success cases. | ||
| */ | ||
| public interface RemoteSuccessCallback extends IterableSuccessCallback { | ||
| void onSuccess(@NonNull IterableResponseObject.RemoteSuccess data); |
Check notice
Code scanning / CodeQL
Confusing overloading of methods Note
| * won't need to use this callback directly. | ||
| */ | ||
| public interface AuthTokenCallback extends IterableSuccessCallback { | ||
| void onSuccess(@NonNull IterableResponseObject.AuthTokenSuccess data); |
Check notice
Code scanning / CodeQL
Confusing overloading of methods Note
| * won't need to use this callback directly. | ||
| */ | ||
| public interface AuthTokenCallback extends IterableSuccessCallback { | ||
| void onSuccess(@NonNull IterableResponseObject.AuthTokenSuccess data); |
Check notice
Code scanning / CodeQL
Useless parameter Note
| * local and remote success cases. | ||
| */ | ||
| public interface LocalSuccessCallback extends IterableSuccessCallback { | ||
| void onSuccess(@NonNull IterableResponseObject.LocalSuccess data); |
Check notice
Code scanning / CodeQL
Confusing overloading of methods Note
| * @param successHandler The callback which returns `success`. | ||
| */ | ||
| public synchronized void setRead(@NonNull IterableInAppMessage message, boolean read, @Nullable IterableHelper.SuccessHandler successHandler, @Nullable IterableHelper.FailureHandler failureHandler) { | ||
| public synchronized void setRead(@NonNull IterableInAppMessage message, boolean read, @Nullable IterableHelper.IterableSuccessCallback successHandler, @Nullable IterableHelper.FailureHandler failureHandler) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
🔹 Jira Ticket(s) if any
✏️ Description
Tests for fixing the double callbacks and related issues