Skip to content

Commit

Permalink
Merge pull request #1060 from adobe/explicitProcessors
Browse files Browse the repository at this point in the history
Be explicit about error conditions in DomAction and HtmlContent processors
  • Loading branch information
jonsnyder authored Oct 19, 2023
2 parents 086e3e2 + 7c00116 commit dd2a632
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,21 @@ export default ({ modules, logger, storeClickMetrics }) => item => {

if (!type) {
logger.warn("Invalid DOM action data: missing type.", item.getData());
return {};
return { setRenderAttempted: false, includeInNotification: false };
}

if (type === "click") {
if (!selector) {
logger.warn("Invalid DOM action data: missing selector.", item.getData());
return {};
return { setRenderAttempted: false, includeInNotification: false };
}
storeClickMetrics({ selector, meta: item.getMeta() });
return { setRenderAttempted: true, includeInNotification: false };
}

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

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ export default ({ modules, logger }) => item => {
const { type, selector } = item.getData() || {};

if (!selector || !type) {
return {};
return { setRenderAttempted: false, includeInNotification: false };
}

if (!modules[type]) {
logger.warn("Invalid HTML content data", item.getData());
return {};
return { setRenderAttempted: false, includeInNotification: false };
}

return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ describe("createProcessDomAction", () => {

it("returns an empty object if the item has no data, and logs missing type", () => {
data = undefined;
expect(processDomAction(item)).toEqual({});
expect(processDomAction(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).toHaveBeenCalledWith(
"Invalid DOM action data: missing type.",
undefined
Expand All @@ -43,7 +46,10 @@ describe("createProcessDomAction", () => {

it("returns an empty object if the item has no type, and logs missing type", () => {
data = {};
expect(processDomAction(item)).toEqual({});
expect(processDomAction(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).toHaveBeenCalledWith(
"Invalid DOM action data: missing type.",
{}
Expand All @@ -52,7 +58,10 @@ describe("createProcessDomAction", () => {

it("returns an empty object if the item has an unknown type, and logs unknown type", () => {
data = { type: "typeC" };
expect(processDomAction(item)).toEqual({});
expect(processDomAction(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).toHaveBeenCalledWith(
"Invalid DOM action data: unknown type.",
{
Expand All @@ -63,7 +72,10 @@ describe("createProcessDomAction", () => {

it("returns an empty object if the item has no selector for a click type, and logs missing selector", () => {
data = { type: "click" };
expect(processDomAction(item)).toEqual({});
expect(processDomAction(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).toHaveBeenCalledWith(
"Invalid DOM action data: missing selector.",
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,25 +24,37 @@ describe("createProcessHtmlContent", () => {

it("returns an empty object if the item has no data", () => {
data = undefined;
expect(processHtmlContent(item)).toEqual({});
expect(processHtmlContent(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).not.toHaveBeenCalled();
});

it("returns an empty object if the item has no type", () => {
data = { selector: ".myselector" };
expect(processHtmlContent(item)).toEqual({});
expect(processHtmlContent(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).not.toHaveBeenCalled();
});

it("returns an empty object if the item has no selector", () => {
data = { type: "mytype" };
expect(processHtmlContent(item)).toEqual({});
expect(processHtmlContent(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).not.toHaveBeenCalled();
});

it("returns an empty object if the item has an unknown type, and logs unknown type", () => {
data = { type: "typeC", selector: ".myselector", content: "mycontent" };
expect(processHtmlContent(item)).toEqual({});
expect(processHtmlContent(item)).toEqual({
setRenderAttempted: false,
includeInNotification: false
});
expect(logger.warn).toHaveBeenCalledWith("Invalid HTML content data", {
type: "typeC",
selector: ".myselector",
Expand Down

0 comments on commit dd2a632

Please sign in to comment.