-
Notifications
You must be signed in to change notification settings - Fork 3
[REM-3440] Viewable impressions tracking #167
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.
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
Changes from all commits
1a4137f
4a3f504
023f64c
c61d3a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ import io.constructor.data.model.browse.* | |
| import io.constructor.data.model.common.ResultGroup | ||
| import io.constructor.data.model.common.TrackingItem | ||
| import io.constructor.data.model.common.VariationsMap | ||
| import io.constructor.data.model.common.ResultsImpressionItem | ||
| import io.constructor.data.model.conversion.ConversionRequestBody | ||
| import io.constructor.data.model.purchase.PurchaseItem | ||
| import io.constructor.data.model.purchase.PurchaseRequestBody | ||
|
|
@@ -25,6 +26,7 @@ import io.constructor.data.model.search.* | |
| import io.constructor.data.model.tracking.GenericResultClickRequestBody | ||
| import io.constructor.data.model.tracking.ItemDetailLoadRequestBody | ||
| import io.constructor.data.model.tracking.MediaImpressionRequestBody | ||
| import io.constructor.data.model.tracking.ResultsImpressionViewRequestBody | ||
| import io.constructor.injection.component.AppComponent | ||
| import io.constructor.injection.component.DaggerAppComponent | ||
| import io.constructor.injection.module.AppModule | ||
|
|
@@ -1949,6 +1951,14 @@ object ConstructorIo { | |
|
|
||
| /** | ||
| * Tracks media impression view events. | ||
| * | ||
| * Example: | ||
| * ``` | ||
| * ConstructorIo.trackMediaImpressionView("banner-123", "placement-456") | ||
| * ``` | ||
| * @param bannerAdId the id of the banner ad viewed | ||
| * @param placementId the id of the placement | ||
| * @param analyticsTags Additional analytics tags to pass | ||
| */ | ||
| fun trackMediaImpressionView(bannerAdId: String, placementId: String, analyticsTags: Map<String, String>? = null) { | ||
| val completable = trackMediaImpressionViewInternal(bannerAdId, placementId, analyticsTags) | ||
|
|
@@ -1959,6 +1969,14 @@ object ConstructorIo { | |
|
|
||
| /** | ||
| * Tracks media impression click events. | ||
| * | ||
| * Example: | ||
| * ``` | ||
| * ConstructorIo.trackMediaImpressionClick("banner-123", "placement-456") | ||
| * ``` | ||
| * @param bannerAdId the id of the banner ad clicked | ||
| * @param placementId the id of the placement | ||
| * @param analyticsTags Additional analytics tags to pass | ||
| */ | ||
| fun trackMediaImpressionClick(bannerAdId: String, placementId: String, analyticsTags: Map<String, String>? = null) { | ||
| val completable = trackMediaImpressionClickInternal(bannerAdId, placementId, analyticsTags) | ||
|
|
@@ -1967,6 +1985,27 @@ object ConstructorIo { | |
| })) | ||
| } | ||
|
|
||
| /** | ||
| * Tracks results impression view events (items that were viewably impressed per MRC guidelines). | ||
| * | ||
| * Example: | ||
| * ``` | ||
| * ConstructorIo.trackResultsImpressionView(items, "shoes") | ||
| * ``` | ||
| * @param items the list of items that were viewably impressed | ||
| * @param searchTerm the search term associated with the impression | ||
| * @param filterName the name of the filter applied | ||
| * @param filterValue the value of the filter applied | ||
| * @param analyticsTags Additional analytics tags to pass | ||
| */ | ||
| fun trackResultsImpressionView(items: List<ResultsImpressionItem>, searchTerm: String? = null, filterName: String? = null, filterValue: String? = null, analyticsTags: Map<String, String>? = null) { | ||
|
Alexey-Pavlov marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important Issue: The method signature allows
This same coupling issue exists on the request body itself. |
||
| if (items.isEmpty()) return | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Silently returning on empty if (items.isEmpty()) {
w("trackResultsImpressionView called with empty items list, skipping")
return
} |
||
| val completable = trackResultsImpressionViewInternal(items, searchTerm, filterName, filterValue, analyticsTags) | ||
| disposable.add(completable.subscribeOn(Schedulers.io()).subscribe({}, { t -> | ||
| e("Results Impression View error: ${t.message}") | ||
| })) | ||
| } | ||
|
|
||
| internal fun trackItemDetailLoadedInternal(itemName: String, customerId: String, variationId: String? = null, sectionName: String? = null, url: String = "Not Available", analyticsTags: Map<String, String>? = null): Completable { | ||
| preferenceHelper.getSessionId(sessionIncrementHandler) | ||
| val section = sectionName ?: preferenceHelper.defaultItemSection | ||
|
|
@@ -2031,6 +2070,29 @@ object ConstructorIo { | |
| return dataManager.trackMediaImpressionClick(preferenceHelper, requestBody) | ||
| } | ||
|
|
||
| internal fun trackResultsImpressionViewInternal(items: List<ResultsImpressionItem>, searchTerm: String? = null, filterName: String? = null, filterValue: String? = null, analyticsTags: Map<String, String>? = null): Completable { | ||
|
Alexey-Pavlov marked this conversation as resolved.
|
||
| if (items.isEmpty()) return Completable.complete() | ||
|
|
||
| preferenceHelper.getSessionId(sessionIncrementHandler) | ||
| val requestBody = ResultsImpressionViewRequestBody( | ||
| items, | ||
| searchTerm, | ||
| filterName, | ||
| filterValue, | ||
| mergeAnalyticsTags(configMemoryHolder.defaultAnalyticsTags, analyticsTags), | ||
| true, | ||
| BuildConfig.CLIENT_VERSION, | ||
| preferenceHelper.id, | ||
| preferenceHelper.getSessionId(), | ||
| preferenceHelper.apiKey, | ||
| configMemoryHolder.userId, | ||
| configMemoryHolder.segments, | ||
| System.currentTimeMillis() | ||
| ) | ||
|
|
||
| return dataManager.trackResultsImpressionView(requestBody) | ||
| } | ||
|
|
||
| /** | ||
| * Tracks generic result click events. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ import io.constructor.data.model.recommendations.RecommendationsResponse | |
| import io.constructor.data.model.search.* | ||
| import io.constructor.data.model.tracking.GenericResultClickRequestBody | ||
| import io.constructor.data.model.tracking.MediaImpressionRequestBody | ||
| import io.constructor.data.model.tracking.ResultsImpressionViewRequestBody | ||
| import io.constructor.data.remote.ApiPaths | ||
| import io.constructor.data.remote.ConstructorApi | ||
| import io.constructor.injection.ConstructorSdk | ||
|
|
@@ -314,6 +315,10 @@ constructor(private val constructorApi: ConstructorApi, @ConstructorSdk private | |
| return constructorApi.trackQuizConversion(quizConversionRequestBody, params.toMap()) | ||
| } | ||
|
|
||
| fun trackResultsImpressionView(resultsImpressionViewRequestBody: ResultsImpressionViewRequestBody, params: Array<Pair<String, String>> = arrayOf()): Completable { | ||
|
constructor-claude-bedrock[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The |
||
| return constructorApi.trackResultsImpressionView(resultsImpressionViewRequestBody, params.toMap()) | ||
| } | ||
|
|
||
| fun trackMediaImpressionView(preferencesHelper: PreferencesHelper, mediaImpressionRequestBody: MediaImpressionRequestBody): Completable { | ||
| val url = buildMediaUrl(preferencesHelper, ApiPaths.URL_MEDIA_IMPRESSION_VIEW_EVENT) | ||
| return constructorApi.trackMediaImpressionView(url, mediaImpressionRequestBody) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package io.constructor.data.model.common | ||
|
|
||
| import com.squareup.moshi.Json | ||
| import com.squareup.moshi.JsonClass | ||
| import java.io.Serializable | ||
|
|
||
| @JsonClass(generateAdapter = true) | ||
| data class ResultsImpressionItem( | ||
| @Json(name = "item_id") val itemId: String, | ||
| @Json(name = "item_name") val itemName: String, | ||
| @Json(name = "variation_id") val variationId: String? = null, | ||
| @Json(name = "sl_campaign_id") val slCampaignId: String? = null, | ||
| @Json(name = "sl_campaign_owner") val slCampaignOwner: String? = null | ||
| ) : Serializable | ||
|
constructor-claude-bedrock[bot] marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| package io.constructor.data.model.tracking | ||
|
|
||
| import com.squareup.moshi.Json | ||
| import com.squareup.moshi.JsonClass | ||
| import io.constructor.data.model.common.ResultsImpressionItem | ||
| import java.io.Serializable | ||
|
|
||
| @JsonClass(generateAdapter = true) | ||
| data class ResultsImpressionViewRequestBody( | ||
| @Json(name = "items") val items: List<ResultsImpressionItem>, | ||
| @Json(name = "search_term") val searchTerm: String?, | ||
| @Json(name = "filter_name") val filterName: String?, | ||
| @Json(name = "filter_value") val filterValue: String?, | ||
| @Json(name = "analytics_tags") val analyticsTags: Map<String, String>?, | ||
| @Json(name = "beacon") val beacon: Boolean = true, | ||
| @Json(name = "c") val c: String, | ||
| @Json(name = "i") val i: String, | ||
| @Json(name = "s") val s: Int, | ||
| @Json(name = "key") val key: String, | ||
| @Json(name = "ui") val ui: String?, | ||
| @Json(name = "us") val us: List<String?>, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Important Issue: The |
||
| @Json(name = "_dt") val _dt: Long? | ||
|
constructor-claude-bedrock[bot] marked this conversation as resolved.
constructor-claude-bedrock[bot] marked this conversation as resolved.
|
||
| ) : Serializable | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| package io.constructor.core | ||
|
|
||
| import android.content.Context | ||
| import io.constructor.data.local.PreferencesHelper | ||
| import io.constructor.data.memory.ConfigMemoryHolder | ||
| import io.constructor.data.model.common.ResultsImpressionItem | ||
| import io.constructor.test.createTestDataManager | ||
| import io.constructor.util.RxSchedulersOverrideRule | ||
| import io.mockk.every | ||
| import io.mockk.mockk | ||
| import okhttp3.mockwebserver.MockResponse | ||
| import okhttp3.mockwebserver.MockWebServer | ||
| import okhttp3.mockwebserver.RecordedRequest | ||
| import org.junit.After | ||
| import org.junit.Before | ||
| import org.junit.Rule | ||
| import org.junit.Test | ||
| import java.net.SocketTimeoutException | ||
| import java.util.concurrent.TimeUnit | ||
| import kotlin.test.assertEquals | ||
|
|
||
| class ConstructorIoResultsImpressionViewTrackingTest { | ||
|
|
||
| @Rule | ||
| @JvmField | ||
| val overrideSchedulersRule = RxSchedulersOverrideRule() | ||
|
|
||
| private lateinit var mockServer: MockWebServer | ||
| private var constructorIo = ConstructorIo | ||
| private val ctx = mockk<Context>() | ||
| private val preferencesHelper = mockk<PreferencesHelper>() | ||
| private val configMemoryHolder = mockk<ConfigMemoryHolder>() | ||
|
|
||
| @Before | ||
| fun setup() { | ||
| mockServer = MockWebServer() | ||
| mockServer.start() | ||
|
|
||
| every { ctx.applicationContext } returns ctx | ||
|
|
||
| every { preferencesHelper.apiKey } returns "copper-key" | ||
| every { preferencesHelper.id } returns "wacko-the-guid" | ||
| every { preferencesHelper.serviceUrl } returns mockServer.hostName | ||
| every { preferencesHelper.port } returns mockServer.port | ||
| every { preferencesHelper.scheme } returns "http" | ||
| every { preferencesHelper.defaultItemSection } returns "Products" | ||
| every { preferencesHelper.getSessionId(any(), any()) } returns 67 | ||
|
|
||
| every { configMemoryHolder.autocompleteResultCount } returns null | ||
| every { configMemoryHolder.defaultAnalyticsTags } returns mapOf("appVersion" to "123", "appPlatform" to "Android") | ||
| every { configMemoryHolder.userId } returns "player-three" | ||
| every { configMemoryHolder.testCellParams } returns emptyList() | ||
| every { configMemoryHolder.segments } returns emptyList() | ||
|
|
||
| val config = ConstructorIoConfig("dummyKey") | ||
| val dataManager = createTestDataManager(preferencesHelper, configMemoryHolder) | ||
|
|
||
| constructorIo.testInit(ctx, config, dataManager, preferencesHelper, configMemoryHolder) | ||
| } | ||
|
|
||
| @After | ||
| fun tearDown() { | ||
| mockServer.shutdown() | ||
| } | ||
|
|
||
| @Test | ||
| fun trackResultsImpressionView() { | ||
|
constructor-claude-bedrock[bot] marked this conversation as resolved.
constructor-claude-bedrock[bot] marked this conversation as resolved.
constructor-claude-bedrock[bot] marked this conversation as resolved.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: There is no test for the empty-items guard behavior. A test like @Test
fun trackResultsImpressionViewWithEmptyItems() {
val observer = ConstructorIo.trackResultsImpressionViewInternal(emptyList()).test()
observer.assertComplete()
val request = mockServer.takeRequest(1, TimeUnit.SECONDS)
assertNull(request) // no request should be made
} |
||
| val mockResponse = MockResponse().setResponseCode(204) | ||
| mockServer.enqueue(mockResponse) | ||
|
|
||
| val items = listOf( | ||
| ResultsImpressionItem("item-1", "Item One"), | ||
| ResultsImpressionItem("item-2", "Item Two", variationId = "var-2") | ||
| ) | ||
| val observer = ConstructorIo.trackResultsImpressionViewInternal(items).test() | ||
| observer.assertComplete() | ||
| val request = awaitRequest() | ||
| assertEquals("POST", request.method) | ||
| assert(request.path!!.startsWith("/v2/behavioral_action/impression_view")) | ||
|
|
||
| val requestBody = getRequestBody(request) | ||
| assertEquals("true", requestBody["beacon"]) | ||
| assertEquals("wacko-the-guid", requestBody["i"]) | ||
| assertEquals("67", requestBody["s"]) | ||
| assert(requestBody["items"]!!.contains("item_id:item-1")) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: The item assertions use |
||
| assert(requestBody["items"]!!.contains("item_name:Item One")) | ||
| assert(requestBody["items"]!!.contains("item_id:item-2")) | ||
| assert(requestBody["items"]!!.contains("variation_id:var-2")) | ||
| } | ||
|
|
||
| @Test | ||
| fun trackResultsImpressionViewWithSearchTerm() { | ||
| val mockResponse = MockResponse().setResponseCode(204) | ||
| mockServer.enqueue(mockResponse) | ||
|
|
||
| val items = listOf(ResultsImpressionItem("item-1", "Item One", slCampaignId = "camp-1", slCampaignOwner = "owner-a")) | ||
| val observer = ConstructorIo.trackResultsImpressionViewInternal(items, searchTerm = "shoes").test() | ||
| observer.assertComplete() | ||
| val request = awaitRequest() | ||
|
|
||
| val requestBody = getRequestBody(request) | ||
| assertEquals("shoes", requestBody["search_term"]) | ||
| assert(requestBody["items"]!!.contains("sl_campaign_id:camp-1")) | ||
| assert(requestBody["items"]!!.contains("sl_campaign_owner:owner-a")) | ||
| } | ||
|
|
||
| @Test | ||
| fun trackResultsImpressionViewWithFilters() { | ||
| val mockResponse = MockResponse().setResponseCode(204) | ||
| mockServer.enqueue(mockResponse) | ||
|
|
||
| val items = listOf(ResultsImpressionItem("item-1", "Item One")) | ||
| val observer = ConstructorIo.trackResultsImpressionViewInternal(items, filterName = "category_id", filterValue = "shoes-123").test() | ||
| observer.assertComplete() | ||
| val request = awaitRequest() | ||
|
|
||
| val requestBody = getRequestBody(request) | ||
| assertEquals("category_id", requestBody["filter_name"]) | ||
| assertEquals("shoes-123", requestBody["filter_value"]) | ||
| } | ||
|
|
||
| @Test | ||
| fun trackResultsImpressionViewWithAnalyticsTags() { | ||
| val mockResponse = MockResponse().setResponseCode(204) | ||
| mockServer.enqueue(mockResponse) | ||
|
|
||
| val items = listOf(ResultsImpressionItem("item-1", "Item One")) | ||
| val observer = ConstructorIo.trackResultsImpressionViewInternal(items, analyticsTags = mapOf("test" to "test1", "appVersion" to "150")).test() | ||
| observer.assertComplete() | ||
| val request = awaitRequest() | ||
|
|
||
| val requestBody = getRequestBody(request) | ||
| assertEquals("{appVersion:150,appPlatform:Android,test:test1}", requestBody["analytics_tags"]) | ||
| } | ||
|
|
||
| @Test | ||
| fun trackResultsImpressionView500() { | ||
| val mockResponse = MockResponse().setResponseCode(500).setBody("Internal server error") | ||
| mockServer.enqueue(mockResponse) | ||
|
|
||
| val items = listOf(ResultsImpressionItem("item-1", "Item One")) | ||
| val observer = ConstructorIo.trackResultsImpressionViewInternal(items).test() | ||
| observer.assertError { true } | ||
| val request = awaitRequest() | ||
| assert(request.path!!.startsWith("/v2/behavioral_action/impression_view")) | ||
| } | ||
|
|
||
| @Test | ||
| fun trackResultsImpressionViewTimeout() { | ||
| val mockResponse = MockResponse() | ||
| .setResponseCode(500) | ||
| .setBody("Internal server error") | ||
| .setBodyDelay(5, TimeUnit.SECONDS) | ||
| mockServer.enqueue(mockResponse) | ||
|
|
||
| val items = listOf(ResultsImpressionItem("item-1", "Item One")) | ||
| val observer = ConstructorIo.trackResultsImpressionViewInternal(items).test() | ||
| observer.awaitDone(6, TimeUnit.SECONDS) | ||
| observer.assertError(SocketTimeoutException::class.java) | ||
| val request = awaitRequest() | ||
| assert(request.path!!.startsWith("/v2/behavioral_action/impression_view")) | ||
| } | ||
|
|
||
| private fun awaitRequest(): RecordedRequest { | ||
| return mockServer.takeRequest(1, TimeUnit.SECONDS) | ||
| ?: throw AssertionError("Expected request but timed out") | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.