<amp-consent> add machtedGeoGroups to request #25699
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
added 22 commits
November 19, 2019 09:05
…html into ccpa_geooverride_config
lannka
requested changes
Nov 20, 2019
added 11 commits
November 22, 2019 09:38
micajuine-ho
force-pushed
the
ccpa_matchedGeoGroups
branch
from
a355f35
to
7d54dcb
Compare
lannka
requested changes
Nov 25, 2019
| @@ -230,6 +242,29 @@ describes.realWin( | |||
| expect(await ampConsent.getConsentRequiredPromise_()).to.be.true; | |||
| }); | |||
|
|
|||
| it('send post request to server with matched group', async () => { | |||
There was a problem hiding this comment.
add a test case that when multiple groups are matched, we send the first one.
lannka
approved these changes
Nov 25, 2019
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 const vbs, refactoring, adding matchedgroup * add this.matchedGeoGroups_, & in request * adding in getConsentRequiredPromiseLegacy_() * Update amp-consent.js * Update consent-config.js * requested changes * suggested changes * getMatchedGeoGroups method * getMatchedGeoGroups() in amp-consent * 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 * 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 * suggested changes * adding tests * adding tests * nit changes from last pr * suggested changes + tests
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.
Task 5 of #25623.
Add
matchedGeoGroupsto the the checkConsentHref request request body.Previous task(s):
1: #25676 (merged)
2: #25689 (merged)
3 & 4: #25696 (merged)