Skip to content

Conversation

@esezen
Copy link
Contributor

@esezen esezen commented Aug 12, 2025

Purpose

Adds optional seedItemIds parameter to strengthen recommendation tracking by providing seed item.

Changes Made

Implementation

  • JavaScript (src/modules/tracker.js): Added parameter validation and snake_case conversion to seed_item_ids
  • TypeScript (src/types/tracker.d.ts): Added seedItemIds?: string[] | string | number type definition for flexible input
  • Tests (spec/src/modules/tracker.js): Validation and error handling tests

#375

@esezen esezen requested a review from jjl014 as a code owner August 12, 2025 12:31
@esezen esezen requested a review from a team August 12, 2025 12:31
Copy link
Contributor

@jjl014 jjl014 left a comment

Choose a reason for hiding this comment

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

LGTM! 🚀 Just left a minor comment

* @param {number} [networkParameters.timeout] - Request timeout (in milliseconds)
* @param {string} [parameters.slCampaignId] - Pass campaign id of sponsored listing
* @param {string} [parameters.slCampaignOwner] - Pass campaign owner of sponsored listing
* @param {string[]|string|number} [parameters.seedItemIds] - Item ID(s) to be used as seed
Copy link
Contributor

@jjl014 jjl014 Aug 15, 2025

Choose a reason for hiding this comment

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

should we keep this as string[]|string to stay consistent with the results fn?

EDIT: Nvm, it looks like we did this for the view event as well. Feel free to ignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually made it just string[] at first to have a tighter type but we already used this type in view event so I didn't want to make it inconsistent

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha. I was thinking of having it just be string[] as well when I first looked over things, but makes sense to leave it to keep things consistent with the other event

@esezen esezen merged commit b3b4702 into master Aug 18, 2025
7 of 10 checks passed
@esezen esezen deleted the cdx-210-client-javascript-add-optional-seed-item-ids-to-resulyts branch August 18, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants