-
Notifications
You must be signed in to change notification settings - Fork 8
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
Update Rubicon Adapter support for 1.0 mediaTypes - release/0.34.5 (HB-2228) #14
Conversation
…nse method changes for single response
… test to query string param concatenation
…e for sinon 2.0 compatibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a couple of pretty minor comments.
modules/rubiconBidAdapter.js
Outdated
if (spec.hasVideoMediaType(bid)) { | ||
// support instream only | ||
if (utils.deepAccess(bid, `mediaTypes.${VIDEO}`) && utils.deepAccess(bid, `mediaTypes.${VIDEO}.context`) !== 'instream' || | ||
typeof params.video !== 'object' || !params.video.size_id) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't this be simplified to if (utils.deepAccess(bid, `mediaTypes.${VIDEO}.context`) !== 'instream' || !utils.deepAccess(params, 'video.size_id')) { ...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may break the backwards compatibility, when we use bidRequest.mediaType === VIDEO
, utils.deepAccess(bid, 'mediaTypes.${VIDEO}') && utils.deepAccess(bid, 'mediaTypes.${VIDEO}.context') !== 'instream' || typeof params.video !== 'object' || !params.video.size_id
can return false but utils.deepAccess(bid, 'mediaTypes.${VIDEO}.context') !== 'instream' || !utils.deepAccess(params, 'video.size_id')
will always return true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@idettman Sorry my comments is not clear, the original code utils.deepAccess(bid, 'mediaTypes.${VIDEO}') && utils.deepAccess(bid, 'mediaTypes.${VIDEO}.context') !== 'instream' || typeof params.video !== 'object' || !params.video.size_id
should be the correct one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying, I should have read more carefully. Fix committed
|
||
let serverRequests = spec.buildRequests(bidderRequest.bids, bidderRequest); | ||
const foundSlotsCount = serverRequests[0].data.indexOf('&slots=10&'); | ||
expect(foundSlotsCount !== -1).to.equal(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we also verify that the bidRequest
param has only 10 requests?
…dia-type-updata # Conflicts: # modules/rubiconBidAdapter.js # test/spec/modules/rubiconBidAdapter_spec.js
…rds compatibility
Updated 'parseSizes' to fix an error found by the added tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* First implementation of the AdRoll adapter (#1) * Fix request and bid id (#5) * Send Zone ID (#6) * Add age check before fastbid eval (#7) * Add age check before fastbid eval * Fix linting * Add date check (#8) * Add date exists check * Remove logging statement * Fix bidRequest validation (#9) * Fix deprecated function usage (#10) * [SENG-2757] remove custom function from adapter (#11) * remove loadExternalScript function * add adroll to the adloader whitelist * Handle nextroll id (#12) * Handle nextroll id * Remove double nesting in user obj * Revert change to publisherTagAvailable * Rename adroll -> nextroll (#14) * Rename fastbid -> pubtag functions and variables (#15) * Improve coverage of tests * Add docs * Add docs * Improve sizes and add sellerid * Add maintainer email * Fix CI problem * Fix IE tests * Replace second instance of find * Fix types used in the doc Match prebid/prebid.github.io#1796 * Remove unused fields in spec * Add ccpa support * Remove external script usage * Remove IP field * Remove pubtag key * Rename imports; Remove getUserSync function; Remove unused code; Use url.parse function Co-authored-by: Juan Bono <juanbono94@gmail.com> Co-authored-by: Ricardo Azpeitia Pimentel <ricardo.azpeitia@nextroll.com>
…ebid#9125) * Improve Digital adapter: publisher endpoint, addtl consent, syncs (#14) - add bidders to sync url when extend mode enabled - set ConsentedProvidersSettings when extend mode enabled - dynamically generated AD_SERVER_URL when publisherId available * Code refactored * Minor changes Co-authored-by: Faisal Islam <93644923+faisalvs@users.noreply.github.com> Co-authored-by: Faisal Islam <faisal.islam@vivacomsolutions.com>
* Improve Digital adapter: publisher endpoint, addtl consent, syncs (#14) - add bidders to sync url when extend mode enabled - set ConsentedProvidersSettings when extend mode enabled - dynamically generated AD_SERVER_URL when publisherId available * Code refactored * Minor changes * Fix an issue where uppercase </SCRIPT> tags broke the JS on page, as they were not properly escaped * fixed tests --------- Co-authored-by: Faisal Islam <93644923+faisalvs@users.noreply.github.com> Co-authored-by: Faisal Islam <faisal.islam@vivacomsolutions.com> Co-authored-by: Jozef Bartek <j.bartek@improvedigital.com> Co-authored-by: Jozef Bartek <31618107+jbartek25@users.noreply.github.com>
) * collect EIDs for bid request * add ad slot positioning to payload * RPO-2012: Update local storage name-spacing for c_uid (#8) * Updates c_uid namespacing to be more specific for concert * fixes unit tests * remove console.log * RPO-2012: Add check for shared id (#9) * Adds check for sharedId * Updates cookie name * remove trailing comma * [RPO-3152] Enable Support for GPP Consent (#12) * Adds gpp consent integration to concert bid adapter * Update tests to check for gpp consent string param * removes user sync endpoint and tests * updates comment * cleans up consentAllowsPpid function * comment fix * rename variables for clarity * fixes conditional logic for consent allows function (#13) * [RPO-3262] Update getUid function to check for pubcid and sharedid (#14) * Update getUid function to check for pubcid and sharedid * updates adapter version --------- Co-authored-by: antoin <antoin.campbell@voxmedia.com> Co-authored-by: Antoin <antoinfive@gmail.com>
* pageURL pull from topmostLocation * Kargo: Support for client hints (#9) * Starting SUA support * Kargo: Adding support for client hints * Adding tests for sua * Kargo: Update referer logic * Refactor of Kargo Prebid adapter. * PR comments addressed. * Feedback addressed. * Pr comments addressed. * Continuing refactor of Kargo Bid adapter. * Logic adjustment to exclude values when not present. Relying on server defaults. * Updating unit tests. * PR feedback addressed. * Refactoring bid adapter functions. * PR feedback addressed. * Additional refactoring. * Refactoring for each to use Object entries. * Minor fixes. * Minor fixes. * Minor fixes. * TDID and linting updates * Conflicts resolved with master. * Re-adding raw CRB storage (#14) * Updating shared IDs object name * Fixing missing ad markup * Removing package json changes. Fixing unit tests broken by recent changes. * Linting * send requestCount even when it is 0 for BTO (#18) * Reverting package.json change * Reverting package-lock.json changes * Cleanup * Test cleanup * Test fix Test fix All tests fixed * Adding test for TDID * Resolving merge issue --------- Co-authored-by: Neil Flynn <nflynn@kargo.com> Co-authored-by: Julian Gan <juliangan07@gmail.com>
…ebid#10356) * collect EIDs for bid request * add ad slot positioning to payload * RPO-2012: Update local storage name-spacing for c_uid (#8) * Updates c_uid namespacing to be more specific for concert * fixes unit tests * remove console.log * RPO-2012: Add check for shared id (#9) * Adds check for sharedId * Updates cookie name * remove trailing comma * [RPO-3152] Enable Support for GPP Consent (#12) * Adds gpp consent integration to concert bid adapter * Update tests to check for gpp consent string param * removes user sync endpoint and tests * updates comment * cleans up consentAllowsPpid function * comment fix * rename variables for clarity * fixes conditional logic for consent allows function (#13) * [RPO-3262] Update getUid function to check for pubcid and sharedid (#14) * Update getUid function to check for pubcid and sharedid * updates adapter version * [RPO-3405] Add browserLanguage to request meta object --------- Co-authored-by: antoin <antoin.campbell@voxmedia.com> Co-authored-by: Antoin <antoinfive@gmail.com> Co-authored-by: Brett Bloxom <38990705+BrettBlox@users.noreply.github.com>
IMPORTANT! Do not merge this PR, for code review only
Type of change
Description of change
Adds support of 1.0 "mediaTypes" param to the Rubicon Adapter
Other information
HB-2228