UOL - BidAdapter release candidate #2850

lotani-uolinc · 2018-07-14T01:34:24Z

Type of change

New bidder adapter

Description of change

New bidAdapter proposal from UOL - Universo Online.

test parameters for validating bids

{
  bidder: 'uol',
  params: {
    placementId: 1231244,
    test: true,
    cpmFactor: 2
  }
}
``

- contact email of the adapter’s maintainer
   [email protected]

- [x] official adapter submission

mkendall07 · 2018-07-14T01:43:15Z

This pull request introduces 1 alert when merging fc26641 into ed28094 - view on LGTM.com

new alerts:

1 for Superfluous trailing arguments

Comment posted by LGTM.com

jsnellbaker · 2018-07-16T15:45:12Z

@lotani-uolinc Can you please look into the LGTM error when you get the chance?

lotani-uolinc · 2018-07-16T16:50:48Z

@jsnellbaker, sure thing! Thanks for the feedback.

jsnellbaker

@lotani-uolinc While you're looking into the LGTM error, I checked the rest of the adapter code. I found a few areas that should be reviewed further.

Additionally, I noticed in your docs PR you had included the gdpr_supported flag. However, I don't see the normal code that reads the buildRequest's bidderRequest.gdprConsent object and pass it along in your request object. Is this code missing or was the flag set incorrectly in the docs PR? Please confirm.

Thanks.

jsnellbaker · 2018-07-16T16:12:20Z

modules/uolBidAdapter.js

@@ -0,0 +1,202 @@
+import * as utils from 'src/utils';
+// import {config} from 'src/config';


this comment can be removed.

jsnellbaker · 2018-07-16T16:53:05Z

modules/uolBidAdapter.js

