-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Smart Ad Server: Fix bug when multi bids #2170
Conversation
I tried testing this with the parameters from #1908 but I'm not getting any bids back Here's the request body:
|
Hey @mike-chowla, Indeed the parameters in this PR are out dated, and were actually changed in the md file. the right ones are
They just weren't updated in the PR message at the time. Sorry about that. |
I tried with the new params and am still not getting any bids back. Response is 200 status code, content length 0 Request body:
|
Hey @mike-chowla, I don't get it, I tried with your exact request body (copy/paste) with a rest client and i get the response. I also tried with the test page locally and it works just fine. |
Hello @mike-chowla, As I said it works when i try, could you try again ? Also, If it still doesn't work could you show the Url you are calling ? |
Hello @mike-chowla I know it's a bit annoying but i don't understand why you don't have any responses. |
Hi @mike-chowla,
Thank you. |
Could you please provide a HAR file so we could compare with our test? Do you have any errors in the console? In any case for us it is a valid JSON as a response, the test doesn't return any creative to show. Thank you. |
The test parameters need to return a valid ad. HAR File: There's actually 2 issues. The 0.05 cpm you are sending back is getting put in the zero price bucket which causes ads not to display with hello_world.html. I can hack around that but you should really send a test ad back with a higher cpm to stay out of that bucket. The adapter code returns ad field as the creative and as that field is coming back as JSON, it's not a valid creative Here's what I'm getting back.
|
Hi @mike-chowla ,
Could you please test again? Thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LTGM. Test ad works . 2 slots works. Would be a good idea to add a test for the multiple ad slot case
* 'master' of https://github.com/prebid/Prebid.js: EngageBDR New Bid Adapter (prebid#2309) [FEAT] adunit sizes support (prebid#2320) Support aliases in prebidServer (prebid#2257) Changing default currency file to https (prebid#2306) Update stalebot labels (prebid#2319) Enhance location detection within utils (prebid#2167) if cache markup is not enabled, set it to the default value 0 (prebid#2302) Serverbid Bid Adapter: Added archon alias (prebid#2293) Smart Ad Server: Fix bug when multi bids (prebid#2170) NEW adapter AdtelligentBidAdapter (prebid#2137) add optional param to bridgewellBidAdapter (prebid#2289) Increment Pre Version Prebid 1.6.0 Release Unit test fixes (prebid#2301) PBS videoCacheKey and vastUrl (prebid#2101) Add Oneplanetonly Bid Adapter (prebid#2269) firing new adRenderFailed event when renderAd() fails (prebid#2210) Add Content Ignite adapter (prebid#2268) add hb_cache_id, hb_uuid should be deprecated and replaced by hb_cache_id (prebid#2273) Update Yieldlab adapter and add official maintainer (prebid#2231) Update for Media.net adapter (prebid#2232) Update to Rubicon Adapter for mediaTypes support (prebid#2272) message formatting (prebid#2285) Yieldbot impression image creation fix (prebid#2277) Updated Bid params (prebid#2275) Audience Network: Add 'pbv' and 'cb' query params (prebid#2252) Add e-planning analytics adapter (prebid#2211) Add vastUrl for Gamma Adapter Video (prebid#2261) update params for test bid (prebid#2267) Updated adUnitCode (prebid#2262)
Type of change
Description of change
Support multi bids
Other information
See issue #2166