-
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
ucfunnel adapter support CCPA and remove utils.js in adapter #4541
Conversation
2. Replace var with let 3. Put JSON.parse(JSON.stringify()) into try catch block
Merge the latest changes 9/25/2017
documentation PR linked for support ccpa prebid/prebid.github.io#1677 |
193e6b5
to
2864ddd
Compare
@idettman documentation PR linked for support ccpa prebid/prebid.github.io#1677 and I update the test case. What I need to do next? |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@idettman @jsnellbaker documentation PR linked for support ccpa prebid/prebid.github.io#1677 and I update the test case. What I need to do next? |
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.
Hi @ucfunnel
I took a look through the code and found a few items that should be addressed. Can you please take a look?
modules/ucfunnelBidAdapter.js
Outdated
return bids.map(bid => { | ||
return { | ||
method: 'GET', | ||
url: location.protocol + spec.ENDPOINT, |
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.
As part of the 3.0 changes, requests always need to be https
instead of being dynamic. Can you please hard-code the protocol here (and in the related unit tests)?
modules/ucfunnelBidAdapter.js
Outdated
} | ||
|
||
if (isVideoMediaType && videoContext === 'outstream') { | ||
utils.logWarn('Warning: outstream video is not supported yet'); |
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.
This utils
reference is still present, even though the import is gone. Can you either remove this message or re-add the import?
modules/ucfunnelBidAdapter.js
Outdated
/* | ||
if (utils.isArray(requestSizes) && requestSizes.length === 2 && !utils.isArray(requestSizes[0])) { | ||
return [parseInt(requestSizes[0], 10), parseInt(requestSizes[1], 10)]; | ||
} else | ||
*/ |
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.
If this code is no longer needed, can you please remove it?
@jsnellbaker I fixed the problem that should be addressed. |
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 making the updates; LGTM now.
…idVersion1.2.0 * 'master' of https://github.com/prebid/Prebid.js: (22 commits) fix lint errors in unit test file (prebid#4702) Add Revcontent Adapter (prebid#4654) Changed data structure in Platform One Analytic Adapter (prebid#4647) increment pre version Prebid 3.2.0 Release Add static API option to the consentManagementUsp module. (prebid#4685) replace all xhr stubs with global xhr stub to prevent all requests (prebid#4687) Add CCPA us_privacy support to spotxBidAdapter (prebid#4689) ucfunnel adapter support CCPA and remove utils.js in adapter (prebid#4541) freewheelSSPBidAdapter (prebid#4645) Add CCPA support to Beachfront adapter (prebid#4673) add seedingAlliance Adapter (prebid#4614) Changed analytics data structure in YuktaMedia Analytic Adapter (prebid#4659) Add eplanning adapter for prebid 3.0 compliant and CCPA and GDPR support (prebid#4643) Bidder schain support (prebid#4551) Added CCPA support and GDPR compliance to Cedato adapter (prebid#4683) pass us privacy consent string to request (prebid#4581) Prebid 3 Admixer (prebid#4615) Pass uspConsent in bidRequest (prebid#4675) Advertly: New Bidder Adapter Submission (prebid#4496) ...
…4541) * Add a new ucfunnel Adapter and test page * Add a new ucfunnel Adapter and test page * 1. Use prebid lib in the repo to keep updated 2. Replace var with let 3. Put JSON.parse(JSON.stringify()) into try catch block * utils.getTopWindowLocation is a function * Change to modules from adapters * Migrate to module design * [Dev Fix] Remove width and height which can be got from ad unit id * Update ucfunnelBidAdapter to fit into new spec * Correct the endpoint. Fix the error of query string * Add test case for ucfunnelBidAdapter * Fix lint error * Update version number * Combine all checks on bid request * Add GDPR support for ucfunnel adapter * Add in-stream video and native support for ucfunnel adapter * Remove demo page. Add more test cases. * Change request method from POST to GET * Remove unnecessary comment * Support vastXml and vastUrl for video request * update TTL to 30 mins * Avoid using arrow function which is not discuraged in mocha * ucfunnel tdid support * ucfunnel fix error message in debug mode * ucfunnel adapter support CCPA Co-authored-by: Ryan Chou <ryanchou0210@gmail.com>
Type of change
Description of change
Be sure to test the integration with your adserver using the Hello World sample page.
For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:
Other information