Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
20 changes: 20 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -652,3 +652,23 @@ ConstructorIo.trackMediaImpressionClick(
bannerAdId = "abc123",
placementId = "home"
)
```

### Results Impression View Events

```kotlin
// Track sponsored results impression view
val items = listOf(
ResultsImpressionItem(
itemId = "product-789",
itemName = "Premium Sneakers",
variationId = "product-789-white",
slCampaignId = "campaign-1",
slCampaignOwner = "owner-a"
)
)
ConstructorIo.trackResultsImpressionView(
items = items,
searchTerm = "sneakers"
)
```
62 changes: 62 additions & 0 deletions library/src/main/java/io/constructor/core/ConstructorIo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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) {
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
Comment thread
Alexey-Pavlov marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important Issue: The method signature allows filterName and filterValue to be passed independently (one without the other), but they are semantically coupled — a filter name without a value (or vice versa) is likely invalid or meaningless. Either:

  1. Accept a Pair<String, String>? or a dedicated FilterPair data class to enforce they are always provided together, or
  2. Add a runtime validation (similar to the items.isEmpty() guard) and document that both must be provided together.

This same coupling issue exists on the request body itself.

if (items.isEmpty()) return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Silently returning on empty items in the public method is good for safety, but it could mask caller bugs — a caller may accidentally pass an empty list and never know the event was dropped. Consider adding a log warning (using the existing e(...) or w(...) log pattern used elsewhere in this file) before returning, so it's detectable during development:

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
Expand Down Expand Up @@ -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 {
Comment thread
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.
*
Expand Down
5 changes: 5 additions & 0 deletions library/src/main/java/io/constructor/data/DataManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The params parameter (Array<Pair<String, String>> = arrayOf()) is never used by the caller — trackResultsImpressionViewInternal in ConstructorIo.kt invokes dataManager.trackResultsImpressionView(requestBody) with no second argument, and there is no mechanism to pass query params through the public API. This dead parameter adds noise and could confuse future maintainers. Either remove it (matching the actual usage), or if query param extensibility is intentional, document it clearly.

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)
Expand Down
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
Comment thread
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?>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important Issue: The us field type List<String?> allows nullable elements in the list, which is consistent with other request body classes in the codebase (e.g., MediaImpressionRequestBody, GenericResultClickRequestBody). However, _dt is typed as Long? but System.currentTimeMillis() is always passed — it is never null at the call site. Consider making it Long (non-nullable) to better express the intent, matching what's guaranteed by trackResultsImpressionViewInternal. (The same issue exists in MediaImpressionRequestBody, but introducing nullable where it isn't needed here would continue the pattern inconsistency.)

@Json(name = "_dt") val _dt: Long?
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
) : Serializable
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,5 @@ object ApiPaths {
const val URL_QUIZ_RESULTS = "v1/quizzes/%s/results"
const val URL_MEDIA_IMPRESSION_VIEW_EVENT = "v2/ad_behavioral_action/display_ad_view"
const val URL_MEDIA_IMPRESSION_CLICK_EVENT = "v2/ad_behavioral_action/display_ad_click"
const val URL_RESULTS_IMPRESSION_VIEW_EVENT = "v2/behavioral_action/impression_view"
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.constructor.data.model.browse.*
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.data.model.conversion.ConversionRequestBody
import io.constructor.data.model.purchase.PurchaseRequestBody
import io.constructor.data.model.quiz.*
Expand Down Expand Up @@ -144,6 +145,9 @@ interface ConstructorApi {
@POST
fun trackMediaImpressionClick(@Url url: String, @Body mediaImpressionRequestBody: MediaImpressionRequestBody): Completable

@POST(ApiPaths.URL_RESULTS_IMPRESSION_VIEW_EVENT)
fun trackResultsImpressionView(@Body body: ResultsImpressionViewRequestBody, @QueryMap params: Map<String, String>): Completable

@GET
fun getQuizNextQuestion(@Url quizUrl: String): Single<Result<ResponseBody>>

Expand Down
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() {
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 trackResultsImpressionViewWithEmptyItems that verifies no HTTP request is made when items is empty would close this gap and document the intended behavior:

@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"))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: The item assertions use contains("item_id:item-1") which is a substring check on the raw serialized string and could produce false positives (e.g., an item with id "item-1-extra" would also match "item_id:item-1"). This is a limitation of the shared getRequestBody test helper. Consider adding more specific assertions or a comment explaining the testing limitation. Also, the test for trackResultsImpressionView (the basic case) does not verify the key ("copper-key") or ui ("player-three") fields that are verified in similar tests in the codebase — adding those would improve confidence in the full request body construction.

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")
}
}
Loading