🐛 amp-story-shopping does not display CTA if all tags are invalid and…

… Fixes remote shopping data loading on refresh with page attachment open (#38035) * added fixed for shopping tag rendering * added shopping json patch that fixed regression * resolved promise .all * updated template * refactored state update * fixed promise resolution * added more descriptive error message * cahnged order of ternary * added history check for page refresh * updated hsitory service to store all data, not jsut the activeData * added shopping element * added shopping attachment logic for page refresh, added back active product only * added set history state test * cleaned up some minor nits * added unit test for shopping tag history * history stub * added shopping if clause optimizations * added dependencies * added dependencies * added dependencies * updated unit tests * updated unit tests * updated unit tests * removed check * browser test safari * browser test safari * added better url checks * added stuff
ampproject · Apr 19, 2022 · 2ef2d96 · 2ef2d96
1 parent 79b5c24
commit 2ef2d96
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 11 deletions.
diff --git a/build-system/test-configs/dep-check-config.js b/build-system/test-configs/dep-check-config.js
@@ -252,8 +252,10 @@ exports.rules = [
       'extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js->extensions/amp-story/1.0/amp-story-store-service.js',
       'extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js->extensions/amp-story/1.0/variable-service.js',
       'extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js->extensions/amp-story/1.0/story-analytics.js',
+      'extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js->extensions/amp-story/1.0/history.js',
       'extensions/amp-story-shopping/0.1/amp-story-shopping-tag.js->extensions/amp-story/1.0/variable-service.js',
       'extensions/amp-story-shopping/0.1/amp-story-shopping-tag.js->extensions/amp-story/1.0/story-analytics.js',
+      'extensions/amp-story-shopping/0.1/amp-story-shopping-tag.js->extensions/amp-story/1.0/history.js',
 
       // Interactive components that depend on story functionality.
       'extensions/amp-story-interactive/0.1/amp-story-interactive-abstract.js->extensions/amp-story/1.0/amp-story-store-service.js',

diff --git a/extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js b/extensions/amp-story-shopping/0.1/amp-story-shopping-attachment.js
@@ -6,6 +6,7 @@ import {Services} from '#service';
 import {LocalizedStringId_Enum} from '#service/localization/strings';
 
 import {localizeTemplate} from 'extensions/amp-story/1.0/amp-story-localization-service';
+import {HistoryState, setHistoryState} from 'extensions/amp-story/1.0/history';
 
 import {formatI18nNumber, loadFonts} from './amp-story-shopping';
 import {
@@ -82,17 +83,25 @@ export class AmpStoryShoppingAttachment extends AMP.BaseElement {
       this.pageEl_.querySelectorAll('amp-story-shopping-tag')
     );
 
-    getShoppingConfig(this.element, this.pageEl_.id).then((config) =>
-      storeShoppingConfig(this.pageEl_, config)
-    );
-
     if (this.shoppingTags_.length === 0) {
-      return;
+      return Promise.reject(
+        new Error(`No shopping tags on page ${this.pageEl_.id}.`)
+      );
     }
 
-    return this.localizationService_
-      .getLocalizedStringAsync(
-        LocalizedStringId_Enum.AMP_STORY_SHOPPING_CTA_LABEL
+    return getShoppingConfig(this.element, this.pageEl_.id)
+      .then((config) => {
+        if (Object.keys(config).length === 0) {
+          return Promise.reject(
+            new Error(`No valid shopping data on page ${this.pageEl_.id}.`)
+          );
+        }
+        storeShoppingConfig(this.pageEl_, config);
+      })
+      .then(() =>
+        this.localizationService_.getLocalizedStringAsync(
+          LocalizedStringId_Enum.AMP_STORY_SHOPPING_CTA_LABEL
+        )
       )
       .then((ctaText) => {
         this.attachmentEl_ = (
@@ -110,9 +119,6 @@ export class AmpStoryShoppingAttachment extends AMP.BaseElement {
 
   /** @override */
   layoutCallback() {
-    if (this.shoppingTags_.length === 0) {
-      return;
-    }
     loadFonts(this.win, FONTS_TO_LOAD);
     // Update template on attachment state update or shopping data update.
     this.storeService_.subscribe(
@@ -299,6 +305,8 @@ export class AmpStoryShoppingAttachment extends AMP.BaseElement {
     this.storeService_.dispatch(Action.ADD_SHOPPING_DATA, {
       'activeProductData': shoppingData,
     });
+
+    setHistoryState(this.win, HistoryState.SHOPPING_DATA, shoppingData);
   }
 
   /**

diff --git a/extensions/amp-story-shopping/0.1/amp-story-shopping-tag.js b/extensions/amp-story-shopping/0.1/amp-story-shopping-tag.js
@@ -8,6 +8,8 @@ import {
 
 import {Services} from '#service';
 
+import {HistoryState, setHistoryState} from 'extensions/amp-story/1.0/history';
+
 import {formatI18nNumber, loadFonts} from './amp-story-shopping';
 
 import {CSS as shoppingSharedCSS} from '../../../build/amp-story-shopping-shared-0.1.css';
@@ -170,6 +172,8 @@ export class AmpStoryShoppingTag extends AMP.BaseElement {
       'activeProductData': this.tagData_,
     });
 
+    setHistoryState(this.win, HistoryState.SHOPPING_DATA, this.tagData_);
+
     this.variableService_.onVariableUpdate(
       AnalyticsVariable.STORY_SHOPPING_PRODUCT_ID,
       this.tagData_.productId

diff --git a/extensions/amp-story-shopping/0.1/test/test-amp-story-shopping-attachment.js b/extensions/amp-story-shopping/0.1/test/test-amp-story-shopping-attachment.js
@@ -2,6 +2,9 @@ import * as Preact from '#core/dom/jsx';
 
 import {Services} from '#service';
 
+import {HistoryState} from 'extensions/amp-story/1.0/history';
+import * as history from 'extensions/amp-story/1.0/history';
+
 import '../../../amp-story/1.0/amp-story';
 import '../../../amp-story/1.0/amp-story-page';
 import '../amp-story-shopping';
@@ -275,5 +278,21 @@ describes.realWin(
         StoryAnalyticsEvent.SHOPPING_PDP_VIEW
       );
     });
+
+    it('should call history service on Product Listing Page card click', async () => {
+      const historyStub = env.sandbox.stub(history, 'setHistoryState');
+
+      await layoutShoppingImplAndAttachmentChildImpl();
+      storeService.dispatch(Action.TOGGLE_PAGE_ATTACHMENT_STATE, true);
+      const plpCard = attachmentChildEl.querySelector(
+        '.i-amphtml-amp-story-shopping-plp-card'
+      );
+      plpCard.dispatchEvent(new Event('click'));
+      expect(historyStub).to.have.been.called.calledWith(
+        win,
+        HistoryState.SHOPPING_DATA,
+        shoppingData.items[0]
+      );
+    });
   }
 );
diff --git a/extensions/amp-story-shopping/0.1/test/test-amp-story-shopping-tag.js b/extensions/amp-story-shopping/0.1/test/test-amp-story-shopping-tag.js
@@ -5,6 +5,11 @@ import '../amp-story-shopping';
 import {Services} from '#service';
 import {LocalizationService} from '#service/localization';
 
+import '../../../amp-story-page-attachment/0.1/amp-story-page-attachment';
+
+import * as history from 'extensions/amp-story/1.0/history';
+import {HistoryState} from 'extensions/amp-story/1.0/history';
+
 import {registerServiceBuilder} from '../../../../src/service-helpers';
 import {
   Action,
@@ -128,11 +133,27 @@ describes.realWin(
 
     it('should call analytics service on tag click', async () => {
       const trigger = env.sandbox.stub(analytics, 'triggerEvent');
+      env.sandbox.stub(history, 'setHistoryState');
       await setupShoppingTagAndData();
       await shoppingTag.shoppingTagEl_.click();
       expect(trigger).to.have.been.calledWith(
         StoryAnalyticsEvent.SHOPPING_TAG_CLICK
       );
     });
+
+    it('should call history service on tag click', async () => {
+      const tagData = {
+        'productTitle': 'Spectacular Spectacles',
+      };
+
+      const historyStub = env.sandbox.stub(history, 'setHistoryState');
+      await setupShoppingTagAndData();
+      await shoppingTag.shoppingTagEl_.click();
+      expect(historyStub).to.have.been.called.calledWith(
+        win,
+        HistoryState.SHOPPING_DATA,
+        tagData
+      );
+    });
   }
 );
diff --git a/extensions/amp-story/1.0/amp-story.js b/extensions/amp-story/1.0/amp-story.js
@@ -1009,6 +1009,17 @@ export class AmpStory extends AMP.BaseElement {
                 attachmentImpl.open(false /** shouldAnimate */)
               );
           }
+
+          const shoppingData = getHistoryState(
+            this.win,
+            HistoryState.SHOPPING_DATA
+          );
+
+          if (shoppingData) {
+            this.storeService_.dispatch(Action.ADD_SHOPPING_DATA, {
+              'activeProductData': shoppingData,
+            });
+          }
         }
 
         if (

diff --git a/extensions/amp-story/1.0/history.js b/extensions/amp-story/1.0/history.js
@@ -11,6 +11,7 @@ export const LOCAL_STORAGE_KEY = 'amp-story-state';
 export const HistoryState = {
   ATTACHMENT_PAGE_ID: 'ampStoryAttachmentPageId',
   NAVIGATION_PATH: 'ampStoryNavigationPath',
+  SHOPPING_DATA: 'ampStoryShoppingData',
 };
 
 /**