Justpremium Adapter #1227

mkalafior · 2017-05-24T14:45:47Z

Type of change

New bidder adapter

Description of change

New adapter from Justpremium

test parameters for validating bids

// example 1
{
  bidder: 'justpremium',
  params: {
    zone: 34604
  }
}
// example 2
{
  bidder: 'justpremium',
  params: {
    zone: 34604,
    allow: ['lb']
  }
}
// example 3
{
  bidder: 'justpremium',
  params: {
    zone: 34604,
    exclude: ['lb']
  }
}

contact email of the adapter’s maintainer
sebastian.widelak@justpremium.com
official adapter submission

jaiminpanchal27 · 2017-06-01T20:21:50Z

@mkalafior I see that your external js file http://d2nvliyzbo36lk.cloudfront.net/adp/bc.js playing around with pbjs global and i also see reference to winningBids
Can you elaborate what you are doing in minified js with pbjs

mkalafior · 2017-06-02T08:14:27Z

@jaiminpanchal27 sure, I was doing two things there (was as this is deprecated part of the code kept only for backward compatibility). At first I was detecting under which variable prebid is stored (one of our test publisher is using different name than pbjs) then I was checking if our bid was delivered on time (by listening on prebid timeout event). And at the end I was also gathering info about winning bids to understand why we lose (by tracking winning bids cpm, delivery time etc). Thats all.

jaiminpanchal27 · 2017-06-05T16:35:29Z

@mkalafior no bidder adapters is allowed to track winning bids that are not their own. So update your js code accordingly.

mkalafior · 2017-06-06T09:32:04Z

@jaiminpanchal27 i updated the code, please check it and tell me if its ok for you now.

jaiminpanchal27

I was able to get bid response back but didn't see ad loading. Can you check on that.
Also i left few comments.
Thanks

jaiminpanchal27 · 2017-06-14T19:37:58Z

src/adapters/justpremium.js

+          zone: zone,
+          hostname: d.location.hostname,
+          protocol: d.location.protocol.replace(':', ''),
+          sw: top.screen.width,


I am not sure but i think top.screen will throw error in cross domain iframe. Can you also confirm other usage of top in this function.

Will check that.

jaiminpanchal27 · 2017-06-14T19:50:35Z

src/adapters/justpremium.js

