New Content Ignite adapter #2268

jdrucey · 2018-03-15T14:00:36Z

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

Adding a new adapter for Content Ignite
Initially based on AdButler as used by our backend, but soon to introduce video and native

test parameters for validating bids

{
  bidder: 'contentignite',
  params: {
    accountID: '168237',
    zoneID: '299680'
  }
}

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

contact email of the adapter’s maintainer: jamie@contentignite.com
official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

A link to a PR on the docs repo: New Content Ignite docs prebid.github.io#670

Other information

bretg

@jdrucey - please back out the changes to integrationExamples/gpt/hello_world.html.

jdrucey · 2018-03-16T09:01:16Z

@bretg Reverted those hello world changes! Cheers.

jsnellbaker

Hi @jdrucey Thanks for submitting this new adapter. While overall things seem to be good, there are a few items that should be reviewed/updated. Can you please review and make the recommended changes?

On a separate note - while I was testing the adapter using the hello_world page, I initially wasn't seeing the bid render on the page even though there was a response coming back from your system. When I looked further, I noticed the issue was due differences in the requested/received sizes of the ad unit. I had the hello_world page configured for 300x250 or 300x600 ads, where the test params were configured for 728x90. I realized the bid wasn't accepted due to the internal check you perform in the interpretResponse, so it was silently rejected (ie nothing in the console statements like a warning/info message).

While it's not explicitly required, it may be helpful to send out a message (using utils.logWarn or utils.logInfo) when something doesn't pass a check so it's easier to debug later.

jsnellbaker · 2018-03-16T12:57:25Z

modules/contentigniteBidAdapter.js

@@ -0,0 +1,112 @@
+'use strict'


I'm guessing this was copied over from the AdButler adapter, but this isn't really needed. This is actually automatically applied onto the adapter files when pbjs is built and the file's are converted to comply with the setting.

As a general note, the file should ideally be written with ES6 standards (eg replacing vars with let and const where appropriate, ending statements with ;). I realize there's a bit to change in this regard, but it's a recommendation we've been asking for all 1.0 adapters to keep the code updated and consistent.

Ive now introduced arrow functions, const/let usage and reformatted with semi-colons. Would be really neat if there could be a Prettier config file added to the Prebid.js project. It is something we use a lot our end to auto-format js, and while it is very opinionated (by design) there are some settings for project based preference, like the semi-colons in this case. Not sure if this has been considered before, but just an idea!

jsnellbaker · 2018-03-16T12:59:37Z

modules/contentigniteBidAdapter.js

+      maxCPM = utils.getBidIdParameter('maxCPM', bidObj.params)
+      width = parseInt(serverResponse.width)
+      height = parseInt(serverResponse.height)
+      console.log(serverResponse.cpm)


Could you please remove this console.log statement?

Sorry about that! i could have sworn i removed it! ha

jsnellbaker · 2018-03-16T13:01:52Z

modules/contentigniteBidAdapter.js

+    var bidResponse = {}
+    var isCorrectSize = false
+    var isCorrectCPM = true
+    var CPM


Does CPM need to be in all caps? Can you make it consistent with the other variables syntax (camelCase)?

Have reverted to lowercase cpm

jsnellbaker · 2018-03-16T13:03:20Z

modules/contentigniteBidAdapter.js

