Skip to content

Commit 40e8677

Browse files
dompuiudompuiu
authored
PDCL-14700 Do not show containers when there is one proposition that will do a redirect. (#1424)
* PDCL-14700 Do not show containers when there is one proposition that will do a redirect. * Correct typo. * Add changeset.
1 parent 2b83909 commit 40e8677

File tree

3 files changed

+254
-1
lines changed

3 files changed

+254
-1
lines changed

‎.changeset/huge-rabbits-crash.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@adobe/alloy-core": patch
3+
---
4+
5+
Do not remove prehiding snippet when one redirect proposition is returned from the server.

‎packages/core/src/components/Personalization/createFetchDataHandler.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ governing permissions and limitations under the License.
1111
*/
1212
import { groupBy, isNonEmptyArray } from "../../utils/index.js";
1313
import PAGE_WIDE_SCOPE from "../../constants/pageWideScope.js";
14+
import { REDIRECT_ITEM } from "../../constants/schema.js";
1415

1516
const DECISIONS_HANDLE = "personalization:decisions";
1617

@@ -84,6 +85,7 @@ export default ({
8485
[...pagePropositions, ...currentViewPropositions],
8586
nonRenderedPropositions,
8687
));
88+
8789
if (isNonEmptyArray(pagePropositions)) {
8890
logger.logOnContentRendering({
8991
status: "rendering-started",
@@ -117,7 +119,16 @@ export default ({
117119
// Render could take a long time especially if one of the renders
118120
// is waiting for html to appear on the page. We show the containers
119121
// immediately, and whatever renders quickly will not have flicker.
120-
showContainers();
122+
// However, skip showing containers if there's only one page proposition with a single REDIRECT_ITEM
123+
const shouldSkipShowContainers =
124+
pagePropositions.length === 1 &&
125+
pagePropositions[0]
126+
.getItems()
127+
.every((p) => p.getSchema() === REDIRECT_ITEM);
128+
129+
if (!shouldSkipShowContainers) {
130+
showContainers();
131+
}
121132
} else {
122133
({ returnedPropositions, returnedDecisions } = processPropositions(
123134
[],

‎packages/core/test/unit/specs/components/Personalization/createFetchDataHandler.spec.js‎

Lines changed: 237 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import injectCreateProposition from "../../../../../src/components/Personalizati
1616
import flushPromiseChains from "../../../helpers/flushPromiseChains.js";
1717
import defer from "../../../../../src/utils/defer.js";
1818
import createNotificationHandler from "../../../../../src/components/Personalization/createNotificationHandler.js";
19+
import { REDIRECT_ITEM } from "../../../../../src/constants/schema.js";
1920

2021
describe("Personalization::createFetchDataHandler", () => {
2122
let prehidingStyle;
@@ -246,4 +247,240 @@ describe("Personalization::createFetchDataHandler", () => {
246247
viewName: undefined,
247248
});
248249
});
250+
251+
it("should not show containers when there is one page proposition with only REDIRECT_ITEM", async () => {
252+
personalizationDetails.isRenderDecisions.mockReturnValue(true);
253+
const renderDeferred = defer();
254+
processPropositions = () => {
255+
return {
256+
render: () => renderDeferred.promise,
257+
returnedPropositions: [
258+
{
259+
id: "redirect-handle",
260+
scope: "__view__",
261+
items: [
262+
{
263+
schema: REDIRECT_ITEM,
264+
data: { content: "https://example.com" },
265+
},
266+
],
267+
renderAttempted: true,
268+
},
269+
],
270+
returnedDecisions: [],
271+
};
272+
};
273+
run();
274+
response.getPayloadsByType.mockReturnValue([
275+
{
276+
id: "redirect-handle",
277+
scope: "__view__",
278+
items: [
279+
{
280+
schema: REDIRECT_ITEM,
281+
data: { content: "https://example.com" },
282+
},
283+
],
284+
},
285+
]);
286+
cacheUpdate.update.mockReturnValue([]);
287+
expect(showContainers).not.toHaveBeenCalled();
288+
returnResponse();
289+
// showContainers should NOT be called because we have a single redirect item
290+
expect(showContainers).not.toHaveBeenCalled();
291+
renderDeferred.resolve([
292+
{
293+
id: "redirect-handle",
294+
},
295+
]);
296+
await flushPromiseChains();
297+
// Still should not be called after render completes
298+
expect(showContainers).not.toHaveBeenCalled();
299+
});
300+
301+
it("should not show containers when there is one page proposition with multiple REDIRECT_ITEMs", async () => {
302+
personalizationDetails.isRenderDecisions.mockReturnValue(true);
303+
const renderDeferred = defer();
304+
processPropositions = () => {
305+
return {
306+
render: () => renderDeferred.promise,
307+
returnedPropositions: [
308+
{
309+
id: "redirect-handle",
310+
scope: "__view__",
311+
items: [
312+
{
313+
schema: REDIRECT_ITEM,
314+
data: { content: "https://example.com" },
315+
},
316+
{
317+
schema: REDIRECT_ITEM,
318+
data: { content: "https://example2.com" },
319+
},
320+
],
321+
renderAttempted: true,
322+
},
323+
],
324+
returnedDecisions: [],
325+
};
326+
};
327+
run();
328+
response.getPayloadsByType.mockReturnValue([
329+
{
330+
id: "redirect-handle",
331+
scope: "__view__",
332+
items: [
333+
{
334+
schema: REDIRECT_ITEM,
335+
data: { content: "https://example.com" },
336+
},
337+
{
338+
schema: REDIRECT_ITEM,
339+
data: { content: "https://example2.com" },
340+
},
341+
],
342+
},
343+
]);
344+
cacheUpdate.update.mockReturnValue([]);
345+
expect(showContainers).not.toHaveBeenCalled();
346+
returnResponse();
347+
// showContainers should NOT be called because all items are redirects
348+
expect(showContainers).not.toHaveBeenCalled();
349+
renderDeferred.resolve([
350+
{
351+
id: "redirect-handle",
352+
},
353+
]);
354+
await flushPromiseChains();
355+
// Still should not be called after render completes
356+
expect(showContainers).not.toHaveBeenCalled();
357+
});
358+
359+
it("should show containers when there is one page proposition with mixed item types", async () => {
360+
personalizationDetails.isRenderDecisions.mockReturnValue(true);
361+
const renderDeferred = defer();
362+
processPropositions = () => {
363+
return {
364+
render: () => renderDeferred.promise,
365+
returnedPropositions: [
366+
{
367+
id: "mixed-handle",
368+
scope: "__view__",
369+
items: [
370+
{
371+
schema: REDIRECT_ITEM,
372+
data: { content: "https://example.com" },
373+
},
374+
{
375+
schema: "https://ns.adobe.com/personalization/dom-action",
376+
data: { type: "setHtml" },
377+
},
378+
],
379+
renderAttempted: true,
380+
},
381+
],
382+
returnedDecisions: [],
383+
};
384+
};
385+
run();
386+
response.getPayloadsByType.mockReturnValue([
387+
{
388+
id: "mixed-handle",
389+
scope: "__view__",
390+
items: [
391+
{
392+
schema: REDIRECT_ITEM,
393+
data: { content: "https://example.com" },
394+
},
395+
{
396+
schema: "https://ns.adobe.com/personalization/dom-action",
397+
data: { type: "setHtml" },
398+
},
399+
],
400+
},
401+
]);
402+
cacheUpdate.update.mockReturnValue([]);
403+
expect(showContainers).not.toHaveBeenCalled();
404+
returnResponse();
405+
// showContainers SHOULD be called because we have mixed item types
406+
expect(showContainers).toHaveBeenCalled();
407+
renderDeferred.resolve([
408+
{
409+
id: "mixed-handle",
410+
},
411+
]);
412+
await flushPromiseChains();
413+
});
414+
415+
it("should show containers when there are multiple page propositions", async () => {
416+
personalizationDetails.isRenderDecisions.mockReturnValue(true);
417+
const renderDeferred = defer();
418+
processPropositions = () => {
419+
return {
420+
render: () => renderDeferred.promise,
421+
returnedPropositions: [
422+
{
423+
id: "redirect-handle",
424+
scope: "__view__",
425+
items: [
426+
{
427+
schema: REDIRECT_ITEM,
428+
data: { content: "https://example.com" },
429+
},
430+
],
431+
renderAttempted: true,
432+
},
433+
{
434+
id: "other-handle",
435+
scope: "__view__",
436+
items: [
437+
{
438+
schema: "https://ns.adobe.com/personalization/dom-action",
439+
data: { type: "setHtml" },
440+
},
441+
],
442+
renderAttempted: true,
443+
},
444+
],
445+
returnedDecisions: [],
446+
};
447+
};
448+
run();
449+
response.getPayloadsByType.mockReturnValue([
450+
{
451+
id: "redirect-handle",
452+
scope: "__view__",
453+
items: [
454+
{
455+
schema: REDIRECT_ITEM,
456+
data: { content: "https://example.com" },
457+
},
458+
],
459+
},
460+
{
461+
id: "other-handle",
462+
scope: "__view__",
463+
items: [
464+
{
465+
schema: "https://ns.adobe.com/personalization/dom-action",
466+
data: { type: "setHtml" },
467+
},
468+
],
469+
},
470+
]);
471+
cacheUpdate.update.mockReturnValue([]);
472+
expect(showContainers).not.toHaveBeenCalled();
473+
returnResponse();
474+
// showContainers SHOULD be called because we have multiple propositions
475+
expect(showContainers).toHaveBeenCalled();
476+
renderDeferred.resolve([
477+
{
478+
id: "redirect-handle",
479+
},
480+
{
481+
id: "other-handle",
482+
},
483+
]);
484+
await flushPromiseChains();
485+
});
249486
});

0 commit comments

Comments
 (0)