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

Yieldmo Bid Adapter Update #2054

Merged
merged 10 commits into from
Jan 31, 2018
Merged

Yieldmo Bid Adapter Update #2054

merged 10 commits into from
Jan 31, 2018

Conversation

MelodyLi2017
Copy link
Contributor

@MelodyLi2017 MelodyLi2017 commented Jan 22, 2018

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

Updating Yieldmo Prebid Adapter to meet Prebid 1.0 specs

  • test parameters for validating bids
{
       code: 'div-gpt-ad-1460505748561-0', 
       sizes: [[300, 250], [300,600]],
       bids: [{
         bidder: 'yieldmo',
         params: {
           placementId: '1779781193098233305' // string with at most 19 characters (may include numbers only) 
         }
       }]
   }

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

Melody Li added 7 commits January 22, 2018 13:04
environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

merge conflicts

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

cleaning up the rebase

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file
environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

merge conflicts

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

cleaning up the rebase

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

fix tests'

fixed rebase and updated markdown file

fixed linting error

fixed test

test fix
environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

merge conflicts

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

cleaning up the rebase

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file
Copy link
Collaborator

@jsnellbaker jsnellbaker left a comment

Choose a reason for hiding this comment

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

Hello @merrdy,

Thanks for submitting this adapter! Overall the code looks fine to me, however I'm having some trouble trying to test the adapter on the hello_world.html page. When I looked in the console, I'm seeing an Access-Control-Allow-Origin error when it's attempting to call the yieldmo server. Can you take a look into this on your end?

@MelodyLi2017
Copy link
Contributor Author

Hey @jsnellbaker,

The issue was that the id in the placementId field is currently not in production, so you were probably getting a 204 when pinging our server. If you comment out that id, you should be able to get an ad. Is that alright? Otherwise I can ask our ad servers to send a response for that placementId. Thanks!

@jsnellbaker
Copy link
Collaborator

Hey @merrdy,

I did try to comment out the placementId (and the params object afterwards), but neither of these seemed to help. I was still receiving the 204 response from the server. Can you try to open up/publish the placement? Thanks!

@MelodyLi2017
Copy link
Contributor Author

MelodyLi2017 commented Jan 29, 2018

@jsnellbaker

Okay, I'll talk to ad serving. The other issue might be that you have to be emulating mobile in order to get an ad. So you might have to refresh after switching to mobile view in order to get an ad. Sorry I thought I mentioned the mobile mode in the notes above, but I guess I didn't!

@MelodyLi2017
Copy link
Contributor Author

@jsnellbaker

Okay, I've updated the docs and the description above such that the placementId given should result in an ad. Don't forget to emulate mobile mode when you're trying to get the ad. We only serve on mobile, so an ad request from desktop will get a 204. Thanks for being patient with me about this!

@jsnellbaker
Copy link
Collaborator

@merrdy

Thanks for submitting the changes and helping to enable the ad unit for testing. I verified the delivery is working when I'm emulating and not when I'm using my desktop (as you noted).

LGTM

@MelodyLi2017
Copy link
Contributor Author

@jsnellbaker

Okay, thanks!

@jaiminpanchal27
Copy link
Collaborator

Need docs PR to merge. 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. Thank you for contributing

@MelodyLi2017
Copy link
Contributor Author

@jaiminpanchal27

We've reopened our pr to update the docs here: prebid/prebid.github.io#469 (comment).

Thanks!

@jaiminpanchal27 jaiminpanchal27 merged commit c654ebc into prebid:master Jan 31, 2018
dluxemburg pushed a commit to Genius/Prebid.js that referenced this pull request Jul 17, 2018
* added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

merge conflicts

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

cleaning up the rebase

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

* fix tests'

* fixed rebase and updated markdown file

* added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

merge conflicts

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

cleaning up the rebase

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

fix tests'

fixed rebase and updated markdown file

fixed linting error

fixed test

test fix

* added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

merge conflicts

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

cleaning up the rebase

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

fixed unti tests

added usersync and interpretresponse

environments

finished building request

markdown and test cases pr valid bid requests

all tests but user sync

added mraid check and catch for window.top

added height and width, updated tests, fixed interpretResponse, updated md file

* fix tests'

* fixed rebase and updated markdown file

* update placementId

* lint error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants