DO NOT MERGE!!! All changes from last release for Nina #1042

jonsnyder · 2023-09-23T22:19:40Z

Description

DO NOT MERGE!!! This is all the changes for the new release.

Top and bottom events
matching views in view cache with lowercase
Includes bug fixes PRs Fix show containers timing #1040 and Fix empty view change issues #1041

Related Issue

Motivation and Context

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (non-breaking change which does not add functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

I have signed the Adobe Open Source CLA or I'm an Adobe employee.
I have made any necessary test changes and all tests pass.
I have run the Sandbox successfully.

…notifications

…mbining notifications" This reverts commit 8a97a0b.

* TNT-48367 Make SPA view names lowercase

…scope type.

Support for top and bottom of page events. Combine personalization display notifications together.

…t for rendering to complete.

…pAndBottom3

ninaceban · 2023-09-25T20:25:45Z

src/components/Personalization/createFetchDataHandler.js

 }) => {
-  return ({ decisionsDeferred, personalizationDetails, event, onResponse }) => {
+  return ({ cacheUpdate, personalizationDetails, event, onResponse }) => {
    if (personalizationDetails.isRenderDecisions()) {
      hideContainers(prehidingStyle);


Add else clause here to call showContainers. Customers implementing async with pre-hiding snippet will have a hidden page till the timeout function from the snippet is called.

Fixed in #1058

ninaceban · 2023-09-25T20:30:05Z

src/components/Personalization/createOnClickHandler.js

@@ -35,10 +35,10 @@ export default ({
        const xdm = { eventType: INTERACT };
        const scope = decisionsMeta[0].scope;

-        if (scope !== PAGE_WIDE_SCOPE) {
+        if (isNonEmptyString(scope) && scope !== PAGE_WIDE_SCOPE) {


the scope would never be empty

Fixed in #1056

ninaceban · 2023-09-25T20:31:51Z

src/components/Personalization/createOnClickHandler.js

          xdm.web = {
            webPageDetails: {
-              viewName: scope
+              viewName: scope.toLowerCase()


scope is stored lowercase, no need to lowercase again.

Fixed in #1056

ninaceban · 2023-09-25T20:39:14Z

src/components/Personalization/createViewCacheManager.js

+          }
+        },
+        items: [
+          {


Default Content Item schema is used in Target to indicate that user has entered a "control" experience, using it liberally will cause confusion.

ninaceban · 2023-09-25T20:59:58Z

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.


let's move this logic where the event is triggered instead, this functions are calling event API, to merge to an existing event

Fixed in #1061

ninaceban · 2023-09-25T21:03:14Z

src/components/Personalization/handlers/createProcessDomAction.js

+  }
+
+  if (!modules[type]) {
+    logger.warn("Invalid DOM action data: unknown type.", item.getData());


In such cases we should set renderAttempted: false, includeInNotifications:false

Fixed in #1060

ninaceban · 2023-09-25T21:06:44Z

src/components/Personalization/handlers/createProcessPropositions.js

+          const errorMessage = `Failed to execute action ${item.toString()}. ${message} ${stack}`;
+          logger.error(errorMessage);
+        }
+        return false;


logger level should be warrning

Fixed in #1058

ninaceban · 2023-09-25T21:09:16Z

src/components/Personalization/handlers/createProcessPropositions.js

+    let renderedItems = [];
+    let nonRenderedItems = [];
+    let itemRenderers = [];
+    let atLeastOneWithNotification = false;


so many flags, a decision diagram would be very helpful for CE engineers debugging an issue.

ninaceban · 2023-09-25T21:12:32Z

src/components/Personalization/index.js

+    [schema.HTML_CONTENT_ITEM]: createProcessHtmlContent({ modules, logger }),
+    [schema.REDIRECT_ITEM]: createProcessRedirect({
+      logger,
+      executeRedirect: url => window.location.replace(url),


extract this to a function

Fixed in #1061

ninaceban · 2023-09-25T21:19:22Z

src/components/Personalization/handlers/createProcessRedirect.js

+
+  const render = () => {
+    return collect({ decisionsMeta: [item.getMeta()] }).then(() => {
+      executeRedirect(content);


Render would be called only when renderDecisions:true, otherwise it should be just part of the array of propositions returned.
Collect should be called here with documentMayUnload argument.
You should try/catch errors here and log the error and showContainers.

If renderDecisions is false, processRedirect isn't even called, and the proposition is just returned as part of the returned propositions. See

alloy/src/components/Personalization/createFetchDataHandler.js

Line 63 in bf813fa

if (personalizationDetails.isRenderDecisions()) {

.

Changing it to use collect is fixed in #1061

The try/catch is done in

alloy/src/components/Personalization/handlers/createProcessPropositions.js

Line 23 in bf813fa

.catch(error => {

The showContainers is in

alloy/src/components/Personalization/createFetchDataHandler.js

Line 77 in bf813fa

showContainers();

ninaceban · 2023-09-25T21:22:50Z

src/components/Personalization/handlers/injectCreateProposition.js

+          return "page";
+        }
+        if (scopeType === "view") {
+          return "view";


as this gets part of the proposition design, extract this into an enum of proposition types.

This is fixed in #1056

ninaceban · 2023-09-25T21:27:07Z

src/components/Personalization/validateApplyPropositionsOptions.js


 export const EMPTY_PROPOSITIONS = { propositions: [] };

 export default ({ logger, options }) => {
  const applyPropositionsOptionsValidator = objectOf({
-    propositions: arrayOf(objectOf(anything())).nonEmpty(),
-    metadata: objectOf(anything())
+    propositions: arrayOf(objectOf(anything())),


this validator should be oneOf:

viewName

propositions && metadata

If you don't send in any propositions or a viewName, then this command does nothing. That's not an error. Additionally it is a valid use case to send in propositions without metadata. These could be page_wide_scope propositions that were not rendered automatically.

ninaceban

Generally, I would recommend breaking this into a few smaller PRs, it touches one of the most impactful and feature rich components, Personalization. Mixing the whole component refactoring and adding a big feature on top of that becomes pretty complicated for the PR reviewer to understand the changes and how it will impact the backward compatibility.
The src/components/Personalization/handlers/createProcessPropositions.js is pretty complex, would be good to break it into smaller descriptive pieces and comments. A design spec would also help CE rotation engineers that will potentially have to debug a related issue.

While testing Target use-cases, have found a few use-cases that needs to be fixed:

when renderDecisions: false the redirect offer should not be executed, the proposition should be returned in the result
the notification should be a collect instead of interact call
when renderDecisions:false the prehiding snippet should be removed(showContainers should be called)
when a click from a surface occurs the surface scope should not be sent in aviewName, viewName will be populated only when the proposition scopeType:view
when something fails, we log warnings, not errors

As improvements:

the functional tests for AJO should be improved to have the assertions for rendering, click notifications viewName I have mentioned above.
add test for renderDecisions:false and redirect offer being returned
add a functional test to validate that prehiding style is removed:
-- when no personalization payload is returned
-- when renderDecisions is false
-- when an exception while executing

ninaceban · 2023-09-26T07:55:59Z

src/core/createEvent.js

+  deduplicateArray
+} from "../utils";
+
+const getXdmPropositions = xdm => {


experience.decisioning is strictly Personalization related. Core should remain Personalization agnostic. Move this logic to Personalization component.

ninaceban · 2023-09-26T08:03:29Z

src/core/createEvent.js

+        !userXdm.web ||
+        !userXdm.web.webPageDetails ||
+        !userXdm.web.webPageDetails.viewName
+      ) {


in case viewName is undefined it will anyways be returned as undefined. No need for extra condition. Please revert this change.

Fixed in #1061

ninaceban · 2023-09-26T08:04:26Z

src/core/createEvent.js

@@ -63,8 +81,23 @@ export default () => {
        return;
      }

+      const newPropositions = deduplicateArray(


this logic too should be moved to Personalization.

ninaceban · 2023-09-26T08:14:44Z

src/components/Personalization/createOnClickHandler.js

@@ -35,10 +35,10 @@ export default ({
        const xdm = { eventType: INTERACT };
        const scope = decisionsMeta[0].scope;

-        if (scope !== PAGE_WIDE_SCOPE) {
+        if (isNonEmptyString(scope) && scope !== PAGE_WIDE_SCOPE) {


the viewName would be populated only when the proposition was for an SPA view, add to this if clause another one that checks if scopeType: view.

Fixed in #1056

jonsnyder · 2023-10-18T17:32:03Z

I've addressed most of the comments here in these PRs:
#1056
#1057
#1058
#1060
#1061

Outstanding are still these comments:

Creating a decision diagram and making processPropositions easier to understand. Documenting how to create a new renderer.
Moving proposition deduplication out of core and into Personalization component.
Figuring out a better way to deal with empty view change notifications without having to reuse default content items.

jonsnyder and others added 30 commits February 6, 2023 12:52

WIP add handlers for each kind of activity

22d1d81

Finalize personalization handlers

efe6c16

Merge remote-tracking branch 'origin/main' into personalizationRefactor

6c33b2d

Add component tests with support for fetch, viewChange, and apply

3aeff9d

Refactor handlePropositions to render

63cdfb0

Remove unused files

4c337ec

fix some tests

f648376

Update all unit tests

1cc8e88

Fix functional tests, and fix issues found when running functional tests

520a0c8

Remove fetch related features, only including refactor and combining …

8a97a0b

…notifications

Add unit tests for new handler files

76038af

Revert "Remove fetch related features, only including refactor and co…

443745b

…mbining notifications" This reverts commit 8a97a0b.

Save display notifications and send them at the bottom of page hit

aa0474a

Fix default value for sendDisplayNotifications

0ca03c8

Add viewName option to applyPropositions

9a9835d

Add showContainers call after rendering in fetch data handler

daea723

Merge branch 'combineNotifications' into topAndBottom2

09e1a19

TNT-48367 Make SPA view names lowercase (#1027)

3fbc4d9

* TNT-48367 Make SPA view names lowercase

Change proposition to be immutable. Render based on schema. Group by …

8c56124

…scope type.

Remove unused code, fix unit tests for personalization handlers

e9b5acf

Add license header

9b664c0

Fix bugs found running functional tests

50dba9c

Merge remote-tracking branch 'origin/main' into topAndBottom2

a8dd78f

Update scope lowercase logic to only apply to the view cache.

ca33032

Merge pull request #1036 from adobe/topAndBottom2

0bf960d

Support for top and bottom of page events. Combine personalization display notifications together.

[skip ci] 2.19.0-beta.0

b3ab160

[skip ci] update self devDependency to 2.19.0-beta.0

1960692

Call show containers immediately after we start rendering. Do not wai…

5b29072

…t for rendering to complete.

Add unit test for showing containers at the right time

2e08077

Fix empty view change issues

bda7033

Merge remote-tracking branch 'origin/fixShowContainersTiming' into to…

300cf11

…pAndBottom3

ninaceban reviewed Sep 25, 2023

View reviewed changes

ninaceban requested changes Sep 26, 2023

View reviewed changes

carterworks marked this pull request as draft September 26, 2023 17:16

This was referenced Oct 18, 2023

Fix an issue where web.webPageDetails.viewName was included in the click notification. #1056

Merged

Fix empty view change issues #1041

Merged

jonsnyder closed this Nov 9, 2023

jonsnyder deleted the topAndBottom3 branch November 9, 2023 21:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DO NOT MERGE!!! All changes from last release for Nina #1042

DO NOT MERGE!!! All changes from last release for Nina #1042

jonsnyder commented Sep 23, 2023

ninaceban Sep 25, 2023 •

edited

Loading

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023 •

edited

Loading

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023 •

edited

Loading

ninaceban Sep 25, 2023 •

edited

Loading

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023

ninaceban Sep 25, 2023

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023 •

edited

Loading

jonsnyder Oct 18, 2023

ninaceban Sep 25, 2023

jonsnyder Oct 18, 2023

ninaceban left a comment

ninaceban Sep 26, 2023

ninaceban Sep 26, 2023

jonsnyder Oct 18, 2023

ninaceban Sep 26, 2023

ninaceban Sep 26, 2023

jonsnyder Oct 18, 2023

jonsnyder commented Oct 18, 2023 •

edited

Loading

DO NOT MERGE!!! All changes from last release for Nina #1042

DO NOT MERGE!!! All changes from last release for Nina #1042

Conversation

jonsnyder commented Sep 23, 2023

Description

Related Issue

Motivation and Context

Screenshots (if appropriate):

Types of changes

Checklist:

ninaceban Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ninaceban Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ninaceban Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

ninaceban Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ninaceban Sep 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ninaceban left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonsnyder commented Oct 18, 2023 • edited Loading

ninaceban Sep 25, 2023 •

edited

Loading

ninaceban Sep 25, 2023 •

edited

Loading

ninaceban Sep 25, 2023 •

edited

Loading

ninaceban Sep 25, 2023 •

edited

Loading

ninaceban Sep 25, 2023 •

edited

Loading

jonsnyder commented Oct 18, 2023 •

edited

Loading