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
<amp-consent> Local storage initial check #25765
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…html into ccpa_geooverride_config
lannka
reviewed
Nov 25, 2019
lannka
reviewed
Nov 25, 2019
micajuine-ho
force-pushed
the
ccpa_local_storage
branch
from
November 25, 2019 23:15
5f5651d
to
7eba8ed
Compare
lannka
requested changes
Nov 25, 2019
lannka
approved these changes
Nov 27, 2019
lannka
reviewed
Nov 27, 2019
caroqliu
pushed a commit
to caroqliu/amphtml
that referenced
this pull request
Dec 5, 2019
* added experiment flag * added configs * changes to prod/canary config * cleaning up * more clean up * merge logic in consent-config * adding in userAssert for consentRequired * Update canary-config.json * Update prod-config.json * Update amp-consent.js * cleaning up * deprecate promptIfUnknownForGeo * consent intialize promise * config merge then validate * check if geoService can't identify * verify consentRequired from endpoint * removed consentState to string * adding comment * bug fixing and geoGroupUnknown * adding in getConsentRequiredPromiseLegacy_() * Update amp-consent.js * requested changes * suggested changes * suggested fixes & fixing unit tests * unit tests, better userAssert messages * adding tests * Suggested changes to tests + bug fixes * starting to write tests for promptunknowngeo * unit tests * remove function*, cleaning up * sandbox change * a little more cleaning up * cleaning * cleaning up logic * fixing tests and promises * testing * bug fix * unit test for cr * test for non remote consentRequired * migrating unit tests * Cleaning code, adding thorough test * get local storage first * local storage priority and tests * adding tests * move logic to consentRequiredPromise * Small changes * fixing tests * fixed tests, added config check * Suggested Changes * cleaning
micajuine-ho
pushed a commit
to micajuine-ho/amphtml
that referenced
this pull request
Dec 27, 2019
* added experiment flag * added configs * changes to prod/canary config * cleaning up * more clean up * merge logic in consent-config * adding in userAssert for consentRequired * Update canary-config.json * Update prod-config.json * Update amp-consent.js * cleaning up * deprecate promptIfUnknownForGeo * consent intialize promise * config merge then validate * check if geoService can't identify * verify consentRequired from endpoint * removed consentState to string * adding comment * bug fixing and geoGroupUnknown * adding in getConsentRequiredPromiseLegacy_() * Update amp-consent.js * requested changes * suggested changes * suggested fixes & fixing unit tests * unit tests, better userAssert messages * adding tests * Suggested changes to tests + bug fixes * starting to write tests for promptunknowngeo * unit tests * remove function*, cleaning up * sandbox change * a little more cleaning up * cleaning * cleaning up logic * fixing tests and promises * testing * bug fix * unit test for cr * test for non remote consentRequired * migrating unit tests * Cleaning code, adding thorough test * get local storage first * local storage priority and tests * adding tests * move logic to consentRequiredPromise * Small changes * fixing tests * fixed tests, added config check * Suggested Changes * cleaning
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add initial local storage check to amp-consent flow
for #25623
Previous task(s):
1: #25676 (merged)
2: #25689 (merged)
3 & 4: #25696 (merged)
5: #25699 (merged)