Index Exchange Bid Adapter: bidder params.size not required; read video parameters from ad unit #6691

punkiller · 2021-05-03T23:25:55Z

…aTypes

Type of change

[ x ] Bugfix
[ x ] Feature
[ x ] Does this change affect user-facing APIs or examples documented on http://prebid.org?

Description of change

The bidder level param : params.size is no longer required.
The only required bidder level param will be siteId.
The information about sizes will be read from the adunit mediaTypes object.

Be sure to test the integration with your adserver using the Hello World sample page.

contact email of the adapter’s maintainer
[ x ] official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

A link to a PR on the docs repo at https://github.com/prebid/prebid.github.io/

Other information

…aTypes

patmmccann · 2021-05-06T12:14:45Z

Your adapter appears to read most video parameters from the ad unit, can you also change your .MD file to reflect?

patmmccann · 2021-05-11T11:42:45Z

modules/ixBidAdapter.js

  const videoAdUnitAllowlist = [
    'mimes', 'minduration', 'maxduration', 'protocols', 'protocol',
    'startdelay', 'placement', 'linearity', 'skip', 'skipmin',
    'skipafter', 'sequence', 'battr', 'maxextended', 'minbitrate',
    'maxbitrate', 'boxingallowed', 'playbackmethod', 'playbackend',
-    'delivery', 'pos', 'companionad', 'api', 'companiontype', 'ext'
+    'delivery', 'pos', 'companionad', 'api', 'companiontype', 'ext',
+    'playerSize'


What happens if someone sets h and w on the ad unit?

Good point.

I'll make the change to include that in the allowlist and consider the [w,h] values for size.
A question with the spec ...
I see that w and h are recommended and playerSize is optional.
https://docs.prebid.org/dev-docs/adunit-reference.html#adUnit-video-example

In the case that [playerSize] is not equal to [w,h] , should [w,h] override playerSize ?

W and h are in the openrtb spec; playersize is not.

You can choose what to prefer; whichever you choose, perhaps indicate in doc

Pbs adapter taking approach of preferring h/w if both specified; 1833 in pbs

nice, will check it out

patmmccann · 2021-05-12T14:11:06Z

@punkiller @ix-prebid-development confirming you noticed request that the .md file instream example be updated to show mime types, minduration, maxduration and protocols as parameters can come from mediatypes and not just bidder params.

https://github.com/prebid/Prebid.js/blob/master/modules/ixBidAdapter.md

This will solve the only remaining ix issue for #5966 inclusion

This pr will likely affect that file as well where you discuss player size and height at the bottom.

punkiller · 2021-05-12T14:55:25Z

Hi @patmmccann
Thanks for the reminder.
I will add the documentation today, will also try and undraft this PR.

patmmccann · 2021-05-14T15:08:34Z

Thanks for the .MD updates, removing ix from list of adapters requiring video changes!

punkiller · 2021-05-17T12:32:45Z

You're welcome. Thanks for the suggestions.

modules/ixBidAdapter.js

patmmccann · 2021-05-17T17:52:21Z

Looks like this needs a docs fix on https://docs.prebid.org/dev-docs/bidders/ix for video params as well

punkiller · 2021-05-20T16:11:52Z

@mlb7687 Thanks for the review. Ive resolved your comments.
@robertrmartinez please feel free to review this ...

punkiller · 2021-05-20T17:10:49Z

@patmmccann Acknowledging your docs change request. I am expecting a review here of the .md file as well, so once thats resolved, ill make the docs repo PR

patmmccann · 2021-05-20T17:18:35Z

@patmmccann Acknowledging your docs change request. I am expecting a review here of the .md file as well, so once thats resolved, ill make the docs repo PR

Ty, to be more specific, the example config Json is what I noticed

patmmccann · 2021-06-08T18:22:32Z

also reviewed by @mlb7687

ncolletti · 2021-07-26T20:07:10Z

modules/ixBidAdapter.js

+    const propInVideoRef = paramsVideoRef && paramsVideoRef.hasOwnProperty(property);
+
+    if (!propInMediaType && !propInVideoRef) {
+      utils.logError('IX Bid Adapter: ' + property + ' is not included in either the adunit or params level');


@punkiller (or @lksharma and @umakajan for OOO backup) - Could this line be updated to a logWarn?

This pr duplicates this code execution in buildRequests where previously it was only in isBidRequestValid, the latter makes sense for an error since no request would go out. However when it is run within buildRequests there are instances where we are seeing errors for your adapter but requests are still made.

Maybe a better suggestion would be to move the logging to the buildRequests area instead of in this function?

@ix-prebid-support -- for visibility to the thread

@ncolletti would you be able to provide us with a test page where our team can debug this issue?

We are not able to reproduce this behaviour at this moment

bretg · 2021-08-13T16:45:48Z

docs PR prebid/prebid.github.io#3200

…eo parameters from ad unit (prebid#6691) * not relying on ix bidder params.size, reading from ad unit level mediaTypes * video impression should have size * deprecating params.size, reading mediaTypes.video * mediaTypes.video.[w/h] + update the documentation * Updating the documentation examples * addressing Mikes feedback * IE-11 does not support object.entries() * cleaning up the video params check

not relying on ix bidder params.size, reading from ad unit level medi…

214c076

…aTypes

mkendall07 requested a review from robertrmartinez May 4, 2021 13:39

mkendall07 assigned mkendall07 and robertrmartinez and unassigned mkendall07 May 4, 2021

patmmccann added maintenance minor labels May 6, 2021

punkiller added 2 commits May 6, 2021 07:41

video impression should have size

159e8e7

Merge branch 'master' into ixReadMediaTypes

c566494

patmmccann reviewed May 11, 2021

View reviewed changes

punkiller added 3 commits May 12, 2021 17:45

deprecating params.size, reading mediaTypes.video

8db7d58

mediaTypes.video.[w/h] + update the documentation

e379f58

Updating the documentation examples

990bcd9

punkiller marked this pull request as ready for review May 13, 2021 00:57

patmmccann mentioned this pull request May 14, 2021

Support video params at the adunit level #6512

Closed

mlb7687 suggested changes May 17, 2021

View reviewed changes

punkiller added 4 commits May 18, 2021 13:03

addressing Mikes feedback

f26c7e8

Merge branch 'master' into ixReadMediaTypes

b0a7e4d

IE-11 does not support object.entries()

235cb3a

cleaning up the video params check

e36e15f

patmmccann changed the title ~~IX bidder params.size not required, reading from ad unit level mediaTypes~~ Index Exchange Bid Adapter: bidder params.size not required; read from ad unit level mediaTypes May 25, 2021

patmmccann changed the title ~~Index Exchange Bid Adapter: bidder params.size not required; read from ad unit level mediaTypes~~ Index Exchange Bid Adapter: bidder params.size not required; read video parameters from ad unit May 25, 2021

patmmccann approved these changes Jun 8, 2021

View reviewed changes

patmmccann merged commit 6dd4c8f into prebid:master Jun 8, 2021

ncolletti reviewed Jul 26, 2021

View reviewed changes

umakajan mentioned this pull request Aug 12, 2021

IX Bid Adapter: Update ix docs for size validation prebid/prebid.github.io#3200

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Index Exchange Bid Adapter: bidder params.size not required; read video parameters from ad unit #6691

Index Exchange Bid Adapter: bidder params.size not required; read video parameters from ad unit #6691

punkiller commented May 3, 2021

patmmccann commented May 6, 2021

patmmccann May 11, 2021

punkiller May 11, 2021

patmmccann May 11, 2021

patmmccann May 14, 2021 •

edited

Loading

punkiller May 14, 2021

patmmccann commented May 12, 2021

punkiller commented May 12, 2021

patmmccann commented May 14, 2021

punkiller commented May 17, 2021

patmmccann commented May 17, 2021

punkiller commented May 20, 2021

punkiller commented May 20, 2021

patmmccann commented May 20, 2021

patmmccann commented Jun 8, 2021

ncolletti Jul 26, 2021

ncolletti Jul 27, 2021

umakajan Aug 10, 2021

bretg commented Aug 13, 2021

Index Exchange Bid Adapter: bidder params.size not required; read video parameters from ad unit #6691

Index Exchange Bid Adapter: bidder params.size not required; read video parameters from ad unit #6691

Conversation

punkiller commented May 3, 2021

Type of change

Description of change

Other information

patmmccann commented May 6, 2021

patmmccann May 11, 2021

Choose a reason for hiding this comment

punkiller May 11, 2021

Choose a reason for hiding this comment

patmmccann May 11, 2021

Choose a reason for hiding this comment

patmmccann May 14, 2021 • edited Loading

Choose a reason for hiding this comment

punkiller May 14, 2021

Choose a reason for hiding this comment

patmmccann commented May 12, 2021

punkiller commented May 12, 2021

patmmccann commented May 14, 2021

punkiller commented May 17, 2021

patmmccann commented May 17, 2021

punkiller commented May 20, 2021

punkiller commented May 20, 2021

patmmccann commented May 20, 2021

patmmccann commented Jun 8, 2021

ncolletti Jul 26, 2021

Choose a reason for hiding this comment

ncolletti Jul 27, 2021

Choose a reason for hiding this comment

umakajan Aug 10, 2021

Choose a reason for hiding this comment

bretg commented Aug 13, 2021

patmmccann May 14, 2021 •

edited

Loading