Synacormedia: Added video support to adapter.#3695
Conversation
Added playerSize to video test page.
amuraco
force-pushed
the
synacormedia-video
branch
3 times, most recently
from
f113a35 to
856a15a
Compare
amuraco
force-pushed
the
synacormedia-video
branch
from
856a15a to
1c0eaf0
Compare
idettman
left a comment
idettman
left a comment
There was a problem hiding this comment.
Overall looks good, just a few comments/questions
modules/synacormediaBidAdapter.js
Outdated
| let size0 = size[0]; | ||
| let size1 = size[1]; | ||
| let imp = { | ||
| id: videoOrBannerKey.substring(0, 1) + bid.bidId + '-' + size0 + 'x' + size1, |
There was a problem hiding this comment.
Use template strings if possible in Prebid, so line 68 is changed to something like:
id: `${videoOrBannerKey.substring(0,1)}${bid.bidId}-${size0}x${size1}`
modules/synacormediaBidAdapter.js
Outdated
| }, | ||
| interpretResponse: function(serverResponse) { | ||
| var updateMacros = (bid, r) => { | ||
| return r ? r.replace(/\${AUCTION_PRICE}/g, parseFloat(bid.price)) : r; |
There was a problem hiding this comment.
Since replace expects a string type for the replacement argument, the parseFloat will be converted back to a string. Could you remove parseFloat or is it used to format the price?
return r ? r.replace(/\${AUCTION_PRICE}/g, bid.price) : r;
modules/synacormediaBidAdapter.js
Outdated
| width: parseInt(width, 10), | ||
| height: parseInt(height, 10), | ||
| creativeId: seatbid.seat + '~' + bid.crid, | ||
| creativeId: seatbid.seat + '_' + bid.crid, |
There was a problem hiding this comment.
A template string could be used here:
creativeId: `${seatbid.seat}_${bid.crid}`,
modules/synacormediaBidAdapter.js
Outdated
| currency: 'USD', | ||
| netRevenue: true, | ||
| mediaType: BANNER, | ||
| mediaType: (isVideo ? VIDEO : BANNER), |
There was a problem hiding this comment.
Just a recommendation, but you could simplify by removing the parens here:
mediaType: isVideo ? VIDEO : BANNER,
|
I have updated the PR and addressed the comments. |
|
Just bumping this to see if it can be re-reviewed with the recommended changes. |
* Synacormedia: Added video support to adapter. Added playerSize to video test page. * Synacormedia: Updated test case for IE/Edge * Synacormedia: Updates based on PR review
3 participants
Type of change
Description of change
Added support for Video bids to Synacormedia bid adapter
contact email of the adapter’s maintainer: eng-demand@synacor.com
Documentation PR: Synacormedia: Added video parameters to documentaion. prebid.github.io#1236