+const UOL_LOG_HEADER = 'UOL Bidder Error: '
+export const spec = {
+  code: BIDDER_CODE,
+  supportedMediaTypes: ['banner'], // not required since it is default


Could import/use the defined mediaTypes from src/mediaTypes. Eg import { BANNER } from 'src/mediaTypes'; This way you can ensure the values are read correctly.

jsnellbaker · 2018-07-16T16:57:57Z

modules/uolBidAdapter.js

+     */
+  getUserSyncs: function(syncOptions, serverResponses) {
+    const syncs = [];
+    if (syncOptions.iframeEnabled && serverResponses.body.trackingPixel) {


serverResponses is an array of objects, not a single object. This latter half of the code would result in undefined errors. Could you update this logic?

jsnellbaker · 2018-07-16T16:58:34Z

modules/uolBidAdapter.js

+    */
+  onTimeout: function(data) {
+    // Bidder specifc code
+    return true;


Is there any more you would wish to do in the case of a timeout?

For the time being we have no plans hence, Im removing the method call.

jsnellbaker · 2018-07-16T19:37:09Z

@lotani-uolinc I saw your response in github before in regards to GDPR support, but I'm not seeing it anymore (not sure what happened).

If you're still not currently planning on supporting GDPR - could you please make a change to your docs PR?

The field in the docs PR is an internal variable for the prebid site that will list that adapter in a certain table of GDPR compliant adapters. Until your adapter supports GDPR, can you either remove the variable or set it to false?

lotani-uolinc · 2018-07-16T19:39:43Z

@jsnellbaker, no problem. We have set it to false and will release it soon. Thanks in advance.

…andidate

lotani-uolinc · 2018-07-16T20:29:28Z

@jsnellbaker, we have addressed the changes requested. Could you please evaluate?
We are at your disposal for any further changes.
Thanks!

jsnellbaker · 2018-07-17T13:24:29Z

@lotani-uolinc Thanks for making the updates. They look fine to me. In addition the test param information seems to be working successfully as well.

Something I noticed while testing with browserstack in preparation for the merge were a series of errors from the unit tests. There seemed to be two sets of errors occurring in different browsers (safari mainly) that I've copied down below for reference. Can you take a look and see if something can be done to address these errors?

UOL Bid Adapter
    buildRequests
      buildRequest geolocation param
        �[31m✗ �[39m�[31mshould contain user coordinates if (i) DNT is off; (ii) browser supports implementation; (iii) localStorage contains geolocation history�[39m
	undefined is not an object (evaluating 'navigator.permissions.query')
	grantTriangulation@webpack:///test/spec/modules/uolBidAdapter_spec.js:96:46 <- test/test_index.js:129093:46
	webpack:///test/spec/modules/uolBidAdapter_spec.js:177:8 <- test/test_index.js:129172:27

        �[31m✗ �[39m�[31mshould not contain user coordinates if localStorage is empty�[39m
	undefined is not an object (evaluating 'navigator.permissions.query')
	denyTriangulation@webpack:///test/spec/modules/uolBidAdapter_spec.js:104:46 <- test/test_index.js:129101:46
	webpack:///test/spec/modules/uolBidAdapter_spec.js:186:8 <- test/test_index.js:129181:26

        �[31m✗ �[39m�[31mshould not contain user coordinates if browser doesnt support permission query�[39m
	undefined is not an object (evaluating 'navigator.permissions.query')
	removeQuerySupport@webpack:///test/spec/modules/uolBidAdapter_spec.js:112:46 <- test/test_index.js:129109:46
	webpack:///test/spec/modules/uolBidAdapter_spec.js:195:8 <- test/test_index.js:129190:27

UOL Bid Adapter
    buildRequests
      buildRequest geolocation param
        �[31m✗ �[39m�[31mshould contain user coordinates if (i) DNT is off; (ii) browser supports implementation; (iii) localStorage contains geolocation history�[39m
	TypeError: Unable to get property 'query' of undefined or null reference
	   at grantTriangulation (webpack:///test/spec/modules/uolBidAdapter_spec.js:96:6 <- test/test_index.js:129093:7)
	   at Anonymous function (webpack:///test/spec/modules/uolBidAdapter_spec.js:177:8 <- test/test_index.js:129172:9)

        �[31m✗ �[39m�[31mshould not contain user coordinates if localStorage is empty�[39m
	TypeError: Unable to get property 'query' of undefined or null reference
	   at denyTriangulation (webpack:///test/spec/modules/uolBidAdapter_spec.js:104:6 <- test/test_index.js:129101:7)
	   at Anonymous function (webpack:///test/spec/modules/uolBidAdapter_spec.js:186:8 <- test/test_index.js:129181:9)

        �[31m✗ �[39m�[31mshould not contain user coordinates if browser doesnt support permission query�[39m
	TypeError: Unable to get property 'query' of undefined or null reference
	   at removeQuerySupport (webpack:///test/spec/modules/uolBidAdapter_spec.js:112:6 <- test/test_index.js:129109:7)
	   at Anonymous function (webpack:///test/spec/modules/uolBidAdapter_spec.js:195:8 <- test/test_index.js:129190:9)

lotani-uolinc · 2018-07-17T16:19:39Z

@jsnellbaker, thanks for the review. We are now looking into alternatives for the safari issue.

lotani-uolinc · 2018-07-17T17:24:40Z

@jsnellbaker, as a fix, we disabled geolocation tests for engines that do not implement permission and geolocation APIs.
Feel free to reach if any changes are required.

jsnellbaker · 2018-07-17T17:51:48Z

@lotani-uolinc That's fine; it looks like everything is settled.

lotani-uolinc · 2018-07-17T18:29:27Z

@jsnellbaker, nice! Thanks for your time.

UOL - BidAdapter release candidate

fc26641

jsnellbaker self-assigned this Jul 16, 2018

jsnellbaker self-requested a review July 16, 2018 14:10

jsnellbaker added the needs review label Jul 16, 2018

jsnellbaker mentioned this pull request Jul 16, 2018

UOL - bidAdapter release candidate docs prebid/prebid.github.io#889

Merged

jsnellbaker requested changes Jul 16, 2018

View reviewed changes

jsnellbaker added needs update and removed needs review labels Jul 16, 2018

UOL - BidAdapter - applying requested changes/fixes for the release c…

2bdb6ff

…andidate

UOL - BidAdapter - disabled geolocation tests for unsupported browsers

952996f

jsnellbaker approved these changes Jul 17, 2018

View reviewed changes

jsnellbaker added LGTM and removed needs update labels Jul 17, 2018

jsnellbaker merged commit a658ae7 into prebid:master Jul 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UOL - BidAdapter release candidate #2850

UOL - BidAdapter release candidate #2850

lotani-uolinc commented Jul 14, 2018 •

edited

Loading

mkendall07 commented Jul 14, 2018

jsnellbaker commented Jul 16, 2018

lotani-uolinc commented Jul 16, 2018

jsnellbaker left a comment

jsnellbaker Jul 16, 2018

jsnellbaker Jul 16, 2018

jsnellbaker Jul 16, 2018

jsnellbaker Jul 16, 2018

lotani-uolinc Jul 16, 2018

jsnellbaker commented Jul 16, 2018

lotani-uolinc commented Jul 16, 2018

lotani-uolinc commented Jul 16, 2018 •

edited

Loading

jsnellbaker commented Jul 17, 2018

lotani-uolinc commented Jul 17, 2018

lotani-uolinc commented Jul 17, 2018

jsnellbaker commented Jul 17, 2018

lotani-uolinc commented Jul 17, 2018

		@@ -0,0 +1,202 @@
		import * as utils from 'src/utils';
		// import {config} from 'src/config';

UOL - BidAdapter release candidate #2850

UOL - BidAdapter release candidate #2850

Conversation

lotani-uolinc commented Jul 14, 2018 • edited Loading

Type of change

Description of change

mkendall07 commented Jul 14, 2018

jsnellbaker commented Jul 16, 2018

lotani-uolinc commented Jul 16, 2018

jsnellbaker left a comment

Choose a reason for hiding this comment

jsnellbaker Jul 16, 2018

Choose a reason for hiding this comment

jsnellbaker Jul 16, 2018

Choose a reason for hiding this comment

jsnellbaker Jul 16, 2018

Choose a reason for hiding this comment

jsnellbaker Jul 16, 2018

Choose a reason for hiding this comment

lotani-uolinc Jul 16, 2018

Choose a reason for hiding this comment

jsnellbaker commented Jul 16, 2018

lotani-uolinc commented Jul 16, 2018

lotani-uolinc commented Jul 16, 2018 • edited Loading

jsnellbaker commented Jul 17, 2018

lotani-uolinc commented Jul 17, 2018

lotani-uolinc commented Jul 17, 2018

jsnellbaker commented Jul 17, 2018

lotani-uolinc commented Jul 17, 2018

lotani-uolinc commented Jul 14, 2018 •

edited

Loading

lotani-uolinc commented Jul 16, 2018 •

edited

Loading