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

Update Improve Digital adapter for Prebid 1.0 #1728

Merged
merged 4 commits into from
Oct 26, 2017

Conversation

jbartek25
Copy link
Contributor

Type of change

  • Other

Description of change

Improve Digital adapter changes for Prebid 1.0.

Copy link
Contributor

@dbemiller dbemiller left a comment

Choose a reason for hiding this comment

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

Looks overall fine... just one small comment from me

}
bid.ad = `${nurl}<script>${bidObject.adm}</script>`;
bid.adId = bidObject.id;
bid.bidderCode = spec.code;
Copy link
Contributor

Choose a reason for hiding this comment

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

Our docs are being updated now, but... you don't need to set this property. In fact if you do, it'll break adapter aliasing.

if (bidObject.nurl && bidObject.nurl.length > 0) {
nurl = `<img src="${bidObject.nurl}" width="0" height="0" style="display:none">`;
}
bid.ad = `${nurl}<script>${bidObject.adm}</script>`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add required param creativeId to bid response. http://prebid.org/dev-docs/bidder-adapter-1.html

Copy link
Contributor

@dbemiller dbemiller left a comment

Choose a reason for hiding this comment

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

Actually... one more change, because #1742 got merged.

The first argument to interpretResponse now looks like this:

{
  body: responseBody,
  headers: {
    get: function(header) { /* returns a header from the HTTP response */ }
  }
}

You'll have to update the spec so that it looks through that object as well now.

@jbartek25
Copy link
Contributor Author

All requested changes done

@dbemiller dbemiller merged commit c730f95 into prebid:master Oct 26, 2017
Millerrok pushed a commit to Vertamedia/Prebid.js that referenced this pull request Oct 31, 2017
* 'master' of https://github.com/prebid/Prebid.js: (22 commits)
  Update GetIntent adapter to 1.0 version (prebid#1721)
  Add `usePaymentRule` param to AN bidders (prebid#1778)
  New hooks API (replaces monkey-patching for currency) (prebid#1683)
  Change prebidServer to call client user syncs if they exist (prebid#1734)
  Fix Centro adapter to allow requests of the same units (prebid#1746)
  add vastUrl + media type for video bids Prebid Server (prebid#1739)
  Update adxcg adapter for prebid 1.0 (prebid#1741)
  Update yieldmoBid adapter request url (prebid#1771)
  Upgrade Quantcast adapter for Prebid 1.0 (prebid#1753)
  Fidelity Media Adapter update. Prebid v1.0 (prebid#1719)
  Kargo Adapter for Prebid 1.0 (prebid#1729)
  updated for prebid 1.0 api (prebid#1722)
  Add AdOcean adapter (prebid#1735)
  Update Conversant adapter to Prebid 1.0 (prebid#1711)
  Fix test-coverage bug (prebid#1765)
  Migrating TrustX adapter to 1.0 (prebid#1709)
  Update Improve Digital adapter for Prebid 1.0 (prebid#1728)
  Fixed the argument type on getUserSyncs. (prebid#1767)
  nanointeractive bid adapter (prebid#1627)
  Validating bid response params (prebid#1738)
  ...
mattpr pushed a commit to mattpr/Prebid.js that referenced this pull request Oct 31, 2017
* Update Improve Digital adapter for Prebid 1.0

* Removed bidderCode from bids

* Added creativeId to bid response; updated format of the first argument of interpretResponse
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* Update Improve Digital adapter for Prebid 1.0

* Removed bidderCode from bids

* Added creativeId to bid response; updated format of the first argument of interpretResponse
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.

4 participants