-
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
add smms adapter #4439
add smms adapter #4439
Conversation
8a2acc0
to
7730b7c
Compare
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.
@songtungmtp Please see inline comments
modules/smmsBidAdapter.js
Outdated
let valid = !!(bid.params.placementId); | ||
if (valid && bid.params.hasOwnProperty('currency')) { | ||
if (SUPPORTED_CURRENCIES.indexOf(bid.params.currency) === -1) { | ||
console.log('Invalid currency type, we support only JPY and USD!'); |
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.
Use utils functions for logging
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.
I changed to utils.logError
modules/smmsBidAdapter.js
Outdated
'cur': bid.params.hasOwnProperty('currency') ? bid.params.currency : DEFAULT_CURRENCY, | ||
'ua': navigator.userAgent, | ||
'adtk': _encodeURIComponent(g.lat ? '0' : '1'), | ||
'loc': utils.getTopWindowUrl(), |
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.
This will be removed in 3.0 please use refererInfo.referer
instead
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.
I changed to refererInfo.referer
.
modules/smmsBidAdapter.js
Outdated
import { registerBidder } from '../src/adapters/bidderFactory'; | ||
|
||
const BIDDER_CODE = 'smms'; | ||
const ENDPOINT_BANNER = '//bidder.mediams.mb.softbank.jp/api/v1/prebid/banner'; |
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.
Banner and native endpoints has to be https to be in 3.0
It would be great if you can update your endpoints to be https. We have a PR up to update all adapters endpoint
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.
yes, I added https as protocol
modules/smmsBidAdapter.js
Outdated
|
||
export const spec = { | ||
code: BIDDER_CODE, | ||
aliases: ['smms'], // short code |
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.
aliases is an optional property. Both your bidder code and alias used here are same. You can remove this if not needed
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.
I removed this
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 |
e2cba57
to
04a7c76
Compare
I created a new PR for updating docs repo. Please give it a look, thank you! |
Type of change
Description of change
Be sure to test the integration with your adserver using the Hello World sample page.
Other information