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

New Ad4Game adapter for Prebid.js 1.0 #1797

Merged
merged 2 commits into from
Nov 6, 2017

Conversation

adilets
Copy link
Contributor

@adilets adilets commented Nov 3, 2017

Type of change

Description of change

Adding new bidder adapter for A4G

  • test parameters for validating bids
{
  bidder: 'a4g',
  params: {
    zoneId: 59304,
    deliveryUrl: 'http://dev01.ad4game.com/v1/bid'
}

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

Other information

@adilets
Copy link
Contributor Author

adilets commented Nov 3, 2017

Hi @matthewlane. I created new PR from (#1650).

About response, you should use real domain instead of localhost when you testing. In my case I use xmmorpg.com domain. I rewrite localhost to this domain.

Thank you

@dbemiller
Copy link
Contributor

About response, you should use real domain instead of localhost when you testing. In my case I use xmmorpg.com domain. I rewrite localhost to this domain.

hey @adilets, could you add a note about this in your .md file? It's nice to have more permanent documentation about caveats like this when we re-test 6 months from now.

@dbemiller dbemiller requested a review from ndhimehta November 3, 2017 13:55
@matthewlane matthewlane assigned matthewlane and unassigned ndhimehta Nov 3, 2017
@matthewlane matthewlane requested review from matthewlane and removed request for ndhimehta November 3, 2017 17:13
Copy link
Collaborator

@matthewlane matthewlane left a comment

Choose a reason for hiding this comment

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

I'm able to receive bid responses from your endpoint after rewriting localhost to the test domain. This uncovered a problem with bid id matching but I've noted a fix for this below

const bidResponses = [];
utils._each(serverResponses.body, function(response) {
const bidResponse = {
requestId: request.bidId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

request is whatever was returned from the buildRequests function, which currently isn't returning a bidId. This causes the bids for your adapter to not get matched up, making it appear there was no bid response even though your endpoint is successfully returning bids. Make sure to make bidId available so that this will work, see other comment for possible approaches


return {
method: 'GET',
url: deliveryUrl,
Copy link
Collaborator

Choose a reason for hiding this comment

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

One way to do it is to add bidId to this object, and set it's value to the bid id of validBidRequests (bidId: validBidRequests[0] && validBidRequests[0].bidId in that case). Then bidId will be available on the request parameter in interpretResponse and line 61 will work without modification. You can do it however you'd prefer though

]
}
];
```
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 a note about rewriting localhost to the test domain in order to receive bids

@adilets
Copy link
Contributor Author

adilets commented Nov 6, 2017

@matthewlane Thanks for your review and comments. I updated PR.

@matthewlane matthewlane merged commit 6107cc8 into prebid:master Nov 6, 2017
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.

5 participants