[CI 5051] Add itemID to select tracking

[crgee1]

Add itemID to select tracking event

[constructor-claude-bedrock]

Code Review Summary

This PR adds itemID parameter support to the trackAutocompleteSelect method. The implementation is clean and follows the existing patterns in the codebase. The changes are straightforward: adding the new optional parameter through the call chain from the public API down to the request data struct, and including appropriate test coverage.

Detailed Feedback

[File: AutocompleteClient/FW/Logic/Worker/ConstructorIO.swift Lines: 344-358] Missing Documentation for New Parameter

The public API documentation comment is missing the itemID parameter. The Parameters section should be updated to include:

- itemID: Identifier of the selected item (optional)

The current documentation lists:

• searchTerm
• originalQuery
• sectionName
• group (unlabeled in the docs)
• resultID
• completionHandler

But itemID is not documented, which could cause confusion for API consumers.

[File: AutocompleteClient/FW/Logic/Worker/ConstructorIO.swift Line: 357] Usage Example Should Be Updated

Consider updating the usage example to demonstrate the new itemID parameter. While it's optional, showing how to use it would help developers understand when and how to provide this value.

[File: AutocompleteClientTests/FW/Logic/Worker/ConstructorIOIntegrationTests.swift Line: 68] Test Using Misleading Variable Name

The integration test passes customerID for the itemID parameter:

self.constructor.trackAutocompleteSelect(..., itemID: customerID, ...)

While this works functionally (both are strings), using customerID (defined as "10001" at line 28) for an itemID parameter is semantically confusing. Consider either:

1. Using a dedicated itemID test constant (e.g., fileprivate let itemID = "item-123")
2. Reusing itemName if that's more appropriate
3. Adding a comment explaining why customerID represents an item ID in this context

This would improve test readability and prevent confusion about the purpose of each identifier.

[Positive] Consistent Implementation Pattern

The implementation correctly follows the established pattern:

• Optional parameter with default nil value
• Properly threaded through the init chain
• Correctly passed to requestBuilder.set(itemID:) which handles the nil case
• Good test coverage with the new testTrackAutocompleteSelect_WithItemID() test

[Positive] Test Coverage

The new unit test testTrackAutocompleteSelect_WithItemID() properly verifies that the itemID is correctly added to the query string as item_id=0123456789. The regex pattern correctly validates the parameter is present in the request URL.

Conclusion

This is a solid implementation that adds the requested functionality cleanly. The main improvements needed are:

1. Required: Add documentation for the itemID parameter in the public API method comment
2. Recommended: Update the usage example to show the new parameter
3. Minor: Use a more semantically appropriate variable in the integration test

Once the documentation is updated, this will be ready to merge.

[constructor-claude-bedrock]

Code Review Summary

This PR adds support for the itemID parameter to the trackAutocompleteSelect method. The implementation is clean, follows existing patterns in the codebase, and includes appropriate test coverage. The changes are well-structured and maintain backward compatibility by making the new parameter optional.

Detailed Feedback

[File: AutocompleteClient/FW/Logic/Request/CIOTrackAutocompleteSelectData.swift Lines: L25, L31, L37, L47]

✅ Good: The itemID property is correctly added as an optional String?, following the same pattern as resultID. The initialization and decoration logic are properly implemented.

[File: AutocompleteClient/FW/Logic/Worker/ConstructorIO.swift Lines: L353, L361-362]

✅ Good: The public API is updated correctly with:

• The new itemID parameter added as optional with a default value of nil
• Documentation updated to describe the parameter
• Usage example updated to show the new parameter
• Backward compatibility maintained

[File: AutocompleteClientTests/FW/Logic/Worker/ConstructorIOTrackAutocompleteSelectTests.swift Lines: L50-60]

✅ Good: A dedicated unit test testTrackAutocompleteSelect_WithItemID was added that:

• Validates the item_id query parameter is correctly added to the request URL
• Follows the existing test pattern for testTrackAutocompleteSelect_WithResultID
• Uses proper HTTP stubbing with regex matching

[File: AutocompleteClientTests/FW/Logic/Worker/ConstructorIOIntegrationTests.swift Line: L68]

⚠️ Minor Concern: The integration test now passes itemID: customerID to the trackAutocompleteSelect call. While this technically works, there's a semantic mismatch:

• customerID is defined as "10001" (line 28) and represents a customer/user identifier
• itemID should represent a product/item identifier

Suggestion: Consider using a more semantically appropriate test value for clarity:

// Add near other test constants (around line 42)
fileprivate let itemID = "item_123"

// Then in the test (line 68)
self.constructor.trackAutocompleteSelect(searchTerm: searchTerm, originalQuery: originalQuery, sectionName: sectionName, group: group, resultID: resultID, itemID: itemID, completionHandler: { response in

This would make the test more self-documenting and avoid confusion between customer IDs and item IDs. However, this is a minor issue and doesn't affect functionality.

Conclusion

The implementation is solid and follows best practices:

• ✅ Backward compatible (new parameter is optional)
• ✅ Consistent with existing patterns (resultID implementation)
• ✅ Properly tested with both unit and integration tests
• ✅ Documentation updated with examples
• ✅ Reuses existing RequestBuilder.set(itemID:) method

The only minor suggestion is improving test data semantics in the integration test. Overall, this is high-quality code ready for merge. Great work! 🎉

[Alexey-Pavlov]

LGTM!

[HHHindawy]

LGTM!