+      })
+      if (isCorrectCPM && isCorrectSize) {
+        bidResponse.requestId = bidObj.bidId
+        bidResponse.bidderCode = spec.code


The bidderCode does not need to be populated here; it's actually managed by the prebid code in our bidderFactory's code. Can you please remove this line?

This has now been removed

jdrucey · 2018-03-16T16:09:13Z

Hi @jsnellbaker, thanks for taking the time to look over the code, we appreciate it.
All your suggestions are very sensible, and i agree it is good to bring this code up-to-date with regards to ES6 standards, i certainly prefer it myself.
I have pushed the fixes and changes so let me know if all is as expected.

jsnellbaker

@jdrucey Thanks for making the changes.

LGTM

* 'master' of https://github.com/prebid/Prebid.js: EngageBDR New Bid Adapter (prebid#2309) [FEAT] adunit sizes support (prebid#2320) Support aliases in prebidServer (prebid#2257) Changing default currency file to https (prebid#2306) Update stalebot labels (prebid#2319) Enhance location detection within utils (prebid#2167) if cache markup is not enabled, set it to the default value 0 (prebid#2302) Serverbid Bid Adapter: Added archon alias (prebid#2293) Smart Ad Server: Fix bug when multi bids (prebid#2170) NEW adapter AdtelligentBidAdapter (prebid#2137) add optional param to bridgewellBidAdapter (prebid#2289) Increment Pre Version Prebid 1.6.0 Release Unit test fixes (prebid#2301) PBS videoCacheKey and vastUrl (prebid#2101) Add Oneplanetonly Bid Adapter (prebid#2269) firing new adRenderFailed event when renderAd() fails (prebid#2210) Add Content Ignite adapter (prebid#2268) add hb_cache_id, hb_uuid should be deprecated and replaced by hb_cache_id (prebid#2273) Update Yieldlab adapter and add official maintainer (prebid#2231) Update for Media.net adapter (prebid#2232) Update to Rubicon Adapter for mediaTypes support (prebid#2272) message formatting (prebid#2285) Yieldbot impression image creation fix (prebid#2277) Updated Bid params (prebid#2275) Audience Network: Add 'pbv' and 'cb' query params (prebid#2252) Add e-planning analytics adapter (prebid#2211) Add vastUrl for Gamma Adapter Video (prebid#2261) update params for test bid (prebid#2267) Updated adUnitCode (prebid#2262)

* initial Content Ignite adapter * revert hello world example * Fixes for latest prebid standards * reformatting with modern keywords

initial Content Ignite adapter

f935651

bretg requested a review from jsnellbaker March 15, 2018 17:10

bretg assigned jsnellbaker Mar 15, 2018

bretg added the needs review label Mar 15, 2018

bretg requested changes Mar 15, 2018

View reviewed changes

revert hello world example

0b6c288

jsnellbaker reviewed Mar 16, 2018

View reviewed changes

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

jamiedruce added 2 commits March 16, 2018 15:26

Fixes for latest prebid standards

41683f3

reformatting with modern keywords

20ccfc0

jsnellbaker approved these changes Mar 19, 2018

View reviewed changes

jsnellbaker added LGTM and removed needs update labels Mar 19, 2018

matthewlane approved these changes Mar 19, 2018

View reviewed changes

matthewlane merged commit 28e58ff into prebid:master Mar 19, 2018

dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018

Add Content Ignite adapter (prebid#2268)

e91f51b

* initial Content Ignite adapter * revert hello world example * Fixes for latest prebid standards * reformatting with modern keywords

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Content Ignite adapter #2268

New Content Ignite adapter #2268

jdrucey commented Mar 15, 2018

bretg left a comment

jdrucey commented Mar 16, 2018

jsnellbaker left a comment •

edited

Loading

jsnellbaker Mar 16, 2018

jdrucey Mar 16, 2018

jsnellbaker Mar 16, 2018

jdrucey Mar 16, 2018

jsnellbaker Mar 16, 2018

jdrucey Mar 16, 2018

jsnellbaker Mar 16, 2018

jdrucey Mar 16, 2018

jdrucey commented Mar 16, 2018

jsnellbaker left a comment

New Content Ignite adapter #2268

New Content Ignite adapter #2268

Conversation

jdrucey commented Mar 15, 2018

Type of change

Description of change

Other information

bretg left a comment

Choose a reason for hiding this comment

jdrucey commented Mar 16, 2018

jsnellbaker left a comment • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdrucey commented Mar 16, 2018

jsnellbaker left a comment

Choose a reason for hiding this comment

jsnellbaker left a comment •

edited

Loading