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.

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

Track conversions on in-app message campaigns without a clickthrough URL #7306

Merged
merged 5 commits into from
Feb 4, 2021
Merged

Track conversions on in-app message campaigns without a clickthrough URL #7306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62351f8
Start tracking conversions on in-app message campaigns that don't hav…
christibbs Jan 14, 2021
88f89c4
Add test coverage, use FIRIAMMessageDefinition in implementation for …
christibbs Jan 14, 2021
df8a5c8
Add CHANGELOG
christibbs Jan 14, 2021
2155c7e
Update CHANGELOG
christibbs Jan 14, 2021
4564975
Merge branch 'master' into fix-conversion-tracking
christibbs Feb 4, 2021
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
3 changes: 3 additions & 0 deletions FirebaseInAppMessaging/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# 2021-2 -- v7.7.0
- [fixed] Fixed conversion tracking for in-app messages with a conversion event but not a button / action URL (#7306).
christibbs marked this conversation as resolved.
Show resolved Hide resolved

# 2021-1 -- v7.5.0
- [fixed] Fixed failed assertion causing app to crash during test on device flow (#7299).

Expand Down
6 changes: 4 additions & 2 deletions FirebaseInAppMessaging/Sources/Analytics/FIRIAMAnalyticsEventLoggerImpl.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,11 @@ - (void)logFAEventsForMessageActionWithCampaignID:(NSString *)campaignID
name:kFAEventNameForAction
parameters:params];
}
}

// set a special user property so that conversion events can be queried based on that
// for reporting purpose
- (void)logConversionTrackingEventForCampaignID:(NSString *)campaignID {
// Set a special user property so that conversion events can be queried based on that
// for reporting purposes.
NSString *conversionTrackingUserPropertyValue =
[NSString stringWithFormat:@"%@%@", kFAUserPropertyPrefixForFIAM, campaignID];

Expand Down
16 changes: 16 additions & 0 deletions FirebaseInAppMessaging/Sources/Flows/FIRIAMDisplayExecutor.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ - (void)messageClicked:(FIRInAppMessagingDisplayMessage *)inAppMessage
@"Logging analytics event for url following %@",
success ? @"succeeded" : @"failed");
}];

// Also start tracking conversions.
[self.analyticsEventLogger
logConversionTrackingEventForCampaignID:_currentMsgBeingDisplayed.renderData.messageID];
}
}

Expand Down Expand Up @@ -232,13 +236,25 @@ - (void)impressionDetectedForMessage:(FIRInAppMessagingDisplayMessage *)inAppMes
// Displayed long enough to be a valid impression.
[self recordValidImpression:_currentMsgBeingDisplayed.renderData.messageID
withMessageName:_currentMsgBeingDisplayed.renderData.name];

if ([self shouldTrackConversionsOnImpressionForCurrentInAppMessage:_currentMsgBeingDisplayed]) {
[self.analyticsEventLogger
logConversionTrackingEventForCampaignID:_currentMsgBeingDisplayed.renderData.messageID];
}
} else {
FIRLogDebug(kFIRLoggerInAppMessaging, @"I-IAM400011",
@"A test message. Record the test message impression event.");
return;
}
}

- (BOOL)shouldTrackConversionsOnImpressionForCurrentInAppMessage:
Copy link

Choose a reason for hiding this comment

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

Shouldn't this also include that the message should have conversion event defined?

If it doesn't we don't need to track it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We weren't checking on a conversion event previously, from comments (and piecing together the code) it looks like we currently always track the conversion and then presumably handle reporting on the backend. Do you think we need to check for this now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're waiting on Analytics to confirm how to proceed.

(FIRIAMMessageDefinition *)inAppMessage {
// If the message has no action URL, an impression is enough to start tracking conversions.
id<FIRIAMMessageContentData> contentData = inAppMessage.renderData.contentData;
return contentData.actionURL == nil && contentData.secondaryActionURL == nil;
}

