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

Rubicon Bid Adapter: PAAPI/Fledge support #10671

Merged
merged 10 commits into from
Nov 8, 2023
Merged

Rubicon Bid Adapter: PAAPI/Fledge support #10671

merged 10 commits into from
Nov 8, 2023

Conversation

spotxslagle
Copy link
Contributor

Type of change

  • [ x ] Feature

Description of change

Rubicon Bid Adapter support for passing PAAPI/Fledge auction configs

Other information

@@ -716,6 +720,16 @@ export const spec = {
}, []).sort((adA, adB) => {
return (adB.cpm || 0.0) - (adA.cpm || 0.0);
});

let fledgeAuctionConfigs = responseObj.component_auction_config?.map(config => {
return { config, bidId: config.bidId }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this bidId returned by AE have to match the bidId prebid has for the bidRequest?

Where is DV+ getting this bidId?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. They're getting it from the fastlane param we pass in the adapter

modules/rubiconBidAdapter.js Outdated Show resolved Hide resolved
return { config, bidId: config.bidId }
});

if (fledgeAuctionConfigs) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

return {bids, fledgeAuctionConfigs} should work, core will ignore the second property if it's not an array and still pick up the bids

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While that would be better, it breaks some unit tests it's not worth fixing right now. I'll plan to deal with this when we aren't trying to get this out so quickly

@ChrisHuie ChrisHuie removed the request for review from patmmccann November 8, 2023 14:18
@spotxslagle
Copy link
Contributor Author

@dgirardi would you mind giving this another look? I'm hoping to get this merged today. Thank you!

Copy link
Collaborator

@dgirardi dgirardi left a comment

Choose a reason for hiding this comment

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

LGTM!

@spotxslagle
Copy link
Contributor Author

@ChrisHuie Can this be merged? I'd like to get it in the release this week

@robertrmartinez robertrmartinez merged commit 680f924 into master Nov 8, 2023
4 checks passed
@robertrmartinez robertrmartinez deleted the rbaFledge branch November 8, 2023 16:53
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.

4 participants