Fire pixel without measure #3

sachinsurfs · 2021-06-14T09:21:19Z

Type of change

Bugfix
Feature
New 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

For internal team : https://docs.google.com/document/d/17gw3hnOSTZgHKqNXXWZJSdO6QG3bOyjKpgXxzstH3Pc/edit#

Be sure to test the integration with your adserver using the Hello World sample page.

contact email of the adapter’s maintainer
official adapter submission

For any changes that affect user-facing APIs or example code documented on http://prebid.org, please provide:

A link to a PR on the docs repo at prebid#userId.md

Other information

modules/quantcastIdSystem.js

mhammond-quantcast · 2021-06-14T22:09:02Z

modules/quantcastIdSystem.js

    // Consent signals are currently checked on the server side.
    let fpa = storage.getCookie(QUANTCAST_FPA);
+
+    const coppa = coppaDataHandler.getCoppa();


is coppaDataHandler always available?

Yes, coppaDataHandler is a Prebid utility function. It checks for Coppa configuration parameter that the publisher is expected to pass. When not present, it returns undefined.

mhammond-quantcast · 2021-06-14T22:11:20Z

modules/quantcastIdSystem.js

+      fpa = '';
+      storage.setCookie(QUANTCAST_FPA, fpa, expired, '/', domain, null);
+    } else if (!fpa) {
+      var expires = new Date(now.getTime() + (cookieExpTime * 86400000)).toGMTString();


somewhere we need to be checking we have a valid clientId before we set fpa or call quantserve

(please add tests to cover this)

So this isn't your fault and this is in suit with the current status-quo in Prebid.js, but this is a little maddening.

> git grep '* 24' modules/apstreamBidAdapter.js: var daysSinceApEpoch = Math.floor(timeDiff / (1000 * 3600 * 24)); modules/categoryTranslation.js: if (!mappingData || timestamp() > mappingData.lastUpdated + refreshInDays * 24 * 60 * 60 * 1000) { modules/criteoIdSystem.js:const cookiesMaxAge = 13 * 30 * 24 * 60 * 60 * 1000; modules/id5IdSystem.js: return (new Date(Date.now() + (1000 * 60 * 60 * 24 * expDays))).toUTCString(); modules/lotamePanoramaIdSystem.js:const DAY_MS = 60 * 60 * 24 * 1000; modules/parrableIdSystem.js:const ONE_YEAR_MS = 364 * 24 * 60 * 60 * 1000; modules/pubCommonIdSystem.js: const expiresStr = (new Date(Date.now() + (storage.expires * (60 * 60 * 24 * 1000)))).toUTCString(); modules/userId/index.js: const expiresStr = (new Date(Date.now() + (storage.expires * (60 * 60 * 24 * 1000)))).toUTCString(); modules/userId/index.js: const expiresStr = (new Date(Date.now() + (CONSENT_DATA_COOKIE_STORAGE_CONFIG.expires * (60 * 60 * 24 * 1000)))).toUTCString();

@ssmccoy - Yeah, this could have been one common const somewhere.

Btw, Any objections to also change 86400000 to 60 * 60 * 24 * 1000? I think it is at least a little clearer.

modules/quantcastIdSystem.js

ssmccoy

Few questions.

ssmccoy · 2021-06-21T21:01:23Z

modules/quantcastIdSystem.js

+      fpa = '';
+      storage.setCookie(QUANTCAST_FPA, fpa, expired, '/', domain, null);
+    } else if (!fpa) {
+      var expires = new Date(now.getTime() + (cookieExpTime * 86400000)).toGMTString();


So this isn't your fault and this is in suit with the current status-quo in Prebid.js, but this is a little maddening.

> git grep '* 24' modules/apstreamBidAdapter.js: var daysSinceApEpoch = Math.floor(timeDiff / (1000 * 3600 * 24)); modules/categoryTranslation.js: if (!mappingData || timestamp() > mappingData.lastUpdated + refreshInDays * 24 * 60 * 60 * 1000) { modules/criteoIdSystem.js:const cookiesMaxAge = 13 * 30 * 24 * 60 * 60 * 1000; modules/id5IdSystem.js: return (new Date(Date.now() + (1000 * 60 * 60 * 24 * expDays))).toUTCString(); modules/lotamePanoramaIdSystem.js:const DAY_MS = 60 * 60 * 24 * 1000; modules/parrableIdSystem.js:const ONE_YEAR_MS = 364 * 24 * 60 * 60 * 1000; modules/pubCommonIdSystem.js: const expiresStr = (new Date(Date.now() + (storage.expires * (60 * 60 * 24 * 1000)))).toUTCString(); modules/userId/index.js: const expiresStr = (new Date(Date.now() + (storage.expires * (60 * 60 * 24 * 1000)))).toUTCString(); modules/userId/index.js: const expiresStr = (new Date(Date.now() + (CONSENT_DATA_COOKIE_STORAGE_CONFIG.expires * (60 * 60 * 24 * 1000)))).toUTCString();

modules/quantcastIdSystem.js

ssmccoy · 2021-06-21T21:02:36Z

modules/quantcastIdSystem.js


 const QUANTCAST_FPA = '__qca';
+const DEFAULT_COOKIE_EXP_TIME = 392; // (13 months - 2 days)
+const PREBID_PCODE = 'p-KceJUEvXN48CE'; // Not associated with a real account
+const DOMAIN_QSERVE = 'https://pixel.quantserve.com/pixel';


QSERVE_URL ? This isn't really a domain.

Question: Do we want to use a protocol relative URL instead? I mean maybe not these days, but it is common practice.
Note: If we did, it would be QSERVE_URL = "//pixel.quantserve.com/pixel";

Hmm, the advantage of this is that the URL would have the same protocol as the site it is on (versus always being https), right? I think that makes sense.

i think we need to use secure these days for certain cookie based stuff to work in Chrome? (might be mistaken about that). I guess that's less of a concern here, although i suppose these requests will set 3rd party cookies where they can.

My vote would just be to keep this secure i think. Does that ever cause issues if done on a non-secure site?

We should use QSERVE_URL as Scott suggests though.

modules/quantcastIdSystem.js

sachinsurfs · 2021-06-22T16:31:36Z

modules/quantcastIdSystem.js

+      usPrivacyParamString = `&us_privacy=${US_PRIVACY_STRING}`;
+    }
+
+    let url = DOMAIN_QSERVE +


I just had removed 'et' and 'tzo' in the previous commit - b31c0f9

Only cause the general consensus currently is that we want to keep the number of parameters we send through prebid minimum in our first iteration unless we have strong justification for needing it

modules/quantcastIdSystem.js

sachinsurfs · 2021-06-22T16:31:36Z

modules/quantcastIdSystem.js

+      usPrivacyParamString = `&us_privacy=${US_PRIVACY_STRING}`;
+    }
+
+    let url = DOMAIN_QSERVE +


I just had removed 'et' and 'tzo' in the previous commit - b31c0f9

Only cause the general consensus currently is that we want to keep the number of parameters we send through prebid minimum in our first iteration unless we have strong justification for needing it

ac-qc

lgtm

mhammond-quantcast · 2021-06-23T18:43:06Z

lgtm on the issues i'd raised - please get to resolution on the outstanding comments from Scott

ssmccoy

Some final comments.

ssmccoy · 2021-06-23T20:54:25Z

modules/quantcastIdSystem.js

+    var gdprParamStrings;
+
+    if (!fpa) {
+      var expires = new Date(now.getTime() + (cookieExpDays * 86400000)).toGMTString();


Seems weird to call now.getTime() twice in a row like this. Also weird to use string interpolation below and not here.

Why not do this instead?

const DAY_MS = 86400000; /* ... *snip* ... */

var et = now.getTime(); var expires = new Date(et + (cookieExpDays * DAY_MS)).toGMTString(); var rand = Math.round(Math.random() * 2147483647); fpa = "B0-${rand}-${et}";

Otherwise 👍

mhammond-quantcast · 2021-06-25T13:14:31Z

let's use a more meaningful title for the PR when you make it on prebid proper

Sachin Shastri added 2 commits June 14, 2021 02:11

add code for firing pixel without measure

7df1c4b

add more unit tests

05ec12c

mhammond-quantcast reviewed Jun 14, 2021

View reviewed changes

modules/quantcastIdSystem.js Outdated Show resolved Hide resolved

mhammond-quantcast reviewed Jun 14, 2021

View reviewed changes

modules/quantcastIdSystem.js Show resolved Hide resolved

mhammond-quantcast reviewed Jun 14, 2021

View reviewed changes

modules/quantcastIdSystem.js Show resolved Hide resolved

mhammond-quantcast reviewed Jun 14, 2021

View reviewed changes

correct gdpr logic; don't fire when no consent

8439525

mhammond-quantcast reviewed Jun 16, 2021

View reviewed changes

modules/quantcastIdSystem.js Outdated Show resolved Hide resolved

mhammond-quantcast reviewed Jun 16, 2021

View reviewed changes

modules/quantcastIdSystem.js Outdated Show resolved Hide resolved

Sachin Shastri added 4 commits June 17, 2021 15:04

add clientId check

7ba5778

address more pr comments

17a4058

add glvid

35a9e38

remove et / tzo

b31c0f9

ssmccoy suggested changes Jun 21, 2021

View reviewed changes

mhammond-quantcast reviewed Jun 22, 2021

View reviewed changes

modules/quantcastIdSystem.js Outdated Show resolved Hide resolved

rename vars; update ccpa check

7a94f13

sachinsurfs commented Jun 22, 2021

View reviewed changes

ac-qc approved these changes Jun 22, 2021

View reviewed changes

move to protocol relative url

22f2532

ssmccoy reviewed Jun 23, 2021

View reviewed changes

refactor code

b03709b

mhammond-quantcast approved these changes Jun 25, 2021

View reviewed changes

sachinsurfs merged commit 09680be into master Jun 26, 2021

sachinsurfs deleted the prebid-without-measure_ branch June 26, 2021 03:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fire pixel without measure #3

Fire pixel without measure #3

sachinsurfs commented Jun 14, 2021 •

edited

Loading

mhammond-quantcast Jun 14, 2021

sachinsurfs Jun 22, 2021

mhammond-quantcast Jun 14, 2021

mhammond-quantcast Jun 14, 2021

ssmccoy Jun 21, 2021

sachinsurfs Jun 22, 2021

ssmccoy left a comment

ssmccoy Jun 21, 2021

ssmccoy Jun 21, 2021

ssmccoy Jun 21, 2021

sachinsurfs Jun 23, 2021 •

edited

Loading

mhammond-quantcast Jun 23, 2021 •

edited

Loading

sachinsurfs Jun 22, 2021 •

edited

Loading

sachinsurfs Jun 22, 2021 •

edited

Loading

ac-qc left a comment

mhammond-quantcast commented Jun 23, 2021

ssmccoy left a comment

ssmccoy Jun 23, 2021

ssmccoy Jun 23, 2021

mhammond-quantcast commented Jun 25, 2021

Fire pixel without measure #3

Fire pixel without measure #3

Conversation

sachinsurfs commented Jun 14, 2021 • edited Loading

Type of change

Description of change

Other information

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssmccoy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sachinsurfs Jun 23, 2021 • edited Loading

Choose a reason for hiding this comment

mhammond-quantcast Jun 23, 2021 • edited Loading

Choose a reason for hiding this comment

sachinsurfs Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

sachinsurfs Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

ac-qc left a comment

Choose a reason for hiding this comment

mhammond-quantcast commented Jun 23, 2021

ssmccoy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mhammond-quantcast commented Jun 25, 2021

sachinsurfs commented Jun 14, 2021 •

edited

Loading

sachinsurfs Jun 23, 2021 •

edited

Loading

mhammond-quantcast Jun 23, 2021 •

edited

Loading

sachinsurfs Jun 22, 2021 •

edited

Loading

sachinsurfs Jun 22, 2021 •

edited

Loading