Skip to content

Vibrant Media Bid Adapter: onBidWon pixel trigger#8191

Merged
patmmccann merged 3 commits intoprebid:masterfrom
Vibrant-Media:bidwon_pixel
Apr 13, 2022
Merged

Vibrant Media Bid Adapter: onBidWon pixel trigger#8191
patmmccann merged 3 commits intoprebid:masterfrom
Vibrant-Media:bidwon_pixel

Conversation

@sardusmatt
Copy link
Contributor

Type of change

  • Bugfix
  • Feature
  • New 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

Triggered Vibrant internal pixel endpoint on bidWon event handler

Other information

onBidWon: function(bidData) {
logInfo('Bid won: ' + JSON.stringify(bidData));
if (bidData && bidData.meta && bidData.meta.wp) {
triggerPixel(`${bidData.meta.wp}${bidData.status}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you make sure biddata.wp is a url?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bidData.meta.wp is guaranteed to be a valid url, since it's an internal endpoint we use for reporting purposes (bids/won bids ratio and similar stats), generated server-side because we use several dynamic query string parameters in it.
We could add regex-based URL validation for bidData.meta.wp in the adapter though. Would that meet your expectations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ChrisHuie should we go ahead and add regex-based URL validation to our adapter, to formally check bidData.meta.wp has the right format?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That works :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ChrisHuie updated accordingly, thanks :)

@ChrisHuie ChrisHuie self-requested a review March 24, 2022 10:30
@ChrisHuie ChrisHuie self-assigned this Mar 24, 2022
@patmmccann patmmccann merged commit c6b10ec into prebid:master Apr 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments