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

rubiconBidAdapter.js: ensure unique imp IDs for twin adunits #12303

Closed
wants to merge 1 commit into from

Conversation

spotxslagle
Copy link
Contributor

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

adUnitCodeCount[adUnitCode] = 1;
}
// update adunit map so we can translate back to original adunit code
impIdMap[imp.id] = adUnitCode;
Copy link
Collaborator

@robertrmartinez robertrmartinez Oct 8, 2024

Choose a reason for hiding this comment

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

not sure we need both adUnitCodeCount and impIdMap

would this just work something like

const adUnitCode = imp.id;
if (impIdMap[adUnitCode]) {
      imp.id = `${adUnitCode}-${impIdMap[adUnitCode]}`; // I like adding a - to keep them separate
      impIdMap[adUnitCode] += 1;
} else {
     impIdMap[adUnitCode] = 1;
}

Then just reset like you do after toORTB

impIdMap = {};

Might be missing something.

I think the imp function is called one after other so we do not need to handle other auctions or something.

@patmmccann patmmccann changed the title Update rubiconBidAdapter.js rubiconBidAdapter.js: ensure unique imp IDs for twin adunits Oct 8, 2024
@patmmccann
Copy link
Collaborator

you guys are on my #12171 tasklist, can you fix as part of this or your next pr?

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