- (void)displayErrorForMessage:(FIRInAppMessagingDisplayMessage *)inAppMessage
error:(NSError *)error {
__weak id<FIRInAppMessagingDisplayDelegate> appSideDelegate = self.inAppMessaging.delegate;
Expand Down
9 changes: 9 additions & 0 deletions FirebaseInAppMessaging/Sources/Private/Flows/FIRIAMAnalyticsEventLogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,5 +57,14 @@ typedef NS_ENUM(NSInteger, FIRIAMAnalyticsLogEventType) {
withCampaignName:(NSString *)campaignName
eventTimeInMs:(nullable NSNumber *)eventTimeInMs
completion:(void (^)(BOOL success))completion;

@optional
/**
* Adds an analytics log indicating that a campaign has been interacted with, and is therefore
* considered responsible for future conversion events as defined by the app developer during
* campaign creation. Only necessary if the implementing class talks to Firebase Analytics.
*/
- (void)logConversionTrackingEventForCampaignID:(NSString *)campaignID;

@end
NS_ASSUME_NONNULL_END
38 changes: 37 additions & 1 deletion FirebaseInAppMessaging/Tests/Unit/FIRIAMDisplayExecutorTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ @interface FIRIAMDisplayExecutor (Testing)
imageData:(FIRInAppMessagingImageData *)imageData
landscapeImageData:(nullable FIRInAppMessagingImageData *)landscapeImageData
triggerType:(FIRInAppMessagingDisplayTriggerType)triggerType;

- (BOOL)shouldTrackConversionsOnImpressionForCurrentInAppMessage:
(FIRIAMMessageDefinition *)inAppMessage;
@end

@interface FIRIAMDisplayExecutorTests : XCTestCase
Expand Down Expand Up @@ -799,7 +802,7 @@ - (void)testAnalyticsTrackingOnTestMessageDismissCase {
OCMVerifyAll((id)self.mockAnalyticsEventLogger);
}

- (void)testAnalyticsTrackingImpressionOnValidImpressionDetectedCase {
- (void)testAnalyticsTrackingImpressionOnValidImpressionDetectedCaseWithActionURL {
// This setup allows next message to be displayed from display interval perspective.
OCMStub([self.mockTimeFetcher currentTimestampInSeconds])
.andReturn(DISPLAY_MIN_INTERVALS * 60 + 100);
Expand All @@ -818,6 +821,39 @@ - (void)testAnalyticsTrackingImpressionOnValidImpressionDetectedCase {
initWithDelegateInteraction:FIRInAppMessagingDelegateInteractionImpressionDetected];
self.displayExecutor.messageDisplayComponent = display;

// M2 has an action URL. Conversion shouldn't be tracked yet.
XCTAssertFalse(
[self.displayExecutor shouldTrackConversionsOnImpressionForCurrentInAppMessage:self.m2]);

[self.displayExecutor checkAndDisplayNextAppForegroundMessage];
OCMVerifyAll((id)self.mockAnalyticsEventLogger);
}

- (void)testAnalyticsTrackingImpressionOnValidImpressionDetectedCaseWithoutActionURL {
// This setup allows next message to be displayed from display interval perspective.
OCMStub([self.mockTimeFetcher currentTimestampInSeconds])
.andReturn(DISPLAY_MIN_INTERVALS * 60 + 100);

// not expecting triggering analytics recording
OCMExpect([self.mockAnalyticsEventLogger
logAnalyticsEventForType:FIRIAMAnalyticsEventMessageImpression
forCampaignID:[OCMArg isEqual:self.m5.renderData.messageID]
withCampaignName:[OCMArg any]
eventTimeInMs:[OCMArg any]
completion:[OCMArg any]]);

[self.clientMessageCache setMessageData:@[ self.m5 ]];

FIRIAMMessageDisplayForTesting *display = [[FIRIAMMessageDisplayForTesting alloc]
initWithDelegateInteraction:FIRInAppMessagingDelegateInteractionImpressionDetected];
self.displayExecutor.messageDisplayComponent = display;

// M5 has no action URL. Conversion should be tracked after impression.
OCMExpect([self.mockAnalyticsEventLogger
logConversionTrackingEventForCampaignID:[OCMArg isEqual:self.m5.renderData.messageID]]);
XCTAssertTrue(
[self.displayExecutor shouldTrackConversionsOnImpressionForCurrentInAppMessage:self.m5]);

[self.displayExecutor checkAndDisplayNextAppForegroundMessage];
OCMVerifyAll((id)self.mockAnalyticsEventLogger);
}
Expand Down