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

Adnuntius Bid Adaptor: handling deals better #9943

Merged
merged 1 commit into from
May 22, 2023

Conversation

antosarho
Copy link
Contributor

@antosarho antosarho commented May 12, 2023

Type of change

  • Feature

  • Code style update (formatting, local variables)

  • Refactoring (no functional changes, no api changes)

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

Description of change

This change adds better support for deals to the Adnuntius Bid Adaptor.

It adds a new maxDeals parameter, which defaults to 0, that allows the adaptor to return responses for both the highest bidding advertisement AND (when maxDeals > 0) deal advertisements.

Other information

This change introduces a new optional parameter, which will require an update to the adaptor documentation but it does not affect functionality for existing users.

@ChrisHuie ChrisHuie requested a review from osazos May 16, 2023 17:19
@ChrisHuie ChrisHuie changed the title Adnuntius Bid Adaptor: Handling deals better Adnuntius Bid Adaptor: handling deals better May 16, 2023
Copy link
Collaborator

@osazos osazos left a comment

Choose a reason for hiding this comment

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

The prebid.github.io PR is missing, it would be nice to open it in the same time.

By the way, LGTM for the code review.

modules/adnuntiusBidAdapter.js Outdated Show resolved Hide resolved
@antosarho
Copy link
Contributor Author

The prebid.github.io PR is missing, it would be nice to open it in the same time.

By the way, LGTM for the code review.

And the doco update is here (done by a co-worker): prebid/prebid.github.io#4591

@antosarho antosarho requested a review from osazos May 22, 2023 10:35
@osazos osazos removed the needs docs label May 22, 2023
@osazos osazos merged commit be23ae2 into prebid:master May 22, 2023
mscottnelson added a commit to 33Across/Prebid.js that referenced this pull request Jun 2, 2023
* 33across_analytics_adapter:
  Adman Adapter: remove useless parameter (prebid#9967)
  Rise Bid Adapter: support sua and plcmt params. (prebid#9996)
  clean.io RTD Module: Use loadExternalScript function instead of insertElement() method to insert the Clean.io script. (prebid#9991)
  Bump socket.io-parser from 4.2.1 to 4.2.3 (prebid#9992)
  AdHash Bid Adapter: changes to support preroll videos (prebid#9870)
  RTB House Bid Adapter: docs update - FLEDGE support (prebid#9986)
  Core: fix  in ortb -> legacy native asset conversion (prebid#9923)
  Adnuntius Bid Adaptor: Handling deals better (prebid#9943)
  FreeWheel SSP Adapter: add support for video.plcmt (prebid#9978)
  PubMatic Bid Adapter: Added support for video.plcmt (prebid#9979)
  Criteo Bid Adapter: add support for video.plcmt (prebid#9975)
  add SSP Copper6 alias adapter (prebid#9972)
  Criteo bid adapter: Add video outstream renderer (prebid#9955)
  PBS Bid Adapter: add context to emitted seatnonbid event (prebid#9804)
  GumGum Bid Adapter : Id5 integration suppress link type for ttd (prebid#9924)
  Increment version to 7.51.0-pre
  Prebid 7.50.0 release
  Docs Integration Examples : add Events UI page for Video Module  (prebid#9934)
  LiveIntent UserId module: add support for distributorId configuration parameter (prebid#9963)
  Underdog Media Bid Adapter: Update ttl & referer information (prebid#9826)
  eps_aliasing - adding in aliases for epsilon (formerly conversant) for bidder and analytics (prebid#9961)
  Add mobile client hint 33x adapter (prebid#9958)
  confiant Rtd Provider : add message type check (prebid#9950)
  Criteo Bid Adapter: add support for imp.rwdd (prebid#9964)
  Yahoo SSP adapter support for extra site publisher info. (prebid#9921)
  RubiconBidAdapter: sync parseSize algorithm for isBidRequestValid and ortb conversion (prebid#9957)

Ticket: IDG-829
@robertrmartinez
Copy link
Collaborator

Hello @antosarho

I work for Magnite and while testing a newer version of prebid I noticed that one of our pubs who uses an alias of your adapter had their spend through it drop to zero.

This is the only PR for your adapter that is between the two versions in question (7.50.0 and 7.54.0)

I just wanted to reach out and confirm if this is expected on your side?

I see the code Here: https://github.com/Adnuntius/Prebid.js/blob/3b7c8d62318d71bc95431fbaaffbb4765f36cc88/modules/adnuntiusBidAdapter.js#L179-L188

which seems to want to filter out any alias bidders.

Is there something the publisher needs to change, such as this new param maxDeals which defaults to zero now? Which is not something sent previously?

Or is this a known thing and alias bidders are no longer able to go through adnuntius?

Thanks!

@antosarho
Copy link
Contributor Author

antosarho commented Jul 14, 2023

Hello @robertrmartinez

I must apologise for the regression sneaking in and I thank you for being so thorough to pinpoint exactly where the problem lies.

I've now created this pull request that will address the issue: #10232

Unfortunately there is no fallback option to allow the use of aliases with the adnuntius adaptor in 7.54.0.

Apologies once more, and thanks again.

Cheers!

@robertrmartinez
Copy link
Collaborator

Hey glad to help @antosarho

One more question, one of our publishers uses an alias named calt

Will this still work? From looking at the PR it seams you only want aliases named adndeal{1-5}

So, is this something publishers need to migrate to? Understanding other alias names will not work?

Thanks!

@antosarho
Copy link
Contributor Author

Hey @robertrmartinez,

The intention is to treat specific aliases in a special way because our ad server does something specific in these cases.

If the aliases are not adndeal{1-5}, it should work like normal -- that, at least, is the intention.

So if an alias is specified with pbjs.aliasBidder('adnuntius', 'alt-adnuntius');, then it should work fine.

Is that how magnite code specifies the alias? Or is there another way we should cater for?

Thanks again!

@robertrmartinez
Copy link
Collaborator

Hey @antosarho

You are right, at a quick glance I misinterpreted the code change in #10232

It is really saying

"If the bidder is NOT one of the deal aliases, add its bid response"

I thought it was the reverse!

Thanks for the quick fix!

musikele pushed a commit to rubicon-project/Prebid.js that referenced this pull request Aug 28, 2023
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