CriteoId User Module #4287

Swiiip · 2019-10-11T14:09:17Z

Add CriteoId User Module

Type of change

[x ] Feature

Description of change

This PR allows adapters to have access to a Criteo ID via the user module. It replaces the CriteoRTUS User Module currently available. We will remove it in a following PR and update adapters and documentation accordingly.

jaiminpanchal27

@Swiiip Please see inline comments.
Also please submit a PR to the docs repo to list Criteo ID system here http://prebid.org/dev-docs/modules/userId.html. Thank you for contributing

jaiminpanchal27 · 2019-10-25T20:58:26Z

modules/criteoIdSystem.js

+const pastDateString = new Date(0).toString();
+const expirationString = new Date(utils.timestamp() + cookiesMaxAge).toString();
+
+function areCookiesWriteable() {


Can you use cookiesAreEnabled from utils https://github.com/prebid/Prebid.js/blob/master/src/utils.js#L908

cookiesAreEnabled function uses the window.navigator.cookieEnabled flag which returns true on Safari even on domains considered as trackers by ITP, which we would like to avoid. That's why we only rely on the read/write check.

jaiminpanchal27 · 2019-10-25T20:58:57Z

modules/criteoIdSystem.js

+  return canWrite;
+}
+
+function extractProtocolHost (url, returnOnlyHost = false) {


Please use https://github.com/prebid/Prebid.js/blob/master/src/url.js#L27

Done 👍, I kept the formatting part in the module, but the parsing part uses url.js function.

jaiminpanchal27 · 2019-10-25T21:56:42Z

modules/criteoIdSystem.js

+   * @param {SubmoduleParams} [configParams]
+   * @returns {function(callback:function)}
+   */
+  getId(configParams) {


I tried this on hello_world page and it does not work. Here is the fiddle https://jsfiddle.net/t2c3nfq4/

Here jsdoc says it returns function but I see an object on line 134. On first call when there is nothing in localStorage/cookies, getId will return {'criteoId: undefined'}. User id module calls submodule.getId() from here https://github.com/prebid/Prebid.js/blob/master/modules/userId/index.js#L391 and here https://github.com/prebid/Prebid.js/blob/master/modules/userId/index.js#L418. Your response is not satisfying none of the scenarios.

Can you share a working test page ? More details here http://prebid.org/dev-docs/modules/userId.html

I updated the code from getId() and decode(id).
getId() returns an object {id: xxx} where xxx can either be an object {criteoId: <criteoId>} or undefined
decode(id) returns the id as-is

The jsfiddler should work now.

Swiiip · 2019-10-28T16:52:34Z

The updated doc for the Criteo ID module is available here: prebid/prebid.github.io#1575

jaiminpanchal27 · 2019-11-04T03:24:12Z

@Swiiip Everything looks good. Just one more missing part.
Please add your filename here https://github.com/prebid/Prebid.js/blob/master/modules/.submodules.json so build system automatically pulls userId module when criteoIdSystem is included in build command.

And also pull master from upstream to resolve this conflict.

Thanks

pm-harshad-mane · 2019-11-04T19:06:38Z

Hello @Swiiip ,

CI job is failing, can you merge the upstream/master in your branch and try again? this may pass the CI job.

jaiminpanchal27

LGTM

Changes: - Use of url parsing function from url lib - Update the return type of getId() - Update the jsdoc to reflect the real return types

pm-harshad-mane · 2019-11-05T18:44:33Z

Hello @Swiiip ,

Few questions for you,

As a bidder adapter, should we support both criteoId and criteoRtusId? criteortusIdSystem.js is still present in the master
For criteortus a bidders had to code like userId.criteortus.${BIDDER_CODE}.userid. For new criteoId userId.criteoId will work, right?
if a bidder is passing criteoid in ORTB format like prebidServerBidAdapter, what should be value for source paarameter? should it be criteo.com ? Refer Liveintent code here
https://github.com/prebid/Prebid.js/blob/master/modules/prebidServerBidAdapter/index.js#L733

@jaiminpanchal27, @jsnellbaker
Does Criteo need to add support for their id in prebidServerBidAdapter like LiveIntent, Parrable has added code? https://github.com/prebid/Prebid.js/blob/master/modules/prebidServerBidAdapter/index.js#L733

Swiiip · 2019-11-06T16:38:55Z

Hello @pm-harshad-mane,

Thank you for your interest in this module.

As a bidder adapter, you should rather use the CriteoId module than the CriteoRTUS module. The latter is still supported but it is deprecated and is redundant with the CriteoId module. It will be removed soon from prebid.
You are right, to have a better uderstanding of the module, you can check the updated documentation currently in review here: Replace Criteo Rtus module doc with Criteo ID module doc prebid.github.io#1575.
I'm not familiar with this format/intergration and I'm not sure Criteo supports it for the moment, let me ask around and come back to you.

jaiminpanchal27 self-assigned this Oct 11, 2019

Swiiip force-pushed the criteoId_module branch 3 times, most recently from cf3db2b to d4fafb5 Compare October 24, 2019 16:07

jaiminpanchal27 requested changes Oct 25, 2019

View reviewed changes

Swiiip marked this pull request as ready for review October 28, 2019 16:46

Swiiip force-pushed the criteoId_module branch 3 times, most recently from 776e0fd to ca69371 Compare November 4, 2019 16:57

jaiminpanchal27 approved these changes Nov 4, 2019

View reviewed changes

jaiminpanchal27 requested a review from jsnellbaker November 4, 2019 19:09

Hugo Duthil added 4 commits November 5, 2019 10:28

Add CriteoId module

f39d274

Update the return type of getId in Criteo Id module

97f90fe

Changes: - Use of url parsing function from url lib - Update the return type of getId() - Update the jsdoc to reflect the real return types

Fix failing tests for Criteo user module

c4355aa

Add CriteoIdSystem submodule to .submodule.json.

9745fa2

Swiiip force-pushed the criteoId_module branch from ca69371 to 9745fa2 Compare November 5, 2019 09:29

jsnellbaker approved these changes Nov 5, 2019

View reviewed changes

jsnellbaker merged commit 23c2eba into prebid:master Nov 5, 2019

bmwcmw mentioned this pull request Nov 18, 2019

Provide criteoId to server by user.ext.eids #4478

Merged

9 tasks

dbirkbeck mentioned this pull request Feb 13, 2020

AppNexus bid adapter no longer passes Criteo user ID #4860

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CriteoId User Module #4287

CriteoId User Module #4287

Swiiip commented Oct 11, 2019

jaiminpanchal27 left a comment

jaiminpanchal27 Oct 25, 2019

Swiiip Oct 28, 2019

jaiminpanchal27 Oct 25, 2019

Swiiip Oct 28, 2019

jaiminpanchal27 Oct 25, 2019

Swiiip Oct 28, 2019

Swiiip commented Oct 28, 2019

jaiminpanchal27 commented Nov 4, 2019

pm-harshad-mane commented Nov 4, 2019

jaiminpanchal27 left a comment

pm-harshad-mane commented Nov 5, 2019 •

edited

Loading

Swiiip commented Nov 6, 2019

CriteoId User Module #4287

CriteoId User Module #4287

Conversation

Swiiip commented Oct 11, 2019

Add CriteoId User Module

Type of change

Description of change

jaiminpanchal27 left a comment

Choose a reason for hiding this comment

jaiminpanchal27 Oct 25, 2019

Choose a reason for hiding this comment

Swiiip Oct 28, 2019

Choose a reason for hiding this comment

jaiminpanchal27 Oct 25, 2019

Choose a reason for hiding this comment

Swiiip Oct 28, 2019

Choose a reason for hiding this comment

jaiminpanchal27 Oct 25, 2019

Choose a reason for hiding this comment

Swiiip Oct 28, 2019

Choose a reason for hiding this comment

Swiiip commented Oct 28, 2019

jaiminpanchal27 commented Nov 4, 2019

pm-harshad-mane commented Nov 4, 2019

jaiminpanchal27 left a comment

Choose a reason for hiding this comment

pm-harshad-mane commented Nov 5, 2019 • edited Loading

Swiiip commented Nov 6, 2019

pm-harshad-mane commented Nov 5, 2019 •

edited

Loading