Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add eplanningBidAdapter #2003

Merged
merged 3 commits into from
Jan 22, 2018
Merged

Add eplanningBidAdapter #2003

merged 3 commits into from
Jan 22, 2018

Conversation

Nistenf
Copy link
Contributor

@Nistenf Nistenf commented Dec 27, 2017

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

This PR adds the eplanningBidAdapter, which was removed because the old version did not comply with the new 1.0 requirements.

  • test parameters for validating bids
var adUnits = [{
    code: 'div-gpt-ad-1460505748561-0',
    sizes: [[300, 250]],
    bids: [{
      bidder: 'eplanning',
      params: {
        t: 1
      }
    }]
}];

Other information

The bidder parameters have not changed so the documentation doesn't need to be updated.
An important note: the adserver code needs a small update (it will be done in the next few days) before it can correctly answer in the format the adapter expects (the test adunit does work correctly, though), I'm submitting the PR now so we can be sure the adapter is correctly verified.

Thanks!

@sebaperez
Copy link
Contributor

Hi @dbemiller, could this be reviewed? I know you are busy but we would like to be compatible with 1.0 asap

@dbemiller
Copy link
Contributor

dbemiller commented Jan 2, 2018

@sebaperez I do minimal work on Prebid.js these days... my main focus is on Prebid Server.

Most everyone involved on both projects was out last week, but one of the Prebid.js maintainers will get to it whenever they have bandwidth.

@mike-chowla
Copy link
Contributor

The user sync generating a 404 because the url is being set to the string 'sync' which causes the browser to try load an object called 'sync' relative to the current page

        syncs.push({
           type: 'image',
           url: 'sync',
          });

@mike-chowla
Copy link
Contributor

The other thing I noted, the returned test ad is just text. Most adapters test ad is an image

@Nistenf
Copy link
Contributor Author

Nistenf commented Jan 4, 2018

@mike-chowla Thanks for the feedback. I just fixed the sync, and changed the test ad to an image (this needs to be deployed)

@mike-chowla
Copy link
Contributor

Doesn't look like the test ad image deployed but otherwise looks good

Copy link
Contributor

@mike-chowla mike-chowla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With fixes looks, good.

@sebaperez
Copy link
Contributor

@mike-chowla thank you so much for reviewing!
Can we move foward with this? The image ad for testing will be deployed in the following days. However seems that the code itself is OK. Forgive me for insisting, but we really want to be compatible with 1.0

@mike-chowla
Copy link
Contributor

@sebaperez, I've already enter my review as 'approved' so you are good from my end. I understand waiting to get your 1.0 code out :-)

Copy link
Contributor

@mike-chowla mike-chowla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jaiminpanchal27 jaiminpanchal27 merged commit c1d86ea into prebid:master Jan 22, 2018
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Jan 23, 2018
* 'master' of https://github.com/prebid/Prebid.js: (23 commits)
  Update Atomx adapter for Prebid v1.0 (prebid#2026)
  Add vi bid adapter (prebid#2020)
  Add eplanningBidAdapter (prebid#2003)
  OpenX Adapter: Update to support mediaTypes field, instead of the deprecated mediaType field (prebid#1974)
  Separate bids & won calls (prebid#2015)
  1.0 adapter support for mantis (prebid#1840)
  Media.net adapter added (prebid#2038)
  GumGum Adapter for Prebid.js 1.0 (prebid#1966)
  Serverbid Bid Adapter: updated docs and ad sizes (prebid#2023)
  Adding districtm as an alias (prebid#2018)
  Use sudo to workaround Travis regression (prebid#2041)
  Fix uncached video bids triggering callback early (prebid#2017)
  Re-implemented RhythmOne audit beacon in prebid 1.0 interface (prebid#1953)
  Add NasmediaAdmixer adapter for Perbid.js 1.0 (prebid#1937)
  Update Adform adapter to Prebid v1.0 (prebid#1947)
  Upgrade Admixer adapter for Prebid 1.0 (prebid#1755)
  multiformat size validation checks (prebid#1964)
  Gjirafa Bidder Adapter (prebid#1944)
  pin gulp-connect at non-broken version (prebid#2008)
  Added dynamic ttl property for One Display and One Mobile. (prebid#2004)
  ...
@rmloveland
Copy link
Contributor

Added the needs-docs label since it's not showing up on http://prebid.org/download.html in the 1.2.0 dropdown due to lack of the page having the "prebid 1.0 support" key in the page's YAML header.

@sebaperez
Copy link
Contributor

sebaperez commented Feb 1, 2018

@rmloveland I just opened a pr in prebid.org website repo (ref prebid/prebid.github.io#586)

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

* Add new parameter to request

* Fix sync URL parameter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants