Adding Video Support to Conversant Adapter #1153

trex-conversant · 2017-04-24T21:48:54Z

Added support for video
Added tagid parameter
Added pos parameter
Changed API endpoint
Added support for multiple sizes per ad unit
Updated tests.

Type of change

[x ] Feature
[x ] Code style update (formatting, local variables)

Description of change

Added in support for video, the bulk of this PR. Added in additional parameters which required minor changes. Now supporting multiple sizes per ad unit via new API endpoint.

Other information

dbemiller · 2017-04-28T15:10:17Z

Hey. Since your params are changing, could you also open a PR to update the docs?

We'll also need some valid bid params we can use for testing. See the PR process

trex-conversant · 2017-04-28T23:45:48Z

Hi @dbemiller you can use site ID 108060 to test with. And thank for the info on the doc PR, will do.

dbemiller · 2017-05-02T14:17:28Z

I'm not seeing any valid bids coming from your adapter... using this cannibalized source code from the example page:

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8" />
    <title>Prebid.js video adUnit example</title>

    <!-- videojs -->
    <link rel="stylesheet" href="http://vjs.zencdn.net/5.9.2/video-js.css">
    <script type="text/javascript" src="http://vjs.zencdn.net/5.9.2/video.js"></script>

    <!-- videojs-vast-vpaid -->
    <link href="https://cdnjs.cloudflare.com/ajax/libs/videojs-vast-vpaid/2.0.2/videojs.vast.vpaid.min.css" rel="stylesheet">
    <script src="https://cdnjs.cloudflare.com/ajax/libs/videojs-vast-vpaid/2.0.2/videojs_5.vast.vpaid.min.js"></script>

    <!-- prebid.js -->
    <script src="../../build/dev/prebid.js" async></script>


    <script>

      var pbjs = pbjs || {};
      pbjs.que = pbjs.que || [];

      /*
       Prebid Video adUnit
       */

      var videoAdUnit = {
        code: 'video1',
        sizes: [640,480],
        mediaType: 'video',
        bids: [
          {
            bidder: 'conversant',
            params: {
              site_id: '108060',
              video: {
                skipppable: true,
                playback_method: ['auto_play_sound_off']
              }
            }
          }
        ]
      };

      pbjs.que.push(function(){
        pbjs.addAdUnits(videoAdUnit);
        pbjs.requestBids({
          timeout : 700,
          bidsBackHandler : function(bids) {
            console.log("BIDS ARE:");
            console.log(bids);
          }
        });
      });

      pbjs.bidderSettings =
        {
          standard: {
            adserverTargeting: [
              {
                key: "hb_bidder",
                val: function (bidResponse) {
                  return bidResponse.bidderCode;
                }
              }, {
                key: "hb_adid",
                val: function (bidResponse) {
                  return bidResponse.adId;
                }
              }, {
                key: "hb_pb",
                val: function (bidResponse) {
                  return "10.00";
                }
              }, {
                key: "hb_size",
                val: function (bidResponse) {
                  return bidResponse.size;

                }
              }
            ]
          }
        };

    </script>
</head>
<body></body>
</html>

Am I missing something?

dbemiller · 2017-05-01T20:22:33Z

src/adapters/conversant.js

+        var banner = Array.isArray(format) ? {format: format} : {w: adW, h: adH};
+
+        if(pos !== '')
+	      banner.pos = pos;


The code coverage on all these optional params is pretty thin... these could do with some more unit tests. Definitely less than 80% coverage on the changed code here.

dbemiller · 2017-05-01T20:39:13Z

src/adapters/conversant.js

