Equativ Bid Adapter: initial release, Smartadserver Bid Adapter: take lowest floor to send to endpoint #12326
Conversation
add support of dsa
restore topics
DSA fix for UT
krzysztofequativ
changed the title
krzysztofequativ
changed the title
|
@patmmccann, could you share when the review could be perform, please? We have a pilot publisher for the new adapter and would like to schedule the work. Thanks in advance for your feedback. |
|
Review feedback on slack |
modules/equativBidAdapter.js
Outdated
| delete imp.dt; | ||
|
|
||
| imp.bidfloor = imp.bidfloor || spec.getMinFloor(bidRequest); | ||
| imp.secure = Number(window.location.protocol === 'https:'); |
There was a problem hiding this comment.
Window doesn't matter, the safeframe is always secure
There was a problem hiding this comment.
Updated. However, I see many other adapters which calculate the secure property in the same way. Does it mean that they also should be updated?
modules/equativBidAdapter.js
Outdated
| * @param bidRequest | ||
| * @returns {number} | ||
| */ | ||
| getMinFloor: (bidRequest) => { |
There was a problem hiding this comment.
You guys have this same code block in two adapters, please import it
modules/smartadserverBidAdapter.js
Outdated
| }); | ||
|
|
||
| return isPlainObject(floor) && !isNaN(floor.floor) ? floor.floor : DEFAULT_FLOOR; | ||
| return floors.length ? Math.min(...floors) : DEFAULT_FLOOR; |
There was a problem hiding this comment.
line 357 below is throwing a warning:
/home/circleci/Prebid.js/modules/smartadserverBidAdapter.js
357:1 warning The type 'syncs' is undefined jsdoc/no-undefined-types
|
Thanks @patmmccann for your immediate response. I've fixed all the mentioned points. Please continue reviewing. |
| deepSetValue(req, 'site.publisher.id', bid.params.networkId); | ||
| } | ||
|
|
||
| const pid = storage.getCookie(PID_COOKIE_NAME); |
There was a problem hiding this comment.
why not check local storage?
There was a problem hiding this comment.
Indeed, we're going to improve it in the next update.
Type of change