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

PBS Bid Adapter: Fix duplicate imp.id #6933

Merged
merged 3 commits into from
Jun 4, 2021

Conversation

FilipStamenkovic
Copy link
Contributor

Type of change

  • Bugfix

Description of change

This PR suggests deduplication of imp.id by adding suffix -2 to the imp.id, if duplication is detected.
Change bidIdMap so that the keys are in format imp.id-bidder instead of adUnit.code-bidder.

Other information

Fixes #6910

Copy link
Contributor

@hnkhandev hnkhandev left a comment

Choose a reason for hiding this comment

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

Since this is not limited to just two sets of identical adunits (e.g adding native could make it three sets), I would recommend using a map/obj to keep track of the imp ids. With the current proposed changes, it would label triple adUnits as '-2-3' instead of '-3'.

let impIds = {};
...
...
...
let id = adUnit.code;
if (!(id in impIds)) {
   impIds[id] = 1;
} else {
   impIds[id]++;
   id = `${id}-${impIds[id]}`;
}  

@FilipStamenkovic
Copy link
Contributor Author

@hnkhandev Thanks for the review, it should be fixed now. I overlooked how my while loop would behave for 'triplets'.

Copy link
Contributor

@hnkhandev hnkhandev left a comment

Choose a reason for hiding this comment

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

Looks good. I can't spot any further issues.

@ChrisHuie ChrisHuie self-requested a review June 4, 2021 17:10
@ChrisHuie ChrisHuie self-assigned this Jun 4, 2021
@ChrisHuie ChrisHuie merged commit fbe94af into prebid:master Jun 4, 2021
@FilipStamenkovic FilipStamenkovic deleted the s2s_twin_codes branch June 4, 2021 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prebid Server BidAdapter doesn't support 'twinned' ad units
3 participants