freewheelSSPBidAdapter #4645

xwang202 · 2019-12-18T23:34:17Z

It seems our previous adapter has been deleted by one of your changes. I am pushing this file again with changes of updating the freewheelSSPBidAdapter based on Prebid 3.0 changes and adding max and min size limit as per client's requirement.

Please be aware that the file name has been changed from previously freewheel-sspBidAdapter.js to freewheelSSPBidAdapter.js

Type of change

Prebid 3.0 changes
Max and min size limit as per client's requirement

Description of change

update the protocol to use HTTPS only
use banner.sizes instead of sizes
update the getUserSync method
add maximum and minimum sizes limit as per client's requirement

…bid 3.0 changes, add max and min size limit as per client's requirement

lgtm-com · 2019-12-18T23:42:24Z

This pull request introduces 1 alert when merging cd6267a into 927d36e - view on LGTM.com

new alerts:

1 for Superfluous trailing arguments

lgtm-com · 2019-12-19T00:09:07Z

This pull request introduces 1 alert when merging 59dfcd5 into 927d36e - view on LGTM.com

new alerts:

1 for Superfluous trailing arguments

xwang202 · 2019-12-19T00:28:37Z

Hi @jsnellbaker , it seems our adapter freewheel-sspBidAdapter.js was deleted by one of your changes several days ago. It already impacted some of our clients. I pushed the new one and made some changes under the prebid 3.0. (Please be aware we changed the file name to freewheelSSPBidAdapter.js, hope it won't break the building process). Can you please help to review or at least put the original file back?

Thanks,
Xuan

xwang202 · 2019-12-19T00:50:00Z

Hi @jsnellbaker ,

I just noticed you released the legacy version 2.44.1 today. We will tell our client to use the legacy one so far.

Thanks,
Xuan

jsnellbaker · 2019-12-19T15:39:01Z

@xwang202
Renaming the file just here could impact some things. If you still want to do that, can you please also update adapter name in the 2.44.x-legacy branch (including the .md file and your adapter's _spec.js test file)? Further you'd also have to update the bidder file in the docs repo to keep everything consistent.

If you don't want to change the name in all these places, can you please go back to the original name of the adapter.js file in this PR?

Outside of the naming changes, can you also please re-add your adapter's test spec file here?

xwang202 · 2019-12-19T22:46:31Z

Thanks for your response @jsnellbaker . We will use the original file name. I've updated both js and md files. Please help to review again.

Thanks,
Xuan

xwang202 · 2020-01-02T15:48:30Z

Hi @jsnellbaker ,

I've updated both code and tests files. Can you please help to re-run the CICD workflow?

Thanks,
Xuan

xwang202 · 2020-01-02T17:00:13Z

Hi @jsnellbaker ,

To add to my previous comment, we are gonna do the CCPA release next week.

Thanks,
Xuan

jsnellbaker

Hi @xwang202

Thanks for renaming the files back to the original name. There were some items that I saw in-line that need to be addressed.

In addition to the below, it seems the unit test file (ie the adapter's _spec.js) isn't present in this PR. Can you please add this back here? These tests are needed for your adapter to be re-added.

Thanks.

jsnellbaker · 2020-01-02T17:16:32Z

modules/freewheel-sspBidAdapter.js

+      }
+    }
+
+    var location = utils.getTopWindowUrl();


This function doesn't exist anymore in Prebid 3.x. Can you take a look at the bidderRequest.refererInfo property (see more info here)? It may have the information you were using here instead.

jsnellbaker · 2020-01-02T17:17:11Z

modules/freewheel-sspBidAdapter.js

@@ -0,0 +1,372 @@
+import * as utils from '../src/utils';
+import { registerBidder } from '../src/adapters/bidderFactory';
+// import { config } from 'src/config';


Can you please remove this commented line if it's no longer needed?

….getTopWindowUrl(), remove the unused code

lgtm-com · 2020-01-02T19:45:17Z

This pull request introduces 1 alert when merging 76a625a into 34416c4 - view on LGTM.com

new alerts:

1 for Superfluous trailing arguments

jsnellbaker · 2020-01-02T20:12:39Z

@xwang202 Can you take a look at the test failures in the circleci job? I think one/several of the unit tests need to be updated to include the new refererInfo object in the bidderRequest variable.

Additionally, would you be able to look at the LGTM warning? I'd like you to confirm that the extra variable you're passing in that noted function isn't needed.

Thanks!

lgtm-com · 2020-01-02T22:38:11Z

This pull request introduces 1 alert when merging 382d914 into 773a004 - view on LGTM.com

new alerts:

1 for Superfluous trailing arguments

jsnellbaker

Hi @xwang202

Thanks for making the updates. I think we're okay to merge this PR at this point.

To confirm on a point you made earlier - did you want me to wait on merging so you can include the CCPA compliance as part of this PR? Or did you want me to merge it now and you'll just open a separate PR later?

Please confirm.

xwang202 · 2020-01-03T17:01:01Z

Hi @jsnellbaker ,

Thanks for your confirmation. For the LGTM warning, we still want to keep our previous code. Please merge this PR at your first convenience.

For CCPA compliance, I will create another PR next week after we did the CCPA release on our end.

Thanks,
Xuan

jsnellbaker · 2020-01-03T18:48:51Z

Thanks for the confirmation.

…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) ...

