Skip to content

ITBL-1286 Supports messageID for push opens on android. #6

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.

Sign up for GitHub

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

Merged
davidtruong merged 1 commit into from
Sep 15, 2016
Merged

ITBL-1286 Supports messageID for push opens on android. #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions iterableapi/src/main/java/com/iterable/iterableapi/IterableApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,14 @@ protected void registerForPush(String iterableAppId, String gcmProjectNumber, bo
* @param campaignId
* @param templateId
*/
protected void trackPushOpen(int campaignId, int templateId) {
protected void trackPushOpen(int campaignId, int templateId, String messageId) {
Copy link

@jontsai jontsai Aug 30, 2016

Choose a reason for hiding this comment

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

are we guaranteed to always have a messageId?

Copy link
Contributor Author

@davidtruong davidtruong Aug 30, 2016

Choose a reason for hiding this comment

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

Are you always passing down a messageId along with every message sent from our servers (similar to campaignID/templateId)? If not then I'll check for an optional messageID.

Copy link

@jontsai jontsai Aug 30, 2016

Choose a reason for hiding this comment

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

Yeah, I think it should always be set. In case you find that we aren't, lmk and I'll fix it.

JSONObject requestJSON = new JSONObject();

try {
requestJSON.put(IterableConstants.KEY_EMAIL, _email);
requestJSON.put(IterableConstants.KEY_CAMPAIGNID, campaignId);
requestJSON.put(IterableConstants.KEY_TEMPLATE_ID, templateId);

requestJSON.put(IterableConstants.KEY_MESSAGE_ID, messageId);
}
catch (JSONException e) {
e.printStackTrace();
Expand Down
1 change: 1 addition & 0 deletions iterableapi/src/main/java/com/iterable/iterableapi/IterableConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public final class IterableConstants {
public static final String KEY_RECIPIENT_EMAIL = "recipientEmail";
public static final String KEY_SEND_AT = "sendAt";
public static final String KEY_TEMPLATE_ID = "templateId";
public static final String KEY_MESSAGE_ID = "messageId";
Copy link

@jontsai jontsai Aug 30, 2016

Choose a reason for hiding this comment

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

looks like spacing didn't line up

public static final String KEY_TOKEN = "token";
public static final String KEY_PLATFORM = "platform";
public static final String KEY_APPLICATIONNAME = "applicationName";
Expand Down
7 changes: 7 additions & 0 deletions iterableapi/src/main/java/com/iterable/iterableapi/IterableNotificationData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
class IterableNotificationData {
private int campaignId;
private int templateId;
private String messageId;
private boolean isGhostPush;

IterableNotificationData(String data){
Expand All @@ -22,6 +23,10 @@ class IterableNotificationData {
templateId = iterableJson.getInt(IterableConstants.KEY_TEMPLATE_ID);
}

if (iterableJson.has(IterableConstants.KEY_MESSAGE_ID)) {
messageId = iterableJson.getString(IterableConstants.KEY_MESSAGE_ID);
}

if (iterableJson.has(IterableConstants.IS_GHOST_PUSH)) {
isGhostPush = iterableJson.getBoolean(IterableConstants.IS_GHOST_PUSH);
}
Expand All @@ -40,6 +45,8 @@ public int getTemplateId()
return this.templateId;
}

public String getMessageId() { return this.messageId; }
Copy link

@jontsai jontsai Aug 30, 2016

Choose a reason for hiding this comment

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

this should be fine going forward right? before we make the change on the server, the messageId won't exist yet. so... does this need to be made an option? or we just synchronize our deploy of server code with release of SDK update?

Copy link
Contributor Author

@davidtruong davidtruong Aug 30, 2016

Choose a reason for hiding this comment

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

The server code should go out first since the old SDK won't process anything related to messageID. I'll deploy the mobile SDKs afterward.


public boolean getIsGhostPush()
{
return this.isGhostPush;
Expand Down
2 changes: 1 addition & 1 deletion iterableapi/src/main/java/com/iterable/iterableapi/IterablePushOpenReceiver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public void onReceive(Context context, Intent intent) {
IterableNotificationData iterableNotificationData = new IterableNotificationData(iterableDataString);
if (IterableApi.sharedInstance != null) {
IterableApi.sharedInstance.setNotificationData(iterableNotificationData);
IterableApi.sharedInstance.trackPushOpen(iterableNotificationData.getCampaignId(), iterableNotificationData.getTemplateId());
IterableApi.sharedInstance.trackPushOpen(iterableNotificationData.getCampaignId(), iterableNotificationData.getTemplateId(), iterableNotificationData.getMessageId());
Copy link
Contributor

@dontgitit dontgitit Aug 30, 2016

Choose a reason for hiding this comment

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

seems like it would make sense to use the iterableNotificationData that you just set? Or pass in the whole object? Rather than each param? But this is fine too

}
}
}
Expand Down