+        bidmanager.addBidResponse(bid.placementCode, bidder.createBid(function (ad) {
+          let bidObject;
+          if (!ad) {
+            bidObject = bidfactory.createBid(2);


Please add bid object as second parameter in createBid call. Also it would be great if you can use STATUS.GOOD constant instead of 1 defined in constants.json file

jaiminpanchal27 · 2017-06-14T19:52:54Z

src/adapters/justpremium.js

+      bidmanager.addBidResponse(bid.placementCode, bidObject);
+      bid = findBid(zone, reqBids);
+    }
+    console.error(err);


Use utils.logError

jaiminpanchal27 · 2017-06-14T19:55:17Z

src/adapters/justpremium.js

+  }
+
+  function loadResources() {
+    if (toLoad.length > 0) {


Any reason toLoad is array. If not than you can update this code

In development mode, instead of one file, few separated ones are loaded. For this purpose I would prefer to leave it as it is. Of course if its ok for you.

mkalafior · 2017-06-27T14:24:02Z

@jaiminpanchal27 I updated the code and also applied most of your suggestions (I only left toLoad as an array).
But one remark here. As we are not able to serve an ad from cross origin iframe, I added a condition to detect such execution and to return empty bid then. Hope its ok for you.
Additionally I prepared a demo with our adapter here.

mkendall07 · 2017-07-11T19:52:09Z

modules/justpremiumBidAdapter.js

+        bidmanager.addBidResponse(bid.placementCode, bidder.createBid(function (ad) {
+          let bidObject;
+          if (!ad) {
+            bidObject = bidfactory.createBid(CONSTANTS.STATUS.NO_BID);


Need to pass through the bidObj from the request. Take a look at other adapters for examples

mkendall07

Pass through bidObj so we can get the request bid.id

mkalafior · 2017-08-01T07:20:40Z

@mkendall07, @jaiminpanchal27 are you able to check my last commit? I followed the instruction given by @mkendall07. Is there anything else which is blocking us to merge this PR?

ehulsbosch · 2017-08-14T06:43:32Z

@mkendall07 could you please review the change that was requested?

mkendall07

LGTM. Please note that as of prebid 1.0, we won't accept bidders that load external JS for fetching demand. Only XHR requests will be supported.

mkendall07 · 2017-08-14T11:28:08Z

This is merged into master. Please submit a PR to the docs repo to add a file for your adapter to the bidders directory so your adapter's params will appear on the bidders page. Thank you for contributing

* Justpremium adapter and unit tests. * Fix test suit. * Performance improvements. * Changes requested in pull request review. * Register justpremium adapter in adaptermanager * pass through bid from request * fix linting errors

mkalafior added 2 commits May 24, 2017 14:26

Justpremium adapter and unit tests.

8aa8f9c

Fix test suit.

00a2ce6

jaiminpanchal27 self-assigned this May 30, 2017

Performance improvements.

0816e98

protonate requested review from jaiminpanchal27 and protonate June 9, 2017 17:27

jaiminpanchal27 added the in progress label Jun 14, 2017

jaiminpanchal27 requested changes Jun 14, 2017

View reviewed changes

jaiminpanchal27 added needs update and removed in progress labels Jun 14, 2017

mkalafior added 3 commits June 27, 2017 15:16

Changes requested in pull request review.

6cc1617

Merge with upstream

4917b85

Register justpremium adapter in adaptermanager

5fefb28

Merge remote-tracking branch 'upstream/master'

92da906

jaiminpanchal27 previously approved these changes Jul 11, 2017

View reviewed changes

jaiminpanchal27 added LGTM and removed needs update labels Jul 11, 2017

mkendall07 reviewed Jul 11, 2017

View reviewed changes

mkendall07 added needs update and removed LGTM labels Jul 11, 2017

mkendall07 requested changes Jul 11, 2017

View reviewed changes

mkalafior added 2 commits July 20, 2017 08:41

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

83d5022

pass through bid from request

16a7b42

mkalafior dismissed jaiminpanchal27’s stale review via 16a7b42 July 20, 2017 08:38

jaiminpanchal27 assigned mkendall07 and unassigned jaiminpanchal27 Aug 1, 2017

mkalafior added 2 commits August 2, 2017 13:27

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

b943b79

fix linting errors

d8ddfd0

mkendall07 approved these changes Aug 14, 2017

View reviewed changes

mkendall07 merged commit 9581c00 into prebid:master Aug 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Justpremium Adapter #1227

Justpremium Adapter #1227

mkalafior commented May 24, 2017

jaiminpanchal27 commented Jun 1, 2017

mkalafior commented Jun 2, 2017 •

edited

Loading

jaiminpanchal27 commented Jun 5, 2017

mkalafior commented Jun 6, 2017

jaiminpanchal27 left a comment

jaiminpanchal27 Jun 14, 2017

mkalafior Jun 21, 2017

jaiminpanchal27 Jun 14, 2017

jaiminpanchal27 Jun 14, 2017

jaiminpanchal27 Jun 14, 2017

mkalafior Jun 21, 2017

mkalafior commented Jun 27, 2017

mkendall07 Jul 11, 2017

mkendall07 left a comment

mkalafior commented Aug 1, 2017

ehulsbosch commented Aug 14, 2017

mkendall07 left a comment

mkendall07 commented Aug 14, 2017

Justpremium Adapter #1227

Justpremium Adapter #1227

Conversation

mkalafior commented May 24, 2017

Type of change

Description of change

jaiminpanchal27 commented Jun 1, 2017

mkalafior commented Jun 2, 2017 • edited Loading

jaiminpanchal27 commented Jun 5, 2017

mkalafior commented Jun 6, 2017

jaiminpanchal27 left a comment

Choose a reason for hiding this comment

jaiminpanchal27 Jun 14, 2017

Choose a reason for hiding this comment

mkalafior Jun 21, 2017

Choose a reason for hiding this comment

jaiminpanchal27 Jun 14, 2017

Choose a reason for hiding this comment

jaiminpanchal27 Jun 14, 2017

Choose a reason for hiding this comment

jaiminpanchal27 Jun 14, 2017

Choose a reason for hiding this comment

mkalafior Jun 21, 2017

Choose a reason for hiding this comment

mkalafior commented Jun 27, 2017

mkendall07 Jul 11, 2017

Choose a reason for hiding this comment

mkendall07 left a comment

Choose a reason for hiding this comment

mkalafior commented Aug 1, 2017

ehulsbosch commented Aug 14, 2017

mkendall07 left a comment

Choose a reason for hiding this comment

mkendall07 commented Aug 14, 2017

mkalafior commented Jun 2, 2017 •

edited

Loading