[REM-3440] Viewable impressions tracking

[ZSnake]

Viewable impressions tracking

For context, we're introducing viewable impressions tracking for Sponsored listings to get a better outlook on wether a sponsored product was actually viewed by the end user. This PR will introduce trackResultsImpressionView to track viewable impressions

References

• RFC
• Project

[Copilot]

Pull request overview

This PR introduces a new public tracking API to report “viewable” (MRC-style) sponsored results impressions from the Android SDK, wiring it through the API layer and adding unit test coverage plus README usage docs.

Changes:

• Added ConstructorIo.trackResultsImpressionView(...) (and internal implementation) to send the new impression-view tracking event.
• Added new request/DTO models (ResultsImpressionViewRequestBody, ResultsImpressionItem) and a new API path + Retrofit endpoint wiring.
• Added a dedicated unit test suite and documented usage in README.md.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
README.md	Documents how to call trackResultsImpressionView with ResultsImpressionItem instances.
library/src/test/java/io/constructor/core/ConstructorIoResultsImpressionViewTrackingTest.kt	Adds unit tests validating request path, payload fields, and error/timeout behavior for the new event.
library/src/main/java/io/constructor/data/remote/ConstructorApi.kt	Adds Retrofit API method for the results impression view endpoint.
library/src/main/java/io/constructor/data/remote/ApiPaths.kt	Introduces URL_RESULTS_IMPRESSION_VIEW_EVENT constant.
library/src/main/java/io/constructor/data/model/tracking/ResultsImpressionViewRequestBody.kt	Adds Moshi-serializable request body model for the new tracking call.
library/src/main/java/io/constructor/data/model/common/ResultsImpressionItem.kt	Adds Moshi-serializable item payload model used by the new tracking event.
library/src/main/java/io/constructor/data/DataManager.kt	Wires DataManager method to call the new Retrofit API endpoint.
library/src/main/java/io/constructor/core/ConstructorIo.kt	Adds public trackResultsImpressionView API and internal request construction + dispatch.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Alexey-Pavlov]

Thanks for working on it! Left a few comments. Could you please take a look when you get a chance?

[Alexey-Pavlov]

Thanks for the changes!

[constructor-claude-bedrock]

Code Review

This PR introduces trackResultsImpressionView for viewable impression tracking of sponsored listings, following established patterns in the codebase closely. The implementation is clean and the test coverage is solid.

Inline comments: 6 discussions added

Overall Assessment: ⚠️ Needs Work

[constructor-claude-bedrock]

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.)

[constructor-claude-bedrock]

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.

[constructor-claude-bedrock]

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.

[constructor-claude-bedrock]

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
}

[constructor-claude-bedrock]

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.

[constructor-claude-bedrock]

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
}