Fix empty view change issues

src/components/Personalization/createApplyPropositions.js

@@ -10,7 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag
 governing permissions and limitations under the License.
 */
 
-import { isNonEmptyArray, isObject } from "../../utils";
+import { isNonEmptyArray, isObject, defer } from "../../utils";
 import { DOM_ACTION, HTML_CONTENT_ITEM } from "./constants/schema";
 import PAGE_WIDE_SCOPE from "../../constants/pageWideScope";
 
@@ -75,6 +75,11 @@ export default ({
   };
 
   return ({ propositions = [], metadata = {}, viewName }) => {
+    // We need to immediately call concat so that subsequent sendEvent
+    // calls will wait for applyPropositions to complete before executing.
+    const displayNotificationsDeferred = defer();
+    pendingDisplayNotifications.concat(displayNotificationsDeferred.promise);
+
     const propositionsToExecute = preparePropositions({
       propositions,
       metadata
@@ -93,7 +98,7 @@ export default ({
           ...additionalPropositions
         ]);
 
-        pendingDisplayNotifications.concat(render());
+        render().then(displayNotificationsDeferred.resolve);
 
         return {
           propositions: returnedPropositions

src/components/Personalization/createComponent.js

@@ -10,10 +10,11 @@ OF ANY KIND, either express or implied. See the License for the specific languag
 governing permissions and limitations under the License.
 */
 
-import { noop } from "../../utils";
+import { noop, flatMap } from "../../utils";
 import createPersonalizationDetails from "./createPersonalizationDetails";
 import { AUTHORING_ENABLED } from "./constants/loggerMessage";
 import validateApplyPropositionsOptions from "./validateApplyPropositionsOptions";
+import { PropositionEventType } from "./constants/propositionEventType";
 
 export default ({
   getPageLocation,
@@ -27,7 +28,8 @@ export default ({
   showContainers,
   applyPropositions,
   setTargetMigration,
-  pendingNotificationsHandler
+  mergeDecisionsMeta,
+  pendingDisplayNotifications
 }) => {
   return {
     lifecycle: {
@@ -65,9 +67,9 @@ export default ({
           logger
         });
 
-        const handlerPromises = [];
+        const decisionsMetaPromises = [];
         if (personalizationDetails.shouldAddPendingDisplayNotifications()) {
-          handlerPromises.push(pendingNotificationsHandler({ event }));
+          decisionsMetaPromises.push(pendingDisplayNotifications.clear());
         }
 
         if (personalizationDetails.shouldFetchData()) {
@@ -84,7 +86,7 @@ export default ({
           });
         } else if (personalizationDetails.shouldUseCachedData()) {
           // eslint-disable-next-line consistent-return
-          handlerPromises.push(
+          decisionsMetaPromises.push(
             viewChangeHandler({
               personalizationDetails,
               event,
@@ -93,9 +95,19 @@ export default ({
             })
           );
         }
-        // We can wait for personalization to be applied and for
-        // the fetch data request to complete in parallel.
-        return Promise.all(handlerPromises);
+
+        // This promise.all waits for both the pending display notifications to be resolved
+        // (i.e. the top of page call to finish rendering) and the view change handler to
+        // finish rendering anything for this view.
+        return Promise.all(decisionsMetaPromises).then(decisionsMetas => {
+          // We only want to call mergeDecisionsMeta once, but we can get the propositions
+          // from two places: the pending display notifications and the view change handler.
+          mergeDecisionsMeta(
+            event,
+            flatMap(decisionsMetas, dms => dms),
+            PropositionEventType.DISPLAY
+          );
+        });
       },
       onClick({ event, clickedElement }) {
         onClickHandler({ event, clickedElement });

src/components/Personalization/createViewChangeHandler.js

@@ -10,10 +10,8 @@ OF ANY KIND, either express or implied. See the License for the specific languag
 governing permissions and limitations under the License.
 */
 
-import { PropositionEventType } from "./constants/propositionEventType";
-
-export default ({ mergeDecisionsMeta, processPropositions, viewCache }) => {
-  return ({ personalizationDetails, event, onResponse }) => {
+export default ({ processPropositions, viewCache }) => {
+  return ({ personalizationDetails, onResponse }) => {
     let returnedPropositions;
     let returnedDecisions;
     const viewName = personalizationDetails.getViewName();
@@ -25,26 +23,21 @@ export default ({ mergeDecisionsMeta, processPropositions, viewCache }) => {
       };
     });
 
-    return viewCache
-      .getView(viewName)
-      .then(propositions => {
-        let render;
-        if (personalizationDetails.isRenderDecisions()) {
-          ({
-            render,
-            returnedPropositions,
-            returnedDecisions
-          } = processPropositions(propositions));
-          return render();
-        }
-        ({ returnedPropositions, returnedDecisions } = processPropositions(
-          [],
-          propositions
-        ));
-        return [];
-      })
-      .then(decisionsMeta => {
-        mergeDecisionsMeta(event, decisionsMeta, PropositionEventType.DISPLAY);
-      });
+    return viewCache.getView(viewName).then(propositions => {
+      let render;
+      if (personalizationDetails.isRenderDecisions()) {
+        ({
+          render,
+          returnedPropositions,
+          returnedDecisions
+        } = processPropositions(propositions));
+        return render();
+      }
+      ({ returnedPropositions, returnedDecisions } = processPropositions(
+        [],
+        propositions
+      ));
+      return [];
+    });
   };
 };

src/components/Personalization/event.js

@@ -19,6 +19,11 @@ export const mergeDecisionsMeta = (
   eventType,
   eventLabel = ""
 ) => {
+  // Do not send a display notification with no decisions. Even empty view changes
+  // should include a proposition.
+  if (decisionsMeta.length === 0) {
+    return;
+  }
   const xdm = {
     _experience: {
       decisioning: {

src/components/Personalization/index.js

@@ -31,7 +31,6 @@ import remapHeadOffers from "./dom-actions/remapHeadOffers";
 import createPreprocess from "./dom-actions/createPreprocess";
 import injectCreateProposition from "./handlers/injectCreateProposition";
 import createAsyncArray from "./utils/createAsyncArray";
-import createPendingNotificationsHandler from "./createPendingNotificationsHandler";
 import * as schema from "./constants/schema";
 import processDefaultContent from "./handlers/processDefaultContent";
 import { isPageWideSurface } from "./utils/surfaceUtils";
@@ -80,10 +79,6 @@ const createPersonalization = ({ config, logger, eventManager }) => {
   });
 
   const pendingDisplayNotifications = createAsyncArray();
-  const pendingNotificationsHandler = createPendingNotificationsHandler({
-    pendingDisplayNotifications,
-    mergeDecisionsMeta
-  });
   const fetchDataHandler = createFetchDataHandler({
     prehidingStyle,
     showContainers,
@@ -101,7 +96,6 @@ const createPersonalization = ({ config, logger, eventManager }) => {
     getClickMetasBySelector
   });
   const viewChangeHandler = createViewChangeHandler({
-    mergeDecisionsMeta,
     processPropositions,
     viewCache
   });
@@ -126,7 +120,8 @@ const createPersonalization = ({ config, logger, eventManager }) => {
     showContainers,
     applyPropositions,
     setTargetMigration,
-    pendingNotificationsHandler
+    mergeDecisionsMeta,
+    pendingDisplayNotifications
   });
 };
 

test/unit/specs/components/Personalization/createComponent.spec.js

@@ -24,6 +24,8 @@ describe("Personalization", () => {
   let event;
   let personalizationComponent;
   let setTargetMigration;
+  let mergeDecisionsMeta;
+  let pendingDisplayNotifications;
   let cacheUpdate;
 
   const build = () => {
@@ -36,7 +38,9 @@ describe("Personalization", () => {
       mergeQuery,
       viewCache,
       showContainers,
-      setTargetMigration
+      setTargetMigration,
+      mergeDecisionsMeta,
+      pendingDisplayNotifications
     });
   };
 
@@ -62,6 +66,11 @@ describe("Personalization", () => {
     cacheUpdate = jasmine.createSpyObj("cacheUpdate", ["update", "cancel"]);
     viewCache.createCacheUpdate.and.returnValue(cacheUpdate);
     setTargetMigration = jasmine.createSpy("setTargetMigration");
+    mergeDecisionsMeta = jasmine.createSpy("mergeDecisionsMeta");
+    pendingDisplayNotifications = jasmine.createSpyObj(
+      "pendingDisplayNotifications",
+      ["clear"]
+    );
 
     build();
   });

test/unit/specs/components/Personalization/createViewChangeHandler.spec.js

@@ -11,12 +11,10 @@ governing permissions and limitations under the License.
 */
 
 import createViewChangeHandler from "../../../../../src/components/Personalization/createViewChangeHandler";
-import { PropositionEventType } from "../../../../../src/components/Personalization/constants/propositionEventType";
 import { CART_VIEW_DECISIONS } from "./responsesMock/eventResponses";
 import injectCreateProposition from "../../../../../src/components/Personalization/handlers/injectCreateProposition";
 
 describe("Personalization::createViewChangeHandler", () => {
-  let mergeDecisionsMeta;
   let processPropositions;
   let viewCache;
 
@@ -27,7 +25,6 @@ describe("Personalization::createViewChangeHandler", () => {
   let createProposition;
 
   beforeEach(() => {
-    mergeDecisionsMeta = jasmine.createSpy("mergeDecisionsMeta");
     processPropositions = jasmine.createSpy("processPropositions");
     viewCache = jasmine.createSpyObj("viewCache", ["getView"]);
 
@@ -46,16 +43,16 @@ describe("Personalization::createViewChangeHandler", () => {
 
   const run = async () => {
     const viewChangeHandler = createViewChangeHandler({
-      mergeDecisionsMeta,
       processPropositions,
       viewCache
     });
-    await viewChangeHandler({
+    const decisionsMeta = await viewChangeHandler({
       event,
       personalizationDetails,
       onResponse
     });
-    return onResponse.calls.argsFor(0)[0]();
+    const result = onResponse.calls.argsFor(0)[0]();
+    return { decisionsMeta, result };
   };
 
   it("should trigger render if renderDecisions is true", async () => {
@@ -70,14 +67,11 @@ describe("Personalization::createViewChangeHandler", () => {
       returnedDecisions: CART_VIEW_DECISIONS
     });
 
-    const result = await run();
+    const { decisionsMeta, result } = await run();
 
     expect(processPropositions).toHaveBeenCalledTimes(1);
-    expect(mergeDecisionsMeta).toHaveBeenCalledWith(
-      "myevent",
-      "decisionMeta",
-      PropositionEventType.DISPLAY
-    );
+    expect(decisionsMeta).toEqual("decisionMeta");
+
     expect(result.decisions).toEqual(CART_VIEW_DECISIONS);
   });
 });

test/unit/specs/components/Personalization/topLevel/buildAlloy.js

@@ -30,7 +30,6 @@ import { createCallbackAggregator, assign } from "../../../../../../src/utils";
 import injectCreateProposition from "../../../../../../src/components/Personalization/handlers/injectCreateProposition";
 import createProcessPropositions from "../../../../../../src/components/Personalization/handlers/createProcessPropositions";
 import createAsyncArray from "../../../../../../src/components/Personalization/utils/createAsyncArray";
-import createPendingNotificationsHandler from "../../../../../../src/components/Personalization/createPendingNotificationsHandler";
 import * as schema from "../../../../../../src/components/Personalization/constants/schema";
 import createProcessDomAction from "../../../../../../src/components/Personalization/handlers/createProcessDomAction";
 import createProcessHtmlContent from "../../../../../../src/components/Personalization/handlers/createProcessHtmlContent";
@@ -115,10 +114,6 @@ const buildComponent = ({
   });
 
   const pendingDisplayNotifications = createAsyncArray();
-  const pendingNotificationsHandler = createPendingNotificationsHandler({
-    pendingDisplayNotifications,
-    mergeDecisionsMeta
-  });
   const fetchDataHandler = createFetchDataHandler({
     prehidingStyle,
     showContainers,
@@ -136,7 +131,6 @@ const buildComponent = ({
     getClickMetasBySelector
   });
   const viewChangeHandler = createViewChangeHandler({
-    mergeDecisionsMeta,
     processPropositions,
     viewCache
   });
@@ -161,7 +155,8 @@ const buildComponent = ({
     showContainers,
     applyPropositions,
     setTargetMigration,
-    pendingNotificationsHandler
+    mergeDecisionsMeta,
+    pendingDisplayNotifications
   });
 };