Skip to content

Commit

Permalink
PDCL-12483 Add conflict resolution support for In-App messages. (#1194)
Browse files Browse the repository at this point in the history
* WIP

* another approach

* this too

* cleaner

* fix tests

* fix bug

* Small optimization.

* Add propositionAction.

* Fix test count.

* Add option to run a specific test in watch mode.

* Add test for conflict resolution testing.

* Rename file.

* Add comment.

* Use isNonEmptyArray utility.

---------

Co-authored-by: Jason Waters <[email protected]>
  • Loading branch information
dompuiu and jasonwaters authored Oct 28, 2024
1 parent 5031e7d commit 3f64390
Show file tree
Hide file tree
Showing 16 changed files with 496 additions and 21 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"format": "prettier --write \"*.{html,js,cjs,mjs,jsx}\" \"{sandbox,src,test,scripts}/**/*.{html,js,cjs,mjs,jsx}\"",
"test": "npm run test:unit && npm run test:scripts && npm run test:functional",
"test:unit": "karma start karma.conf.cjs --single-run",
"test:unit:debug": "karma start --browsers=Chrome --single-run=false --debug",
"test:unit:debug": "karma start karma.conf.cjs --browsers=Chrome --single-run=false --debug",
"test:unit:watch": "karma start",
"test:unit:saucelabs:local": "karma start karma.saucelabs.conf.cjs --single-run",
"test:unit:coverage": "karma start --single-run --reporters spec,coverage",
Expand Down
2 changes: 1 addition & 1 deletion scripts/specs/updatePackageVersion.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe("updatePackageVersion", () => {
expect(logger.info).toHaveBeenCalledOnceWith(
"Updating package.json with version 1.2.3.",
);
expect(exec).toHaveBeenCalledTimes(4);
expect(exec).toHaveBeenCalledTimes(5);
});

it("doesn't update the package version", async () => {
Expand Down
17 changes: 15 additions & 2 deletions scripts/watchFunctionalTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ program.addOption(
.default("chrome"),
);

program.addOption(
new Option(
"--specsPath <specsPath...>",
"configures the test runner to run tests from the specified files",
),
);

program.parse();

const argv = program.opts();
Expand Down Expand Up @@ -68,8 +75,14 @@ const effectByEventCode = {
} else {
firstBuildComplete = true;
const testcafe = await createTestCafe();
const liveRunner = testcafe.createLiveModeRunner();
await liveRunner.browsers(argv.browsers).run();
let liveRunner = testcafe.createLiveModeRunner();
liveRunner = liveRunner.browsers(argv.browsers);

if (argv.specsPath) {
liveRunner = await liveRunner.src(argv.specsPath);
}

await liveRunner.run();
await testcafe.close();
}
},
Expand Down
17 changes: 14 additions & 3 deletions src/components/Personalization/createNotificationHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/
import { defer } from "../../utils/index.js";
import { SUPPRESS } from "../../constants/eventType.js";
import isNonEmptyArray from "../../utils/isNonEmptyArray.js";

export default (collect, renderedPropositions) => {
return (isRenderDecisions, isSendDisplayEvent, viewName) => {
Expand All @@ -24,10 +26,19 @@ export default (collect, renderedPropositions) => {
return renderedPropositionsDeferred.resolve;
}

return (decisionsMeta) => {
if (decisionsMeta.length > 0) {
return (decisionsMetaDisplay = [], decisionsMetaSuppressed = []) => {
if (isNonEmptyArray(decisionsMetaDisplay)) {
collect({
decisionsMeta,
decisionsMeta: decisionsMetaDisplay,
viewName,
});
}

if (isNonEmptyArray(decisionsMetaSuppressed)) {
collect({
decisionsMeta: decisionsMetaSuppressed,
eventType: SUPPRESS,
propositionAction: { reason: "Conflict" },
viewName,
});
}
Expand Down
45 changes: 43 additions & 2 deletions src/components/Personalization/createOnDecisionHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,26 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/

import { MESSAGE_IN_APP } from "../../constants/schema.js";

// When multiple In-App messages propositions are returned, we need to show only one
// of them (the one with lowest rank). This function keep track of the number of
// times it was called. It returns false for the first proposition that contains
// In-App messages items, true afterwards.
const createShouldSuppressDisplay = () => {
let count = 0;
return (proposition) => {
const { items = [] } = proposition;

if (!items.some((item) => item.schema === MESSAGE_IN_APP)) {
return false;
}
count += 1;

return count > 1;
};
};

export default ({
processPropositions,
createProposition,
Expand All @@ -23,8 +43,10 @@ export default ({
const { sendDisplayEvent = true } = personalization;
const viewName = event ? event.getViewName() : undefined;

const shouldSuppressDisplay = createShouldSuppressDisplay();

const propositionsToExecute = propositions.map((proposition) =>
createProposition(proposition, true),
createProposition(proposition, true, shouldSuppressDisplay(proposition)),
);

const { render, returnedPropositions } = processPropositions(
Expand All @@ -36,7 +58,26 @@ export default ({
sendDisplayEvent,
viewName,
);
render().then(handleNotifications);

const propositionsById = propositionsToExecute.reduce(
(tot, proposition) => {
tot[proposition.getId()] = proposition;
return tot;
},
{},
);

render().then((decisionsMeta) => {
const decisionsMetaDisplay = decisionsMeta.filter(
(meta) => !propositionsById[meta.id].shouldSuppressDisplay(),
);

const decisionsMetaSuppressed = decisionsMeta.filter((meta) =>
propositionsById[meta.id].shouldSuppressDisplay(),
);

handleNotifications(decisionsMetaDisplay, decisionsMetaSuppressed);
});

return Promise.resolve({
propositions: returnedPropositions,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ const isValidInAppMessage = (data, logger) => {
export default ({ modules, logger }) => {
return (item) => {
const data = item.getData();
const meta = { ...item.getProposition().getNotification() };
const proposition = item.getProposition();
const meta = { ...proposition.getNotification() };
const shouldSuppressDisplay = proposition.shouldSuppressDisplay();

if (!data) {
logger.warn("Invalid in-app message data: undefined.", data);
Expand All @@ -74,10 +76,12 @@ export default ({ modules, logger }) => {

return {
render: () => {
return modules[type]({
...data,
meta,
});
return shouldSuppressDisplay
? null
: modules[type]({
...data,
meta,
});
},
setRenderAttempted: true,
includeInNotification: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export default ({ schemaProcessors, logger }) => {
item = items[i];
({ render, setRenderAttempted, includeInNotification, onlyRenderThis } =
processItem(item));

if (onlyRenderThis) {
returnedPropositions = [];
returnedDecisions = [];
Expand All @@ -99,6 +100,7 @@ export default ({ schemaProcessors, logger }) => {
atLeastOneWithNotification = includeInNotification;
break;
}

if (render) {
itemRenderers.push(wrapRenderWithLogging(render, item));
}
Expand Down Expand Up @@ -195,6 +197,7 @@ export default ({ schemaProcessors, logger }) => {
false,
);
});

const render = () => {
return Promise.all(renderers.map((renderer) => renderer())).then(
(metas) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,11 @@ export default ({ preprocess, isPageWideSurface }) => {
};
};

return (payload, visibleInReturnedItems = true) => {
return (
payload,
visibleInReturnedItems = true,
shouldSuppressDisplay = false,
) => {
const { id, scope, scopeDetails, items = [] } = payload;
const { characteristics: { scopeType } = {} } = scopeDetails || {};

Expand Down Expand Up @@ -85,6 +89,9 @@ export default ({ preprocess, isPageWideSurface }) => {
toJSON() {
return payload;
},
shouldSuppressDisplay() {
return shouldSuppressDisplay;
},
addToReturnValues(
propositions,
decisions,
Expand Down
9 changes: 7 additions & 2 deletions src/components/RulesEngine/createDecisionProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,15 @@ export default ({ eventRegistry }) => {
}
};

const evaluate = (context = {}) =>
Object.values(payloadsBasedOnActivityId)
const evaluate = (context = {}) => {
const sortedPayloadsBasedOnActivityId = Object.values(
payloadsBasedOnActivityId,
).sort(({ rank: rankA }, { rank: rankB }) => rankA - rankB);

return sortedPayloadsBasedOnActivityId
.map((payload) => payload.evaluate(context))
.filter((payload) => payload.items.length > 0);
};

const addPayloads = (personalizationPayloads) => {
personalizationPayloads.forEach(addPayload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export default (payload, eventRegistry, decisionHistory) => {
}

return {
rank: payload?.scopeDetails?.rank || Infinity,
evaluate,
isEvaluable: items.length > 0,
};
Expand Down
1 change: 1 addition & 0 deletions src/constants/eventType.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ export const DISPLAY = "decisioning.propositionDisplay";
export const INTERACT = "decisioning.propositionInteract";
export const TRIGGER = "decisioning.propositionTrigger";
export const DISMISS = "decisioning.propositionDismiss";
export const SUPPRESS = "decisioning.propositionSuppressDisplay";
export const EVENT_TYPE_TRUE = 1;
6 changes: 5 additions & 1 deletion src/constants/propositionEventType.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,26 +10,30 @@ OF ANY KIND, either express or implied. See the License for the specific languag
governing permissions and limitations under the License.
*/

import { DISPLAY, INTERACT, TRIGGER, DISMISS } from "./eventType.js";
import { DISPLAY, INTERACT, TRIGGER, DISMISS, SUPPRESS } from "./eventType.js";

export const PropositionEventType = {
DISPLAY: "display",
INTERACT: "interact",
TRIGGER: "trigger",
DISMISS: "dismiss",
SUPPRESS: "suppressDisplay",
};

const eventTypeToPropositionEventTypeMapping = {
[DISPLAY]: PropositionEventType.DISPLAY,
[INTERACT]: PropositionEventType.INTERACT,
[TRIGGER]: PropositionEventType.TRIGGER,
[DISMISS]: PropositionEventType.DISMISS,
[SUPPRESS]: PropositionEventType.SUPPRESS,
};

const propositionEventTypeToEventTypeMapping = {
[PropositionEventType.DISPLAY]: DISPLAY,
[PropositionEventType.INTERACT]: INTERACT,
[PropositionEventType.TRIGGER]: TRIGGER,
[PropositionEventType.DISMISS]: DISMISS,
[PropositionEventType.SUPPRESS]: SUPPRESS,
};

export const getPropositionEventType = (eventType) =>
Expand Down
Loading

0 comments on commit 3f64390

Please sign in to comment.