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
| registerForPush(); //TODO: FIX, THE LOGIN NEVER CALLS THE CALLBACK IF THE AUTOPUSH REGISTRATION IS TRUE | ||
| } | ||
| if (_setUserSuccessCallbackHandler != null) { // todo: why is there an else if it can be both true | ||
| _setUserSuccessCallbackHandler.onSuccess(new JSONObject()); // passing blank json object here as onSuccess is @Nonnull |
There was a problem hiding this comment.
Problem here is we need the registerPush() to be passing a completion handler so that we can pass the correct onSuccess Object
There was a problem hiding this comment.
I am not sure I understand it, the registerPush should be passing a completion handler to where? The else if that was here basically meant that the completionHandler was never called and what caused the double callback only when autoRegisterForPush was false
There was a problem hiding this comment.
Thanks for asking that question. I didnt do a good job up there.
What I meant was, because the registerForPush() is itself a void method, it is a fire and forget method. The next line of code will not wait for success of failure from that. Therefore, calling a successCallbackHandler right after registerForPush() may give incorrect result for the parent method setEmail(email, successahandler, failureHandler)
To mitigate that, we might have to make registerDevice async as well where we get result back and decide to call success or failure callback.
Perhaps in setEmail case, we directly call the callback from api level and registerForPush() doesnt need callback. Free to hear more
🔹 Jira Ticket(s) if any
✏️ Description
Tests for fixing the double callbacks and related issues