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.

Sign up for GitHub

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

Jump to bottom

Add AdgenerationBidAdapter #2034

Merged
merged 5 commits into from
Jan 30, 2018
Merged

Add AdgenerationBidAdapter #2034

merged 5 commits into from
Jan 30, 2018

Conversation

adg-dev
Copy link
Contributor

@adg-dev adg-dev commented Jan 16, 2018 •
edited
Loading

Type of change

  • New bidder adapter

Description of change

Adding Adgeneraton bid adapter for Prebid 1.0.

  • test parameters for validation bids.
{
  bidder: 'adg',
  params: {
    id: '58278',
    width: '300',
    height: '250'
  }
}

@mkendall07 mkendall07 self-assigned this Jan 18, 2018
mkendall07
mkendall07 requested changes Jan 23, 2018
View reviewed changes
modules/adgenerationBidAdapter.js Outdated
let serverRequests = [];
for (let i = 0, len = validBidRequests.length; i < len; i++) {
const validReq = validBidRequests[i];
const url = validReq.params.debug ? 'http://api-test.scaleout.jp/adsv/v1' : 'https://d.socdm.com/adsv/v1';
Copy link
Member

Choose a reason for hiding this comment

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

prefer these strings to be constants.

modules/adgenerationBidAdapter.js Outdated
export const spec = {
code: ADG_BIDDER_CODE,
aliases: ['adg'], // short code
supportedMediaTypes: [NATIVE],
Copy link
Member

Choose a reason for hiding this comment

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

If you are supporting more than native, I.E. banner, you need to set it explicitly here.

modules/adgenerationBidAdapter.js
let script = document.createElement(`script`);
script.type = 'text/javascript';
script.innerHTML = `(function(){ new APV.VideoAd(${JSON.stringify(apvVideoAdParam)}).load('${vastXml.replace(/\r?\n/g, '')}'); })();`;
return script.outerHTML;
Copy link
Member

Choose a reason for hiding this comment

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

can you explain what this is doing/why it's needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

VAST data, in our sever responses , is converted by CSS Sprites etc.

@mkendall07
Copy link
Member

@adg-dev
Thanks for the PR. Left a few comments. Also 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.

@mkendall07 mkendall07 added this to the Prebid 1.3 release milestone Jan 24, 2018
@banakemi
Copy link
Contributor

Hi, @mkendall07
Thanks for review.
Fix your comments and our docs. (prebid/prebid.github.io#575)

@mkendall07
Copy link
Member

@banakemi
Thanks. Can you please drop width and height from your params? These can be obtained from the bid request directly.

@banakemi
Copy link
Contributor

banakemi commented Jan 28, 2018 •
edited
Loading

@mkendall07
Thanks!
...Are you talking about interpretResponse methods?
If so, I guess those params are needed to display the banner.
Could you tell me which part you're talking about, please?

@mkendall07
Copy link
Member

I mean from your bidder params:

params: {
    id: '58278',
    width: '300',
    height: '250'
  }

The adunit will have these defined already

@mkendall07 mkendall07 merged commit 40627d7 into prebid:master Jan 30, 2018
@ghost ghost mentioned this pull request Feb 14, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@banakemi banakemi banakemi left review comments

@mkendall07 mkendall07 mkendall07 approved these changes

Assignees

@mkendall07 mkendall07

Labels
needs docs needs update
Projects
None yet
Milestone
Prebid 1.3 release
Development

Successfully merging this pull request may close these issues.
3 participants
@adg-dev @mkendall07 @banakemi