Removed geoOverride flag (#26724)

ampproject · Feb 11, 2020 · 7c483dd · 7c483dd
1 parent 15b60a3
commit 7c483dd
Show file tree

Hide file tree

Showing 8 changed files with 2 additions and 98 deletions.
diff --git a/build-system/global-configs/canary-config.json b/build-system/global-configs/canary-config.json
@@ -14,7 +14,6 @@
   "amp-ad-ff-adx-ady": 0.01,
   "amp-auto-ads-adsense-holdout": 0.1,
   "amp-auto-ads-no-op-experiment": 0.05,
-  "amp-consent-geo-override": 1,
   "amp-consent-v2": 1,
   "amp-mega-menu": 1,
   "amp-nested-menu": 1,

diff --git a/build-system/global-configs/prod-config.json b/build-system/global-configs/prod-config.json
@@ -14,7 +14,6 @@
   "amp-ad-ff-adx-ady": 0.01,
   "amp-auto-ads-adsense-holdout": 0.1,
   "amp-auto-ads-no-op-experiment": 0.05,
-  "amp-consent-geo-override": 1,
   "amp-consent-v2": 1,
   "amp-mega-menu": 1,
   "amp-nested-menu": 1,

diff --git a/extensions/amp-consent/0.1/amp-consent.js b/extensions/amp-consent/0.1/amp-consent.js
@@ -26,7 +26,6 @@ import {ConsentPolicyManager} from './consent-policy-manager';
 import {ConsentStateManager} from './consent-state-manager';
 import {ConsentUI} from './consent-ui';
 import {Deferred} from '../../../src/utils/promise';
-import {GEO_IN_GROUP} from '../../amp-geo/0.1/amp-geo-in-group';
 import {
   NOTIFICATION_UI_MANAGER,
   NotificationUiManager,
@@ -38,7 +37,7 @@ import {
   resolveRelativeUrl,
 } from '../../../src/url';
 import {dev, devAssert, user, userAssert} from '../../../src/log';
-import {dict, hasOwn} from '../../../src/utils/object';
+import {dict} from '../../../src/utils/object';
 import {getData} from '../../../src/event-helper';
 import {getServicePromiseForDoc} from '../../../src/service';
 import {isEnumValue} from '../../../src/types';
@@ -425,11 +424,7 @@ export class AmpConsent extends AMP.BaseElement {
    */
   init_() {
     this.passSharedData_();
-    if (isExperimentOn(this.win, 'amp-consent-geo-override')) {
-      this.syncRemoteConsentState_();
-    } else {
-      this.maybeSetDirtyBit_();
-    }
+    this.syncRemoteConsentState_();
 
     this.getConsentRequiredPromise_()
       .then(isConsentRequired => {
@@ -454,9 +449,6 @@ export class AmpConsent extends AMP.BaseElement {
    * @return {!Promise<boolean>}
    */
   getConsentRequiredPromise_() {
-    if (!isExperimentOn(this.win, 'amp-consent-geo-override')) {
-      return this.getConsentRequiredPromiseLegacy_();
-    }
     return this.consentStateManager_
       .getConsentInstanceInfo()
       .then(storedInfo => {
@@ -476,41 +468,6 @@ export class AmpConsent extends AMP.BaseElement {
       });
   }
 
-  /**
-   * Returns a promise that resolve when amp-consent knows
-   * if the consent is required.
-   * @return {!Promise<boolean>}
-   */
-  getConsentRequiredPromiseLegacy_() {
-    let consentRequiredPromise = null;
-    if (this.consentConfig_['promptIfUnknownForGeoGroup']) {
-      const geoGroup = this.consentConfig_['promptIfUnknownForGeoGroup'];
-      consentRequiredPromise = this.isConsentRequiredGeo_(geoGroup);
-    } else {
-      consentRequiredPromise = this.getConsentRemote_().then(
-        remoteConfigResponse => {
-          if (
-            !remoteConfigResponse ||
-            !hasOwn(remoteConfigResponse, 'promptIfUnknown')
-          ) {
-            this.user().error(
-              TAG,
-              'Expecting promptIfUnknown from ' +
-                'checkConsentHref when promptIfUnknownForGeoGroup is not ' +
-                'specified'
-            );
-            // Set to false if not defined
-            return false;
-          }
-          return !!remoteConfigResponse['promptIfUnknown'];
-        }
-      );
-    }
-    return consentRequiredPromise.then(required => {
-      return !!required;
-    });
-  }
-
   /**
    * Blindly pass sharedData
    */
@@ -526,18 +483,6 @@ export class AmpConsent extends AMP.BaseElement {
     this.consentStateManager_.setConsentInstanceSharedData(sharedDataPromise);
   }
 
-  /**
-   * Set dirtyBit of the local consent value based on server response
-   */
-  maybeSetDirtyBit_() {
-    const responsePromise = this.getConsentRemote_();
-    responsePromise.then(response => {
-      if (response && !!response['forcePromptOnNext']) {
-        this.consentStateManager_.setDirtyBit();
-      }
-    });
-  }
-
   /**
    * Clear cache for server side decision and then sync.
    */
@@ -582,18 +527,6 @@ export class AmpConsent extends AMP.BaseElement {
     }
   }
 
-  /**
-   * Returns a promise that if user is in the given geoGroup
-   * @param {string} geoGroup
-   * @return {Promise<boolean>}
-   */
-  isConsentRequiredGeo_(geoGroup) {
-    return Services.geoForDocOrNull(this.element).then(geo => {
-      userAssert(geo, 'requires <amp-geo> to use promptIfUnknownForGeoGroup');
-      return geo.isInCountryGroup(geoGroup) == GEO_IN_GROUP.IN;
-    });
-  }
-
   /**
    * Get localStored consent info, and send request to get consent from endpoint
    * if there is checkConsentHref specified.

diff --git a/extensions/amp-consent/0.1/test-e2e/test-amp-consent-client-side.js b/extensions/amp-consent/0.1/test-e2e/test-amp-consent-client-side.js
@@ -26,7 +26,6 @@ describes.endtoend(
   {
     testUrl:
       'http://localhost:8000/test/manual/amp-consent/amp-consent-basic-uses.amp.html#amp-geo=de',
-    experiments: ['amp-consent-geo-override'],
     // TODO (micajuineho): Add shadow-demo after #25985 is fixed and viewer-demo when...
     environments: ['single'],
   },

diff --git a/extensions/amp-consent/0.1/test-e2e/test-amp-consent-server-side-expire-cache.js b/extensions/amp-consent/0.1/test-e2e/test-amp-consent-server-side-expire-cache.js
@@ -28,7 +28,6 @@ describes.endtoend(
   {
     testUrl:
       'http://localhost:8000/test/manual/amp-consent/amp-consent-basic-uses.amp.html#amp-geo=mx',
-    experiments: ['amp-consent-geo-override'],
     // TODO (micajuineho): Add shadow-demo after #25985 is fixed, and viewer-demo when...
     environments: ['single'],
   },

diff --git a/extensions/amp-consent/0.1/test-e2e/test-amp-consent-server-side.js b/extensions/amp-consent/0.1/test-e2e/test-amp-consent-server-side.js
@@ -26,7 +26,6 @@ describes.endtoend(
   {
     testUrl:
       'http://localhost:8000/test/manual/amp-consent/amp-consent-basic-uses.amp.html#amp-geo=us',
-    experiments: ['amp-consent-geo-override'],
     // TODO (micajuineho): Add shadow-demo after #25985 is fixed and viewer-demo when...
     environments: ['single'],
   },

diff --git a/extensions/amp-consent/0.1/test/test-amp-consent.js b/extensions/amp-consent/0.1/test/test-amp-consent.js
@@ -241,13 +241,6 @@ describes.realWin(
 
     describe('geo-override server communication', () => {
       let ampConsent;
-      beforeEach(() => {
-        toggleExperiment(win, 'amp-consent-geo-override', true);
-      });
-
-      afterEach(() => {
-        toggleExperiment(win, 'amp-consent-geo-override', false);
-      });
 
       it('checks local storage before making sever request', async () => {
         const config = {
@@ -825,12 +818,7 @@ describes.realWin(
         });
       });
 
-      afterEach(() => {
-        toggleExperiment(win, 'amp-consent-geo-override', false);
-      });
-
       it('should not show promptUI if local storage has decision', async () => {
-        toggleExperiment(win, 'amp-consent-geo-override', true);
         const config = {
           'consentInstanceId': 'abc',
           'consentRequired': 'remote',
@@ -945,7 +933,6 @@ describes.realWin(
 
         describe('hide/show postPromptUI with local storage', () => {
           beforeEach(() => {
-            toggleExperiment(win, 'amp-consent-geo-override', true);
             defaultConfig = dict({
               'consentInstanceId': 'ABC',
               'consentRequired': true,
@@ -959,10 +946,6 @@ describes.realWin(
             ampConsent = new AmpConsent(consentElement);
           });
 
-          afterEach(() => {
-            toggleExperiment(win, 'amp-consent-geo-override', false);
-          });
-
           it('hides postPromptUI with no local storage decision', async () => {
             await ampConsent.buildCallback();
             expect(postPromptUI).to.have.display('none');

diff --git a/tools/experiments/experiments-config.js b/tools/experiments/experiments-config.js
@@ -67,13 +67,6 @@ export const EXPERIMENTS = [
     spec: 'https://github.com/ampproject/amphtml/issues/26432',
     cleanupIssue: 'https://github.com/ampproject/amphtml/issues/26432',
   },
-  {
-    id: 'amp-consent-geo-override',
-    name: 'AMP consent modified to support CCPA',
-    spec:
-      'https://github.com/ampproject/amphtml/blob/lannka-consent-design/extensions/amp-consent/amp-consent.md',
-    cleanupIssue: 'https://github.com/ampproject/amphtml/issues/25623',
-  },
   {
     id: 'amp-sidebar-v2',
     name: 'Updated sidebar component with nested menu and animations',