* freewheelSSPBidAdapter Update the freewheelSSPBidAdapter based on Prebid 3.0 changes, add max and min size limit as per client's requirement * freewheelSSPBidAdapter fix the path issue * freewheelSSPBidAdapter fix the indentation * update the test * rename freewheel bid adapter name * freewheelSSPBidAdapter use bidderRequest.refererInfo instead of utils.getTopWindowUrl(), remove the unused code * freewheelSSPBidAdapter add spec file * freewheelSSPBidAdapter update the gdpr and refererInfo condition check

freewheelSSPBidAdapter Update the freewheelSSPBidAdapter based on Pre…

cd6267a

…bid 3.0 changes, add max and min size limit as per client's requirement

xwang202 added 2 commits December 18, 2019 18:56

freewheelSSPBidAdapter fix the path issue

30cce3c

freewheelSSPBidAdapter fix the indentation

59dfcd5

jsnellbaker added needs unit tests needs update labels Dec 19, 2019

jsnellbaker self-requested a review December 19, 2019 16:13

jsnellbaker self-assigned this Dec 19, 2019

xwang202 added 2 commits December 19, 2019 17:38

update the test

eae2a4a

rename freewheel bid adapter name

54ab273

jsnellbaker requested changes Jan 2, 2020

View reviewed changes

xwang202 added 2 commits January 2, 2020 14:34

freewheelSSPBidAdapter use bidderRequest.refererInfo instead of utils…

2fad698

….getTopWindowUrl(), remove the unused code

freewheelSSPBidAdapter add spec file

76a625a

freewheelSSPBidAdapter update the gdpr and refererInfo condition check

382d914

jsnellbaker approved these changes Jan 3, 2020

View reviewed changes

jsnellbaker merged commit c015d79 into prebid:master Jan 3, 2020

jsnellbaker removed needs unit tests needs update labels Jan 3, 2020

jsnellbaker added the LGTM label Jan 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

freewheelSSPBidAdapter #4645

freewheelSSPBidAdapter #4645

xwang202 commented Dec 18, 2019 •

edited

Loading

lgtm-com bot commented Dec 18, 2019

lgtm-com bot commented Dec 19, 2019

xwang202 commented Dec 19, 2019

xwang202 commented Dec 19, 2019

jsnellbaker commented Dec 19, 2019

xwang202 commented Dec 19, 2019

xwang202 commented Jan 2, 2020

xwang202 commented Jan 2, 2020

jsnellbaker left a comment

jsnellbaker Jan 2, 2020

xwang202 Jan 2, 2020

jsnellbaker Jan 2, 2020

xwang202 Jan 2, 2020

lgtm-com bot commented Jan 2, 2020

jsnellbaker commented Jan 2, 2020

lgtm-com bot commented Jan 2, 2020

jsnellbaker left a comment

xwang202 commented Jan 3, 2020

jsnellbaker commented Jan 3, 2020

freewheelSSPBidAdapter #4645

freewheelSSPBidAdapter #4645

Conversation

xwang202 commented Dec 18, 2019 • edited Loading

Type of change

Description of change

lgtm-com bot commented Dec 18, 2019

lgtm-com bot commented Dec 19, 2019

xwang202 commented Dec 19, 2019

xwang202 commented Dec 19, 2019

jsnellbaker commented Dec 19, 2019

xwang202 commented Dec 19, 2019

xwang202 commented Jan 2, 2020

xwang202 commented Jan 2, 2020

jsnellbaker left a comment

Choose a reason for hiding this comment

jsnellbaker Jan 2, 2020

Choose a reason for hiding this comment

xwang202 Jan 2, 2020

Choose a reason for hiding this comment

jsnellbaker Jan 2, 2020

Choose a reason for hiding this comment

xwang202 Jan 2, 2020

Choose a reason for hiding this comment

lgtm-com bot commented Jan 2, 2020

jsnellbaker commented Jan 2, 2020

lgtm-com bot commented Jan 2, 2020

jsnellbaker left a comment

Choose a reason for hiding this comment

xwang202 commented Jan 3, 2020

jsnellbaker commented Jan 3, 2020

xwang202 commented Dec 18, 2019 •

edited

Loading