TNT-43012 Return default content items #802

XDex · 2022-02-04T16:27:00Z

Note: DO NOT MERGE until target-upstream v0.0.56 with support for characteristics -> scopeType is released to production.

Description

Support default content proposition items delivered for control experiences with no offer content.

Related Issue

https://jira.corp.adobe.com/browse/TNT-43012

Motivation and Context

Support default content propositions in the response, so that customers know when a control experience is selected.

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.

jfkhoury · 2022-02-07T18:34:22Z

src/components/Personalization/createPersonalizationDetails.js

@@ -41,7 +42,12 @@ export default ({ renderDecisions, decisionScopes, event, viewCache }) => {
        scopes.push(PAGE_WIDE_SCOPE);
      }

-      const schemas = [HTML_CONTENT_ITEM, JSON_CONTENT_ITEM, REDIRECT_ITEM];
+      const schemas = [
+        DEFAULT_CONTENT_ITEM,


I like the explicit nature of this PR.

I wish there was a way for Target Upstream to figure out which schemas to act on without a change to the Web SDK which enable us to add more schemas in the future without a change to the SDKs..

@jfkhoury We can make an optimization in this regard in case when content for all available schemas should be retrieved by not specifying schemas array at all in the request - in this case content for all available schemas will be returned implicitly from Target Upstream.
Thus, only when the page wide scope is not included in the request would we need to set the schemas explicitly to all values other than DOM_ACTION. Let me know if this would be desired and I'll open a ticket for it..

@XDex the optimization you suggested would still require a code change when a new schema was added. Target upstream should know that only when the "view" scope is requested, it should return DOM_ACTION schemas. If this were the case the Web SDK would only need to be updated when new schemas needed to be handled by the SDK. Currently only redirect, and dom action schemas are handled. The rest are passed through.

If a new pass-through schema were added, this code would need to be updated even though the SDK isn't doing anything with it.

Right, let's just leave it as-is then for now, the schemas being always explicitly specified in the request.

5amfung · 2022-02-08T22:54:51Z

@XDex Your PR title is referring to the wrong TNT number.

ninaceban

@XDex great job!
I saw you are checking that renderAttempted=false although there was nothing to render, and renderAttempted is confusing in this situation, it is an experience that was executed on the page because of the nature of __view__ scope. The customers are chaining the sendEvent promise to filter the executed experiences, collect the response tokens and send them to third parties.
I would say that in case renderDecisions=true we should add the renderAttempted=true flag and send a display notification.

XDex · 2022-02-10T16:53:54Z

@ninaceban As we discussed today, I'll wait for your A4T PR to get merged before adding the mods to mark default content items as renderable actions

test/functional/specs/Personalization/C28761.js

jonsnyder · 2022-02-14T21:52:54Z

src/components/Personalization/createPersonalizationDetails.js

@@ -41,7 +42,12 @@ export default ({ renderDecisions, decisionScopes, event, viewCache }) => {
        scopes.push(PAGE_WIDE_SCOPE);
      }

-      const schemas = [HTML_CONTENT_ITEM, JSON_CONTENT_ITEM, REDIRECT_ITEM];
+      const schemas = [
+        DEFAULT_CONTENT_ITEM,


@XDex the optimization you suggested would still require a code change when a new schema was added. Target upstream should know that only when the "view" scope is requested, it should return DOM_ACTION schemas. If this were the case the Web SDK would only need to be updated when new schemas needed to be handled by the SDK. Currently only redirect, and dom action schemas are handled. The rest are passed through.

If a new pass-through schema were added, this code would need to be updated even though the SDK isn't doing anything with it.

XDex · 2022-02-23T14:12:49Z

@ninaceban @jonsnyder I've implemented support for default-content items when renderDecisions=true, with proper grouping to view/mbox scopes, please take another look when you get a chance.
Note - This PR requires the support of characteristics -> scopeType to be released in Target upstream before being merged, see https://jira.corp.adobe.com/browse/TNT-43696
Thank you!

jonsnyder

These changes look good. I would like Nina to review before we merge though.

ninaceban

@XDex Thank you Andrei. I see you have added a functional test, could you also add unit tests that assert a proper grouping of the decisions based on the use-cases:

default content item is returned for a view
default content item is returned for a decision scope
default content item is returned for __view__ scope

ninaceban · 2022-03-07T13:24:53Z

src/components/Personalization/groupDecisions.js

+  return {
+    pageWideScopeDecisions,
+    nonPageWideScopeDecisions,
+    viewScopeDecisions


why we need to return the viewScopeDecisions here?

ninaceban · 2022-03-07T14:29:18Z

src/components/Personalization/groupDecisions.js

-      decisionsGroupedByDomActionSchema.unmatchedDecisions
+    viewDecisions: viewScopeDecisions,
+    nonAutoRenderableDecisions: decisionsGroupedByRenderableSchemas.unmatchedDecisions.concat(
+      nonPageWideScopeDecisions


I would suggest to keep the old order of extracting the decisions:

Extract the redirect decisions by Redirect Schema(no changes needed).

Extract from remaining decisions from step 1 the renderable decisions (by DOM_ACTION, DEFAULT_CONTENT_ITEM schema and scopeType: view).

Extract page-wide scope decisions and viewDecisions from the renderableDecisions from step 2, by __view__ scope.

Remaining decisions from step 2 are the form-based composed decisions.

Otherwise, it becomes extremely hard to understand why we group then concat at the end the decisions, then also maintain the code.

XDex requested review from ninaceban and jonsnyder February 4, 2022 16:27

jfkhoury requested review from liljenback, shammowla and carterworks February 7, 2022 18:32

jfkhoury reviewed Feb 7, 2022

View reviewed changes

jfkhoury approved these changes Feb 7, 2022

View reviewed changes

XDex changed the title ~~TNT-43421 Return default content items~~ TNT-43012 Return default content items Feb 9, 2022

ninaceban requested changes Feb 9, 2022

View reviewed changes

jonsnyder reviewed Feb 14, 2022

View reviewed changes

XDex force-pushed the TNT-43012 branch from 77cf1eb to 4c2d5c3 Compare February 23, 2022 13:52

jonsnyder approved these changes Feb 25, 2022

View reviewed changes

ninaceban requested changes Mar 7, 2022

View reviewed changes

TNT-43421 Return default content items

8b9c348

XDex force-pushed the TNT-43012 branch from 4c2d5c3 to 8b9c348 Compare March 9, 2022 16:33

jonsnyder merged commit 477503e into adobe:main Mar 9, 2022

github-actions bot mentioned this pull request Mar 9, 2022

2.9.0 #826

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TNT-43012 Return default content items #802

TNT-43012 Return default content items #802

XDex commented Feb 4, 2022 •

edited

Loading

jfkhoury Feb 7, 2022

XDex Feb 8, 2022

jonsnyder Feb 14, 2022

XDex Feb 23, 2022

5amfung commented Feb 8, 2022

ninaceban left a comment •

edited

Loading

XDex commented Feb 10, 2022

jonsnyder Feb 14, 2022

XDex commented Feb 23, 2022

jonsnyder left a comment

ninaceban left a comment

ninaceban Mar 7, 2022

ninaceban Mar 7, 2022

TNT-43012 Return default content items #802

TNT-43012 Return default content items #802

Conversation

XDex commented Feb 4, 2022 • edited Loading

Description

Related Issue

Motivation and Context

Screenshots (if appropriate):

Types of changes

Checklist:

jfkhoury Feb 7, 2022

Choose a reason for hiding this comment

XDex Feb 8, 2022

Choose a reason for hiding this comment

jonsnyder Feb 14, 2022

Choose a reason for hiding this comment

XDex Feb 23, 2022

Choose a reason for hiding this comment

5amfung commented Feb 8, 2022

ninaceban left a comment • edited Loading

Choose a reason for hiding this comment

XDex commented Feb 10, 2022

jonsnyder Feb 14, 2022

Choose a reason for hiding this comment

XDex commented Feb 23, 2022

jonsnyder left a comment

Choose a reason for hiding this comment

ninaceban left a comment

Choose a reason for hiding this comment

ninaceban Mar 7, 2022

Choose a reason for hiding this comment

ninaceban Mar 7, 2022

Choose a reason for hiding this comment

XDex commented Feb 4, 2022 •

edited

Loading

ninaceban left a comment •

edited

Loading