realvuBidAdapter by tchibirev · Pull Request #1571 · prebid/Prebid.js

tchibirev · 2017-09-07T18:07:38Z

Type of change

Description of change

New 'realvu' Bid Adapter

test parameters for validating bids

{
  bidder: 'realvu',
  params: {
     partnerId: '1Y',
     placementId: '9339508'
  }
}

Be sure to test the integration with your adserver using the Hello World sample page.

contact email of the adapter’s maintainer
it@realvu.com
official adapter submission

Other information

realvuAnalyticsAdapter.js initial merge

matthewlane

Thanks for the PR. I wasn't able to validate bids with the provided test parameters, please advise if those are still the correct params to test with. Also a few comments below for your review

matthewlane · 2017-09-11T20:57:38Z

modules/realvuBidAdapter.js

+    var callbackId = bid_request.bidId;
+    // utils.logMessage('realvuBidAdapter boost callback "' + callbackId + '", rez.realvu=' + rez.realvu);
+    if (rez.realvu === 'yes') {
+      var adap = new AppnexusAdapter();


Importing and using other adapters isn't generally recommended as it may result in unexpected behavior. What is the desired behavior here? Would creating a new alias bid adapter in appnexusBidAdapter fit this case?

Creating a new alias is not exactly what we need. RealVu adaper define either an ad unit is in view or not. Only if an ad unit is in view realvu adapter calls Appnexus. If the unit is out of view "no bid" is responded without request to server.

appnexusBidAdapter still shouldn't be imported though, but you can copy the buildJPTCall function and include it (or just the required parts) directly in this module

matthewlane · 2017-09-11T21:01:36Z

modules/realvuBidAdapter.js

+      for (var i = 0; i < pbids.length; i++) {
+        var bid_rq = pbids[i];
+        var sizes = utils.parseSizesInput(bid_rq.sizes);
+        top.realvu_boost.addUnitById({


References to top should be placed in a try/catch block to prevent possible uncaught TypeErrors

tchibirev · 2017-09-14T19:09:25Z

try-catch has been added to access top window.

matthewlane

A few more comments for review, and please provide testing parameters we can use to verify bid responses. See this file for an example of the pages we test bidder adapters with

matthewlane · 2017-09-14T20:48:09Z

modules/realvuBidAdapter.js

+    var callbackId = bid_request.bidId;
+    // utils.logMessage('realvuBidAdapter boost callback "' + callbackId + '", rez.realvu=' + rez.realvu);
+    if (rez.realvu === 'yes') {
+      var adap = new AppnexusAdapter();


appnexusBidAdapter still shouldn't be imported though, but you can copy the buildJPTCall function and include it (or just the required parts) directly in this module

matthewlane · 2017-09-14T20:49:07Z

modules/realvuBidAdapter.js

@@ -0,0 +1,64 @@
+import adaptermanager from 'src/adaptermanager';
+
+var utils = require('src/utils.js');


Prefer const for require statements and anywhere else in the module they can be used in place of var

tchibirev · 2017-09-29T20:32:54Z

Following recommendations appnexusBidAdapter was included inline.

tchibirev · 2017-10-06T14:30:53Z

Can we move forward with this adapter?

matthewlane · 2017-10-06T15:53:31Z

We still need test parameters for validating bids with this adapter. The params

{
  bidder: 'realvu',
  params: {
     partnerId: '1Y',
     placementId: '9339508'
  }
}

are not returning anything. Please update these or point us to a testing page and we can review

tchibirev · 2017-10-16T16:48:39Z

Evidently in your test you have referrer=localhost so an ad server responds with no-bid.
Please use this test page: http://control.realvu.net/prebid/realvuBidAdapterTest.aspx

jaiminpanchal27 · 2017-10-16T19:38:57Z

modules/realvuBidAdapter.js

+    var baseAdapter = new Adapter('appnexus');
+    var usersync = false;
+
+    baseAdapter.callBids = function (params) {


Are you using this callBids anywhere ? I think its not getting used, you can remove it.

jaiminpanchal27 · 2017-10-16T19:39:26Z

modules/realvuBidAdapter.js

+
+  // +copy/pasted appnexusBidAdapter, "handleAnCB" replaced with "handleRvAnCB"
+  var RvAppNexusAdapter = function RvAppNexusAdapter() {
+    var baseAdapter = new Adapter('appnexus');


Why do you need baseAdapter here ?

Corrected code is committed

jaiminpanchal27 · 2017-10-16T19:39:38Z

modules/realvuBidAdapter.js

+
+      // var bidsCount = anArr.length;
+
+      // set expected bids count for callback execution


Remove unwanted code

jaiminpanchal27 · 2017-10-16T19:39:45Z

modules/realvuBidAdapter.js

+        var callbackId = bidRequest.bidId;
+        adloader.loadScript(buildJPTCall(bidRequest, callbackId));
+
+        // store a reference to the bidRequest from the callback id


Remove unwanted code

jaiminpanchal27 · 2017-10-16T19:39:57Z

modules/realvuBidAdapter.js

+        // @endif
+      }
+    };
+    /*


Remove unwanted code

jaiminpanchal27 · 2017-10-16T19:41:46Z

test/spec/modules/realvuBidAdapter_spec.js

+    start: 1504628062271
+  };
+
+  var bidResponseStub = sinon.stub(bidmanager, 'addBidResponse');


stub needs to be defined and restored in beforeEach and afterEach hooks to avoid errors.

jaiminpanchal27 · 2017-10-16T19:42:38Z

test/spec/modules/realvuBidAdapter_spec.js

+  adapter = new Adapter();
+
+  describe('load boost', () => {
+    adapter.callBids(REQUEST);


These test cases are not getting executed.
Your test cases need to be in it. describe is grouping of test cases.
This https://gist.github.com/samwize/8877226 can be helpful.

The test cases are reorganized.

jaiminpanchal27 · 2017-10-16T21:23:14Z

@tchibirev Checks are failing due to linting errors. Also check code coverage using gulp test-coverage and gulp view-coverage

tchibirev · 2017-10-17T21:42:19Z

Could you take a look at last commit?

* 'master' of https://github.com/prebid/Prebid.js: (414 commits) Make response headers available to the specs (prebid#1748) add option to run tests in a specific file (prebid#1727) Update JCM Adapter to 1.0 (prebid#1715) Finished an unfinished comment. (prebid#1749) Platform.io Bidder Adapter update. Prebid v1.0. (prebid#1705) Fix window.top.host cross origin issue when in nested iframes. (prebid#1730) fix log message not displaying when referencing missing bidder (prebid#1737) Allow more than one placement from one page (prebid#1692) Justpremium Adapter bugfix (prebid#1716) Updating license (prebid#1717) realvuBidAdapter (prebid#1571) Update JSDoc to call the module `pbjs` (prebid#1572) Update Beachfront adapter for v1.0 (prebid#1675) Update AdButler adapter for Prebid v1.0 (prebid#1664) Increment pre version Fix for prebid#1628 (allowing standard bidCpmAdjustment) (prebid#1645) Prebid 0.31.0 Release Support native click tracking (prebid#1691) Initial commit for video support for pbs (prebid#1706) Fixes: Immediate adapter response may end auction (prebid#1690) ...

tchibirev and others added 12 commits August 29, 2017 17:36

init commit realvuAnalyticsAdapter.js

c775f65

Merge branch 'rvaa'

b65e020

realvuAnalyticsAdapter.js initial merge

realvuBidAdapter.js initial commit

4cdb383

realvuBidAdapter fixed for latest version

3806507

Merge remote-tracking branch 'upstream/master'

94aed8f

realvuBidAdapter unit_id:bid.code

e6cc8ba

Merge branch 'master' of https://github.com/tchibirev/Prebid.js

08d0162

tabs and spaces

975df01

Merge remote-tracking branch 'upstream/master'

c8cc17f

test implemented

1d9b120

postpone analytics

e3d099c

Merge remote-tracking branch 'upstream/master'

af90c50

protonate requested review from dbemiller and matthewlane September 7, 2017 19:23

protonate assigned matthewlane Sep 7, 2017

matthewlane reviewed Sep 11, 2017

View reviewed changes

tchibirev added 2 commits September 14, 2017 07:33

try-catch for top

185856f

Merge remote-tracking branch 'upstream/master'

5032ebf

matthewlane reviewed Sep 14, 2017

View reviewed changes

matthewlane added the needs update label Sep 14, 2017

Igor Tchibirev added 4 commits September 25, 2017 09:43

Merge remote-tracking branch 'upstream/master'

c021a4e

Merge branch 'master' of https://github.com/tchibirev/Prebid.js

9c02629

Merge remote-tracking branch 'upstream/master'

d6f473b

appnexusBidAdapter inline

98b2ef1

var replaced with const in require statements

89e7c70

matthewlane approved these changes Oct 16, 2017

View reviewed changes

matthewlane added LGTM and removed needs update labels Oct 16, 2017

jaiminpanchal27 requested changes Oct 16, 2017

View reviewed changes

jaiminpanchal27 added needs update and removed LGTM labels Oct 16, 2017

Igor Tchibirev added 2 commits October 16, 2017 16:02

Merge remote-tracking branch 'upstream/master'

571cb4f

unwanted code removed, test fixed

4d2374f

Igor Tchibirev added 3 commits October 17, 2017 11:43

Merge remote-tracking branch 'upstream/master'

4b30b35

lint and test-coverage fix

1e0c403

lint fix

a316cfc

mkendall07 added needs review and removed needs update labels Oct 19, 2017

jaiminpanchal27 approved these changes Oct 20, 2017

View reviewed changes

jaiminpanchal27 merged commit 37b218a into prebid:master Oct 20, 2017

jsnellbaker mentioned this pull request Nov 30, 2017

add new adapters since 0.30.0 release prebid/headerbid-expert#1

Merged

		@@ -0,0 +1,64 @@
		import adaptermanager from 'src/adaptermanager';

		var utils = require('src/utils.js');


		// var bidsCount = anArr.length;

		// set expected bids count for callback execution

Conversation

tchibirev commented Sep 7, 2017

Type of change

Description of change

Other information

Uh oh!

matthewlane left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tchibirev commented Sep 14, 2017

Uh oh!

matthewlane left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tchibirev commented Sep 29, 2017

Uh oh!

tchibirev commented Oct 6, 2017

Uh oh!

matthewlane commented Oct 6, 2017

Uh oh!

tchibirev commented Oct 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaiminpanchal27 commented Oct 16, 2017

Uh oh!

tchibirev commented Oct 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

matthewlane left a comment •

edited

Loading