-
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
Adtelligent bid adapter updated to v3.0 #4703
Conversation
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.
Overall looks good. Some minor changes requested. Thanks
modules/adtelligentBidAdapter.js
Outdated
import {Renderer} from '../src/Renderer'; | ||
import findIndex from 'core-js/library/fn/array/find-index'; | ||
|
||
const URL = '//hb.adtelligent.com/auction/'; |
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.
All endpoints has to be secure in 3.0. Please add https
modules/adtelligentBidAdapter.js
Outdated
import {Renderer} from '../src/Renderer'; | ||
import findIndex from 'core-js/library/fn/array/find-index'; | ||
|
||
const URL = '//hb.adtelligent.com/auction/'; |
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.
All endpoints are required to be secure in Prebid 3.0. Please update your endpoints
modules/adtelligentBidAdapter.js
Outdated
} | ||
|
||
serverResponse.bids.forEach(serverBid => { | ||
const requestId = findIndex(bidderRequest.bids, (bidRequest) => { |
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.
Any strong reason to use find-index from core-js. We have a white list of core-js modules here https://github.com/prebid/Prebid.js/blob/master/allowedModules.js#L2
Can you update the logic ?
Also, we have a custom eslint rule but not sure why it did not catch this import. I will look into this.
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.
nope, just left from previous dev =)
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.
so, can i use Array.prototype.findIndex ?
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.
@GeneGenie Array.prototype.findIndex does not support Internet explorer and hence the previous dev used core-js to support IE. I would suggest to use find
from core-js
Here is the updated code which worked fine on my test page. Do test in on your end as well.
const request = find(bidderRequest.bids, (bidRequest) => {
return bidRequest.bidId === serverBid.requestId;
});
if (serverBid.cpm !== 0 && request !== undefined) {
const bid = createBid(serverBid, getMediaType(request));
bids.push(bid);
}
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.
fixed
Type of change
Description of change
Remove deprecated code
Tests updated
Do not trigger same usersyncs across all auctions