Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public class IterableApi {
private IterableNotificationData _notificationData;
private String _deviceId;
private boolean _firstForegroundHandled;
private ResultCallbackHandler callbackHandler;
Copy link
Member

Choose a reason for hiding this comment

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

Trying to see if callback handler should be stored globally in this file. When is the value assigned to this variable? Also because many requests can trigger at same time, this might lead to some race conditions


IterableApiClient apiClient = new IterableApiClient(new IterableApiAuthProvider());
private @Nullable IterableInAppManager inAppManager;
Expand Down Expand Up @@ -681,14 +682,15 @@ public void trackPushOpen(int campaignId, int templateId, @NonNull String messag
/**
* Consumes an InApp message.
* @param messageId
* @param callbackHandler The callback which returns `success` or `failure`.
*/
public void inAppConsume(@NonNull String messageId) {
public void inAppConsume(@NonNull String messageId, @Nullable ResultCallbackHandler callbackHandler) {
IterableInAppMessage message = getInAppManager().getMessageById(messageId);
if (message == null) {
IterableLogger.e(TAG, "inAppConsume: message is null");
return;
}
inAppConsume(message, null, null);
inAppConsume(message, null, null, callbackHandler);
IterableLogger.printInfo();
}

Expand All @@ -700,13 +702,13 @@ public void inAppConsume(@NonNull String messageId) {
* @param message message object
* @param source An enum describing how the in App delete was triggered
* @param clickLocation The module in which the action happened
* @param callbackHandler The callback which returns `success` or `failure`.
*/
public void inAppConsume(@NonNull IterableInAppMessage message, @Nullable IterableInAppDeleteActionType source, @Nullable IterableInAppLocation clickLocation) {
public void inAppConsume(@NonNull IterableInAppMessage message, @Nullable IterableInAppDeleteActionType source, @Nullable IterableInAppLocation clickLocation, @Nullable ResultCallbackHandler callbackHandler) {
Copy link
Member

Choose a reason for hiding this comment

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

Will need a additional method with new parameter instead of changing this one, as this is a public method which could be used currently - to avoid breaking changes

if (!checkSDKInitialization()) {
return;
}

apiClient.inAppConsume(message, source, clickLocation, inboxSessionId);
apiClient.inAppConsume(message, source, clickLocation, inboxSessionId, callbackHandler);
Copy link
Member

Choose a reason for hiding this comment

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

two callbackHandlers (success and failure) instead of one for consistency.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ void trackInAppDelivery(@NonNull IterableInAppMessage message) {
}
}

public void inAppConsume(@NonNull IterableInAppMessage message, @Nullable IterableInAppDeleteActionType source, @Nullable IterableInAppLocation clickLocation, @Nullable String inboxSessionId) {
public void inAppConsume(@NonNull IterableInAppMessage message, @Nullable IterableInAppDeleteActionType source, @Nullable IterableInAppLocation clickLocation, @Nullable String inboxSessionId, @Nullable final ResultCallbackHandler callbackHandler) {
Copy link
Member

Choose a reason for hiding this comment

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

Use two separate callbacks.
One for success and failure.
And reuse - IterableHelper.success and failure callbacks in the method signature.

JSONObject requestJSON = new JSONObject();

try {
Expand All @@ -333,8 +333,22 @@ public void inAppConsume(@NonNull IterableInAppMessage message, @Nullable Iterab
if (clickLocation == IterableInAppLocation.INBOX) {
addInboxSessionID(requestJSON, inboxSessionId);
}

sendPostRequest(IterableConstants.ENDPOINT_INAPP_CONSUME, requestJSON);

sendPostRequest(IterableConstants.ENDPOINT_INAPP_CONSUME, requestJSON, new IterableHelper.SuccessHandler() {
@Override
public void onSuccess(@NonNull JSONObject data) {
if (callbackHandler != null) {
callbackHandler.sendResult(true);
}
}
}, new IterableHelper.FailureHandler() {
@Override
public void onFailure(@NonNull String reason, @Nullable JSONObject data) {
if (callbackHandler != null) {
callbackHandler.sendResult(false);
}
}
});
} catch (JSONException e) {
e.printStackTrace();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ private void processMessageRemoval() {
}

if (message.isMarkedForDeletion() && !message.isConsumed()) {
IterableApi.sharedInstance.getInAppManager().removeMessage(message);
IterableApi.sharedInstance.getInAppManager().removeMessage(message, null);
Copy link
Member

Choose a reason for hiding this comment

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

This can stay like before. The new method with new parameters will be for explicit use by developers.

}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,13 @@ public synchronized int getUnreadInboxMessagesCount() {
* Set the read flag on an inbox message
* @param message Inbox message object retrieved from {@link IterableInAppManager#getInboxMessages()}
* @param read Read state flag. true = read, false = unread
* @param callbackHandler The callback which returns `success` or `failure`.
*/
public synchronized void setRead(@NonNull IterableInAppMessage message, boolean read) {
public synchronized void setRead(@NonNull IterableInAppMessage message, boolean read, @Nullable ResultCallbackHandler callbackHandler) {
Copy link
Member

Choose a reason for hiding this comment

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

Marking as read / setRead can be a separate PR of its own. This PR can confine to removeMessage

message.setRead(read);
if(callbackHandler != null){
callbackHandler.sendResult(true);
}
Copy link
Member

Choose a reason for hiding this comment

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

This will be part of next PR for setRead

notifyOnChange();
}

Expand Down Expand Up @@ -249,17 +253,18 @@ public void execute(Uri url) {
/**
* Remove message from the list
* @param message The message to be removed
* @param callbackHandler The callback which returns `success` or `failure`.
*/
public synchronized void removeMessage(@NonNull IterableInAppMessage message) {
public synchronized void removeMessage(@NonNull IterableInAppMessage message, @Nullable ResultCallbackHandler callbackHandler) {
Copy link
Member

Choose a reason for hiding this comment

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

ResultCallbackHandler -> IterableCallbackHandler for success and failure -> in a new method

message.setConsumed(true);
api.inAppConsume(message.getMessageId());
api.inAppConsume(message.getMessageId(), callbackHandler);
notifyOnChange();
}

public synchronized void removeMessage(@NonNull IterableInAppMessage message, @NonNull IterableInAppDeleteActionType source, @NonNull IterableInAppLocation clickLocation) {
public synchronized void removeMessage(@NonNull IterableInAppMessage message, @NonNull IterableInAppDeleteActionType source, @NonNull IterableInAppLocation clickLocation, @Nullable ResultCallbackHandler callbackHandler) {
Copy link
Member

Choose a reason for hiding this comment

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

ResultCallbackHandler > Iterable success and failure callback handler.
A new method needed

IterableLogger.printInfo();
message.setConsumed(true);
api.inAppConsume(message, source, clickLocation);
api.inAppConsume(message, source, clickLocation, callbackHandler);
notifyOnChange();
}

Expand Down
Copy link
Member

Choose a reason for hiding this comment

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

public interface SuccessCallback<T> {
void onSuccess(T result);
}
public interface FailureCallback {
void onFailure(Throwable throwable);
}

Wondering if these ^ existing callbacks can be reused for callbacks instead of creating new Interface ? or will it be good to have them for sanity/separation...

Copy link
Member

Choose a reason for hiding this comment

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

To use Iterable pre existing callbacks

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.iterable.iterableapi;

/**
* Handler to pass result of success/failure for setemail/setuserid
*/
public interface ResultCallbackHandler {

/**
* Callback called when registertoken API hits
* @param success can be true or false based on result of registertoken API success/failure
*/
void sendResult(boolean success);
}
Original file line number Diff line number Diff line change
Expand Up @@ -78,18 +78,57 @@ public void testInboxMessageOrdering() throws Exception {
assertEquals("message4", inboxMessages.get(1).getMessageId());
}

@Test
public void testRemoveMessage() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

🙌

dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(IterableTestUtils.getResourceString("inapp_payload_inbox_multiple.json")));
IterableInAppManager inAppManager = IterableApi.getInstance().getInAppManager();
inAppManager.syncInApp();
shadowOf(getMainLooper()).idle();
List<IterableInAppMessage> inboxMessages = inAppManager.getInboxMessages();
assertEquals(2, inboxMessages.size());
assertEquals(1, inAppManager.getUnreadInboxMessagesCount());
inAppManager.removeMessage(inboxMessages.get(0), IterableInAppLocation.inbox, IterableInAppDeleteSource.deleteButton, new IterableHelper.SuccessHandler() {
@Override
Copy link
Member

Choose a reason for hiding this comment

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

Reverify this block to see if it works as expected as the callback has different handler in the definition above.

public void onSuccess() {
assertEquals(1, inAppManager.getInboxMessages().size());
assertEquals("message2", inAppManager.getInboxMessages().get(0).getMessageId());
}
});
assertEquals(1, inAppManager.getInboxMessages().size());
assertEquals("message2", inAppManager.getInboxMessages().get(0).getMessageId());
}

@Test
public void testSetRead() throws Exception {
// Set up mock response
Copy link
Member

Choose a reason for hiding this comment

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

Test set read can be part of next PR

dispatcher.enqueueResponse("/inApp/getMessages", new MockResponse().setBody(IterableTestUtils.getResourceString("inapp_payload_inbox_multiple.json")));

// Initialize in-app manager and wait for messages to be synced
IterableInAppManager inAppManager = IterableApi.getInstance().getInAppManager();
inAppManager.syncInApp();
shadowOf(getMainLooper()).idle();

// Get inbox messages
List<IterableInAppMessage> inboxMessages = inAppManager.getInboxMessages();

// Verify initial state
assertEquals(1, inAppManager.getUnreadInboxMessagesCount());
assertEquals(2, inboxMessages.size());
assertFalse(inboxMessages.get(0).isRead());
assertTrue(inboxMessages.get(1).isRead());
inAppManager.setRead(inboxMessages.get(0), true);

// Set first message as read with a callback
boolean[] callbackCalled = { false };
inAppManager.setRead(inboxMessages.get(0), true, success -> {
callbackCalled[0] = true;
assertTrue(success);
});

// Wait for callback to be called
shadowOf(getMainLooper()).idle();

// Verify that callback was called and that message is marked as read
assertTrue(callbackCalled[0]);
assertEquals(0, inAppManager.getUnreadInboxMessagesCount());
assertEquals(2, inAppManager.getInboxMessages().size());
assertTrue(inboxMessages.get(0).isRead());
Expand Down