-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Prebid Core: refactor bidderSettings to have only one entry point #7712
Changes from all commits
1282769
2a45317
1d71442
19966d0
057dbc9
b0d9ca6
284ab3a
5e4c268
152296a
5abbe1b
f6c7468
0a30d07
a7f28f5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { convertCamelToUnderscore, isArray, isNumber, isPlainObject, logError, logInfo, deepAccess, logMessage, convertTypes, isStr, getParameterByName, deepClone, chunk, logWarn, getBidRequest, createTrackPixelHtml, isEmpty, transformBidderParamKeywords, getMaxValueFromArray, fill, getMinValueFromArray, isArrayOfNums, isFn, isAllowZeroCpmBidsEnabled } from '../src/utils.js'; | ||
import { convertCamelToUnderscore, isArray, isNumber, isPlainObject, logError, logInfo, deepAccess, logMessage, convertTypes, isStr, getParameterByName, deepClone, chunk, logWarn, getBidRequest, createTrackPixelHtml, isEmpty, transformBidderParamKeywords, getMaxValueFromArray, fill, getMinValueFromArray, isArrayOfNums, isFn } from '../src/utils.js'; | ||
import { Renderer } from '../src/Renderer.js'; | ||
import { config } from '../src/config.js'; | ||
import { registerBidder, getIabSubCategory } from '../src/adapters/bidderFactory.js'; | ||
|
@@ -8,6 +8,7 @@ import find from 'core-js-pure/features/array/find.js'; | |
import includes from 'core-js-pure/features/array/includes.js'; | ||
import { OUTSTREAM, INSTREAM } from '../src/video.js'; | ||
import { getStorageManager } from '../src/storageManager.js'; | ||
import { bidderSettings } from '../src/bidderSettings.js'; | ||
|
||
const BIDDER_CODE = 'appnexus'; | ||
const URL = 'https://ib.adnxs.com/ut/v3/prebid'; | ||
|
@@ -292,7 +293,7 @@ export const spec = { | |
serverResponse.tags.forEach(serverBid => { | ||
const rtbBid = getRtbBid(serverBid); | ||
if (rtbBid) { | ||
const cpmCheck = (isAllowZeroCpmBidsEnabled(bidderRequest.bidderCode)) ? rtbBid.cpm >= 0 : rtbBid.cpm > 0; | ||
const cpmCheck = (bidderSettings.get(bidderRequest.bidderCode, 'allowZeroCpmBids') === true) ? rtbBid.cpm >= 0 : rtbBid.cpm > 0; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From readability perspective, to me, personally, previous version was much clearer. But, I guess since that is used in 2,3 places in the entire prebid.js than it's fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't disagree but there's a problem with circular imports when we put everything in |
||
if (cpmCheck && includes(this.supportedMediaTypes, rtbBid.ad_type)) { | ||
const bid = newBid(serverBid, rtbBid, bidderRequest); | ||
bid.mediaType = parseMediaType(rtbBid); | ||
|
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.
Why do we need
@babel/eslint-parser
?Looking at the npm description I see:
Do we need these 'experimental and non-standard' syntax? Or am I missing something?
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.
It's part of the
class
syntax, specifically field declaration and private (#) methods and fields. Btw I care about objects but not really about syntax, I don't mind going either way with this.