33Across UIM: Add cookie storage support for first-party ID #78
Conversation
| function getStoredValue(key, storageConfig = {}) { | ||
| if (storageConfig.type === STORAGE_TYPE_COOKIES && storage.cookiesAreEnabled()) { | ||
| return storage.getCookie(key); | ||
| } else if (storageConfig.type === STORAGE_TYPE_LOCALSTORAGE) { |
There was a problem hiding this comment.
What about checking whether LocalStorage is enabled via localStorageIsEnabled?
There was a problem hiding this comment.
in the case of local storage, each function checks internally if local storage storage is enabled
e.g., here is how setDataInLocalStorage is defined:
const setDataInLocalStorage = function (key, value, done) {
let cb = function (result) {
if (result && result.valid && hasLocalStorage()) { // <-- hasLocalStorage is called
window.localStorage.setItem(key, value);
}
}
return schedule(cb, STORAGE_TYPE_LOCALSTORAGE, done);
}
modules/33acrossIdSystem.js
Outdated
| if (fp) { | ||
| params.fp = fp; | ||
| } | ||
|
|
||
| return params; | ||
| } | ||
|
|
||
| function handleFpId(fpId, storageConfig = {}) { | ||
| if (!fpId) { | ||
| return storage.removeDataFromLocalStorage(STORAGE_FPID_KEY); |
There was a problem hiding this comment.
What about removing from cookie?
There was a problem hiding this comment.
good catch, that part is missing.. working on it.
| const expirationInMs = 60 * 60 * 24 * 1000 * storageConfig.expires; | ||
| const expirationTime = new Date(Date.now() + expirationInMs); | ||
|
|
||
| storage.setCookie(key, value, expirationTime.toUTCString(), 'Lax'); |
There was a problem hiding this comment.
Given that we are not specifying an domain per
Prebid.js/src/storageManager.js
Line 56 in db9d236
There was a problem hiding this comment.
that's correct, the subdomain is included. I've verified this behavior and that's how most of the IdSystems set the cookies, without a hostname, since that part is handled by the storage manager util.
| } | ||
|
|
||
| function storeValue(key, value, storageConfig = {}) { | ||
| if (storageConfig.type === STORAGE_TYPE_COOKIES && storage.cookiesAreEnabled()) { |
There was a problem hiding this comment.
What is storage is default of {} or type is not set? Also what is the expected structure of storageConfig the pub should pass? Have we documented this in the md?
There was a problem hiding this comment.
we won't be storing any values if the storage is {} or the type is not set.. that's the behavior that can be found in the baseID module modules/userId/index.js as well.
The recommended structure is part of the documentation --> https://docs.prebid.org/dev-docs/modules/userid-submodules/33across.html
pbjs.setConfig({
userSync: {
userIds: [{
name: "33acrossId",
params: {
pid: "0010b00002GYU4eBAH" // Example ID
},
storage: {
name: "33acrossId",
type: "html5",
expires: 90,
refreshInSeconds: 8 * 3600
}
}]
}
});
No description provided.