-        adW = bidSizes[0][0];
-        adH = bidSizes[0][1];
+        format = [];
+        utils._each(bidSizes,function(bidSize){


Lots of style issues (here and throughout). Please conform to standard javascript, except for the exclusions listed here . You'll probably be forced to by the CI anyway, since #1111 will probably be merged very soon.

trex-conversant · 2017-05-05T18:21:03Z

PR for Docs: prebid/prebid.github.io#240

trex-conversant · 2017-05-18T00:34:12Z

For testing video, please test with site_id 88563.

mkendall07 · 2017-05-18T21:15:34Z

Please fix linting errors. You can use eslint --fix

dbemiller · 2017-05-24T16:15:52Z

@trex-conversant After that merge, the diff shows 211 files changed. Please rebase, or open a new PR, to get a clean diff.

dbemiller · 2017-06-01T17:07:25Z

@trex-conversant Bids are coming back invalid with site_id 88563...

trex-conversant · 2017-06-01T22:53:37Z

Unfortunate timing, the test campaign just happened to end today, but I've renewed it. It should be good now.

dbemiller · 2017-06-06T13:23:26Z

@trex-conversant Looks like another conflict popped up

* Added support for video * Added tag_id parameter * Added position parameter * Changed API endpoint * Added support for multiple sizes per ad unit

mjacobsonny added the video label Apr 28, 2017

dbemiller self-requested a review April 28, 2017 15:15

dbemiller self-assigned this Apr 28, 2017

dbemiller reviewed May 2, 2017

View reviewed changes

protonate added the needs review label May 2, 2017

dbemiller added needs update and removed needs review labels May 17, 2017

trex-conversant force-pushed the develop branch from 727bd1d to 29eb1d5 Compare May 25, 2017 20:28

dbemiller added needs review and removed needs update labels Jun 1, 2017

dbemiller approved these changes Jun 2, 2017

View reviewed changes

dbemiller added LGTM and removed needs review labels Jun 2, 2017

protonate added this to the Prebid 0.25.0 milestone Jun 2, 2017

Adding Video Support for Conversant Adapter

66f21a0

* Added support for video * Added tag_id parameter * Added position parameter * Changed API endpoint * Added support for multiple sizes per ad unit

trex-conversant force-pushed the develop branch from 29eb1d5 to 66f21a0 Compare June 6, 2017 18:46

protonate approved these changes Jun 7, 2017

View reviewed changes

protonate merged commit 3d174d1 into prebid:master Jun 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding Video Support to Conversant Adapter #1153

Adding Video Support to Conversant Adapter #1153

trex-conversant commented Apr 24, 2017

dbemiller commented Apr 28, 2017 •

edited

Loading

trex-conversant commented Apr 28, 2017

dbemiller commented May 2, 2017 •

edited

Loading

dbemiller May 1, 2017 •

edited

Loading

dbemiller May 1, 2017 •

edited

Loading

trex-conversant commented May 5, 2017

trex-conversant commented May 18, 2017

mkendall07 commented May 18, 2017

dbemiller commented May 24, 2017

dbemiller commented Jun 1, 2017 •

edited

Loading

trex-conversant commented Jun 1, 2017

dbemiller commented Jun 6, 2017

Adding Video Support to Conversant Adapter #1153

Adding Video Support to Conversant Adapter #1153

Conversation

trex-conversant commented Apr 24, 2017

Type of change

Description of change

Other information

dbemiller commented Apr 28, 2017 • edited Loading

trex-conversant commented Apr 28, 2017

dbemiller commented May 2, 2017 • edited Loading

dbemiller May 1, 2017 • edited Loading

Choose a reason for hiding this comment

dbemiller May 1, 2017 • edited Loading

Choose a reason for hiding this comment

trex-conversant commented May 5, 2017

trex-conversant commented May 18, 2017

mkendall07 commented May 18, 2017

dbemiller commented May 24, 2017

dbemiller commented Jun 1, 2017 • edited Loading

trex-conversant commented Jun 1, 2017

dbemiller commented Jun 6, 2017

dbemiller commented Apr 28, 2017 •

edited

Loading

dbemiller commented May 2, 2017 •

edited

Loading

dbemiller May 1, 2017 •

edited

Loading

dbemiller May 1, 2017 •

edited

Loading

dbemiller commented Jun 1, 2017 •

edited

Loading