♻️ Remove geoOverride from mode (#35654)

* Remove geoOverride from mode * Fix test file
ampproject · Aug 13, 2021 · 862f51f · 862f51f
1 parent 1af084a
commit 862f51f
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 23 deletions.
diff --git a/extensions/amp-geo/0.1/amp-geo.js b/extensions/amp-geo/0.1/amp-geo.js
@@ -39,6 +39,7 @@ import {Deferred} from '#core/data-structures/promise';
 import {isJsonScriptTag} from '#core/dom';
 import {isArray, isObject} from '#core/types';
 import {tryParseJson} from '#core/types/object/json';
+import {getHashParams} from '#core/types/string/url';
 
 import {isCanary} from '#experiments';
 
@@ -189,20 +190,18 @@ export class AmpGeo extends AMP.BaseElement {
     const trimmedGeoMatch = GEO_HOTPATCH_STR_REGEX.exec(COUNTRY);
 
     // default country is 'unknown' which is also the zero length case
-    if (
-      getMode(this.win).geoOverride &&
-      (isCanary(this.win) || getMode(this.win).localDev)
-    ) {
+    const geoOverride = getHashParams(this.win)['amp-geo'];
+    if (geoOverride && (isCanary(this.win) || getMode(this.win).localDev)) {
       // debug override case, only works in canary or localdev
       // match to \w characters only to prevent xss vector
       const overrideGeoMatch = GEO_HOTPATCH_STR_REGEX.exec(
-        getMode(this.win).geoOverride.toLowerCase()
+        geoOverride.toLowerCase()
       );
       if (overrideGeoMatch[1]) {
-        this.country_ = overrideGeoMatch[1].toLowerCase();
+        this.country_ = overrideGeoMatch[1];
         if (overrideGeoMatch[2]) {
           // Allow subdivision_ to be customized for testing, not checking us-ca
-          this.subdivision_ = overrideGeoMatch[2].toLowerCase();
+          this.subdivision_ = overrideGeoMatch[2];
         }
         this.mode_ = mode.GEO_OVERRIDE;
       }

diff --git a/extensions/amp-geo/0.1/test/test-amp-geo.js b/extensions/amp-geo/0.1/test/test-amp-geo.js
@@ -99,9 +99,9 @@ describes.realWin(
       geo = new AmpGeo(el);
     });
 
-    afterEach(() => {
-      delete win.__AMP_MODE.geoOverride;
-    });
+    function setGeoOverrideHash(hashValue) {
+      geo.win.location.originalHash = `#amp-geo=${hashValue}`;
+    }
 
     function addConfigElement(opt_elementName, opt_type, opt_textContent) {
       const child = doc.createElement(opt_elementName || 'script');
@@ -189,7 +189,7 @@ describes.realWin(
     });
 
     it('should allow hash to override geo in test', () => {
-      win.__AMP_MODE.geoOverride = 'nz';
+      setGeoOverrideHash('nz');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -210,7 +210,7 @@ describes.realWin(
     });
 
     it('should allow hash to override subdivision in test', () => {
-      win.__AMP_MODE.geoOverride = 'us us-ca';
+      setGeoOverrideHash('us us-ca');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -233,7 +233,7 @@ describes.realWin(
     });
 
     it('should allow preset country groups', () => {
-      win.__AMP_MODE.geoOverride = 'fr';
+      setGeoOverrideHash('fr');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -248,7 +248,7 @@ describes.realWin(
     });
 
     it('should allow preset-us-ca, but not us-ca', () => {
-      win.__AMP_MODE.geoOverride = 'us us-ca';
+      setGeoOverrideHash('us us-ca');
       addConfigElement(
         'script',
         'application/json',
@@ -263,7 +263,7 @@ describes.realWin(
     });
 
     it('should set amp-geo-no-group if no group matches', () => {
-      win.__AMP_MODE.geoOverride = 'za';
+      setGeoOverrideHash('za');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -282,7 +282,7 @@ describes.realWin(
     });
 
     it('should return configured and matched groups in `geo` service', () => {
-      win.__AMP_MODE.geoOverride = 'nz';
+      setGeoOverrideHash('nz');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -296,7 +296,7 @@ describes.realWin(
     });
 
     it('isInCountryGroup works with multiple group targets', () => {
-      win.__AMP_MODE.geoOverride = 'nz';
+      setGeoOverrideHash('nz');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -315,7 +315,7 @@ describes.realWin(
     });
 
     it('isInCountryGroup works with single group targets', () => {
-      win.__AMP_MODE.geoOverride = 'nz';
+      setGeoOverrideHash('nz');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -332,7 +332,7 @@ describes.realWin(
     });
 
     it('should allow uppercase hash to override geo in test', () => {
-      win.__AMP_MODE.geoOverride = 'NZ';
+      setGeoOverrideHash('NZ');
       addConfigElement('script');
       geo.buildCallback();
 
@@ -347,7 +347,7 @@ describes.realWin(
     });
 
     it('should accept uppercase country codes in config', () => {
-      win.__AMP_MODE.geoOverride = 'nz';
+      setGeoOverrideHash('nz');
       addConfigElement(
         'script',
         'application/json',
@@ -385,7 +385,7 @@ describes.realWin(
     });
 
     it('should allow hash to override pre-rendered geo in test', () => {
-      win.__AMP_MODE.geoOverride = 'nz';
+      setGeoOverrideHash('nz');
       doc.body.classList.add('amp-iso-country-mx', 'amp-geo-group-nafta');
       addConfigElement('script');
       geo.buildCallback();

diff --git a/src/mode.js b/src/mode.js
@@ -66,8 +66,6 @@ function getMode_(win) {
     localDev: coreMode.isLocalDev(win),
     development: isModeDevelopment(win, hashParams),
     esm: IS_ESM,
-    // amp-geo override
-    geoOverride: hashParams['amp-geo'],
     test: coreMode.isTest(win),
     rtvVersion: getRtvVersion(win),
   };