-
Notifications
You must be signed in to change notification settings - Fork 0
Permutive RTD: vendorless consent check #319
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
base: master
Are you sure you want to change the base?
Permutive RTD: vendorless consent check #319
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
* local e2e testing * rework concurrency id * clean up inputs * try installing safari * specify shell * try npx * break out build from test for e2e * try safaridriver --enable * adjust build, safaridriver * safari why such a pain * safari * safari... * do not allow parallel tests * try to install chrome 109 * log browsers * deb installation * fix deb installation * extract install-deb * fix install-deb * more flexible version definition * remove safari 15 * disable coverage on chrome 109 * Better job names * bump chrome 109 -> 113, and add logic to test it outside browserstack * reintroduce safari 15.6 * try --no-sandbox * rename custom -> noSandbox * update wait for browserstack to accept variable number of sessions * fix type error
…cts (prebid#14171) * Add weakCachec to objectGuard * add unit test * lint --------- Co-authored-by: Demetrio Girardi <dgirardi@prebid.org>
* Start.io adapter: Change the protocol from http to https * Start.io adapter: Changing content-type to json * Start.io adapter: Changing content-type back to text/plain
…bid#14044) * Add SeenThis Brand Stories module * test: add unit tests for seenthisBrandStories module functions and constants * remove support for loading inside iframe * only allow events of seenthis origin * fix tests; update applying of styling * sort variables * emit billable event on init --------- Co-authored-by: Per Holmäng <per.holmang@gmail.com>
* GreenbidsBidAdapter: fix tests * fix adnuntius & pbsBidAdapter
* Screencore prebid adapter * rearrange code * use lowercase screncore bidder code * fix tests * update tests * trigger CI * Screencore Bid Adapter: add endpointId parameter * Updated adapter to use teqblazeUtils library * Added endpointId parameter support in test parameters * Updated test specs to include endpointId validation * Screencore Bid Adapter: update sync URL to base domain Update SYNC_URL constant to use base domain. The getUserSyncs function from teqblazeUtils will append the appropriate path. * Screencore Bid Adapter: migrate to teqblazeUtils library - Update imports to use buildRequestsBase, interpretResponse, getUserSyncs, isBidRequestValid, and buildPlacementProcessingFunction from teqblazeUtils - Remove storage manager dependency (no longer needed) - Update isBidRequestValid to use placementId/endpointId params validation - Refactor buildRequests to use buildRequestsBase pattern - Rewrite test suite to match teqblazeUtils API: - Simplify test data structures - Update server response format (body as array) - Add tests for placementId/endpointId validation - Update getUserSyncs URL format expectations --------- Co-authored-by: Kostiantyn Karchevsky <kostiantyn.karchevsky@teqblaze.com> Co-authored-by: Demetrio Girardi <dgirardi@prebid.org> Co-authored-by: Patrick McCann <patmmccann@gmail.com>
…r-purpose-4 Permutive Modules: update consent handling
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀 |
|
|
||
| const logger = prefixLog('[PermutiveID]') | ||
|
|
||
| function hasPurposeConsent(userConsent, requiredPurposes, enforceVendorConsent) { |
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.
should probably import this block from /libraries in both modules
…#14197) Co-authored-by: Patrick McCann <patmmccann@gmail.com>
…nt' into codex/remove-consent-check-for-purpose-4-6khb93
…r-purpose-4-6khb93 Permutive Modules: centralize consent gating
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
1 similar comment
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
Clarify the description of enforceVendorConsent parameter in the documentation.
|
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Whoa there, partner! 🌵🤠 We wrangled some duplicated code in your PR:
Reducing code duplication by importing common functions from a library not only makes our code cleaner but also easier to maintain. Please move the common code from both files into a library and import it in each. We hate that we have to mention this, however, commits designed to hide from this utility by renaming variables or reordering an object are poor conduct. We will not look upon them kindly! Keep up the great work! 🚀 |
|
Tread carefully! This PR adds 2 linter errors (possibly disabled through directives):
|
AntonioGargaro
left a comment
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.
LGTM, just a few linter errors that need to be resolved ✅
|
Hey @patmmccann I've raised a PR here for your review to fix some of the linter errors for this PR. |
A bitwise `&` was being used instead of the intended `|`.
* Onetag Adapter: remove screenLeft usage * Update onetagBidAdapter_spec.js
* TargetingData from event * lint * test fix
fix: linter errors
Summary
Testing
Codex Task