Aardvark v1.0 #2507

dejanstrbac · 2018-05-08T16:49:12Z

Type of change

Bugfix
Feature
New bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other

Description of change

Support for Prebid.js > v1.0

test parameters for validating bids

{
    bidder: 'aardvark',
    params: {
       ai: '0000',
       sc: '1234'
    }
}

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

christ@rtk.io @naegelin is the official maintainer and responsible (CTO of RTK.io)

official adapter submission

jsnellbaker

Hi @dejanstrbac Thanks for submitting this new adapter. I was taking a look through the code and found a few things that should be reviewed again.

In addition to the comments in-line below, when I was testing the new adapter with the test params in the .md file - I was get a 'no bid' type of response from the system. Can you take a look into this as well? Ideally we should have a valid test bid that can serve to verify everything's working with a new adapter.

Below is a copy of the response:

[{"nurl": "", "cid": "2ba9426c4afb42", "media": "banner", "error": "No bids received for WygO", "adm": "", "id": "WygO", "cpm": "0.00"}]

jsnellbaker · 2018-05-09T16:13:43Z

modules/aardvarkBidAdapter.js

+        }
+
+        if (bidderRequest && bidderRequest.gdprConsent) {
+          rMap.payload.gdpr = bidderRequest.gdprConsent.gdprApplies;


There may be rare chances where this value may be undefined if the CMP didn't have it defined. Do you want to set your own back-up/default value in that case? If so, I would recommend to check if typeof bidderRequest.gdprConsent.gdprApplies === 'boolean' so that you can accept natural cases where gdprApplies was set to `false.

jsnellbaker · 2018-05-09T16:28:11Z

modules/aardvarkBidAdapter.js

+    const syncs = [];
+    var url = '//' + SYNC_ENDPOINT + '/cs';
+    var gdprApplies = true;
+    if (typeof gdprConsent.gdprApplies === 'boolean') {


please add to the check if gdprConsent exists. On pages without a CMP, this current code would lead to an undefined error.

jsnellbaker · 2018-05-09T16:30:37Z

modules/aardvarkBidAdapter.js

+        currency: rawBid.currency ? rawBid.currency : AARDVARK_CURRENCY,
+        netRevenue: rawBid.netRevenue ? rawBid.netRevenue : true,
+        ttl: rawBid.ttl ? rawBid.ttl : AARDVARK_TTL,
+        exchange: rawBid.ex,


Can remove this exchange field from the bidResponse object. This is not a current field.

jsnellbaker · 2018-05-09T18:05:27Z

modules/aardvarkBidAdapter.js

+const AARDVARK_TTL = 300;
+const AARDVARK_CURRENCY = 'USD';
+
+var hasSynced = false;


could change this to let instead of var.

jsnellbaker · 2018-05-09T18:06:19Z

modules/aardvarkBidAdapter.js

+    var referer = utils.getTopWindowUrl();
+    var pageCategories = [];
+
+    if (window.rtkcategories && Array.isArray(window.rtkcategories)) {


Can you clarify the use-case here for reading this window object?

This is a requirement from several tier 1 publishers in order to allow RTK buyers to target specific inventory. It is currently implemented across Hearst and others

dejanstrbac · 2018-05-10T17:24:53Z

@jsnellbaker updated according to the requested changes - thanks!

jsnellbaker · 2018-05-11T15:12:32Z

modules/aardvarkBidAdapter.js

+
+  interpretResponse: function(serverResponse, bidRequest) {
+    var bidResponses = [];
+    utils._each(serverResponse.body, function(rawBid) {


Can you review the logic in using this utils._each function? It's currently iterating through each of the properties of the response object and evaluating them each as an entire bid object.

This is a copy of the bid response object that I saw when I tested and it treated rawBid as demo_exchange (from the ex field) during the first pass of the function.

{"ex": "demo_exchange", "width": 300, "cpm": 0.01, "cid": "2edd6ea6821b56", "media": "banner", "adm": "<div>Demo Unit</div>", "id": "1234", "height": 250}

We've been updating the demo auction today, sorry about that. That is an array returned from our endpoint, so it gets iterated over as an array, not a map. It is now an array as it should be.

jsnellbaker

Thanks for making the changes; LGTM

* PreBid.js/master: (79 commits) increment prebid version Prebid 1.11.0 release replace find reference with imported version (prebid#2542) GDPR - add consent information to PBS cookie_sync request (prebid#2530) add support for safeframe workflow and new utils method to read adunit sizes (prebid#2523) Smart: GDPR support (prebid#2528) fix getPreparedBidForAuction to look for renderer on correct bid (prebid#2505) PubMatic Adapter: Bug fix to read all bids from seatBid array (prebid#2520) Create GXOne Bid Adapter and tests for it (prebid#2540) Lifestreet: gdpr and consent string parameters (prebid#2537) -GDPR support added in media net bidder (prebid#2538) change AppNexus endpoint to use ORTB (prebid#2532) fixed bug when latitute/longitue are not provided (prebid#2533) added gdpr support to userSync in rubicon adapter (prebid#2531) Show only summary and errors (prebid#2514) Aardvark v1.0 (prebid#2507) Add 1024x768 (size_id = 53) in sizeMap (prebid#2527) Add new Adapter brainyBidAdapter (prebid#2458) OpenX Adapter: GDPR Support (prebid#2504) add support for latLong in rubicon adapter (prebid#2508) ...

* Aardvark v1.0 + Add GDPR support * required modifications * cover case where gdprConsent data is not present * demo auction update * ignore empty bids * accept empty bids

harpere assigned jsnellbaker May 8, 2018

harpere added the needs review label May 8, 2018

dejanstrbac mentioned this pull request May 9, 2018

GDPR support for Aardvark prebid/prebid.github.io#750

Merged

Aardvark v1.0 + Add GDPR support

d5cf40c

dejanstrbac force-pushed the aardvark_adapter_update branch from 3396a45 to d5cf40c Compare May 9, 2018 16:01

jsnellbaker requested changes May 9, 2018

View reviewed changes

jsnellbaker added needs update adapter support not receiving bids and removed needs review labels May 9, 2018

jsnellbaker reviewed May 9, 2018

View reviewed changes

Dejan Strbac added 3 commits May 10, 2018 16:06

required modifications

e9093f7

cover case where gdprConsent data is not present

048f0bd

demo auction update

c52e1d7

ignore empty bids

0718446

jsnellbaker reviewed May 11, 2018

View reviewed changes

accept empty bids

1fe7e68

jsnellbaker approved these changes May 14, 2018

View reviewed changes

jsnellbaker added LGTM and removed needs update not receiving bids labels May 14, 2018

jsnellbaker merged commit 119e590 into prebid:master May 14, 2018

dejanstrbac deleted the aardvark_adapter_update branch May 15, 2018 09:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Aardvark v1.0 #2507

Aardvark v1.0 #2507

dejanstrbac commented May 8, 2018 •

edited

Loading

jsnellbaker left a comment

jsnellbaker May 9, 2018

jsnellbaker May 9, 2018

jsnellbaker May 9, 2018

jsnellbaker May 9, 2018

jsnellbaker May 9, 2018

naegelin May 9, 2018

dejanstrbac commented May 10, 2018

jsnellbaker May 11, 2018

dejanstrbac May 11, 2018

jsnellbaker left a comment

Aardvark v1.0 #2507

Aardvark v1.0 #2507

Conversation

dejanstrbac commented May 8, 2018 • edited Loading

Type of change

Description of change

jsnellbaker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dejanstrbac commented May 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsnellbaker left a comment

Choose a reason for hiding this comment

dejanstrbac commented May 8, 2018 •

edited

Loading