Skip to content

Conversation

@lws803
Copy link
Contributor

@lws803 lws803 commented Aug 25, 2025

Updates beacon to include sponsored listings tracking (campaign id, owner) for recommendations view events.

Continuation from #389

@lws803 lws803 marked this pull request as ready for review August 25, 2025 12:01
@lws803 lws803 requested review from esezen and jjl014 as code owners August 25, 2025 12:01
@esezen esezen requested a review from a team August 25, 2025 12:01
esezen
esezen previously approved these changes Aug 26, 2025
@esezen esezen dismissed their stale review August 26, 2025 15:51

Noticed an issue with tests

Copy link
Contributor

@esezen esezen left a comment

Choose a reason for hiding this comment

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

@lws803 the test is failing with "Invalid parameters" error. You can re-create by running the test you modified locally

{
  message: 'Invalid parameters',
  errors: [
    {
      code: 'value_error',
      message: 'Value error, sl_campaign_id: Extra inputs are not permitted',
      source: ''
    }
  ],
  trace_id: 'Root=1-68add8a0-26d9101a6636ed034f68874f'
}

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.

I'm not sure if we need these changes for the recs result view event.

I think the change would be on the behavioral endpoint to make sure slCampaignId or slCampaignOwner is supported on the individual item level in the items array we send back with the view event.

Comment on lines +1275 to +1276
slCampaignId,
slCampaignOwner,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if we need this here 🤔

I believe we pass slCampaignId and slCampaignOwner via the items array. That seems to be how home24 has it implemented for Search/Browse based on what I see in their beacon. cc @esezen

Screenshot 2025-08-27 at 5 10 37 PM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @jjl014 , just verified this. Thanks for clarifying! Oh yeah it seems that's already supported out of the box for all items schemas. No changes required on either behavioral endpoint or the beacons here 👍.

Will close this PR since it's no longer required.

@lws803 lws803 closed this Sep 2, 2025
@lws803 lws803 deleted the REM-1993/update-recommendation-view-tracking-to-include-sponsored-listings branch September 2, 2025 10:47
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