-
Notifications
You must be signed in to change notification settings - Fork 0
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 taboola user-id module #1
base: master
Are you sure you want to change the base?
Conversation
still in progress please don't merge |
modules/taboolaIdSystem.js
Outdated
const [match] = cookieData.split('&').filter(item => item.startsWith(`${key}=`)); | ||
if (match) { | ||
// eslint-disable-next-line no-unused-vars | ||
const [_, value] = match.split('='); |
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.
const [_, value] = match.split('='); | |
const value = match.split('=')[1]; |
remove
// eslint-disable-next-line no-unused-vars
modules/taboolaBidAdapter.js
Outdated
@@ -277,7 +277,7 @@ function fillTaboolaReqData(bidderRequest, bidRequest, data) { | |||
const {refererInfo, gdprConsent = {}, uspConsent} = bidderRequest; | |||
const site = getSiteProperties(bidRequest.params, refererInfo, bidderRequest.ortb2); | |||
deepSetValue(data, 'device', bidderRequest?.ortb2?.device); | |||
const extractedUserId = userData.getUserId(gdprConsent, uspConsent); | |||
const extractedUserId = bidRequest?.userId?.taboolaId ? bidRequest.userId.taboolaId : userData.getUserId(gdprConsent, uspConsent); |
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.
In today's meeting they mentioned this is 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 will remove it
* NOTE: We already have query params (?app.type=desktop&app.apikey=...), | ||
* so we'll handle extra params carefully. | ||
*/ | ||
const TABOOLA_SYNC_ENDPOINT = 'https://api.taboola.com/1.2/json/taboola-usersync/user.sync?app.type=desktop&app.apikey=e60e3b54fc66bae12e060a4a66536126f26e6cf8'; |
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.
api key?
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.
its part of the endpoint must have parameter, its a dummy api key that we asked to put (hardcoded) in the endpoint url
if (!cookieData) { | ||
return undefined; | ||
} | ||
const [match] = cookieData.split('&').filter(item => item.startsWith(`${key}=`)); |
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.
cookieData undefined check missing
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.
if (!cookieData) {
return undefined;
}
checking for cookieData being falsy (including undefined, null, empty string, etc.)
isn't it enough ?
Type of change
Bugfix
Feature
New bidder adapter
Updated 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
added taboola's new user id submodule
Other information