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
7 changes: 7 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,13 @@ ConstructorIo.trackAutocompleteSelect("Fashionable Toothpicks", "tooth", "Produc

// Track when the user submits a search (searchTerm, originalQuery)
ConstructorIo.trackSearchSubmit("toothpicks", "tooth")

// Track when the user submits a search with additional parameters, i.e. analytics tags
// Request level analytics tags are merged with the default analytics tags passed on initialization
val request = SearchSubmitTrackingData.build("toothpicks", "tooth") {
setAnalyticsTags(mapOf("relatedSearchTerm" to "true"))
}
ConstructorIo.trackSearchSubmit(request)
```

### Search Events
Expand Down
1 change: 1 addition & 0 deletions library/src/main/java/io/constructor/core/Constants.kt
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class Constants {
const val GROUPS_MAX_DEPTH = "groups_max_depth"
const val FILTER_GROUP_ID = "filters[group_id]"
const val PRE_FILTER_EXPRESSION = "pre_filter_expression"
const val ANALYTICS_TAGS = "analytics_tags[%s]"

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: This constant uses a %s format placeholder, making it a format string rather than a plain constant. Consider documenting this expectation with a brief KDoc comment (e.g., /** Format string; supply the tag key as the argument, e.g. ANALYTICS_TAGS.format("myKey") */) to make the intended usage clear to future maintainers. This is especially important since the pattern differs from all other constants in this object, which are plain strings.

}

object QueryValues {
Expand Down
31 changes: 30 additions & 1 deletion library/src/main/java/io/constructor/core/ConstructorIo.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1490,9 +1490,38 @@ object ConstructorIo {
t -> e("Search Submit error: ${t.message}")
}))
}
internal fun trackSearchSubmitInternal(searchTerm: String, originalQuery: String, resultGroup: ResultGroup?): Completable {

/**
* Tracks search submit events.
*
* Example:
* ```
Comment on lines +1494 to +1498
* val request = SearchSubmitTrackingData.build("toothpicks", "tooth") {
* setResultGroup(ResultGroup("Canned Goods", "canned-goods"))
* setAnalyticsTags(mapOf("relatedSearchTerm" to "true"))
* }
* ConstructorIo.trackSearchSubmit(request)
* ```
* @param request the search submit request object holding all the tracking parameters. Any
* analytics tags set on it are merged with the default analytics tags set on
* [ConstructorIoConfig.defaultAnalyticsTags], with request-level values overriding defaults on
* key collision.
*/
fun trackSearchSubmit(request: SearchSubmitTrackingData) {
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
val completable = trackSearchSubmitInternal(request.searchTerm, request.originalQuery, request.resultGroup, request.analyticsTags)
disposable.add(completable.subscribeOn(Schedulers.io()).subscribe({
context.broadcastIntent(Constants.EVENT_QUERY_SENT, Constants.EXTRA_TERM to request.searchTerm)
}, {
t -> e("Search Submit error: ${t.message}")
}))
}

internal fun trackSearchSubmitInternal(searchTerm: String, originalQuery: String, resultGroup: ResultGroup?, analyticsTags: Map<String, String>? = null): Completable {
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
preferenceHelper.getSessionId(sessionIncrementHandler)
val encodedParams: ArrayList<Pair<String, String>> = getEncodedParams(groupId = resultGroup?.groupId, groupDisplayName = resultGroup?.displayName)
mergeAnalyticsTags(configMemoryHolder.defaultAnalyticsTags, analyticsTags)?.forEach { (key, value) ->
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
encodedParams.add(Constants.QueryConstants.ANALYTICS_TAGS.format(key).urlEncode() to value.urlEncode())
}

return dataManager.trackSearchSubmit(searchTerm, arrayOf(
Constants.QueryConstants.ORIGINAL_QUERY to originalQuery,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package io.constructor.data.builder

import io.constructor.data.model.common.ResultGroup

/**
* Create a Search Submit tracking request object utilizing a builder
*/
class SearchSubmitTrackingData(
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 primary constructor is public (the default), which means callers can construct SearchSubmitTrackingData directly, bypassing the builder entirely:

// This compiles and bypasses the builder:
SearchSubmitTrackingData("term", "query", null, mapOf("k" to "v"))

If the intent is to enforce usage through the builder DSL, consider making the primary constructor internal or private. If direct construction is intentional (e.g. for testing or for callers who don't need the DSL), that's fine as-is, but it should be a deliberate choice. Other fetch-side builders in this codebase (e.g. SearchRequest) are similar, so this is consistent if intentional.

val searchTerm: String,
val originalQuery: String,
val resultGroup: ResultGroup? = null,
val analyticsTags: Map<String, String>? = null,
) {
private constructor(builder: Builder) : this(
builder.searchTerm,
builder.originalQuery,
builder.resultGroup,
builder.analyticsTags,
)

companion object {
inline fun build(
searchTerm: String,
originalQuery: String,
block: Builder.() -> Unit = {}
) = Builder(searchTerm, originalQuery).apply(block).build()
}

class Builder(
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
val searchTerm: String,
val originalQuery: String
) {
var resultGroup: ResultGroup? = null
var analyticsTags: Map<String, String>? = null

fun setResultGroup(resultGroup: ResultGroup): Builder = apply { this.resultGroup = resultGroup }
fun setAnalyticsTags(analyticsTags: Map<String, String>): Builder = apply { this.analyticsTags = analyticsTags }
fun build(): SearchSubmitTrackingData = SearchSubmitTrackingData(this)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import io.constructor.data.memory.ConfigMemoryHolder
import io.constructor.data.model.common.ResultGroup
import io.constructor.data.model.purchase.PurchaseItem
import io.constructor.data.model.common.TrackingItem
import io.constructor.data.builder.SearchSubmitTrackingData
import io.constructor.test.createTestDataManager
import io.constructor.util.RxSchedulersOverrideRule
import io.mockk.every
Expand Down Expand Up @@ -340,18 +341,43 @@ class ConstructorIoTrackingTest {
val observer = ConstructorIo.trackSearchSubmitInternal("titanic", "tit", null).test()
observer.assertComplete()
val request = mockServer.takeRequest()
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&analytics_tags%5BappVersion%5D=123&analytics_tags%5BappPlatform%5D=Android&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
assert(request.path!!.startsWith(path))
}

@Test
fun trackSearchSubmitWithAnalyticsTags() {
val mockResponse = MockResponse().setResponseCode(204)
mockServer.enqueue(mockResponse)
val observer = ConstructorIo.trackSearchSubmitInternal("titanic", "tit", null, mapOf("test" to "test1", "appVersion" to "150")).test()
observer.assertComplete()
val request = mockServer.takeRequest()
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&analytics_tags%5BappVersion%5D=150&analytics_tags%5BappPlatform%5D=Android&analytics_tags%5Btest%5D=test1&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
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.

Important Issue: The test asserts a specific URL parameter order that relies on the iteration order of mergeAnalyticsTags. The merge is defaultAnalyticsTags + analyticsTags, which in Kotlin produces a LinkedHashMap preserving the order of the left map, with collisions resolved to the right-map value but keeping the key in the left map's position. So appVersion appears first (from defaults), then appPlatform, then the new test key. This works today because the mock returns mapOf("appVersion" to "123", "appPlatform" to "Android") in that order.

However, relying on query-parameter ordering for correctness is brittle — HTTP servers are not required to treat parameter order as significant, and a future change to the test setup's defaultAnalyticsTags declaration order would silently break this test. Consider asserting on parsed query parameters individually rather than a full string prefix, similar to how body-based tests use assertEquals(requestBody["analytics_tags"], ...). Alternatively, use assertThat(path).contains("analytics_tags%5BappVersion%5D=150") etc. for each expected tag independently.

assert(request.path!!.startsWith(path))
}

@Test
fun trackSearchSubmitWithRequestBuilder() {
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.

Important Issue: There is no error path test for the new public trackSearchSubmit(request: SearchSubmitTrackingData) overload. The existing trackSearchSubmit500 and trackSearchSubmitTimeout tests only exercise trackSearchSubmitInternal directly. While the internal paths are covered, it would be valuable to have at least one test that calls the public API via the builder and verifies the error is logged (or that the completable fails) — consistent with how other public tracking methods in this file are tested. This validates the full subscription wiring in the new overload.

val mockResponse = MockResponse().setResponseCode(204)
mockServer.enqueue(mockResponse)
val request = SearchSubmitTrackingData.build("titanic", "tit") {
setResultGroup(ResultGroup("Movies", "group_id"))
setAnalyticsTags(mapOf("relatedSearchTerm" to "true"))
}
ConstructorIo.trackSearchSubmit(request)
Comment thread
constructor-claude-bedrock[bot] marked this conversation as resolved.
val recordedRequest = mockServer.takeRequest()
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&group%5Bgroup_id%5D=group_id&group%5Bdisplay_name%5D=Movies&analytics_tags%5BappVersion%5D=123&analytics_tags%5BappPlatform%5D=Android&analytics_tags%5BrelatedSearchTerm%5D=true&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
assert(recordedRequest.path!!.startsWith(path))
}

@Test
fun trackSearchSubmit500() {
val mockResponse = MockResponse().setResponseCode(500).setBody("Internal server error")
mockServer.enqueue(mockResponse)
val observer = ConstructorIo.trackSearchSubmitInternal("titanic", "tit", null).test()
observer.assertError { true }
val request = mockServer.takeRequest()
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&analytics_tags%5BappVersion%5D=123&analytics_tags%5BappPlatform%5D=Android&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
assert(request.path!!.startsWith(path))
}

Expand All @@ -363,7 +389,7 @@ class ConstructorIoTrackingTest {
val observer = ConstructorIo.trackSearchSubmitInternal("titanic", "tit", null).test()
observer.assertError(SocketTimeoutException::class.java)
val request = mockServer.takeRequest()
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&analytics_tags%5BappVersion%5D=123&analytics_tags%5BappPlatform%5D=Android&key=copper-key&i=wacko-the-guid&ui=player-three&s=67&c=cioand-2.44.0&_dt="
assert(request.path!!.startsWith(path))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class ConstructorioSegmentsTest {
val observer = ConstructorIo.trackSearchSubmitInternal("titanic", "tit", null).test()
observer.assertComplete()
val request = mockServer.takeRequest()
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&key=aluminium-key&i=koopa-the-guid&ui=player-two&s=14&us=mobile&us=COUNTRY_US&c=cioand-2.44.0&_dt="
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&analytics_tags%5BappVersion%5D=123&analytics_tags%5BappPlatform%5D=Android&key=aluminium-key&i=koopa-the-guid&ui=player-two&s=14&us=mobile&us=COUNTRY_US&c=cioand-2.44.0&_dt="
assert(request.path!!.startsWith(path))
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class ConstructorioTestCellTest {
val observer = ConstructorIo.trackSearchSubmitInternal("titanic", "tit", null).test()
observer.assertComplete()
val request = mockServer.takeRequest()
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&key=aluminium-key&i=koopa-the-guid&ui=player-two&s=14&ef-cellone=vanilla&ef-celltwo=whipped-cream&c=cioand-2.44.0&_dt=";
val path = "/autocomplete/titanic/search?original_query=tit&tr=search&analytics_tags%5BappVersion%5D=123&analytics_tags%5BappPlatform%5D=Android&key=aluminium-key&i=koopa-the-guid&ui=player-two&s=14&ef-cellone=vanilla&ef-celltwo=whipped-cream&c=cioand-2.44.0&_dt=";
assert(request.path!!.startsWith(path))
}

Expand Down
Loading