Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement liveintentRtd module #45

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wi101
Copy link

@wi101 wi101 commented Dec 18, 2024

CM-1589

Tested in this page: https://vdreiling-lab-test.s3.us-east-1.amazonaws.com/prebid1589/liveintentRtdExample.html

Type of change

  • Bugfix

  • Feature

  • New bidder adapter

  • Updated bidder adapter

  • Code style update (formatting, local variables)

  • Refactoring (no functional changes, no api changes)

  • Build related changes

  • CI related changes

  • Does this change affect user-facing APIs or examples documented on http://prebid.org?

  • Other

Description of change

Other information

Copy link

Tread carefully! This PR adds 3 linter warnings (possibly disabled through directives):

  • modules/liveintentRtdProvider.js (+3 warnings)


function onBidRequest(bidRequest, config, userConsent) {
bidRequest.bids.forEach(bid => {
const userIdSegments = { segment: bid.userId.lipbid.segments.map(id => ({ id })) }
Copy link

@3link 3link Dec 19, 2024

Choose a reason for hiding this comment

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

Each path segment in .userId.lipbid.segments is optional. Please use optional chaining (?.)

function onBidRequest(bidRequest, config, userConsent) {
bidRequest.bids.forEach(bid => {
const userIdSegments = { segment: bid.userId.lipbid.segments.map(id => ({ id })) }
const liSegments = [{name: 'liveintent.com', ...userIdSegments}]
Copy link

@3link 3link Dec 19, 2024

Choose a reason for hiding this comment

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

If userIdSegments is empty or undefined, you should not add anything.

if (bidRequest?.ortb2?.user?.data) {
bidRequest.ortb2.user.data = bidRequest.ortb2.user.data.concat(liSegments)
} else {
bidRequest.ortb2 = {...bidRequest.ortb2, ...{user: {data: liSegments}}}
Copy link

Choose a reason for hiding this comment

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

bidRequest.ortb2 can be undefined here. Then the ... operator will fail.

const userIdSegments = { segment: bid.userId.lipbid.segments.map(id => ({ id })) }
const liSegments = [{name: 'liveintent.com', ...userIdSegments}]
if (bidRequest?.ortb2?.user?.data) {
bidRequest.ortb2.user.data = bidRequest.ortb2.user.data.concat(liSegments)
Copy link

Choose a reason for hiding this comment

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

Currently, you iterate over bids, but ignore them and keep concatenating the same data multiple times.

The goal is this: Enrich each bid (bid.ortb2.user.data) according to the segments known to that bid. Additionally enrich the bidRequest (bidRequest.ortb2.user.data) according to the segments known to it.

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.

2 participants