From 060e7075af4218084745688dec0c08ab21ee9dd4 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 25 Feb 2026 14:20:40 -0700 Subject: [PATCH 01/23] Use dependency injection in the Advertising component --- .../components/Advertising/createComponent.js | 42 +- .../createOnBeforeSendEventHandler.js | 136 +++++ .../handlers/onBeforeSendEventHandler.js | 131 ----- .../core/src/components/Advertising/index.js | 33 +- .../Advertising/createComponent.spec.js | 150 +---- .../createOnBeforeSendEventHandler.spec.js | 228 ++++++++ .../handlers/onBeforeSendEventHandler.spec.js | 517 ------------------ .../components/Advertising/index.spec.js | 76 +++ 8 files changed, 485 insertions(+), 828 deletions(-) create mode 100644 packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js delete mode 100644 packages/core/src/components/Advertising/handlers/onBeforeSendEventHandler.js create mode 100644 packages/core/test/unit/specs/components/Advertising/handlers/createOnBeforeSendEventHandler.spec.js delete mode 100644 packages/core/test/unit/specs/components/Advertising/handlers/onBeforeSendEventHandler.spec.js create mode 100644 packages/core/test/unit/specs/components/Advertising/index.spec.js diff --git a/packages/core/src/components/Advertising/createComponent.js b/packages/core/src/components/Advertising/createComponent.js index dea270a30..68c27c9e1 100644 --- a/packages/core/src/components/Advertising/createComponent.js +++ b/packages/core/src/components/Advertising/createComponent.js @@ -10,30 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import createSendAdConversion from "./handlers/sendAdConversion.js"; -import handleOnBeforeSendEvent from "./handlers/onBeforeSendEventHandler.js"; - -export default ({ - logger, - config, - eventManager, - cookieManager, - adConversionHandler, - getBrowser, - consent, -}) => { - const componentConfig = config.advertising; - - const sendAdConversionHandler = createSendAdConversion({ - eventManager, - cookieManager, - adConversionHandler, - logger, - componentConfig, - getBrowser, - consent, - }); - +export default ({ handleOnBeforeSendEvent, sendAdConversionHandler }) => { return { lifecycle: { onComponentsRegistered() { @@ -41,22 +18,7 @@ export default ({ // the configure command from resolving while waiting for consent. sendAdConversionHandler(); }, - onBeforeEvent: ({ event, advertising = {} }) => { - const { state } = consent.current(); - if (state !== "in") { - // Consent not yet granted — skip advertising ID resolution - // but don't block the sendEvent call. - return; - } - return handleOnBeforeSendEvent({ - cookieManager, - logger, - event, - componentConfig, - advertising, - getBrowser, - }); - }, + onBeforeEvent: handleOnBeforeSendEvent, }, }; }; diff --git a/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js b/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js new file mode 100644 index 000000000..d29fd44f6 --- /dev/null +++ b/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js @@ -0,0 +1,136 @@ +/* +Copyright 2023 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ + +import { SURFER_ID, ID5_ID, RAMP_ID } from "../constants/index.js"; +import { AUTO, WAIT } from "../../../constants/consentStatus.js"; +import { CHROME } from "../../../constants/browser.js"; + +const isAdvertisingDisabled = (advertising) => { + return ![AUTO, WAIT].includes( + advertising?.handleAdvertisingData?.toLowerCase(), + ); +}; + +const waitForAdvertisingId = (advertising) => { + return advertising?.handleAdvertisingData?.toLowerCase() === WAIT; +}; + +/** + * @param {Object} params + * @param {Object} params.cookieManager + * @param {Object} params.logger + * @param {Object} params.state + * @param {Object} params.event + * @param {Object} params.componentConfig + * @param {Object} params.advertising + * @param {Function} params.getBrowser + */ +export default ({ + cookieManager, + logger, + componentConfig, + getBrowser, + consent, + collectSurferId, + getID5Id, + getRampId, + appendAdvertisingIdQueryToEvent, + getUrlParams, + isThrottled, +}) => { + /** + * Appends advertising identity IDs to AEP event query if not already added. + */ + return async ({ event, advertising = {} }) => { + const { state } = consent.current(); + if (state !== "in") { + return; + } + const { skwcid, efid } = getUrlParams(); + const isClickThru = !!(skwcid && efid); + if ( + isAdvertisingDisabled(advertising) || + isClickThru || + (isThrottled(SURFER_ID, cookieManager) && + isThrottled(ID5_ID, cookieManager) && + isThrottled(RAMP_ID, cookieManager)) + ) + return; + + try { + const useShortTimeout = waitForAdvertisingId(advertising); + + let rampIdPromise = null; + + if (!getBrowser || getBrowser() !== CHROME) { + rampIdPromise = getRampId( + logger, + componentConfig.rampIdJSPath, + cookieManager, + useShortTimeout, + useShortTimeout, + ); + } + const [surferIdResult, id5IdResult, rampIdResult] = + await Promise.allSettled([ + collectSurferId(cookieManager, getBrowser, useShortTimeout), + getID5Id( + logger, + componentConfig.id5PartnerId, + useShortTimeout, + useShortTimeout, + ), + rampIdPromise, + ]); + + const availableIds = {}; + if ( + surferIdResult.status === "fulfilled" && + surferIdResult.value && + !isThrottled(SURFER_ID, cookieManager) + ) { + availableIds.surferId = surferIdResult.value; + } + if ( + id5IdResult.status === "fulfilled" && + id5IdResult.value && + !isThrottled(ID5_ID, cookieManager) + ) { + availableIds.id5Id = id5IdResult.value; + } + if ( + rampIdResult.status === "fulfilled" && + rampIdResult.value && + !isThrottled(RAMP_ID, cookieManager) + ) { + availableIds.rampId = rampIdResult.value; + } + // If no IDs are available and any ID is throttled, return , because we dont have new info to send + if ( + Object.keys(availableIds).length === 0 && + (isThrottled(SURFER_ID, cookieManager) || + isThrottled(ID5_ID, cookieManager) || + isThrottled(RAMP_ID, cookieManager)) + ) { + return; + } + appendAdvertisingIdQueryToEvent( + availableIds, + event, + cookieManager, + componentConfig, + ); + } catch (error) { + logger.error("Error in onBeforeSendEvent hook:", error); + } + }; +}; diff --git a/packages/core/src/components/Advertising/handlers/onBeforeSendEventHandler.js b/packages/core/src/components/Advertising/handlers/onBeforeSendEventHandler.js deleted file mode 100644 index ccfd0fad6..000000000 --- a/packages/core/src/components/Advertising/handlers/onBeforeSendEventHandler.js +++ /dev/null @@ -1,131 +0,0 @@ -/* -Copyright 2023 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ - -import collectSurferId from "../identities/collectSurferId.js"; -import { getID5Id } from "../identities/collectID5Id.js"; -import { getRampId } from "../identities/collectRampId.js"; -import { - appendAdvertisingIdQueryToEvent, - getUrlParams, - isThrottled, -} from "../utils/helpers.js"; -import { SURFER_ID, ID5_ID, RAMP_ID } from "../constants/index.js"; -import { AUTO, WAIT } from "../../../constants/consentStatus.js"; -import { CHROME } from "../../../constants/browser.js"; - -const isAdvertisingDisabled = (advertising) => { - return ![AUTO, WAIT].includes( - advertising?.handleAdvertisingData?.toLowerCase(), - ); -}; - -const waitForAdvertisingId = (advertising) => { - return advertising?.handleAdvertisingData?.toLowerCase() === WAIT; -}; - -/** - * Appends advertising identity IDs to AEP event query if not already added. - * @param {Object} params - * @param {Object} params.cookieManager - * @param {Object} params.logger - * @param {Object} params.state - * @param {Object} params.event - * @param {Object} params.componentConfig - * @param {Object} params.advertising - * @param {Function} params.getBrowser - */ -export default async function handleOnBeforeSendEvent({ - cookieManager, - logger, - event, - componentConfig, - advertising, - getBrowser, -}) { - const { skwcid, efid } = getUrlParams(); - const isClickThru = !!(skwcid && efid); - if ( - isAdvertisingDisabled(advertising) || - isClickThru || - (isThrottled(SURFER_ID, cookieManager) && - isThrottled(ID5_ID, cookieManager) && - isThrottled(RAMP_ID, cookieManager)) - ) - return; - - try { - const useShortTimeout = waitForAdvertisingId(advertising); - - let rampIdPromise = null; - - if (!getBrowser || getBrowser() !== CHROME) { - rampIdPromise = getRampId( - logger, - componentConfig.rampIdJSPath, - cookieManager, - useShortTimeout, - useShortTimeout, - ); - } - const [surferIdResult, id5IdResult, rampIdResult] = - await Promise.allSettled([ - collectSurferId(cookieManager, getBrowser, useShortTimeout), - getID5Id( - logger, - componentConfig.id5PartnerId, - useShortTimeout, - useShortTimeout, - ), - rampIdPromise, - ]); - - const availableIds = {}; - if ( - surferIdResult.status === "fulfilled" && - surferIdResult.value && - !isThrottled(SURFER_ID, cookieManager) - ) { - availableIds.surferId = surferIdResult.value; - } - if ( - id5IdResult.status === "fulfilled" && - id5IdResult.value && - !isThrottled(ID5_ID, cookieManager) - ) { - availableIds.id5Id = id5IdResult.value; - } - if ( - rampIdResult.status === "fulfilled" && - rampIdResult.value && - !isThrottled(RAMP_ID, cookieManager) - ) { - availableIds.rampId = rampIdResult.value; - } - // If no IDs are available and any ID is throttled, return , because we dont have new info to send - if ( - Object.keys(availableIds).length === 0 && - (isThrottled(SURFER_ID, cookieManager) || - isThrottled(ID5_ID, cookieManager) || - isThrottled(RAMP_ID, cookieManager)) - ) { - return; - } - appendAdvertisingIdQueryToEvent( - availableIds, - event, - cookieManager, - componentConfig, - ); - } catch (error) { - logger.error("Error in onBeforeSendEvent hook:", error); - } -} diff --git a/packages/core/src/components/Advertising/index.js b/packages/core/src/components/Advertising/index.js index 8eb789ba1..fb7efa505 100644 --- a/packages/core/src/components/Advertising/index.js +++ b/packages/core/src/components/Advertising/index.js @@ -18,6 +18,16 @@ import { } from "../../utils/request/index.js"; import createAdConversionHandler from "./handlers/createAdConversionHandler.js"; import createCookieManager from "./utils/advertisingCookieManager.js"; +import createHandleOnBeforeSendEvent from "./handlers/createOnBeforeSendEventHandler.js"; +import createSendAdConversion from "./handlers/sendAdConversion.js"; +import collectSurferId from "./identities/collectSurferId.js"; +import { getID5Id } from "./identities/collectID5Id.js"; +import { getRampId } from "./identities/collectRampId.js"; +import { + appendAdvertisingIdQueryToEvent, + getUrlParams, + isThrottled, +} from "./utils/helpers.js"; const createAdvertising = ({ logger, @@ -27,11 +37,11 @@ const createAdvertising = ({ consent, getBrowser, }) => { + const componentConfig = config.advertising; const cookieManager = createCookieManager({ orgId: config.orgId, logger, }); - const adConversionHandler = createAdConversionHandler({ eventManager, sendEdgeNetworkRequest, @@ -40,15 +50,32 @@ const createAdvertising = ({ createDataCollectionRequestPayload, logger, }); - return createComponent({ + const handleOnBeforeSendEvent = createHandleOnBeforeSendEvent({ + cookieManager, logger, - config, + getBrowser, + consent, + componentConfig, + collectSurferId, + getID5Id, + getRampId, + appendAdvertisingIdQueryToEvent, + getUrlParams, + isThrottled, + }); + const sendAdConversionHandler = createSendAdConversion({ eventManager, cookieManager, adConversionHandler, + logger, + componentConfig, getBrowser, consent, }); + return createComponent({ + handleOnBeforeSendEvent, + sendAdConversionHandler, + }); }; createAdvertising.namespace = "Advertising"; diff --git a/packages/core/test/unit/specs/components/Advertising/createComponent.spec.js b/packages/core/test/unit/specs/components/Advertising/createComponent.spec.js index 707d56803..7d439302b 100644 --- a/packages/core/test/unit/specs/components/Advertising/createComponent.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/createComponent.spec.js @@ -12,81 +12,20 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; -// Mock the onBeforeSendEventHandler -vi.mock( - "../../../../../src/components/Advertising/handlers/onBeforeSendEventHandler.js", - () => ({ - default: vi.fn(), - }), -); - -// Mock the sendAdConversion handler -vi.mock( - "../../../../../src/components/Advertising/handlers/sendAdConversion.js", - () => ({ - default: vi.fn(() => vi.fn()), - }), -); - import createComponent from "../../../../../src/components/Advertising/createComponent.js"; -import handleOnBeforeSendEvent from "../../../../../src/components/Advertising/handlers/onBeforeSendEventHandler.js"; describe("Advertising::createComponent", () => { - let logger; - let config; - let eventManager; - let cookieManager; - let adConversionHandler; - let consent; + let handleOnBeforeSendEvent; + let sendAdConversionHandler; let component; beforeEach(() => { - logger = { - info: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - warn: vi.fn(), - }; - - config = { - advertising: { - advertiserSettings: [ - { advertiserId: "123", enabled: true }, - { advertiserId: "456", enabled: true }, - ], - id5PartnerId: "test-partner", - rampIdJSPath: "/test-path", - }, - }; - - eventManager = { - createEvent: vi.fn(), - }; - - cookieManager = { - getValue: vi.fn(), - setValue: vi.fn(), - }; - - adConversionHandler = { - trackAdConversion: vi.fn(), - }; - - consent = { - awaitConsent: vi.fn().mockResolvedValue(), - current: vi.fn().mockReturnValue({ state: "in", wasSet: true }), - }; - - // Reset mocks - vi.mocked(handleOnBeforeSendEvent).mockReset(); + handleOnBeforeSendEvent = vi.fn(); + sendAdConversionHandler = vi.fn(); component = createComponent({ - logger, - config, - eventManager, - cookieManager, - adConversionHandler, - consent, + handleOnBeforeSendEvent, + sendAdConversionHandler, }); }); @@ -96,86 +35,23 @@ describe("Advertising::createComponent", () => { expect(component.lifecycle).toHaveProperty("onBeforeEvent"); expect(typeof component.lifecycle.onComponentsRegistered).toBe("function"); expect(typeof component.lifecycle.onBeforeEvent).toBe("function"); + expect(component.lifecycle.onBeforeEvent).toBe(handleOnBeforeSendEvent); }); - it("should call onBeforeSendEvent handler with correct parameters including options", async () => { + it("calls sendAdConversionHandler on registration", () => { + component.lifecycle.onComponentsRegistered(); + expect(sendAdConversionHandler).toHaveBeenCalledTimes(1); + }); + + it("delegates onBeforeEvent to injected handler", async () => { const event = { mergeQuery: vi.fn() }; const advertising = { handleAdvertisingData: "wait" }; await component.lifecycle.onBeforeEvent({ event, advertising }); expect(handleOnBeforeSendEvent).toHaveBeenCalledWith({ - cookieManager, - logger, event, - componentConfig: config.advertising, advertising, - getBrowser: undefined, - }); - }); - - it("should handle onBeforeEvent with empty options", async () => { - const event = { mergeQuery: vi.fn() }; - - await component.lifecycle.onBeforeEvent({ event }); - - expect(handleOnBeforeSendEvent).toHaveBeenCalledWith({ - cookieManager, - logger, - event, - componentConfig: config.advertising, - advertising: {}, - getBrowser: undefined, - }); - }); - - it("should handle onBeforeEvent with undefined options", async () => { - const event = { mergeQuery: vi.fn() }; - - await component.lifecycle.onBeforeEvent({ event, advertising: undefined }); - - expect(handleOnBeforeSendEvent).toHaveBeenCalledWith({ - cookieManager, - logger, - event, - componentConfig: config.advertising, - advertising: {}, - getBrowser: undefined, }); }); - - it("should skip onBeforeSendEvent when consent is out", async () => { - consent.current.mockReturnValue({ state: "out", wasSet: true }); - - const event = { mergeQuery: vi.fn() }; - - await component.lifecycle.onBeforeEvent({ event }); - - expect(handleOnBeforeSendEvent).not.toHaveBeenCalled(); - }); - - it("should skip onBeforeSendEvent when consent is pending", async () => { - consent.current.mockReturnValue({ state: "pending", wasSet: false }); - - const event = { mergeQuery: vi.fn() }; - - await component.lifecycle.onBeforeEvent({ event }); - - expect(handleOnBeforeSendEvent).not.toHaveBeenCalled(); - }); - - it("should maintain shared state across calls", async () => { - const event = { mergeQuery: vi.fn() }; - - await component.lifecycle.onBeforeEvent({ event }); - await component.lifecycle.onBeforeEvent({ event }); - - // Both calls should use the same state object - const firstCallState = vi.mocked(handleOnBeforeSendEvent).mock.calls[0][0] - .state; - const secondCallState = vi.mocked(handleOnBeforeSendEvent).mock.calls[1][0] - .state; - - expect(firstCallState).toBe(secondCallState); - }); }); diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/createOnBeforeSendEventHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/createOnBeforeSendEventHandler.spec.js new file mode 100644 index 000000000..51b7d9b5a --- /dev/null +++ b/packages/core/test/unit/specs/components/Advertising/handlers/createOnBeforeSendEventHandler.spec.js @@ -0,0 +1,228 @@ +/* +Copyright 2025 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ + +import { vi, beforeEach, describe, it, expect } from "vitest"; +import { CHROME } from "../../../../../../src/constants/browser.js"; +import createOnBeforeSendEventHandler from "../../../../../../src/components/Advertising/handlers/createOnBeforeSendEventHandler.js"; + +describe("Advertising::createOnBeforeSendEventHandler", () => { + let cookieManager; + let logger; + let event; + let componentConfig; + let getBrowser; + let consent; + let onBeforeEvent; + let collectSurferIdFn; + let getID5IdFn; + let getRampIdFn; + let appendAdvertisingIdQueryToEventFn; + let getUrlParamsFn; + let isThrottledFn; + + beforeEach(() => { + cookieManager = { + getValue: vi.fn(), + setValue: vi.fn(), + }; + + logger = { + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + warn: vi.fn(), + }; + + event = { + mergeQuery: vi.fn(), + }; + + componentConfig = { + id5PartnerId: "test-partner", + rampIdJSPath: "/test-path", + }; + + getBrowser = vi.fn(() => "Firefox"); + + consent = { + current: vi.fn(() => ({ state: "in", wasSet: true })), + }; + + collectSurferIdFn = vi.fn().mockResolvedValue(null); + getID5IdFn = vi.fn().mockResolvedValue(null); + getRampIdFn = vi.fn().mockResolvedValue(null); + getUrlParamsFn = vi.fn().mockReturnValue({ skwcid: null, efid: null }); + appendAdvertisingIdQueryToEventFn = vi.fn((availableIds, currentEvent) => { + const query = { + advertising: { + conversion: { + StitchIds: {}, + }, + }, + }; + + if (availableIds.surferId) { + query.advertising.conversion.StitchIds.SurferId = availableIds.surferId; + } + if (availableIds.id5Id) { + query.advertising.conversion.StitchIds.ID5 = availableIds.id5Id; + } + if (availableIds.rampId) { + query.advertising.conversion.StitchIds.RampIDEnv = availableIds.rampId; + } + + currentEvent.mergeQuery(query); + }); + getUrlParamsFn = vi.fn().mockReturnValue({ skwcid: null, efid: null }); + isThrottledFn = vi.fn().mockReturnValue(false); + + onBeforeEvent = createOnBeforeSendEventHandler({ + cookieManager, + logger, + componentConfig, + getBrowser, + consent, + collectSurferId: collectSurferIdFn, + getID5Id: getID5IdFn, + getRampId: getRampIdFn, + appendAdvertisingIdQueryToEvent: appendAdvertisingIdQueryToEventFn, + getUrlParams: getUrlParamsFn, + isThrottled: isThrottledFn, + }); + }); + + it("returns early when consent is not in", async () => { + consent.current.mockReturnValue({ state: "out", wasSet: true }); + + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "auto" }, + }); + + expect(collectSurferIdFn).not.toHaveBeenCalled(); + expect(getID5IdFn).not.toHaveBeenCalled(); + expect(getRampIdFn).not.toHaveBeenCalled(); + expect(event.mergeQuery).not.toHaveBeenCalled(); + }); + + it("returns early when advertising handling is disabled", async () => { + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "disabled" }, + }); + + expect(collectSurferIdFn).not.toHaveBeenCalled(); + expect(getID5IdFn).not.toHaveBeenCalled(); + expect(getRampIdFn).not.toHaveBeenCalled(); + }); + + it("uses short timeout when handleAdvertisingData is wait", async () => { + collectSurferIdFn.mockResolvedValue("test-surfer-id"); + + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "wait" }, + }); + + expect(collectSurferIdFn).toHaveBeenCalledWith( + cookieManager, + getBrowser, + true, + ); + expect(getID5IdFn).toHaveBeenCalledWith(logger, "test-partner", true, true); + expect(getRampIdFn).toHaveBeenCalledWith( + logger, + "/test-path", + cookieManager, + true, + true, + ); + }); + + it("collects and merges available IDs", async () => { + collectSurferIdFn.mockResolvedValue("test-surfer-id"); + getID5IdFn.mockResolvedValue("test-id5-id"); + getRampIdFn.mockResolvedValue("test-ramp-id"); + + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "auto" }, + }); + + expect(event.mergeQuery).toHaveBeenCalledWith({ + advertising: { + conversion: { + StitchIds: { + SurferId: "test-surfer-id", + ID5: "test-id5-id", + RampIDEnv: "test-ramp-id", + }, + }, + }, + }); + }); + + it("skips RampID in Chrome", async () => { + getBrowser.mockReturnValue(CHROME); + collectSurferIdFn.mockResolvedValue("test-surfer-id"); + + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "auto" }, + }); + + expect(getRampIdFn).not.toHaveBeenCalled(); + }); + + it("returns early for click-through events", async () => { + getUrlParamsFn.mockReturnValue({ skwcid: "a", efid: "b" }); + + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "auto" }, + }); + + expect(collectSurferIdFn).not.toHaveBeenCalled(); + expect(getID5IdFn).not.toHaveBeenCalled(); + expect(getRampIdFn).not.toHaveBeenCalled(); + }); + + it("returns early when all IDs are throttled", async () => { + isThrottledFn.mockReturnValue(true); + + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "auto" }, + }); + + expect(collectSurferIdFn).not.toHaveBeenCalled(); + expect(getID5IdFn).not.toHaveBeenCalled(); + expect(getRampIdFn).not.toHaveBeenCalled(); + }); + + it("logs when unexpected errors are thrown", async () => { + collectSurferIdFn.mockResolvedValue("test-surfer-id"); + appendAdvertisingIdQueryToEventFn.mockImplementation(() => { + throw new Error("boom"); + }); + + await onBeforeEvent({ + event, + advertising: { handleAdvertisingData: "auto" }, + }); + + expect(logger.error).toHaveBeenCalledWith( + "Error in onBeforeSendEvent hook:", + expect.any(Error), + ); + }); +}); diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/onBeforeSendEventHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/onBeforeSendEventHandler.spec.js deleted file mode 100644 index 191ba040e..000000000 --- a/packages/core/test/unit/specs/components/Advertising/handlers/onBeforeSendEventHandler.spec.js +++ /dev/null @@ -1,517 +0,0 @@ -/* -Copyright 2025 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ - -import { vi, beforeEach, describe, it, expect } from "vitest"; -import handleOnBeforeSendEvent from "../../../../../../src/components/Advertising/handlers/onBeforeSendEventHandler.js"; - -// Mock network operations to prevent real network calls -vi.mock("fetch", () => vi.fn()); - -// Mock globalThis fetch and other network APIs -Object.defineProperty(globalThis, "fetch", { - value: vi.fn(() => - Promise.resolve({ - ok: true, - json: () => Promise.resolve({ success: true }), - }), - ), - writable: true, -}); - -// Mock XMLHttpRequest -Object.defineProperty(globalThis, "XMLHttpRequest", { - value: class MockXMLHttpRequest { - open() { - this.readyState = 4; - } - - send() { - this.status = 200; - } - - setRequestHeader() { - this.headers = {}; - } - }, - writable: true, -}); - -// Mock DOM operations to prevent network calls from script loading -if (typeof globalThis.document !== "undefined") { - globalThis.document.createElement = vi.fn(() => ({ - src: "", - height: 0, - width: 0, - frameBorder: 0, - style: { display: "none" }, - addEventListener: vi.fn(), - onerror: vi.fn(), - })); - if (globalThis.document.body) { - globalThis.document.body.appendChild = vi.fn(); - } - if (globalThis.document.head) { - globalThis.document.head.appendChild = vi.fn(); - } -} - -if (typeof globalThis.window !== "undefined") { - globalThis.window.addEventListener = vi.fn(); - globalThis.window.removeEventListener = vi.fn(); - globalThis.window.attachEvent = vi.fn(); - globalThis.window.detachEvent = vi.fn(); - globalThis.window.ats = undefined; - globalThis.window.ID5 = undefined; -} - -// Mock identity collectors to return resolved promises immediately -vi.mock( - "../../../../../../src/components/Advertising/identities/collectSurferId.js", - () => ({ - default: vi.fn(() => Promise.resolve("mock-surfer-id")), - }), -); - -vi.mock( - "../../../../../../src/components/Advertising/identities/collectID5Id.js", - () => ({ - getID5Id: vi.fn(() => Promise.resolve("mock-id5-id")), - }), -); - -vi.mock( - "../../../../../../src/components/Advertising/identities/collectRampId.js", - () => ({ - getRampId: vi.fn(() => Promise.resolve("mock-ramp-id")), - }), -); - -// Mock helpers to prevent network calls -vi.mock( - "../../../../../../src/components/Advertising/utils/helpers.js", - () => ({ - appendAdvertisingIdQueryToEvent: vi.fn((availableIds, event) => { - // Mock the actual behavior of appending advertising IDs to the event query - const query = { - advertising: { - conversion: { - StitchIds: {}, - }, - }, - }; - - if (availableIds.surferId) { - query.advertising.conversion.StitchIds.SurferId = availableIds.surferId; - } - if (availableIds.id5Id) { - query.advertising.conversion.StitchIds.ID5 = availableIds.id5Id; - } - if (availableIds.rampId) { - query.advertising.conversion.StitchIds.RampIDEnv = availableIds.rampId; - } - - event.mergeQuery(query); - }), - normalizeAdvertiser: vi.fn((advertiserSettings) => { - if (!advertiserSettings || !Array.isArray(advertiserSettings)) { - return "UNKNOWN"; - } - - return advertiserSettings - .filter((item) => item && item.enabled === true && item.advertiserId) - .map((item) => item.advertiserId) - .join(", "); - }), - getUrlParams: vi.fn(() => ({ skwcid: null, efid: null })), - isAnyIdUnused: vi.fn(() => true), - markIdsAsConverted: vi.fn(), - isThrottled: vi.fn(() => false), - shouldThrottle: vi.fn(() => false), - }), -); - -describe("Advertising::onBeforeSendEventHandler", () => { - let cookieManager; - let logger; - let state; - let event; - let componentConfig; - let getSurferId; - let getID5Id; - let getRampId; - let getBrowser; - - beforeEach(async () => { - cookieManager = { - getValue: vi.fn(), - setValue: vi.fn(), - }; - - logger = { - info: vi.fn(), - error: vi.fn(), - debug: vi.fn(), - warn: vi.fn(), - }; - - state = { - processedAdvertisingIds: false, - processingAdvertisingIds: false, - }; - - event = { - mergeXdm: vi.fn(), - mergeQuery: vi.fn(), - }; - - componentConfig = { - id5PartnerId: "test-partner", - rampIdJSPath: "/test-path", - dspEnabled: true, - }; - - // Default to a non-Chrome browser so RampID is collected by default - getBrowser = vi.fn().mockReturnValue("Firefox"); - - // Get and reset mock functions - these are already mocked at module level - const surferIdModule = await import( - "../../../../../../src/components/Advertising/identities/collectSurferId.js" - ); - const id5IdModule = await import( - "../../../../../../src/components/Advertising/identities/collectID5Id.js" - ); - const rampIdModule = await import( - "../../../../../../src/components/Advertising/identities/collectRampId.js" - ); - - getSurferId = vi.mocked(surferIdModule.default); - getID5Id = vi.mocked(id5IdModule.getID5Id); - getRampId = vi.mocked(rampIdModule.getRampId); - - // Reset and set default return values - getSurferId.mockReset().mockResolvedValue(null); - getID5Id.mockReset().mockResolvedValue(null); - getRampId.mockReset().mockResolvedValue(null); - }); - - it("should return early when already processed", async () => { - state.processedAdvertisingIds = true; - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: {}, - getBrowser, - }); - - expect(getSurferId).not.toHaveBeenCalled(); - expect(getID5Id).not.toHaveBeenCalled(); - expect(getRampId).not.toHaveBeenCalled(); - expect(event.mergeXdm).not.toHaveBeenCalled(); - }); - - it("should return early when advertising is disabled", async () => { - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: "disabled" }, - getBrowser, - }); - - expect(getSurferId).not.toHaveBeenCalled(); - expect(getID5Id).not.toHaveBeenCalled(); - expect(getRampId).not.toHaveBeenCalled(); - expect(event.mergeQuery).not.toHaveBeenCalled(); - }); - - it("should return early when advertising.handleAdvertisingData is null", async () => { - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: null }, - getBrowser, - }); - - expect(getSurferId).not.toHaveBeenCalled(); - expect(getID5Id).not.toHaveBeenCalled(); - expect(getRampId).not.toHaveBeenCalled(); - expect(event.mergeQuery).not.toHaveBeenCalled(); - }); - - it("should wait for surfer ID when advertising.handleAdvertisingData is 'wait'", async () => { - getSurferId.mockResolvedValue("test-surfer-id"); - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: "wait" }, - getBrowser, - }); - - expect(getSurferId).toHaveBeenCalledWith(cookieManager, getBrowser, true); - expect(getID5Id).toHaveBeenCalledWith(logger, "test-partner", true, true); - expect(getRampId).toHaveBeenCalledWith( - logger, - "/test-path", - cookieManager, - true, - true, - ); - }); - - it("should collect and merge available advertising IDs", async () => { - getSurferId.mockResolvedValue("test-surfer-id"); - getID5Id.mockResolvedValue("test-id5-id"); - getRampId.mockResolvedValue("test-ramp-id"); - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: "auto" }, - getBrowser, - }); - - expect(getSurferId).toHaveBeenCalledWith(cookieManager, getBrowser, false); - expect(getID5Id).toHaveBeenCalledWith(logger, "test-partner", false, false); - expect(getRampId).toHaveBeenCalledWith( - logger, - "/test-path", - cookieManager, - false, - false, - ); - - expect(event.mergeQuery).toHaveBeenCalledWith({ - advertising: { - conversion: { - StitchIds: { - SurferId: "test-surfer-id", - ID5: "test-id5-id", - RampIDEnv: "test-ramp-id", - }, - }, - }, - }); - - expect(state.processedAdvertisingIds).toBe(false); - }); - - it("should only include available IDs", async () => { - getSurferId.mockResolvedValue("test-surfer-id"); - getID5Id.mockResolvedValue(null); - getRampId.mockResolvedValue("test-ramp-id"); - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: "auto" }, - getBrowser, - }); - - expect(event.mergeQuery).toHaveBeenCalledWith({ - advertising: { - conversion: { - StitchIds: { - SurferId: "test-surfer-id", - RampIDEnv: "test-ramp-id", - }, - }, - }, - }); - }); - - it("should handle configuration without ID5", async () => { - getSurferId.mockResolvedValue(null); - getID5Id.mockResolvedValue("test-id5-id"); - getRampId.mockResolvedValue(null); - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig: { - rampIdJSPath: "/test-path", - }, - advertising: { handleAdvertisingData: "auto" }, - getBrowser, - }); - - expect(getSurferId).toHaveBeenCalled(); - expect(getID5Id).toHaveBeenCalledWith(logger, undefined, false, false); - expect(getRampId).toHaveBeenCalledWith( - logger, - "/test-path", - cookieManager, - false, - false, - ); - - expect(event.mergeQuery).toHaveBeenCalledWith({ - advertising: { - conversion: { - StitchIds: { - ID5: "test-id5-id", - }, - }, - }, - }); - }); - - it("should handle empty configuration", async () => { - getSurferId.mockResolvedValue("test-surfer-id"); - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig: {}, - advertising: { handleAdvertisingData: "auto" }, - getBrowser, - }); - - expect(getSurferId).toHaveBeenCalled(); - expect(getID5Id).toHaveBeenCalledWith(logger, undefined, false, false); - expect(getRampId).toHaveBeenCalledWith( - logger, - undefined, - cookieManager, - false, - false, - ); - - expect(event.mergeQuery).toHaveBeenCalledWith({ - advertising: { - conversion: { - StitchIds: { - SurferId: "test-surfer-id", - }, - }, - }, - }); - }); - - it("should handle all null IDs gracefully", async () => { - getSurferId.mockResolvedValue(null); - getID5Id.mockResolvedValue(null); - getRampId.mockResolvedValue(null); - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: {}, - getBrowser, - }); - - expect(event.mergeXdm).not.toHaveBeenCalled(); - expect(state.processedAdvertisingIds).toBe(false); - }); - - it("should handle identity collection errors", async () => { - getSurferId.mockRejectedValue(new Error("Failed to get surfer ID")); - getID5Id.mockRejectedValue(new Error("Failed to get ID5 ID")); - getRampId.mockRejectedValue(new Error("Failed to get Ramp ID")); - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: "auto" }, - getBrowser, - }); - - expect(event.mergeXdm).not.toHaveBeenCalled(); - expect(logger.error).not.toHaveBeenCalled(); - expect(state.processedAdvertisingIds).toBe(false); - }); - - it("should set flag to prevent concurrent processing", async () => { - state.processingAdvertisingIds = true; - - await handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: {}, - getBrowser, - }); - - expect(getSurferId).not.toHaveBeenCalled(); - expect(getID5Id).not.toHaveBeenCalled(); - expect(getRampId).not.toHaveBeenCalled(); - expect(event.mergeXdm).not.toHaveBeenCalled(); - }); - - it("should prevent concurrent calls from processing", async () => { - let resolveFirstCall; - const firstCallPromise = new Promise((resolve) => { - resolveFirstCall = resolve; - }); - - getSurferId.mockImplementation(() => firstCallPromise); - - // Start two concurrent calls - const call1 = handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: "auto" }, - getBrowser, - }); - - const call2 = handleOnBeforeSendEvent({ - cookieManager, - logger, - state, - event, - componentConfig, - advertising: { handleAdvertisingData: "auto" }, - getBrowser, - }); - - // Resolve the first call - resolveFirstCall("test-surfer-id"); - - await Promise.all([call1, call2]); - - // Both calls will execute since processedAdvertisingIds is not set in onBeforeSendEventHandler - expect(getSurferId).toHaveBeenCalledTimes(2); - }); -}); diff --git a/packages/core/test/unit/specs/components/Advertising/index.spec.js b/packages/core/test/unit/specs/components/Advertising/index.spec.js new file mode 100644 index 000000000..ff4f9aba9 --- /dev/null +++ b/packages/core/test/unit/specs/components/Advertising/index.spec.js @@ -0,0 +1,76 @@ +/* +Copyright 2026 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ + +import { vi, beforeEach, afterEach, describe, it, expect } from "vitest"; +import createAdvertising from "../../../../../src/components/Advertising/index.js"; + +describe("Advertising::index", () => { + let advertising; + let logger; + let config; + let eventManager; + let sendEdgeNetworkRequest; + let consent; + let getBrowser; + let getUrlParams; + + beforeEach(() => { + logger = { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }; + config = { + orgId: "org@AdobeOrg", + advertising: { + advertiserSettings: [ + { advertiserId: "123", enabled: true }, + { advertiserId: "456", enabled: true }, + ], + id5PartnerId: "test-partner", + rampIdJSPath: "/test-path", + }, + }; + eventManager = { + createEvent: vi.fn(), + }; + sendEdgeNetworkRequest = vi.fn(); + consent = { + awaitConsent: vi.fn().mockResolvedValue(), + current: vi.fn().mockReturnValue({ state: "in", wasSet: true }), + }; + getBrowser = vi.fn().mockReturnValue("Firefox"); + getUrlParams = vi.fn().mockReturnValue({ skwcid: null, efid: null }); + + advertising = createAdvertising({ + logger, + config, + eventManager, + sendEdgeNetworkRequest, + consent, + getBrowser, + getUrlParams, + }); + }); + + afterEach(() => { + advertising = null; + }); + + it("should create the advertising component", () => { + expect(advertising).toBeDefined(); + expect(advertising).toHaveProperty("lifecycle"); + expect(advertising.lifecycle).toHaveProperty("onComponentsRegistered"); + expect(advertising.lifecycle).toHaveProperty("onBeforeEvent"); + }); +}); From 34cbec0ae1cc41e6fbbe954ba785346efd938987 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 25 Feb 2026 14:45:51 -0700 Subject: [PATCH 02/23] Use dependency injection in the makeSendServiceWorkerTrackingData function --- .../serviceWorkerNotificationClickListener.js | 123 ++++++---- .../makeSendServiceWorkerTrackingData.js | 229 +++++++++--------- packages/core/src/serviceWorker.js | 10 +- ...iceWorkerNotificationClickListener.spec.js | 23 +- .../makeSendServiceWorkerTrackingData.spec.js | 26 +- 5 files changed, 228 insertions(+), 183 deletions(-) diff --git a/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js b/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js index 49079a1ac..ddc443d8d 100644 --- a/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js +++ b/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js @@ -15,7 +15,9 @@ governing permissions and limitations under the License. /** @import { ServiceWorkerLogger } from '../types.js' */ -import makeSendServiceWorkerTrackingData from "../request/makeSendServiceWorkerTrackingData.js"; +import { createMakeSendServiceWorkerTrackingData } from "../request/makeSendServiceWorkerTrackingData.js"; +import readFromIndexedDb from "./readFromIndexedDb.js"; +import uuidv4 from "../../../utils/uuid.js"; /** * @param {string} type @@ -24,61 +26,82 @@ import makeSendServiceWorkerTrackingData from "../request/makeSendServiceWorkerT const canHandleUrl = (type) => ["DEEPLINK", "WEBURL"].includes(type); /** - * @function - * - * @param {Object} options - * @param {ServiceWorkerGlobalScope} options.sw - * @param {NotificationEvent} options.event - * @param {(url: string, options: object) => Promise} options.fetch - * @param {ServiceWorkerLogger} options.logger + * @param {Object} dependencies + * @param {(options: { xdm: Object, actionLabel?: string, applicationLaunches?: number }, utils: { logger: ServiceWorkerLogger, fetch: (url: string, options: object) => Promise }) => Promise} dependencies.makeSendServiceWorkerTrackingData */ -export default ({ event, sw, logger, fetch }) => { - event.notification.close(); +export const createServiceWorkerNotificationClickListener = ({ + makeSendServiceWorkerTrackingData, +}) => { + /** + * @function + * + * @param {Object} options + * @param {ServiceWorkerGlobalScope} options.sw + * @param {NotificationEvent} options.event + * @param {(url: string, options: object) => Promise} options.fetch + * @param {ServiceWorkerLogger} options.logger + */ + return ({ event, sw, logger, fetch }) => { + event.notification.close(); - const data = event.notification.data; - let targetUrl = null; - let actionLabel = null; + const data = event.notification.data; + let targetUrl = null; + let actionLabel = null; - if (event.action) { - const actionIndex = parseInt(event.action.replace("action_", ""), 10); - if (data?.actions?.buttons[actionIndex]) { - const button = data.actions.buttons[actionIndex]; - actionLabel = button.label; - if (canHandleUrl(button.type) && button.uri) { - targetUrl = button.uri; + if (event.action) { + const actionIndex = parseInt(event.action.replace("action_", ""), 10); + if (data?.actions?.buttons[actionIndex]) { + const button = data.actions.buttons[actionIndex]; + actionLabel = button.label; + if (canHandleUrl(button.type) && button.uri) { + targetUrl = button.uri; + } } + } else if ( + canHandleUrl(data?.interaction?.type) && + data?.interaction?.uri + ) { + targetUrl = data.interaction.uri; } - } else if (canHandleUrl(data?.interaction?.type) && data?.interaction?.uri) { - targetUrl = data.interaction.uri; - } - makeSendServiceWorkerTrackingData( - { - // eslint-disable-next-line no-underscore-dangle - xdm: data._xdm.mixins, - actionLabel, - applicationLaunches: 1, - }, - { - logger, - fetch, - }, - ).catch((error) => { - logger.error("Failed to send tracking call:", error); - }); + makeSendServiceWorkerTrackingData( + { + // eslint-disable-next-line no-underscore-dangle + xdm: data._xdm.mixins, + actionLabel, + applicationLaunches: 1, + }, + { + logger, + fetch, + }, + ).catch((error) => { + logger.error("Failed to send tracking call:", error); + }); - if (targetUrl) { - event.waitUntil( - sw.clients.matchAll({ type: "window" }).then((clientList) => { - for (const client of clientList) { - if (client.url === targetUrl && "focus" in client) { - return client.focus(); + if (targetUrl) { + event.waitUntil( + sw.clients.matchAll({ type: "window" }).then((clientList) => { + for (const client of clientList) { + if (client.url === targetUrl && "focus" in client) { + return client.focus(); + } } - } - if (sw.clients.openWindow) { - return sw.clients.openWindow(targetUrl); - } - }), - ); - } + if (sw.clients.openWindow) { + return sw.clients.openWindow(targetUrl); + } + }), + ); + } + }; }; + +const makeSendServiceWorkerTrackingData = + createMakeSendServiceWorkerTrackingData({ + readFromIndexedDb, + uuidv4, + }); + +export default createServiceWorkerNotificationClickListener({ + makeSendServiceWorkerTrackingData, +}); diff --git a/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js b/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js index 91ce1b004..97a03e9b2 100644 --- a/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js +++ b/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js @@ -15,136 +15,145 @@ governing permissions and limitations under the License. /** @import { ServiceWorkerLogger } from '../types.js' */ /** @import { TrackingDataPayload } from '../types.js' */ -import readFromIndexedDb from "../helpers/readFromIndexedDb.js"; -import uuidv4 from "../../../utils/uuid.js"; - /** - * @async - * @function - * @param {Object} options - * @param {Object} options.xdm - * @param {string} [options.actionLabel] - * @param {number} [options.applicationLaunches=0] - * @param {Object} utils - * @param {ServiceWorkerLogger} utils.logger - * @param {(url: string, options: object) => Promise} utils.fetch + * @param {Object} dependencies + * @param {(logger: ServiceWorkerLogger) => Promise} dependencies.readFromIndexedDb + * @param {() => string} dependencies.uuidv4 * - * @returns {Promise} - * @throws {Error} + * @returns {(options: { xdm: Object, actionLabel?: string, applicationLaunches?: number }, utils: { logger: ServiceWorkerLogger, fetch: (url: string, options: object) => Promise }) => Promise} */ -export default async ( - { xdm, actionLabel, applicationLaunches = 0 }, - { logger, fetch }, -) => { - const configData = await readFromIndexedDb(logger); - const { browser, ecid, edgeDomain, edgeBasePath, datastreamId, datasetId } = - configData || {}; - let customActionData = {}; +export const createMakeSendServiceWorkerTrackingData = ({ + readFromIndexedDb, + uuidv4, +}) => { + /** + * @async + * @function + * @param {Object} options + * @param {Object} options.xdm + * @param {string} [options.actionLabel] + * @param {number} [options.applicationLaunches=0] + * @param {Object} utils + * @param {ServiceWorkerLogger} utils.logger + * @param {(url: string, options: object) => Promise} utils.fetch + * + * @returns {Promise} + * @throws {Error} + */ + return async ( + { xdm, actionLabel, applicationLaunches = 0 }, + { logger, fetch }, + ) => { + const configData = await readFromIndexedDb(logger); + const { browser, ecid, edgeDomain, edgeBasePath, datastreamId, datasetId } = + configData || {}; + let customActionData = {}; - if (actionLabel) { - customActionData = { - customAction: { actionID: actionLabel }, - }; - } + if (actionLabel) { + customActionData = { + customAction: { actionID: actionLabel }, + }; + } - const requiredFields = [ - { name: "browser", errorField: "Browser" }, - { name: "ecid", errorField: "ECID" }, - { - name: "edgeDomain", - errorField: "Edge domain", - }, - { - name: "edgeBasePath", - errorField: "Edge base path", - }, - { - name: "datastreamId", - errorField: "Datastream ID", - }, - { - name: "datasetId", - errorField: "Dataset ID", - }, - ]; + const requiredFields = [ + { name: "browser", errorField: "Browser" }, + { name: "ecid", errorField: "ECID" }, + { + name: "edgeDomain", + errorField: "Edge domain", + }, + { + name: "edgeBasePath", + errorField: "Edge base path", + }, + { + name: "datastreamId", + errorField: "Datastream ID", + }, + { + name: "datasetId", + errorField: "Dataset ID", + }, + ]; - try { - for (const field of requiredFields) { - if (!configData[field.name]) { - throw new Error( - `Cannot send tracking call. ${field.errorField} is missing.`, - ); + try { + for (const field of requiredFields) { + if (!configData[field.name]) { + throw new Error( + `Cannot send tracking call. ${field.errorField} is missing.`, + ); + } } - } - const url = `https://${edgeDomain}/${edgeBasePath}/v1/interact?configId=${datastreamId}&requestId=${uuidv4()}`; + const url = `https://${edgeDomain}/${edgeBasePath}/v1/interact?configId=${datastreamId}&requestId=${uuidv4()}`; - /** @type {TrackingDataPayload} */ - const payload = { - events: [ - { - xdm: { - identityMap: { - ECID: [{ id: ecid }], - }, - timestamp: new Date().toISOString(), - pushNotificationTracking: { - ...customActionData, - pushProviderMessageID: uuidv4(), - pushProvider: browser.toLowerCase(), - }, - application: { - launches: { - value: applicationLaunches, + /** @type {TrackingDataPayload} */ + const payload = { + events: [ + { + xdm: { + identityMap: { + ECID: [{ id: ecid }], }, - }, - eventType: actionLabel - ? "pushTracking.customAction" - : "pushTracking.applicationOpened", - _experience: { - ...xdm._experience, - customerJourneyManagement: { - ...xdm._experience.customerJourneyManagement, - pushChannelContext: { - platform: "web", + timestamp: new Date().toISOString(), + pushNotificationTracking: { + ...customActionData, + pushProviderMessageID: uuidv4(), + pushProvider: browser.toLowerCase(), + }, + application: { + launches: { + value: applicationLaunches, }, - messageProfile: { - channel: { - _id: "https://ns.adobe.com/xdm/channels/push", + }, + eventType: actionLabel + ? "pushTracking.customAction" + : "pushTracking.applicationOpened", + _experience: { + ...xdm._experience, + customerJourneyManagement: { + ...xdm._experience.customerJourneyManagement, + pushChannelContext: { + platform: "web", + }, + messageProfile: { + channel: { + _id: "https://ns.adobe.com/xdm/channels/push", + }, }, }, }, }, - }, - meta: { - collect: { - datasetId, + meta: { + collect: { + datasetId, + }, }, }, + ], + }; + + const response = await fetch(url, { + method: "POST", + headers: { + "content-type": "text/plain; charset=UTF-8", }, - ], - }; + body: JSON.stringify(payload), + }); - const response = await fetch(url, { - method: "POST", - headers: { - "content-type": "text/plain; charset=UTF-8", - }, - body: JSON.stringify(payload), - }); + if (!response.ok) { + logger.error( + "Tracking call failed: ", + response.status, + response.statusText, + ); + return false; + } - if (!response.ok) { - logger.error( - "Tracking call failed: ", - response.status, - response.statusText, - ); + return true; + } catch (error) { + logger.error("Error sending tracking call:", error); return false; } - - return true; - } catch (error) { - logger.error("Error sending tracking call:", error); - return false; - } + }; }; diff --git a/packages/core/src/serviceWorker.js b/packages/core/src/serviceWorker.js index 87bae2c1b..082128b7b 100644 --- a/packages/core/src/serviceWorker.js +++ b/packages/core/src/serviceWorker.js @@ -14,7 +14,9 @@ governing permissions and limitations under the License. import serviceWorkerNotificationClickListener from "./components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; import serviceWorkerPushListener from "./components/PushNotifications/helpers/serviceWorkerPushListener.js"; -import makeSendServiceWorkerTrackingData from "./components/PushNotifications/request/makeSendServiceWorkerTrackingData.js"; +import { createMakeSendServiceWorkerTrackingData } from "./components/PushNotifications/request/makeSendServiceWorkerTrackingData.js"; +import readFromIndexedDb from "./components/PushNotifications/helpers/readFromIndexedDb.js"; +import uuidv4 from "./utils/uuid.js"; /* eslint-disable no-console */ /* eslint-disable no-underscore-dangle */ @@ -35,6 +37,12 @@ const logger = { error: (...args) => console.error(logger.namespace, ...args), }; +const makeSendServiceWorkerTrackingData = + createMakeSendServiceWorkerTrackingData({ + readFromIndexedDb, + uuidv4, + }); + /** * @listens install */ diff --git a/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js b/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js index 4833d8ca1..b756f7cba 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js @@ -14,18 +14,15 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; -vi.mock( - "../../../../../../src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js", -); - -import serviceWorkerNotificationClickListener from "../../../../../../src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; -import makeSendServiceWorkerTrackingData from "../../../../../../src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js"; +import { createServiceWorkerNotificationClickListener } from "../../../../../../src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; describe("serviceWorkerNotificationClickListener", () => { let mockEvent; let mockSw; let mockLogger; let mockFetch; + let mockMakeSendServiceWorkerTrackingData; + let serviceWorkerNotificationClickListener; let mockClient; let mockWaitUntil; @@ -49,6 +46,12 @@ describe("serviceWorkerNotificationClickListener", () => { }; mockFetch = vi.fn(); + mockMakeSendServiceWorkerTrackingData = vi.fn().mockResolvedValue(true); + serviceWorkerNotificationClickListener = + createServiceWorkerNotificationClickListener({ + makeSendServiceWorkerTrackingData: + mockMakeSendServiceWorkerTrackingData, + }); mockWaitUntil = vi.fn(); mockEvent = { @@ -65,8 +68,6 @@ describe("serviceWorkerNotificationClickListener", () => { action: null, waitUntil: mockWaitUntil, }; - - vi.mocked(makeSendServiceWorkerTrackingData).mockResolvedValue(true); }); describe("tracking data sending", () => { @@ -78,7 +79,7 @@ describe("serviceWorkerNotificationClickListener", () => { fetch: mockFetch, }); - expect(makeSendServiceWorkerTrackingData).toHaveBeenCalledWith( + expect(mockMakeSendServiceWorkerTrackingData).toHaveBeenCalledWith( { xdm: mockEvent.notification.data._xdm.mixins, actionLabel: null, @@ -110,7 +111,7 @@ describe("serviceWorkerNotificationClickListener", () => { fetch: mockFetch, }); - expect(makeSendServiceWorkerTrackingData).toHaveBeenCalledWith( + expect(mockMakeSendServiceWorkerTrackingData).toHaveBeenCalledWith( { xdm: mockEvent.notification.data._xdm.mixins, actionLabel: "View Details", @@ -125,7 +126,7 @@ describe("serviceWorkerNotificationClickListener", () => { it("logs error when tracking data sending fails", async () => { const testError = new Error("Network error"); - vi.mocked(makeSendServiceWorkerTrackingData).mockRejectedValue(testError); + mockMakeSendServiceWorkerTrackingData.mockRejectedValue(testError); serviceWorkerNotificationClickListener({ event: mockEvent, diff --git a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js index 639fc056a..ad7b107d6 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js @@ -14,18 +14,14 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; -vi.mock( - "../../../../../../src/components/PushNotifications/helpers/readFromIndexedDb.js", -); -vi.mock("../../../../../../src/utils/uuid.js"); - -import makeSendServiceWorkerTrackingData from "../../../../../../src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js"; -import readFromIndexedDb from "../../../../../../src/components/PushNotifications/helpers/readFromIndexedDb.js"; -import uuidv4 from "../../../../../../src/utils/uuid.js"; +import { createMakeSendServiceWorkerTrackingData } from "../../../../../../src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js"; describe("makeSendServiceWorkerTrackingData", () => { let mockLogger; let mockFetch; + let mockReadFromIndexedDb; + let mockUuidv4; + let makeSendServiceWorkerTrackingData; let mockConfigData; beforeEach(() => { @@ -36,6 +32,8 @@ describe("makeSendServiceWorkerTrackingData", () => { }; mockFetch = vi.fn(); + mockReadFromIndexedDb = vi.fn(); + mockUuidv4 = vi.fn(); mockConfigData = { browser: "Chrome", @@ -46,8 +44,14 @@ describe("makeSendServiceWorkerTrackingData", () => { datasetId: "test-dataset-id", }; - vi.mocked(readFromIndexedDb).mockResolvedValue(mockConfigData); - vi.mocked(uuidv4).mockReturnValue("mock-uuid-1234"); + mockReadFromIndexedDb.mockResolvedValue(mockConfigData); + mockUuidv4.mockReturnValue("mock-uuid-1234"); + makeSendServiceWorkerTrackingData = createMakeSendServiceWorkerTrackingData( + { + readFromIndexedDb: mockReadFromIndexedDb, + uuidv4: mockUuidv4, + }, + ); }); describe("successful tracking data sending", () => { @@ -150,7 +154,7 @@ describe("makeSendServiceWorkerTrackingData", () => { it(`returns false when ${field} is missing`, async () => { const incompleteConfigData = { ...mockConfigData }; delete incompleteConfigData[field]; - vi.mocked(readFromIndexedDb).mockResolvedValue(incompleteConfigData); + mockReadFromIndexedDb.mockResolvedValue(incompleteConfigData); const xdm = { _experience: { From 401e19146201617c6d9c2e3fd4b68c2a8d6e0dce Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 25 Feb 2026 14:56:04 -0700 Subject: [PATCH 03/23] Add changeset --- .changeset/green-poets-bow.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changeset/green-poets-bow.md diff --git a/.changeset/green-poets-bow.md b/.changeset/green-poets-bow.md new file mode 100644 index 000000000..a845151cc --- /dev/null +++ b/.changeset/green-poets-bow.md @@ -0,0 +1,2 @@ +--- +--- From e81cf123c9001a4ddd649a7ef1316aeee9770330 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 25 Feb 2026 14:59:25 -0700 Subject: [PATCH 04/23] Update jsdoc comments for advertising component --- .../createOnBeforeSendEventHandler.js | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js b/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js index d29fd44f6..e06a90c62 100644 --- a/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js +++ b/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js @@ -25,14 +25,20 @@ const waitForAdvertisingId = (advertising) => { }; /** - * @param {Object} params - * @param {Object} params.cookieManager - * @param {Object} params.logger - * @param {Object} params.state - * @param {Object} params.event - * @param {Object} params.componentConfig - * @param {Object} params.advertising - * @param {Function} params.getBrowser + * Creates an onBeforeSendEvent handler that appends advertising IDs to events. + * + * @param {Object} dependencies + * @param {Object} dependencies + * @param {Object} dependencies.logger + * @param {Object} dependencies.componentConfig + * @param {Function} dependencies.getBrowser + * @param {Object} dependencies.consent + * @param {Function} dependencies.collectSurferId + * @param {Function} dependencies.getID5Id + * @param {Function} dependencies.getRampId + * @param {Function} dependencies.appendAdvertisingIdQueryToEvent + * @param {Function} dependencies.getUrlParams + * @param {Function} dependencies.isThrottled */ export default ({ cookieManager, @@ -49,6 +55,11 @@ export default ({ }) => { /** * Appends advertising identity IDs to AEP event query if not already added. + * + * @param {Object} options + * @param {Object} options.event + * @param {Object} [options.advertising={}] + * @returns {Promise} */ return async ({ event, advertising = {} }) => { const { state } = consent.current(); From aa46ad872d7f3cfca53b4512f713dea640373166 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Thu, 26 Feb 2026 12:26:03 -0700 Subject: [PATCH 05/23] fix jsdoc comments --- .../Advertising/handlers/createOnBeforeSendEventHandler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js b/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js index e06a90c62..955682a08 100644 --- a/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js +++ b/packages/core/src/components/Advertising/handlers/createOnBeforeSendEventHandler.js @@ -28,7 +28,7 @@ const waitForAdvertisingId = (advertising) => { * Creates an onBeforeSendEvent handler that appends advertising IDs to events. * * @param {Object} dependencies - * @param {Object} dependencies + * @param {Object} dependencies.cookieManager * @param {Object} dependencies.logger * @param {Object} dependencies.componentConfig * @param {Function} dependencies.getBrowser From f2de2fa4ad9cfff63f51325c20cb91cfd8704933 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Thu, 26 Feb 2026 15:39:39 -0700 Subject: [PATCH 06/23] unify dependency injection a bit --- .../serviceWorkerNotificationClickListener.js | 43 ++----- .../makeSendServiceWorkerTrackingData.js | 13 +- packages/core/src/serviceWorker.js | 31 ++--- ...iceWorkerNotificationClickListener.spec.js | 120 ++++-------------- .../makeSendServiceWorkerTrackingData.spec.js | 29 ++--- 5 files changed, 70 insertions(+), 166 deletions(-) diff --git a/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js b/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js index ddc443d8d..5139d2748 100644 --- a/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js +++ b/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js @@ -15,10 +15,6 @@ governing permissions and limitations under the License. /** @import { ServiceWorkerLogger } from '../types.js' */ -import { createMakeSendServiceWorkerTrackingData } from "../request/makeSendServiceWorkerTrackingData.js"; -import readFromIndexedDb from "./readFromIndexedDb.js"; -import uuidv4 from "../../../utils/uuid.js"; - /** * @param {string} type * @returns {boolean} @@ -27,21 +23,22 @@ const canHandleUrl = (type) => ["DEEPLINK", "WEBURL"].includes(type); /** * @param {Object} dependencies - * @param {(options: { xdm: Object, actionLabel?: string, applicationLaunches?: number }, utils: { logger: ServiceWorkerLogger, fetch: (url: string, options: object) => Promise }) => Promise} dependencies.makeSendServiceWorkerTrackingData + * @param {(options: { xdm: Object, actionLabel?: string, applicationLaunches?: number }) => Promise} dependencies.makeSendServiceWorkerTrackingData + * @param {ServiceWorkerGlobalScope} dependencies.sw + * @param {ServiceWorkerLogger} dependencies.logger */ export const createServiceWorkerNotificationClickListener = ({ makeSendServiceWorkerTrackingData, + sw, + logger, }) => { /** * @function * * @param {Object} options - * @param {ServiceWorkerGlobalScope} options.sw * @param {NotificationEvent} options.event - * @param {(url: string, options: object) => Promise} options.fetch - * @param {ServiceWorkerLogger} options.logger */ - return ({ event, sw, logger, fetch }) => { + return ({ event }) => { event.notification.close(); const data = event.notification.data; @@ -64,18 +61,12 @@ export const createServiceWorkerNotificationClickListener = ({ targetUrl = data.interaction.uri; } - makeSendServiceWorkerTrackingData( - { - // eslint-disable-next-line no-underscore-dangle - xdm: data._xdm.mixins, - actionLabel, - applicationLaunches: 1, - }, - { - logger, - fetch, - }, - ).catch((error) => { + makeSendServiceWorkerTrackingData({ + // eslint-disable-next-line no-underscore-dangle + xdm: data._xdm.mixins, + actionLabel, + applicationLaunches: 1, + }).catch((error) => { logger.error("Failed to send tracking call:", error); }); @@ -95,13 +86,3 @@ export const createServiceWorkerNotificationClickListener = ({ } }; }; - -const makeSendServiceWorkerTrackingData = - createMakeSendServiceWorkerTrackingData({ - readFromIndexedDb, - uuidv4, - }); - -export default createServiceWorkerNotificationClickListener({ - makeSendServiceWorkerTrackingData, -}); diff --git a/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js b/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js index 97a03e9b2..100e8632c 100644 --- a/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js +++ b/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js @@ -19,12 +19,15 @@ governing permissions and limitations under the License. * @param {Object} dependencies * @param {(logger: ServiceWorkerLogger) => Promise} dependencies.readFromIndexedDb * @param {() => string} dependencies.uuidv4 - * + * @param {ServiceWorkerLogger} dependencies.logger + * @param {(url: string, options: object) => Promise} dependencies.fetch * @returns {(options: { xdm: Object, actionLabel?: string, applicationLaunches?: number }, utils: { logger: ServiceWorkerLogger, fetch: (url: string, options: object) => Promise }) => Promise} */ export const createMakeSendServiceWorkerTrackingData = ({ readFromIndexedDb, uuidv4, + logger, + fetch, }) => { /** * @async @@ -33,17 +36,11 @@ export const createMakeSendServiceWorkerTrackingData = ({ * @param {Object} options.xdm * @param {string} [options.actionLabel] * @param {number} [options.applicationLaunches=0] - * @param {Object} utils - * @param {ServiceWorkerLogger} utils.logger - * @param {(url: string, options: object) => Promise} utils.fetch * * @returns {Promise} * @throws {Error} */ - return async ( - { xdm, actionLabel, applicationLaunches = 0 }, - { logger, fetch }, - ) => { + return async ({ xdm, actionLabel, applicationLaunches = 0 }) => { const configData = await readFromIndexedDb(logger); const { browser, ecid, edgeDomain, edgeBasePath, datastreamId, datasetId } = configData || {}; diff --git a/packages/core/src/serviceWorker.js b/packages/core/src/serviceWorker.js index 082128b7b..a0c2fb2ec 100644 --- a/packages/core/src/serviceWorker.js +++ b/packages/core/src/serviceWorker.js @@ -12,7 +12,7 @@ governing permissions and limitations under the License. /** @import { ServiceWorkerLogger } from './components/PushNotifications/types.js' */ -import serviceWorkerNotificationClickListener from "./components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; +import { createServiceWorkerNotificationClickListener } from "./components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; import serviceWorkerPushListener from "./components/PushNotifications/helpers/serviceWorkerPushListener.js"; import { createMakeSendServiceWorkerTrackingData } from "./components/PushNotifications/request/makeSendServiceWorkerTrackingData.js"; import readFromIndexedDb from "./components/PushNotifications/helpers/readFromIndexedDb.js"; @@ -41,6 +41,15 @@ const makeSendServiceWorkerTrackingData = createMakeSendServiceWorkerTrackingData({ readFromIndexedDb, uuidv4, + logger, + fetch, + }); + +const serviceWorkerNotificationClickListener = + createServiceWorkerNotificationClickListener({ + makeSendServiceWorkerTrackingData, + sw, + logger, }); /** @@ -63,16 +72,14 @@ sw.addEventListener("activate", (event) => { * @param {PushEvent} event * @returns {Promise} */ -sw.addEventListener("push", (event) => - serviceWorkerPushListener({ event, logger, sw }), -); +sw.addEventListener("push", (event) => serviceWorkerPushListener({ event })); /** * @listens notificationclick * @param {NotificationEvent} event */ sw.addEventListener("notificationclick", (event) => - serviceWorkerNotificationClickListener({ event, sw, logger, fetch }), + serviceWorkerNotificationClickListener({ event }), ); /** @@ -82,16 +89,10 @@ sw.addEventListener("notificationclick", (event) => sw.addEventListener("notificationclose", (event) => { const data = event.notification.data; - makeSendServiceWorkerTrackingData( - { - xdm: data._xdm.mixins, - actionLabel: "Dismiss", - }, - { - logger, - fetch, - }, - ).catch((error) => { + makeSendServiceWorkerTrackingData({ + xdm: data._xdm.mixins, + actionLabel: "Dismiss", + }).catch((error) => { logger.error("Failed to send tracking call:", error); }); }); diff --git a/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js b/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js index b756f7cba..3a223e3f3 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js @@ -20,7 +20,6 @@ describe("serviceWorkerNotificationClickListener", () => { let mockEvent; let mockSw; let mockLogger; - let mockFetch; let mockMakeSendServiceWorkerTrackingData; let serviceWorkerNotificationClickListener; let mockClient; @@ -45,12 +44,13 @@ describe("serviceWorkerNotificationClickListener", () => { error: vi.fn(), }; - mockFetch = vi.fn(); mockMakeSendServiceWorkerTrackingData = vi.fn().mockResolvedValue(true); serviceWorkerNotificationClickListener = createServiceWorkerNotificationClickListener({ makeSendServiceWorkerTrackingData: mockMakeSendServiceWorkerTrackingData, + sw: mockSw, + logger: mockLogger, }); mockWaitUntil = vi.fn(); @@ -72,24 +72,13 @@ describe("serviceWorkerNotificationClickListener", () => { describe("tracking data sending", () => { it("sends tracking data with application launches when no action is taken", async () => { - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); - expect(mockMakeSendServiceWorkerTrackingData).toHaveBeenCalledWith( - { - xdm: mockEvent.notification.data._xdm.mixins, - actionLabel: null, - applicationLaunches: 1, - }, - { - logger: mockLogger, - fetch: mockFetch, - }, - ); + expect(mockMakeSendServiceWorkerTrackingData).toHaveBeenCalledWith({ + xdm: mockEvent.notification.data._xdm.mixins, + actionLabel: null, + applicationLaunches: 1, + }); }); it("sends tracking data with action label when action button is clicked", () => { @@ -104,36 +93,20 @@ describe("serviceWorkerNotificationClickListener", () => { ], }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); - expect(mockMakeSendServiceWorkerTrackingData).toHaveBeenCalledWith( - { - xdm: mockEvent.notification.data._xdm.mixins, - actionLabel: "View Details", - applicationLaunches: 1, - }, - { - logger: mockLogger, - fetch: mockFetch, - }, - ); + expect(mockMakeSendServiceWorkerTrackingData).toHaveBeenCalledWith({ + xdm: mockEvent.notification.data._xdm.mixins, + actionLabel: "View Details", + applicationLaunches: 1, + }); }); it("logs error when tracking data sending fails", async () => { const testError = new Error("Network error"); mockMakeSendServiceWorkerTrackingData.mockRejectedValue(testError); - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); await new Promise((resolve) => setTimeout(resolve, 0)); @@ -155,12 +128,7 @@ describe("serviceWorkerNotificationClickListener", () => { ], }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); const waitUntilCallback = mockWaitUntil.mock.calls[0][0]; await waitUntilCallback; @@ -180,12 +148,7 @@ describe("serviceWorkerNotificationClickListener", () => { ], }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); const waitUntilCallback = mockWaitUntil.mock.calls[0][0]; await waitUntilCallback; @@ -205,12 +168,7 @@ describe("serviceWorkerNotificationClickListener", () => { ], }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); expect(mockWaitUntil).not.toHaveBeenCalled(); }); @@ -223,12 +181,7 @@ describe("serviceWorkerNotificationClickListener", () => { uri: "https://example.com/main", }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); expect(mockWaitUntil).toHaveBeenCalled(); }); @@ -239,12 +192,7 @@ describe("serviceWorkerNotificationClickListener", () => { uri: "myapp://main/path", }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); expect(mockWaitUntil).toHaveBeenCalled(); }); @@ -255,12 +203,7 @@ describe("serviceWorkerNotificationClickListener", () => { uri: "https://example.com/main", }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); expect(mockWaitUntil).not.toHaveBeenCalled(); }); @@ -268,12 +211,7 @@ describe("serviceWorkerNotificationClickListener", () => { it("handles main notification without interaction", () => { delete mockEvent.notification.data.interaction; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); expect(mockWaitUntil).not.toHaveBeenCalled(); }); @@ -286,12 +224,7 @@ describe("serviceWorkerNotificationClickListener", () => { uri: "https://example.com/page", }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); const waitUntilCallback = mockWaitUntil.mock.calls[0][0]; await waitUntilCallback; @@ -308,12 +241,7 @@ describe("serviceWorkerNotificationClickListener", () => { uri: "https://example.com/new-page", }; - serviceWorkerNotificationClickListener({ - event: mockEvent, - sw: mockSw, - logger: mockLogger, - fetch: mockFetch, - }); + serviceWorkerNotificationClickListener({ event: mockEvent }); const waitUntilCallback = mockWaitUntil.mock.calls[0][0]; await waitUntilCallback; diff --git a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js index ad7b107d6..924012d5a 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendServiceWorkerTrackingData.spec.js @@ -50,6 +50,8 @@ describe("makeSendServiceWorkerTrackingData", () => { { readFromIndexedDb: mockReadFromIndexedDb, uuidv4: mockUuidv4, + logger: mockLogger, + fetch: mockFetch, }, ); }); @@ -72,10 +74,10 @@ describe("makeSendServiceWorkerTrackingData", () => { }, }; - const result = await makeSendServiceWorkerTrackingData( - { xdm, applicationLaunches: 1 }, - { logger: mockLogger, fetch: mockFetch }, - ); + const result = await makeSendServiceWorkerTrackingData({ + xdm, + applicationLaunches: 1, + }); expect(result).toBe(true); expect(mockFetch).toHaveBeenCalledWith( @@ -122,10 +124,11 @@ describe("makeSendServiceWorkerTrackingData", () => { }, }; - const result = await makeSendServiceWorkerTrackingData( - { xdm, actionLabel: "Adobe.com", applicationLaunches: 0 }, - { logger: mockLogger, fetch: mockFetch }, - ); + const result = await makeSendServiceWorkerTrackingData({ + xdm, + actionLabel: "Adobe.com", + applicationLaunches: 0, + }); expect(result).toBe(true); @@ -162,10 +165,7 @@ describe("makeSendServiceWorkerTrackingData", () => { }, }; - const result = await makeSendServiceWorkerTrackingData( - { xdm }, - { logger: mockLogger, fetch: mockFetch }, - ); + const result = await makeSendServiceWorkerTrackingData({ xdm }); expect(result).toBe(false); expect(mockLogger.error).toHaveBeenCalledWith( @@ -200,10 +200,7 @@ describe("makeSendServiceWorkerTrackingData", () => { }, }; - await makeSendServiceWorkerTrackingData( - { xdm }, - { logger: mockLogger, fetch: mockFetch }, - ); + await makeSendServiceWorkerTrackingData({ xdm }); const callArgs = mockFetch.mock.calls[0]; const payload = JSON.parse(callArgs[1].body); From d59682bb27d918b2e23928cff5920156ea9f4432 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Mon, 2 Mar 2026 14:07:19 -0700 Subject: [PATCH 07/23] Annotate testing code smells --- packages/core/test/integration/helpers/advertising.js | 7 ++++--- .../specs/Advertising/clickthrough_both.spec.js | 1 + .../Advertising/clickthrough_duplicate_skwcid.spec.js | 1 + .../specs/Advertising/clickthrough_efid.spec.js | 1 + .../specs/Advertising/clickthrough_skwcid.spec.js | 1 + .../integration/specs/Advertising/consent_gate.spec.js | 2 ++ .../integration/specs/Advertising/options_modes.spec.js | 1 + .../Advertising/handlers/clickThroughHandler.spec.js | 6 ++++++ .../Advertising/handlers/createAdConversionHandler.spec.js | 6 +++--- .../Advertising/handlers/viewThroughHandler.spec.js | 6 ++++++ .../specs/components/Advertising/utils/helpers.spec.js | 2 ++ 11 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/core/test/integration/helpers/advertising.js b/packages/core/test/integration/helpers/advertising.js index 4059e831a..06d37fadb 100644 --- a/packages/core/test/integration/helpers/advertising.js +++ b/packages/core/test/integration/helpers/advertising.js @@ -51,9 +51,10 @@ const getRequestBody = (call) => { export const findClickThroughCall = (calls) => calls.find((call) => { const body = getRequestBody(call); - const event = body.events?.[0]; - return ( - event?.xdm?.eventType === ADVERTISING_CONSTANTS.EVENT_TYPES.CLICK_THROUGH + return body.events?.some( + (event) => + event?.xdm?.eventType === + ADVERTISING_CONSTANTS.EVENT_TYPES.CLICK_THROUGH, ); }); diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js index d58bf89aa..57d216405 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js @@ -19,6 +19,7 @@ describe("Advertising - Clickthrough (both)", () => { }) => { worker.use(...[sendEventHandler]); + // FIXME: Mutates shared URL state and never restores it; leaks across specs. const url = new URL(window.location.href); url.searchParams.set("s_kwcid", "test_keyword_123"); url.searchParams.set("ef_id", "test_experiment_456"); diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js index 3be21c11c..e96151133 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js @@ -20,6 +20,7 @@ describe("Advertising - Clickthrough (duplicate s_kwcid)", () => { }) => { worker.use(...[sendEventHandler]); + // FIXME: Mutates shared URL state and never restores it; leaks across specs. const url = new URL(window.location.origin + window.location.pathname); url.searchParams.append("s_kwcid", "AL!first-keyword"); url.searchParams.append("s_kwcid", "AL!second-keyword"); diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js index a5b2fbdab..75f817cf1 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js @@ -20,6 +20,7 @@ describe("Advertising - Clickthrough (ef_id)", () => { worker.use(...[sendEventHandler]); // Simulate URL param BEFORE configure so component startup can detect it + // FIXME: Mutates shared URL state and never restores it; leaks across specs. const url = new URL(window.location.href); url.searchParams.set("ef_id", "test_experiment_456"); window.history.replaceState({}, "", url.toString()); diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js index c3585d6bc..4c050813c 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js @@ -23,6 +23,7 @@ describe("Advertising - Clickthrough (s_kwcid)", () => { worker.use(...[sendEventHandler]); // Simulate URL param BEFORE configure so component startup can detect it + // FIXME: Mutates shared URL state and never restores it; leaks across specs. const url = new URL(window.location.href); url.searchParams.set("s_kwcid", "test_keyword_123"); window.history.replaceState({}, "", url.toString()); diff --git a/packages/core/test/integration/specs/Advertising/consent_gate.spec.js b/packages/core/test/integration/specs/Advertising/consent_gate.spec.js index d10b2f985..4c79c2758 100644 --- a/packages/core/test/integration/specs/Advertising/consent_gate.spec.js +++ b/packages/core/test/integration/specs/Advertising/consent_gate.spec.js @@ -43,6 +43,7 @@ const clearUrlParams = () => { }; const cleanupAll = () => { + // FIXME: Cleanup is manually invoked in tests (not afterEach); failures can leak shared state. clearUrlParams(); clearCookie("advertising"); clearCookie("consent"); @@ -212,6 +213,7 @@ describe("Advertising - Consent gate", () => { event.preventDefault(); } }; + // FIXME: Listener cleanup is not in finally; assertion failures can leak this handler. window.addEventListener("unhandledrejection", suppressDeclined); cleanupAll(); diff --git a/packages/core/test/integration/specs/Advertising/options_modes.spec.js b/packages/core/test/integration/specs/Advertising/options_modes.spec.js index 312355625..7631d8b19 100644 --- a/packages/core/test/integration/specs/Advertising/options_modes.spec.js +++ b/packages/core/test/integration/specs/Advertising/options_modes.spec.js @@ -17,6 +17,7 @@ const getNamespacedAdvertisingCookieName = () => { const setAdvertisingCookie = (value) => { const name = getNamespacedAdvertisingCookieName(); + // FIXME: Writes process-global cookie state and this file never clears it between tests. document.cookie = `${name}=${value}; path=/`; }; diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js index ad5b113f3..11d92ecb9 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js @@ -17,9 +17,11 @@ import { LAST_CONVERSION_TIME_KEY, } from "../../../../../../src/components/Advertising/constants/index.js"; +// FIXME: Module-level global mutation leaks across files. // Mock network operations to prevent real network calls vi.mock("fetch", () => vi.fn()); +// FIXME: Overwrites runtime globals without guaranteed restoration. // Mock globalThis fetch and other network APIs Object.defineProperty(globalThis, "fetch", { value: vi.fn(() => @@ -31,6 +33,7 @@ Object.defineProperty(globalThis, "fetch", { writable: true, }); +// FIXME: Overwrites runtime globals without guaranteed restoration. // Mock XMLHttpRequest Object.defineProperty(globalThis, "XMLHttpRequest", { value: class MockXMLHttpRequest { @@ -49,6 +52,7 @@ Object.defineProperty(globalThis, "XMLHttpRequest", { writable: true, }); +// FIXME: Mutates document/window globals at module scope; leaks into unrelated specs. // Mock DOM operations to prevent network calls from script loading if (typeof globalThis.document !== "undefined") { globalThis.document.createElement = vi.fn(() => ({ @@ -71,6 +75,7 @@ if (typeof globalThis.window !== "undefined") { globalThis.window.removeEventListener = vi.fn(); } +// FIXME: Module mocks are leaky; use dependency injection instead. // Mock helpers with all functions that might make network calls vi.mock( "../../../../../../src/components/Advertising/utils/helpers.js", @@ -118,6 +123,7 @@ describe("Advertising::clickThroughHandler", () => { error: vi.fn(), }; + // FIXME: Date.now spy is never restored in this file. const fixedTs = Date.UTC(2024, 0, 1, 0, 0, 0); const mockNow = { valueOf: () => fixedTs, diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js index 21efbf15f..b293aeaf0 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js @@ -13,6 +13,7 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; import createAdConversionHandler from "../../../../../../src/components/Advertising/handlers/createAdConversionHandler.js"; +// FIXME: Module-level global mutation leaks across files. // Mock network operations to prevent real network calls vi.mock("fetch", () => vi.fn()); Object.defineProperty(globalThis, "fetch", { @@ -25,13 +26,12 @@ Object.defineProperty(globalThis, "fetch", { writable: true, }); +// FIXME: Module mocks are leaky; use dependency injection instead. // Mock dependencies vi.mock( "../../../../../../src/utils/request/createDataCollectionRequestPayload.js", ); -vi.mock( - "../../../../../../src/utils/request/createDataCollectionRequest.js", -); +vi.mock("../../../../../../src/utils/request/createDataCollectionRequest.js"); describe("Advertising::createAdConversionHandler", () => { let eventManager; diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js index 06ecae9f8..51a8ba801 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js @@ -28,6 +28,7 @@ Object.defineProperty(globalThis, "fetch", { writable: true, }); +// FIXME: Overwrites runtime globals without guaranteed restoration. // Mock XMLHttpRequest Object.defineProperty(globalThis, "XMLHttpRequest", { value: class MockXMLHttpRequest { @@ -46,6 +47,7 @@ Object.defineProperty(globalThis, "XMLHttpRequest", { writable: true, }); +// FIXME: Mutates document/window globals at module scope; leaks into unrelated specs. // Mock DOM operations to prevent network calls from script loading if (typeof globalThis.document !== "undefined") { globalThis.document.createElement = vi.fn(() => ({ @@ -66,6 +68,7 @@ if (typeof globalThis.document !== "undefined") { } if (typeof globalThis.window !== "undefined") { + // FIXME: Mutates document/window globals at module scope; leaks into unrelated specs. globalThis.window.addEventListener = vi.fn(); globalThis.window.removeEventListener = vi.fn(); globalThis.window.attachEvent = vi.fn(); @@ -74,11 +77,13 @@ if (typeof globalThis.window !== "undefined") { globalThis.window.ID5 = undefined; } +// FIXME: Module mocks are leaky; use dependency injection instead. // Mock dependencies vi.mock( "../../../../../../src/components/Advertising/identities/collectAllIdentities.js", ); +// FIXME: Module mocks are leaky; use dependency injection instead. // Mock helpers to prevent network calls vi.mock( "../../../../../../src/components/Advertising/utils/helpers.js", @@ -167,6 +172,7 @@ describe("Advertising::viewThroughHandler", () => { getBrowser = vi.fn(); + // FIXME: Date.now spy is never restored in this file. const fixedTs = Date.UTC(2024, 0, 1, 0, 0, 0); const mockNow = { valueOf: () => fixedTs, diff --git a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js index 9b9330a5e..d312840d2 100644 --- a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js @@ -28,6 +28,7 @@ import { AD_CONVERSION_VIEW_EVENT_TYPE, } from "../../../../../../src/components/Advertising/constants/index.js"; +// FIXME: Module mocks are leaky; use dependency injection instead. // Mock DOM utilities vi.mock("../../../../../../src/utils/dom/index.js", () => ({ awaitSelector: vi.fn(), @@ -78,6 +79,7 @@ describe("Advertising::helpers", () => { describe("getUrlParams", () => { afterEach(() => { + // FIXME: Mutates shared URL state; this must stay paired with robust cleanup. window.history.pushState({}, "", "/"); }); From b4b29604206958098a4e51349ab6cc92c5ca2791 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Mon, 2 Mar 2026 14:31:16 -0700 Subject: [PATCH 08/23] Annotate more leaky patterns in tests --- .../specs/Target/deduplication_spa.spec.js | 2 + .../components/BrandConcierge/index.spec.js | 73 +++++++++++-------- .../visitorService/awaitVisitorOptIn.spec.js | 1 + .../injectGetEcidFromVisitor.spec.js | 1 + .../createSendPushSubscriptionPayload.spec.js | 1 + .../makeSendPushSubscriptionRequest.spec.js | 1 + .../components/RulesEngine/index.spec.js | 1 + .../core/identity/createIdentity.spec.js | 1 + 8 files changed, 49 insertions(+), 32 deletions(-) diff --git a/packages/core/test/integration/specs/Target/deduplication_spa.spec.js b/packages/core/test/integration/specs/Target/deduplication_spa.spec.js index d7c7b1ed9..b7ec4f2cb 100644 --- a/packages/core/test/integration/specs/Target/deduplication_spa.spec.js +++ b/packages/core/test/integration/specs/Target/deduplication_spa.spec.js @@ -23,6 +23,7 @@ describe("Target Custom Code SPA", () => { worker, }) => { // Counter to track custom code executions + // FIXME: Mutates shared window state; cleanup is manual and not protected by finally. window.customCodeExecutionCount = 0; // Use the custom code handler @@ -62,6 +63,7 @@ describe("Target Custom Code SPA", () => { // and once from cached/persisted offers on second call expect(window.customCodeExecutionCount).toBe(2); + // FIXME: Cleanup runs only on happy path; failures can leak shared window state. // Clean up delete window.customCodeExecutionCount; }); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js index 377915e5d..ca0b63a56 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js @@ -17,6 +17,7 @@ describe("BrandConcierge", () => { let mockDependencies; beforeEach(() => { + // FIXME: Mutates global fetch and never restores original value in this file. // Mock window.fetch window.fetch = vi.fn(); @@ -24,21 +25,21 @@ describe("BrandConcierge", () => { loggingCookieJar: { remove: vi.fn(), get: vi.fn(), - set: vi.fn() + set: vi.fn(), }, logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), logOnBeforeNetworkRequest: vi.fn(), - logOnNetworkError: vi.fn() + logOnNetworkError: vi.fn(), }, eventManager: { - createEvent: vi.fn() + createEvent: vi.fn(), }, consent: { suspend: vi.fn(), - resume: vi.fn() + resume: vi.fn(), }, instanceName: "test-instance", sendEdgeNetworkRequest: vi.fn(), @@ -46,19 +47,19 @@ describe("BrandConcierge", () => { orgId: "testorgid@AdobeOrg", edgeConfigId: "test-edge-config-id", conversation: { - stickyConversationSession: false - } + stickyConversationSession: false, + }, }, lifecycle: { onBeforeEvent: vi.fn(), onBeforeRequest: vi.fn(), - onRequestFailure: vi.fn() + onRequestFailure: vi.fn(), }, cookieTransfer: { - cookiesToPayload: vi.fn() + cookiesToPayload: vi.fn(), }, createResponse: vi.fn(), - apexDomain: "adobe.com" + apexDomain: "adobe.com", }; }); @@ -78,18 +79,18 @@ describe("BrandConcierge", () => { const configWithSticky = { ...mockDependencies.config, conversation: { - stickyConversationSession: false - } + stickyConversationSession: false, + }, }; createConciergeComponent({ ...mockDependencies, - config: configWithSticky + config: configWithSticky, }); expect(mockDependencies.loggingCookieJar.remove).toHaveBeenCalledWith( "kndctr_testorgid_AdobeOrg_bc_session_id", - { domain: "adobe.com" } + { domain: "adobe.com" }, ); }); @@ -97,13 +98,13 @@ describe("BrandConcierge", () => { const configWithSticky = { ...mockDependencies.config, conversation: { - stickyConversationSession: true - } + stickyConversationSession: true, + }, }; createConciergeComponent({ ...mockDependencies, - config: configWithSticky + config: configWithSticky, }); expect(mockDependencies.loggingCookieJar.remove).not.toHaveBeenCalled(); @@ -112,15 +113,21 @@ describe("BrandConcierge", () => { it("sendConversationEvent command has options validator", () => { const component = createConciergeComponent(mockDependencies); - expect(component.commands.sendConversationEvent.optionsValidator).toBeDefined(); - expect(typeof component.commands.sendConversationEvent.optionsValidator).toBe("function"); + expect( + component.commands.sendConversationEvent.optionsValidator, + ).toBeDefined(); + expect( + typeof component.commands.sendConversationEvent.optionsValidator, + ).toBe("function"); }); it("sendConversationEvent command has run function", () => { const component = createConciergeComponent(mockDependencies); expect(component.commands.sendConversationEvent.run).toBeDefined(); - expect(typeof component.commands.sendConversationEvent.run).toBe("function"); + expect(typeof component.commands.sendConversationEvent.run).toBe( + "function", + ); }); }); @@ -128,25 +135,27 @@ describe("BrandConcierge config validators", () => { testConfigValidators({ configValidators: createConciergeComponent.configValidators, validConfigurations: [ - {conversation: { stickyConversationSession: true }}, - {conversation: { stickyConversationSession: false }}, - {conversation: { streamTimeout: 10000 }}, - {conversation: { streamTimeout: 20000 }}, - {conversation: { stickyConversationSession: true, streamTimeout: 10000 }}, - {} + { conversation: { stickyConversationSession: true } }, + { conversation: { stickyConversationSession: false } }, + { conversation: { streamTimeout: 10000 } }, + { conversation: { streamTimeout: 20000 } }, + { + conversation: { stickyConversationSession: true, streamTimeout: 10000 }, + }, + {}, ], invalidConfigurations: [ - {conversation: { stickyConversationSession: "invalid" }}, - {conversation: { stickyConversationSession: 123 }}, - {conversation: { streamTimeout: "invalid" }}, - {conversation: { streamTimeout: -1 }}, - {conversation: { streamTimeout: 1.5 }} + { conversation: { stickyConversationSession: "invalid" } }, + { conversation: { stickyConversationSession: 123 } }, + { conversation: { streamTimeout: "invalid" } }, + { conversation: { streamTimeout: -1 } }, + { conversation: { streamTimeout: 1.5 } }, ], - defaultValues: {} + defaultValues: {}, }); it("provides default values for concierge configuration", () => { const config = createConciergeComponent.configValidators({}); expect(config.conversation.stickyConversationSession).toBe(false); expect(config.conversation.streamTimeout).toBe(10000); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js b/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js index 6507d621e..7001384ae 100644 --- a/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js +++ b/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js @@ -18,6 +18,7 @@ const logger = { }; describe("awaitVisitorOptIn", () => { beforeEach(() => { + // FIXME: Mutates global window.adobe state; keep cleanup robust to avoid cross-spec leaks. window.adobe = undefined; }); afterAll(() => { diff --git a/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js b/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js index daa613a55..3075a03cd 100644 --- a/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js +++ b/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js @@ -29,6 +29,7 @@ Visitor.getInstance = () => { const orgId = "456org"; describe("getEcidFromVisitor", () => { beforeEach(() => { + // FIXME: Mutates global window.Visitor state; keep cleanup robust to avoid cross-spec leaks. window.Visitor = undefined; }); afterAll(() => { diff --git a/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js b/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js index 9263b1fe5..dd843bf1c 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js @@ -12,6 +12,7 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; +// FIXME: Module mocks are leaky; use dependency injection instead. vi.mock( "../../../../../../src/utils/request/createDataCollectionRequestPayload.js", ); diff --git a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js index 2d54079a3..f1fde62ab 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js @@ -12,6 +12,7 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; +// FIXME: Module mocks are leaky; use dependency injection instead. vi.mock( "../../../../../../src/components/PushNotifications/helpers/getPushSubscriptionDetails.js", ); diff --git a/packages/core/test/unit/specs/components/RulesEngine/index.spec.js b/packages/core/test/unit/specs/components/RulesEngine/index.spec.js index 3d13f312d..af7be7211 100644 --- a/packages/core/test/unit/specs/components/RulesEngine/index.spec.js +++ b/packages/core/test/unit/specs/components/RulesEngine/index.spec.js @@ -34,6 +34,7 @@ describe("createRulesEngine:commands:evaluateRulesets", () => { awaitConsent: vi.fn().mockReturnValue(awaitConsentDeferred.promise), }; getBrowser = vi.fn().mockReturnValue("foo"); + // FIXME: Mutates global window.referrer and never restores original value in this file. window.referrer = "https://www.google.com/search?q=adobe+journey+optimizer&oq=adobe+journey+optimizer"; diff --git a/packages/core/test/unit/specs/core/identity/createIdentity.spec.js b/packages/core/test/unit/specs/core/identity/createIdentity.spec.js index 97e8066a0..b5bf0f4c3 100644 --- a/packages/core/test/unit/specs/core/identity/createIdentity.spec.js +++ b/packages/core/test/unit/specs/core/identity/createIdentity.spec.js @@ -12,6 +12,7 @@ governing permissions and limitations under the License. import { describe, it, expect, vi, beforeEach } from "vitest"; +// FIXME: Module mocks are leaky; use dependency injection instead. vi.mock("../../../../../src/utils/createDecodeKndctrCookie.js", () => ({ default: vi.fn(), })); From 518a614e842af0ec9dd7150192c5c4debed3ffcf Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Tue, 3 Mar 2026 07:20:13 -0700 Subject: [PATCH 09/23] Fix formatting & linting of test and script files --- package.json | 6 +- .../BrandConcierge/sendConversationalEvent.js | 49 +++++---- .../test/unit/helpers/cleanUpDomChanges.js | 5 +- .../components/Advertising/index.spec.js | 2 +- .../createBuildEndpointUrl.spec.js | 44 +++++--- .../createConversationServiceRequest.spec.js | 20 ++-- .../createSendConversationEvent.spec.js | 104 ++++++++++-------- ...eateSendConversationServiceRequest.spec.js | 71 ++++++------ .../BrandConcierge/createStreamParser.spec.js | 6 +- .../createTimeoutWrapper.spec.js | 91 +++++++++++---- .../components/BrandConcierge/utils.spec.js | 19 ++-- .../BrandConcierge/validateMessage.spec.js | 1 - .../Consent/createComponent.spec.js | 5 +- .../specs/core/buildAndValidateConfig.spec.js | 5 +- .../core/test/unit/specs/utils/event.spec.js | 5 +- .../prepareConfigOverridesForEdge.spec.js | 2 +- .../validation/createArrayOfValidator.spec.js | 5 +- .../createNoUnknownFieldsValidator.spec.js | 5 +- .../createObjectOfValidator.spec.js | 5 +- .../validation/createRenamedValidator.spec.js | 5 +- 20 files changed, 264 insertions(+), 191 deletions(-) diff --git a/package.json b/package.json index 01d0adea5..a3b8daeef 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "scripts": { "clean": "rimraf dist distTest libEs5 libEs6 types", "lint": "eslint --cache --fix \"*.{js,cjs,mjs,jsx}\" \"{sandboxes/*/src,packages/*/src,test,scripts}/**/*.{js,cjs,mjs,jsx}\"", - "format": "prettier --write \"*.{html,js,cjs,mjs,jsx}\" \"{sandboxes/*/src,packages/*/src,test,scripts}/**/*.{html,js,cjs,mjs,jsx}\"", + "format": "prettier --write \"*.{html,js,cjs,mjs,jsx}\" \"{sandboxes/*/src,packages/*/{src,test,scripts}}/**/*.{html,js,cjs,mjs,jsx}\"", "types": "tsc", "test": "pnpm exec playwright install chromium && vitest run && pnpm run test:scripts", "test:coverage": "rimraf coverage && pnpm exec playwright install chromium && vitest run --coverage", @@ -55,13 +55,13 @@ "./*.{cjs,mjs,js,jsx}": [ "eslint --cache --fix" ], - "./{packages/*/src,test,scripts}/**/*.{cjs,mjs,js,jsx}": [ + "./{packages/*/{src,test,scripts}}/**/*.{cjs,mjs,js,jsx}": [ "eslint --cache --fix" ], "./*.{html,js,cjs,mjs,jsx}": [ "prettier --write" ], - "./{sandboxes/browser,packages/*/src,test,scripts}/**/*.{html,js,cjs,mjs,jsx}": [ + "./{sandboxes/browser,packages/*/{src,test,scripts}}/**/*.{html,js,cjs,mjs,jsx}": [ "prettier --write" ] }, diff --git a/packages/core/test/functional/specs/BrandConcierge/sendConversationalEvent.js b/packages/core/test/functional/specs/BrandConcierge/sendConversationalEvent.js index 14d717512..e9608b205 100644 --- a/packages/core/test/functional/specs/BrandConcierge/sendConversationalEvent.js +++ b/packages/core/test/functional/specs/BrandConcierge/sendConversationalEvent.js @@ -19,12 +19,9 @@ import createAlloyProxy from "../../helpers/createAlloyProxy.js"; import orgMainConfigMain from "../../helpers/constants/configParts/orgMainConfigMain.js"; const networkLogger = createNetworkLogger(); -const config = compose( - orgMainConfigMain, - { - stickyConversationSession: false, - }, -); +const config = compose(orgMainConfigMain, { + stickyConversationSession: false, +}); createFixture({ title: "BrandConcierge - sendConversationEvent", @@ -41,10 +38,10 @@ test.meta({ test("Test 1: C2590433 - Send conversational event with message only", async () => { const alloy = createAlloyProxy(); await alloy.configure(config); - + let streamResponseCalled = false; let capturedResponse = null; - + await alloy.sendConversationEvent({ message: "Hello, I need help with my order", onStreamResponse: (response) => { @@ -59,15 +56,19 @@ test("Test 1: C2590433 - Send conversational event with message only", async () const requestBody = JSON.parse(request.request.body); // Verify message is in the query parameter - await t.expect(requestBody.query.conversation.message).eql("Hello, I need help with my order"); + await t + .expect(requestBody.query.conversation.message) + .eql("Hello, I need help with my order"); await t.expect(requestBody.query.conversation.surfaces).ok(); - + // Verify event has ECID in identityMap await t.expect(requestBody.events[0].xdm.identityMap.ECID).ok(); await t.expect(requestBody.events[0].xdm.identityMap.ECID.length).gte(1); - + // Verify onStreamResponse was called with data - await t.expect(streamResponseCalled).ok("onStreamResponse callback should be called"); + await t + .expect(streamResponseCalled) + .ok("onStreamResponse callback should be called"); }); test.meta({ @@ -100,9 +101,11 @@ test("Test 2: C2590434 - Send conversational event with data object", async () = // Verify data is in the query parameter await t.expect(requestBody.query.conversation.data.type).eql("feedback"); await t.expect(requestBody.query.conversation.data.payload.rating).eql(5); - await t.expect(requestBody.query.conversation.data.payload.comment).eql("Great service!"); + await t + .expect(requestBody.query.conversation.data.payload.comment) + .eql("Great service!"); await t.expect(requestBody.query.conversation.surfaces).ok(); - + // Verify event has ECID in identityMap await t.expect(requestBody.events[0].xdm.identityMap.ECID).ok(); await t.expect(requestBody.events[0].xdm.identityMap.ECID.length).gte(1); @@ -139,11 +142,17 @@ test("Test 3: C2590435 - Send conversational event with feedback in XDM", async const requestBody = JSON.parse(request.request.body); // Verify XDM data is properly merged - await t.expect(requestBody.events[0].xdm.interactionId).eql("test-interaction-123"); - await t.expect(requestBody.events[0].xdm.conversationId).eql("test-conversation-456"); + await t + .expect(requestBody.events[0].xdm.interactionId) + .eql("test-interaction-123"); + await t + .expect(requestBody.events[0].xdm.conversationId) + .eql("test-conversation-456"); await t.expect(requestBody.events[0].xdm.conversation.feedback.rating).eql(4); - await t.expect(requestBody.events[0].xdm.conversation.feedback.comment).eql("Good bot response"); - + await t + .expect(requestBody.events[0].xdm.conversation.feedback.comment) + .eql("Good bot response"); + // Verify event has ECID in identityMap await t.expect(requestBody.events[0].xdm.identityMap.ECID).ok(); await t.expect(requestBody.events[0].xdm.identityMap.ECID.length).gte(1); @@ -206,6 +215,8 @@ test("Test 5: C2590437 - Send conversational event with missing required data fi } // Verify that an error was thrown - await t.expect(errorThrown).ok("Should throw error for missing required data.type field"); + await t + .expect(errorThrown) + .ok("Should throw error for missing required data.type field"); await t.expect(errorMessage).contains("", "Error message should be present"); }); diff --git a/packages/core/test/unit/helpers/cleanUpDomChanges.js b/packages/core/test/unit/helpers/cleanUpDomChanges.js index 82d694fa5..2413a6725 100644 --- a/packages/core/test/unit/helpers/cleanUpDomChanges.js +++ b/packages/core/test/unit/helpers/cleanUpDomChanges.js @@ -9,10 +9,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import { - selectNodes, - removeNode, -} from "../../../src/utils/dom/index.js"; +import { selectNodes, removeNode } from "../../../src/utils/dom/index.js"; /** * Removes container DOM nodes used for all the diff --git a/packages/core/test/unit/specs/components/Advertising/index.spec.js b/packages/core/test/unit/specs/components/Advertising/index.spec.js index ff4f9aba9..87711f7eb 100644 --- a/packages/core/test/unit/specs/components/Advertising/index.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/index.spec.js @@ -51,7 +51,7 @@ describe("Advertising::index", () => { }; getBrowser = vi.fn().mockReturnValue("Firefox"); getUrlParams = vi.fn().mockReturnValue({ skwcid: null, efid: null }); - + advertising = createAdvertising({ logger, config, diff --git a/packages/core/test/unit/specs/components/BrandConcierge/createBuildEndpointUrl.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/createBuildEndpointUrl.spec.js index 8e9348efd..025869507 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/createBuildEndpointUrl.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/createBuildEndpointUrl.spec.js @@ -19,17 +19,17 @@ describe("createBuildEndpointUrl", () => { beforeEach(() => { queryStringMock = { - stringify: vi.fn() + stringify: vi.fn(), }; mockRequest = { getRequestParams: vi.fn().mockReturnValue({ - existingParam: "value" + existingParam: "value", }), getId: vi.fn().mockReturnValue("test-request-id"), getEdgeSubPath: vi.fn().mockReturnValue("/v1/conversation"), getAction: vi.fn().mockReturnValue("chat"), - getDatastreamIdOverride: vi.fn().mockReturnValue(null) + getDatastreamIdOverride: vi.fn().mockReturnValue(null), }; buildEndpointUrl = createBuildEndpointUrl({ queryString: queryStringMock }); @@ -40,12 +40,14 @@ describe("createBuildEndpointUrl", () => { }); it("builds URL with request parameters", () => { - queryStringMock.stringify.mockReturnValue("existingParam=value&requestId=test-request-id&configId=test-datastream"); + queryStringMock.stringify.mockReturnValue( + "existingParam=value&requestId=test-request-id&configId=test-datastream", + ); const result = buildEndpointUrl({ edgeDomain: "edge.adobedc.net", request: mockRequest, - datastreamId: "test-datastream" + datastreamId: "test-datastream", }); expect(mockRequest.getRequestParams).toHaveBeenCalled(); @@ -55,44 +57,54 @@ describe("createBuildEndpointUrl", () => { expect(queryStringMock.stringify).toHaveBeenCalledWith({ existingParam: "value", requestId: "test-request-id", - configId: "test-datastream" + configId: "test-datastream", }); - expect(result).toBe("https://edge.adobedc.net/v1/conversation/chat?existingParam=value&requestId=test-request-id&configId=test-datastream"); + expect(result).toBe( + "https://edge.adobedc.net/v1/conversation/chat?existingParam=value&requestId=test-request-id&configId=test-datastream", + ); }); it("uses datastream override when available", () => { mockRequest.getDatastreamIdOverride.mockReturnValue("override-datastream"); - queryStringMock.stringify.mockReturnValue("existingParam=value&requestId=test-request-id&configId=override-datastream"); + queryStringMock.stringify.mockReturnValue( + "existingParam=value&requestId=test-request-id&configId=override-datastream", + ); const result = buildEndpointUrl({ edgeDomain: "edge.adobedc.net", request: mockRequest, - datastreamId: "default-datastream" + datastreamId: "default-datastream", }); expect(mockRequest.getDatastreamIdOverride).toHaveBeenCalled(); expect(queryStringMock.stringify).toHaveBeenCalledWith({ existingParam: "value", requestId: "test-request-id", - configId: "override-datastream" + configId: "override-datastream", }); - expect(result).toBe("https://edge.adobedc.net/v1/conversation/chat?existingParam=value&requestId=test-request-id&configId=override-datastream"); + expect(result).toBe( + "https://edge.adobedc.net/v1/conversation/chat?existingParam=value&requestId=test-request-id&configId=override-datastream", + ); }); it("handles empty request parameters", () => { mockRequest.getRequestParams.mockReturnValue({}); - queryStringMock.stringify.mockReturnValue("requestId=test-request-id&configId=test-datastream"); + queryStringMock.stringify.mockReturnValue( + "requestId=test-request-id&configId=test-datastream", + ); const result = buildEndpointUrl({ edgeDomain: "edge.adobedc.net", request: mockRequest, - datastreamId: "test-datastream" + datastreamId: "test-datastream", }); expect(queryStringMock.stringify).toHaveBeenCalledWith({ requestId: "test-request-id", - configId: "test-datastream" + configId: "test-datastream", }); - expect(result).toBe("https://edge.adobedc.net/v1/conversation/chat?requestId=test-request-id&configId=test-datastream"); + expect(result).toBe( + "https://edge.adobedc.net/v1/conversation/chat?requestId=test-request-id&configId=test-datastream", + ); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/createConversationServiceRequest.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/createConversationServiceRequest.spec.js index 08feff5a6..03cd10282 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/createConversationServiceRequest.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/createConversationServiceRequest.spec.js @@ -19,7 +19,7 @@ describe("createConversationServiceRequest", () => { beforeEach(() => { mockPayload = { - message: "test message" + message: "test message", }; mockSessionId = "test-session-123"; }); @@ -29,7 +29,7 @@ describe("createConversationServiceRequest", () => { return createConversationServiceRequest({ payload: mockPayload, sessionId: mockSessionId, - ...options + ...options, }); }; @@ -38,7 +38,7 @@ describe("createConversationServiceRequest", () => { it("creates a request with proper payload", () => { const request = createConversationServiceRequest({ payload: mockPayload, - sessionId: mockSessionId + sessionId: mockSessionId, }); expect(request).toBeDefined(); @@ -49,7 +49,7 @@ describe("createConversationServiceRequest", () => { it("provides the appropriate action", () => { const request = createConversationServiceRequest({ payload: mockPayload, - sessionId: mockSessionId + sessionId: mockSessionId, }); expect(request.getAction()).toBe("conversations"); @@ -59,7 +59,7 @@ describe("createConversationServiceRequest", () => { const request = createConversationServiceRequest({ payload: mockPayload, action: "custom-action", - sessionId: mockSessionId + sessionId: mockSessionId, }); expect(request.getAction()).toBe("custom-action"); @@ -68,7 +68,7 @@ describe("createConversationServiceRequest", () => { it("never uses sendBeacon", () => { const request = createConversationServiceRequest({ payload: mockPayload, - sessionId: mockSessionId + sessionId: mockSessionId, }); expect(request.getUseSendBeacon()).toBe(false); @@ -77,20 +77,20 @@ describe("createConversationServiceRequest", () => { it("includes sessionId in request parameters", () => { const request = createConversationServiceRequest({ payload: mockPayload, - sessionId: mockSessionId + sessionId: mockSessionId, }); expect(request.getRequestParams()).toEqual({ - sessionId: mockSessionId + sessionId: mockSessionId, }); }); it("uses correct edge subpath", () => { const request = createConversationServiceRequest({ payload: mockPayload, - sessionId: mockSessionId + sessionId: mockSessionId, }); expect(request.getEdgeSubPath()).toBe("/brand-concierge"); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationEvent.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationEvent.spec.js index 65cf8ff09..45893122a 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationEvent.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationEvent.spec.js @@ -23,7 +23,7 @@ describe("createSendConversationEvent", () => { mergeQuery: vi.fn(), mergeXdm: vi.fn(), finalize: vi.fn(), - mergeMeta: vi.fn() + mergeMeta: vi.fn(), }; mockDependencies = { @@ -35,10 +35,10 @@ describe("createSendConversationEvent", () => { error: vi.fn(), }, eventManager: { - createEvent: vi.fn().mockReturnValue(mockEvent) + createEvent: vi.fn().mockReturnValue(mockEvent), }, consent: { - current: vi.fn().mockReturnValue({state: "in"}) + current: vi.fn().mockReturnValue({ state: "in" }), }, instanceName: "test-instance", sendEdgeNetworkRequest: vi.fn(), @@ -49,22 +49,24 @@ describe("createSendConversationEvent", () => { edgeBasePath: "/ee", datastreamId: "test-datastream", conversation: { - streamTimeout: 10000 - } + streamTimeout: 10000, + }, }, - buildEndpointUrl: vi.fn().mockReturnValue("https://test.adobe.io/conversation"), + buildEndpointUrl: vi + .fn() + .mockReturnValue("https://test.adobe.io/conversation"), lifecycle: { onBeforeEvent: vi.fn().mockResolvedValue(undefined), onBeforeRequest: vi.fn(), - onRequestFailure: vi.fn() + onRequestFailure: vi.fn(), }, cookieTransfer: { cookiesToPayload: vi.fn(), - responseToCookies: vi.fn() + responseToCookies: vi.fn(), }, createResponse: vi.fn(), sendConversationServiceRequest: vi.fn(), - decodeKndctrCookie: vi.fn().mockReturnValue("test-ecid-123") + decodeKndctrCookie: vi.fn().mockReturnValue("test-ecid-123"), }; }); @@ -80,12 +82,14 @@ describe("createSendConversationEvent", () => { status: 200, body: createMockReadableStream([]), }; - mockDependencies.sendConversationServiceRequest.mockResolvedValue(mockResponse); + mockDependencies.sendConversationServiceRequest.mockResolvedValue( + mockResponse, + ); const sendConversationEvent = createSendConversationEvent(mockDependencies); const options = { message: "Hello, I need help with a product", - onStreamResponse: vi.fn() + onStreamResponse: vi.fn(), }; const resultPromise = sendConversationEvent(options); @@ -96,20 +100,22 @@ describe("createSendConversationEvent", () => { conversation: { surfaces: [expect.any(String)], message: "Hello, I need help with a product", - data: undefined - } + data: undefined, + }, }); expect(mockEvent.mergeXdm).toHaveBeenCalledWith({ identityMap: { ECID: [ { - id: "test-ecid-123" - } - ] - } + id: "test-ecid-123", + }, + ], + }, }); expect(mockDependencies.lifecycle.onBeforeEvent).toHaveBeenCalled(); - expect(mockDependencies.sendConversationServiceRequest).toHaveBeenCalled(); + expect( + mockDependencies.sendConversationServiceRequest, + ).toHaveBeenCalled(); return resultPromise; }); }); @@ -120,15 +126,17 @@ describe("createSendConversationEvent", () => { status: 200, body: createMockReadableStream([]), }; - mockDependencies.sendConversationServiceRequest.mockResolvedValue(mockResponse); + mockDependencies.sendConversationServiceRequest.mockResolvedValue( + mockResponse, + ); const sendConversationEvent = createSendConversationEvent(mockDependencies); const options = { xdm: { interactionId: "test-interaction-id", - conversationId: "test-conversation-id" + conversationId: "test-conversation-id", }, - onStreamResponse: vi.fn() + onStreamResponse: vi.fn(), }; const resultPromise = sendConversationEvent(options); @@ -139,16 +147,18 @@ describe("createSendConversationEvent", () => { identityMap: { ECID: [ { - id: "test-ecid-123" - } - ] - } + id: "test-ecid-123", + }, + ], + }, }); expect(mockEvent.mergeXdm).toHaveBeenCalledWith({ interactionId: "test-interaction-id", - conversationId: "test-conversation-id" + conversationId: "test-conversation-id", }); - expect(mockDependencies.sendConversationServiceRequest).toHaveBeenCalled(); + expect( + mockDependencies.sendConversationServiceRequest, + ).toHaveBeenCalled(); return resultPromise; }); }); @@ -159,7 +169,9 @@ describe("createSendConversationEvent", () => { status: 200, body: createMockReadableStream([]), }; - mockDependencies.sendConversationServiceRequest.mockResolvedValue(mockResponse); + mockDependencies.sendConversationServiceRequest.mockResolvedValue( + mockResponse, + ); const sendConversationEvent = createSendConversationEvent(mockDependencies); const options = { @@ -167,10 +179,10 @@ describe("createSendConversationEvent", () => { type: "feedback", payload: { rating: 5, - comment: "Great service!" - } + comment: "Great service!", + }, }, - onStreamResponse: vi.fn() + onStreamResponse: vi.fn(), }; const resultPromise = sendConversationEvent(options); @@ -185,12 +197,14 @@ describe("createSendConversationEvent", () => { type: "feedback", payload: { rating: 5, - comment: "Great service!" - } - } - } + comment: "Great service!", + }, + }, + }, }); - expect(mockDependencies.sendConversationServiceRequest).toHaveBeenCalled(); + expect( + mockDependencies.sendConversationServiceRequest, + ).toHaveBeenCalled(); return resultPromise; }); }); @@ -211,13 +225,15 @@ describe("createSendConversationEvent", () => { }, }, }; - mockDependencies.sendConversationServiceRequest.mockResolvedValue(mockResponse); + mockDependencies.sendConversationServiceRequest.mockResolvedValue( + mockResponse, + ); const sendConversationEvent = createSendConversationEvent(mockDependencies); const onStreamResponse = vi.fn(); const options = { message: "Hello, I need help", - onStreamResponse + onStreamResponse, }; const resultPromise = sendConversationEvent(options); @@ -226,24 +242,24 @@ describe("createSendConversationEvent", () => { // Fast-forward time by 10 seconds to trigger the timeout vi.advanceTimersByTime(10000); - return resultPromise.then(res => { + return resultPromise.then((res) => { // Verify that timeout error was logged expect(mockDependencies.logger.error).toHaveBeenCalledWith( "Stream error occurred", expect.objectContaining({ - message: "Stream timeout: No data received within 10 seconds" - }) + message: "Stream timeout: No data received within 10 seconds", + }), ); // Verify that onStreamResponse was called with the timeout error expect(onStreamResponse).toHaveBeenCalledWith({ error: expect.objectContaining({ - message: "Stream timeout: No data received within 10 seconds" - }) + message: "Stream timeout: No data received within 10 seconds", + }), }); vi.useRealTimers(); }); // await flushPromiseChains(); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationServiceRequest.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationServiceRequest.spec.js index 2c098aeac..0bd0082f7 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationServiceRequest.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/createSendConversationServiceRequest.spec.js @@ -24,32 +24,34 @@ describe("createSendConversationServiceRequest", () => { status: 200, body: { getReader: vi.fn().mockReturnValue({ - read: vi.fn().mockResolvedValue({ done: true, value: undefined }) - }) - } + read: vi.fn().mockResolvedValue({ done: true, value: undefined }), + }), + }, }; mockDependencies = { logger: { logOnBeforeNetworkRequest: vi.fn(), - logOnNetworkError: vi.fn() + logOnNetworkError: vi.fn(), }, - fetch: vi.fn().mockResolvedValue(mockResponse) + fetch: vi.fn().mockResolvedValue(mockResponse), }; mockRequest = { - getPayload: vi.fn().mockReturnValue({ message: "test message" }) + getPayload: vi.fn().mockReturnValue({ message: "test message" }), }; }); it("creates a send conversation service request function", () => { - const sendConversationServiceRequest = createSendConversationServiceRequest(mockDependencies); + const sendConversationServiceRequest = + createSendConversationServiceRequest(mockDependencies); expect(typeof sendConversationServiceRequest).toBe("function"); }); it("makes fetch request with correct parameters", async () => { - const sendConversationServiceRequest = createSendConversationServiceRequest(mockDependencies); + const sendConversationServiceRequest = + createSendConversationServiceRequest(mockDependencies); const requestId = "test-request-id"; const url = "https://test.adobe.io/conversation"; @@ -57,21 +59,22 @@ describe("createSendConversationServiceRequest", () => { requestId, url, request: mockRequest, - streamingEnabled: true + streamingEnabled: true, }); expect(mockDependencies.fetch).toHaveBeenCalledWith(url, { method: "POST", headers: { "Content-Type": "text/plain", - "Accept": "text/event-stream" + Accept: "text/event-stream", }, - body: JSON.stringify({ message: "test message" }) + body: JSON.stringify({ message: "test message" }), }); }); it("uses correct headers for non-streaming requests", async () => { - const sendConversationServiceRequest = createSendConversationServiceRequest(mockDependencies); + const sendConversationServiceRequest = + createSendConversationServiceRequest(mockDependencies); const requestId = "test-request-id"; const url = "https://test.adobe.io/conversation"; @@ -79,34 +82,37 @@ describe("createSendConversationServiceRequest", () => { requestId, url, request: mockRequest, - streamingEnabled: false + streamingEnabled: false, }); expect(mockDependencies.fetch).toHaveBeenCalledWith(url, { method: "POST", headers: { "Content-Type": "text/plain", - "Accept": "text/plain" + Accept: "text/plain", }, - body: JSON.stringify({ message: "test message" }) + body: JSON.stringify({ message: "test message" }), }); }); it("logs network request before sending", async () => { - const sendConversationServiceRequest = createSendConversationServiceRequest(mockDependencies); + const sendConversationServiceRequest = + createSendConversationServiceRequest(mockDependencies); const requestId = "test-request-id"; const url = "https://test.adobe.io/conversation"; await sendConversationServiceRequest({ requestId, url, - request: mockRequest + request: mockRequest, }); - expect(mockDependencies.logger.logOnBeforeNetworkRequest).toHaveBeenCalledWith({ + expect( + mockDependencies.logger.logOnBeforeNetworkRequest, + ).toHaveBeenCalledWith({ url, requestId, - payload: { message: "test message" } + payload: { message: "test message" }, }); }); @@ -116,7 +122,7 @@ describe("createSendConversationServiceRequest", () => { const failedResponse = { ok: false, - status: 500 + status: 500, }; mockDependencies.fetch @@ -124,14 +130,15 @@ describe("createSendConversationServiceRequest", () => { .mockResolvedValueOnce(failedResponse) .mockResolvedValueOnce(mockResponse); - const sendConversationServiceRequest = createSendConversationServiceRequest(mockDependencies); + const sendConversationServiceRequest = + createSendConversationServiceRequest(mockDependencies); const requestId = "test-request-id"; const url = "https://test.adobe.io/conversation"; const resultPromise = sendConversationServiceRequest({ requestId, url, - request: mockRequest + request: mockRequest, }); // Fast-forward through all the timers (2s + 3s delays) @@ -152,21 +159,22 @@ describe("createSendConversationServiceRequest", () => { const failedResponse = { ok: false, - status: 500 + status: 500, }; mockDependencies.fetch .mockResolvedValueOnce(failedResponse) .mockResolvedValueOnce(mockResponse); - const sendConversationServiceRequest = createSendConversationServiceRequest(mockDependencies); + const sendConversationServiceRequest = + createSendConversationServiceRequest(mockDependencies); const requestId = "test-request-id"; const url = "https://test.adobe.io/conversation"; const resultPromise = sendConversationServiceRequest({ requestId, url, - request: mockRequest + request: mockRequest, }); await vi.runAllTimersAsync(); @@ -176,7 +184,7 @@ describe("createSendConversationServiceRequest", () => { requestId, url, payload: { message: "test message" }, - error: expect.any(Error) + error: expect.any(Error), }); vi.useRealTimers(); @@ -188,20 +196,21 @@ describe("createSendConversationServiceRequest", () => { const failedResponse = { ok: false, - status: 500 + status: 500, }; mockDependencies.fetch.mockResolvedValue(failedResponse); - const sendConversationServiceRequest = createSendConversationServiceRequest(mockDependencies); + const sendConversationServiceRequest = + createSendConversationServiceRequest(mockDependencies); const requestId = "test-request-id"; const url = "https://test.adobe.io/conversation"; const resultPromise = sendConversationServiceRequest({ requestId, url, - request: mockRequest - }).catch(error => error); // Catch the error to prevent unhandled rejection + request: mockRequest, + }).catch((error) => error); // Catch the error to prevent unhandled rejection // Process timers and promises await vi.runAllTimersAsync(); @@ -215,4 +224,4 @@ describe("createSendConversationServiceRequest", () => { vi.useRealTimers(); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/createStreamParser.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/createStreamParser.spec.js index d22938610..1d289e5a9 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/createStreamParser.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/createStreamParser.spec.js @@ -124,7 +124,9 @@ describe("createStreamParser", () => { it("handles event types and IDs", async () => { const mockStream = createMockReadableStream([ - new TextEncoder().encode('event: message\ndata: {"text": "Hello"}\nid: 123\n\n'), + new TextEncoder().encode( + 'event: message\ndata: {"text": "Hello"}\nid: 123\n\n', + ), ]); await streamParser(mockStream, { onEvent, onPing, onComplete }); @@ -199,4 +201,4 @@ describe("createStreamParser", () => { expect(onEvent).toHaveBeenCalledWith({ error: expect.any(Error) }); expect(onComplete).toHaveBeenCalledTimes(1); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/createTimeoutWrapper.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/createTimeoutWrapper.spec.js index 1b7fd2e96..90c05041c 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/createTimeoutWrapper.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/createTimeoutWrapper.spec.js @@ -23,7 +23,10 @@ describe("createTimeoutWrapper", () => { it("returns object with onEvent, onPing, and onComplete handlers", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); expect(typeof wrapper.onEvent).toBe("function"); expect(typeof wrapper.onPing).toBe("function"); @@ -32,7 +35,10 @@ describe("createTimeoutWrapper", () => { it("calls callback immediately when data arrives before timeout", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); const testData = { data: "test data" }; wrapper.onEvent(testData); @@ -43,7 +49,10 @@ describe("createTimeoutWrapper", () => { it("resets timeout when data arrives", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Advance to just before timeout vi.advanceTimersByTime(9000); @@ -60,7 +69,10 @@ describe("createTimeoutWrapper", () => { it("resets timeout when ping arrives", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Advance to just before timeout vi.advanceTimersByTime(9000); @@ -80,14 +92,17 @@ describe("createTimeoutWrapper", () => { expect(mockCallback).toHaveBeenCalledTimes(1); expect(mockCallback).toHaveBeenCalledWith({ error: expect.objectContaining({ - message: "Stream timeout: No data received within 10 seconds" - }) + message: "Stream timeout: No data received within 10 seconds", + }), }); }); it("ping does not forward to callback", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); wrapper.onPing(); wrapper.onPing(); @@ -99,7 +114,10 @@ describe("createTimeoutWrapper", () => { it("calls callback with error when timeout occurs", () => { const mockCallback = vi.fn(); - createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Advance time to trigger timeout vi.advanceTimersByTime(10000); @@ -107,14 +125,17 @@ describe("createTimeoutWrapper", () => { expect(mockCallback).toHaveBeenCalledTimes(1); expect(mockCallback).toHaveBeenCalledWith({ error: expect.objectContaining({ - message: "Stream timeout: No data received within 10 seconds" - }) + message: "Stream timeout: No data received within 10 seconds", + }), }); }); it("ignores subsequent onEvent calls after timeout fires", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Trigger timeout vi.advanceTimersByTime(10000); @@ -130,7 +151,10 @@ describe("createTimeoutWrapper", () => { it("ignores subsequent onPing calls after timeout fires", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Trigger timeout vi.advanceTimersByTime(10000); @@ -146,7 +170,10 @@ describe("createTimeoutWrapper", () => { it("allows multiple data calls with timeout reset between each", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); wrapper.onEvent({ data: "chunk 1" }); vi.advanceTimersByTime(5000); @@ -166,7 +193,10 @@ describe("createTimeoutWrapper", () => { it("handles error events from stream", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); const streamError = { error: new Error("Stream error") }; wrapper.onEvent(streamError); @@ -176,7 +206,10 @@ describe("createTimeoutWrapper", () => { it("does not timeout if data arrives just before timeout", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Advance to just before timeout vi.advanceTimersByTime(9999); @@ -193,7 +226,10 @@ describe("createTimeoutWrapper", () => { it("does not timeout if ping arrives just before timeout", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Advance to just before timeout vi.advanceTimersByTime(9999); @@ -209,22 +245,25 @@ describe("createTimeoutWrapper", () => { it("maintains rolling timeout with mixed ping and data events", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // T=0: Start - vi.advanceTimersByTime(3000); // T=3s + vi.advanceTimersByTime(3000); // T=3s // Ping at T=3s wrapper.onPing(); - vi.advanceTimersByTime(4000); // T=7s + vi.advanceTimersByTime(4000); // T=7s // Data at T=7s wrapper.onEvent({ data: "chunk 1" }); - vi.advanceTimersByTime(5000); // T=12s + vi.advanceTimersByTime(5000); // T=12s // Ping at T=12s wrapper.onPing(); - vi.advanceTimersByTime(8000); // T=20s + vi.advanceTimersByTime(8000); // T=20s // Data at T=20s wrapper.onEvent({ data: "chunk 2" }); @@ -237,7 +276,10 @@ describe("createTimeoutWrapper", () => { it("clears timeout when onComplete is called", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Receive some data wrapper.onEvent({ data: "chunk 1" }); @@ -255,7 +297,10 @@ describe("createTimeoutWrapper", () => { it("clears timeout when onComplete is called even with no data", () => { const mockCallback = vi.fn(); - const wrapper = createTimeoutWrapper({ onStreamResponseCallback: mockCallback, streamTimeout: 10000 }); + const wrapper = createTimeoutWrapper({ + onStreamResponseCallback: mockCallback, + streamTimeout: 10000, + }); // Stream completes immediately with no data wrapper.onComplete(); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/utils.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/utils.spec.js index 5b4e1cbab..636c59388 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/utils.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/utils.spec.js @@ -10,7 +10,10 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ import { vi, beforeEach, afterEach, describe, it, expect } from "vitest"; -import { getPageSurface, getConciergeSessionCookie } from "../../../../../src/components/BrandConcierge/utils.js"; +import { + getPageSurface, + getConciergeSessionCookie, +} from "../../../../../src/components/BrandConcierge/utils.js"; describe("BrandConcierge utils", () => { describe("getConciergeSessionCookie", () => { @@ -19,10 +22,10 @@ describe("BrandConcierge utils", () => { beforeEach(() => { mockLoggingCookieJar = { - get: vi.fn() + get: vi.fn(), }; mockConfig = { - orgId: "test-org-id" + orgId: "test-org-id", }; }); @@ -32,11 +35,11 @@ describe("BrandConcierge utils", () => { const result = getConciergeSessionCookie({ loggingCookieJar: mockLoggingCookieJar, - config: mockConfig + config: mockConfig, }); expect(mockLoggingCookieJar.get).toHaveBeenCalledWith( - "kndctr_test-org-id_bc_session_id" + "kndctr_test-org-id_bc_session_id", ); expect(result).toBe(expectedCookieValue); }); @@ -46,12 +49,12 @@ describe("BrandConcierge utils", () => { getConciergeSessionCookie({ loggingCookieJar: mockLoggingCookieJar, - config: mockConfig + config: mockConfig, }); expect(mockLoggingCookieJar.get).toHaveBeenCalledWith( - "kndctr_different-org_bc_session_id" + "kndctr_different-org_bc_session_id", ); }); }); -}); \ No newline at end of file +}); diff --git a/packages/core/test/unit/specs/components/BrandConcierge/validateMessage.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/validateMessage.spec.js index 7c4263401..e76f39925 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/validateMessage.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/validateMessage.spec.js @@ -257,7 +257,6 @@ describe("BrandConcierge::validateMessage", () => { validateMessage({ options }); }).toThrowError(); }); - }); describe("AnyOf Permissive Behavior", () => { diff --git a/packages/core/test/unit/specs/components/Consent/createComponent.spec.js b/packages/core/test/unit/specs/components/Consent/createComponent.spec.js index 0bb66929d..cba7c13c2 100644 --- a/packages/core/test/unit/specs/components/Consent/createComponent.spec.js +++ b/packages/core/test/unit/specs/components/Consent/createComponent.spec.js @@ -12,10 +12,7 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; import createComponent from "../../../../../src/components/Consent/createComponent.js"; -import { - createTaskQueue, - defer, -} from "../../../../../src/utils/index.js"; +import { createTaskQueue, defer } from "../../../../../src/utils/index.js"; import flushPromiseChains from "../../../helpers/flushPromiseChains.js"; const createConsent = (generalConsent) => ({ diff --git a/packages/core/test/unit/specs/core/buildAndValidateConfig.spec.js b/packages/core/test/unit/specs/core/buildAndValidateConfig.spec.js index e019f5c06..182b23e98 100644 --- a/packages/core/test/unit/specs/core/buildAndValidateConfig.spec.js +++ b/packages/core/test/unit/specs/core/buildAndValidateConfig.spec.js @@ -13,10 +13,7 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; import buildAndValidateConfig from "../../../../src/core/buildAndValidateConfig.js"; import createConfig from "../../../../src/core/config/createConfig.js"; -import { - boolean, - objectOf, -} from "../../../../src/utils/validation/index.js"; +import { boolean, objectOf } from "../../../../src/utils/validation/index.js"; describe("buildAndValidateConfig", () => { let options; diff --git a/packages/core/test/unit/specs/utils/event.spec.js b/packages/core/test/unit/specs/utils/event.spec.js index c41e65e33..05980b441 100644 --- a/packages/core/test/unit/specs/utils/event.spec.js +++ b/packages/core/test/unit/specs/utils/event.spec.js @@ -11,10 +11,7 @@ governing permissions and limitations under the License. */ import { vi, beforeEach, describe, it, expect } from "vitest"; -import { - mergeDecisionsMeta, - mergeQuery, -} from "../../../../src/utils/event.js"; +import { mergeDecisionsMeta, mergeQuery } from "../../../../src/utils/event.js"; import { PropositionEventType } from "../../../../src/constants/propositionEventType.js"; describe("Utils::event", () => { diff --git a/packages/core/test/unit/specs/utils/prepareConfigOverridesForEdge.spec.js b/packages/core/test/unit/specs/utils/prepareConfigOverridesForEdge.spec.js index ca19cde41..db3565eea 100644 --- a/packages/core/test/unit/specs/utils/prepareConfigOverridesForEdge.spec.js +++ b/packages/core/test/unit/specs/utils/prepareConfigOverridesForEdge.spec.js @@ -91,7 +91,7 @@ describe("utils:prepareConfigOverridesForEdge", () => { }, other_key: { other_value: true, - } + }, }), ).toEqual({ com_adobe_analytics: { diff --git a/packages/core/test/unit/specs/utils/validation/createArrayOfValidator.spec.js b/packages/core/test/unit/specs/utils/validation/createArrayOfValidator.spec.js index 25e71191b..64ea3bbd4 100644 --- a/packages/core/test/unit/specs/utils/validation/createArrayOfValidator.spec.js +++ b/packages/core/test/unit/specs/utils/validation/createArrayOfValidator.spec.js @@ -11,10 +11,7 @@ governing permissions and limitations under the License. */ import { describe } from "vitest"; -import { - arrayOf, - string, -} from "../../../../../src/utils/validation/index.js"; +import { arrayOf, string } from "../../../../../src/utils/validation/index.js"; import describeValidation from "../../../helpers/describeValidation.js"; describe("validation::arrayOf", () => { diff --git a/packages/core/test/unit/specs/utils/validation/createNoUnknownFieldsValidator.spec.js b/packages/core/test/unit/specs/utils/validation/createNoUnknownFieldsValidator.spec.js index 0bcd84fdb..639a10fb2 100644 --- a/packages/core/test/unit/specs/utils/validation/createNoUnknownFieldsValidator.spec.js +++ b/packages/core/test/unit/specs/utils/validation/createNoUnknownFieldsValidator.spec.js @@ -11,10 +11,7 @@ governing permissions and limitations under the License. */ import { describe } from "vitest"; -import { - objectOf, - string, -} from "../../../../../src/utils/validation/index.js"; +import { objectOf, string } from "../../../../../src/utils/validation/index.js"; import describeValidation from "../../../helpers/describeValidation.js"; describe("validation::noUnknownFields", () => { diff --git a/packages/core/test/unit/specs/utils/validation/createObjectOfValidator.spec.js b/packages/core/test/unit/specs/utils/validation/createObjectOfValidator.spec.js index 9703d4af3..7afb12459 100644 --- a/packages/core/test/unit/specs/utils/validation/createObjectOfValidator.spec.js +++ b/packages/core/test/unit/specs/utils/validation/createObjectOfValidator.spec.js @@ -11,10 +11,7 @@ governing permissions and limitations under the License. */ import { describe } from "vitest"; -import { - objectOf, - string, -} from "../../../../../src/utils/validation/index.js"; +import { objectOf, string } from "../../../../../src/utils/validation/index.js"; import describeValidation from "../../../helpers/describeValidation.js"; describe("validation::objectOf", () => { diff --git a/packages/core/test/unit/specs/utils/validation/createRenamedValidator.spec.js b/packages/core/test/unit/specs/utils/validation/createRenamedValidator.spec.js index 3f9bcb577..f506217a9 100644 --- a/packages/core/test/unit/specs/utils/validation/createRenamedValidator.spec.js +++ b/packages/core/test/unit/specs/utils/validation/createRenamedValidator.spec.js @@ -11,10 +11,7 @@ governing permissions and limitations under the License. */ import { describe } from "vitest"; -import { - objectOf, - string, -} from "../../../../../src/utils/validation/index.js"; +import { objectOf, string } from "../../../../../src/utils/validation/index.js"; import describeValidation from "../../../helpers/describeValidation.js"; describe("validation::renamed", () => { From e2a80b4bbba7c16a8b9113cfb4e7b20c0691cd08 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Tue, 3 Mar 2026 11:06:45 -0700 Subject: [PATCH 10/23] Create and use a location helper for safe url modifications during tests --- .../integration/helpers/utils/location.js | 50 ++++ .../Advertising/clickthrough_both.spec.js | 41 ++-- .../clickthrough_duplicate_skwcid.spec.js | 44 ++-- .../Advertising/clickthrough_efid.spec.js | 37 +-- .../Advertising/clickthrough_skwcid.spec.js | 35 +-- .../specs/Advertising/consent_gate.spec.js | 215 +++++++++--------- 6 files changed, 255 insertions(+), 167 deletions(-) create mode 100644 packages/core/test/integration/helpers/utils/location.js diff --git a/packages/core/test/integration/helpers/utils/location.js b/packages/core/test/integration/helpers/utils/location.js new file mode 100644 index 000000000..56e5f0c7a --- /dev/null +++ b/packages/core/test/integration/helpers/utils/location.js @@ -0,0 +1,50 @@ +/* +Copyright 2026 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ +// @ts-check +/** + * @template T + * @param {(context: { + * currentHref: string, + * applyUrl: (nextUrl: string | URL, state?: unknown) => void, + * }) => T | Promise} operations + * @returns {Promise} + * + * @example + * await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + * const url = new URL(currentHref); + * url.searchParams.set("s_kwcid", "test_keyword_123"); + * applyUrl(url); + * await alloy("sendEvent"); + * }); + */ +export const withTemporaryUrl = async (operations) => { + const originalHref = window.location.href; + const originalState = window.history.state; + const originalUrl = new URL(originalHref); + + /** @type {(nextUrl: string | URL, state?: unknown) => void} */ + const applyUrl = (nextUrl, state = window.history.state) => { + const targetUrl = new URL(String(nextUrl), originalHref); + + if (targetUrl.origin !== originalUrl.origin) { + throw new Error("withTemporaryUrl only supports same-origin URLs."); + } + + window.history.replaceState(state, "", targetUrl.toString()); + }; + + try { + return await operations({ currentHref: originalHref, applyUrl }); + } finally { + window.history.replaceState(originalState, "", originalHref); + } +}; diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js index 57d216405..3c23340f7 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_both.spec.js @@ -1,6 +1,13 @@ /* Copyright 2025 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. */ import { test, describe, expect } from "../../helpers/testsSetup/extend.js"; import { sendEventHandler } from "../../helpers/mswjs/handlers.js"; @@ -10,6 +17,7 @@ import { findClickThroughCall, validateClickThroughCall, } from "../../helpers/advertising.js"; +import { withTemporaryUrl } from "../../helpers/utils/location.js"; describe("Advertising - Clickthrough (both)", () => { test("should send advertising.enrichment_ct when both s_kwcid and ef_id are present", async ({ @@ -19,25 +27,26 @@ describe("Advertising - Clickthrough (both)", () => { }) => { worker.use(...[sendEventHandler]); - // FIXME: Mutates shared URL state and never restores it; leaks across specs. - const url = new URL(window.location.href); - url.searchParams.set("s_kwcid", "test_keyword_123"); - url.searchParams.set("ef_id", "test_experiment_456"); - window.history.replaceState({}, "", url.toString()); + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.searchParams.set("s_kwcid", "test_keyword_123"); + url.searchParams.set("ef_id", "test_experiment_456"); + applyUrl(url); - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - }); + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + }); - await alloy("sendEvent"); + await alloy("sendEvent"); - const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); - const conversionCall = findClickThroughCall(calls); - expect(conversionCall).toBeTruthy(); - validateClickThroughCall(conversionCall, { - sampleGroupId: "test_keyword_123", - experimentId: "test_experiment_456", + const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); + const conversionCall = findClickThroughCall(calls); + expect(conversionCall).toBeTruthy(); + validateClickThroughCall(conversionCall, { + sampleGroupId: "test_keyword_123", + experimentId: "test_experiment_456", + }); }); }); }); diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js index e96151133..2c40d9805 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js @@ -1,6 +1,13 @@ /* Copyright 2025 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. */ import { test, describe, expect } from "../../helpers/testsSetup/extend.js"; @@ -11,6 +18,7 @@ import { findClickThroughCall, validateClickThroughCall, } from "../../helpers/advertising.js"; +import { withTemporaryUrl } from "../../helpers/utils/location.js"; describe("Advertising - Clickthrough (duplicate s_kwcid)", () => { test("should use the first s_kwcid value when duplicates are present", async ({ @@ -20,26 +28,28 @@ describe("Advertising - Clickthrough (duplicate s_kwcid)", () => { }) => { worker.use(...[sendEventHandler]); - // FIXME: Mutates shared URL state and never restores it; leaks across specs. - const url = new URL(window.location.origin + window.location.pathname); - url.searchParams.append("s_kwcid", "AL!first-keyword"); - url.searchParams.append("s_kwcid", "AL!second-keyword"); - url.searchParams.set("ef_id", "test_experiment_456"); - window.history.replaceState({}, "", url.toString()); + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const currentUrl = new URL(currentHref); + const url = new URL(currentUrl.origin + currentUrl.pathname); + url.searchParams.append("s_kwcid", "AL!first-keyword"); + url.searchParams.append("s_kwcid", "AL!second-keyword"); + url.searchParams.set("ef_id", "test_experiment_456"); + applyUrl(url); - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - }); + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + }); - await alloy("sendEvent"); + await alloy("sendEvent"); - const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); - const conversionCall = findClickThroughCall(calls); - expect(conversionCall).toBeTruthy(); - validateClickThroughCall(conversionCall, { - sampleGroupId: "AL!first-keyword", - experimentId: "test_experiment_456", + const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); + const conversionCall = findClickThroughCall(calls); + expect(conversionCall).toBeTruthy(); + validateClickThroughCall(conversionCall, { + sampleGroupId: "AL!first-keyword", + experimentId: "test_experiment_456", + }); }); }); }); diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js index 75f817cf1..416bdcd3b 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_efid.spec.js @@ -1,6 +1,13 @@ /* Copyright 2025 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. */ import { test, describe, expect } from "../../helpers/testsSetup/extend.js"; import { sendEventHandler } from "../../helpers/mswjs/handlers.js"; @@ -10,6 +17,7 @@ import { findClickThroughCall, validateClickThroughCall, } from "../../helpers/advertising.js"; +import { withTemporaryUrl } from "../../helpers/utils/location.js"; describe("Advertising - Clickthrough (ef_id)", () => { test("should send advertising.enrichment_ct when ef_id is present", async ({ @@ -20,23 +28,24 @@ describe("Advertising - Clickthrough (ef_id)", () => { worker.use(...[sendEventHandler]); // Simulate URL param BEFORE configure so component startup can detect it - // FIXME: Mutates shared URL state and never restores it; leaks across specs. - const url = new URL(window.location.href); - url.searchParams.set("ef_id", "test_experiment_456"); - window.history.replaceState({}, "", url.toString()); + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.searchParams.set("ef_id", "test_experiment_456"); + applyUrl(url); - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - }); + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + }); - await alloy("sendEvent"); + await alloy("sendEvent"); - const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); - const conversionCall = findClickThroughCall(calls); - expect(conversionCall).toBeTruthy(); - validateClickThroughCall(conversionCall, { - experimentId: "test_experiment_456", + const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); + const conversionCall = findClickThroughCall(calls); + expect(conversionCall).toBeTruthy(); + validateClickThroughCall(conversionCall, { + experimentId: "test_experiment_456", + }); }); }); }); diff --git a/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js b/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js index 4c050813c..92c8e2391 100644 --- a/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js +++ b/packages/core/test/integration/specs/Advertising/clickthrough_skwcid.spec.js @@ -3,6 +3,11 @@ Copyright 2025 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. */ import { test, describe, expect } from "../../helpers/testsSetup/extend.js"; @@ -13,6 +18,7 @@ import { findClickThroughCall, validateClickThroughCall, } from "../../helpers/advertising.js"; +import { withTemporaryUrl } from "../../helpers/utils/location.js"; describe("Advertising - Clickthrough (s_kwcid)", () => { test("should send advertising.enrichment_ct when s_kwcid is present", async ({ @@ -23,23 +29,24 @@ describe("Advertising - Clickthrough (s_kwcid)", () => { worker.use(...[sendEventHandler]); // Simulate URL param BEFORE configure so component startup can detect it - // FIXME: Mutates shared URL state and never restores it; leaks across specs. - const url = new URL(window.location.href); - url.searchParams.set("s_kwcid", "test_keyword_123"); - window.history.replaceState({}, "", url.toString()); + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.searchParams.set("s_kwcid", "test_keyword_123"); + applyUrl(url); - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - }); + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + }); - await alloy("sendEvent"); + await alloy("sendEvent"); - const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); - const conversionCall = findClickThroughCall(calls); - expect(conversionCall).toBeTruthy(); - validateClickThroughCall(conversionCall, { - sampleGroupId: "test_keyword_123", + const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/); + const conversionCall = findClickThroughCall(calls); + expect(conversionCall).toBeTruthy(); + validateClickThroughCall(conversionCall, { + sampleGroupId: "test_keyword_123", + }); }); }); }); diff --git a/packages/core/test/integration/specs/Advertising/consent_gate.spec.js b/packages/core/test/integration/specs/Advertising/consent_gate.spec.js index 4c79c2758..c2c6ab264 100644 --- a/packages/core/test/integration/specs/Advertising/consent_gate.spec.js +++ b/packages/core/test/integration/specs/Advertising/consent_gate.spec.js @@ -15,6 +15,7 @@ import { test, describe, expect } from "../../helpers/testsSetup/extend.js"; import { sendEventHandler } from "../../helpers/mswjs/handlers.js"; import alloyConfig from "../../helpers/alloy/config.js"; import { createAdvertisingConfig } from "../../helpers/advertising.js"; +import { withTemporaryUrl } from "../../helpers/utils/location.js"; import waitFor from "../../helpers/utils/waitFor.js"; const getNamespacedCookieName = (key) => { @@ -35,16 +36,8 @@ const clearCookie = (key) => { document.cookie = `${name}=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/`; }; -const clearUrlParams = () => { - const url = new URL(window.location.href); - url.searchParams.delete("s_kwcid"); - url.searchParams.delete("ef_id"); - window.history.replaceState({}, "", url.toString()); -}; - const cleanupAll = () => { // FIXME: Cleanup is manually invoked in tests (not afterEach); failures can leak shared state. - clearUrlParams(); clearCookie("advertising"); clearCookie("consent"); }; @@ -90,24 +83,26 @@ describe("Advertising - Consent gate", () => { worker.use(sendEventHandler, setConsentAcceptHandler); // Set URL params for click-through BEFORE configure - const url = new URL(window.location.href); - url.searchParams.set("s_kwcid", "AL!test_kwcid_123"); - url.searchParams.set("ef_id", "test_efid_456"); - window.history.replaceState({}, "", url.toString()); - - // Configure with consent pending — advertising component should NOT - // write any cookies until consent is granted. - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - defaultConsent: "pending", - }); + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.searchParams.set("s_kwcid", "AL!test_kwcid_123"); + url.searchParams.set("ef_id", "test_efid_456"); + applyUrl(url); + + // Configure with consent pending — advertising component should NOT + // write any cookies until consent is granted. + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + defaultConsent: "pending", + }); - // Wait enough time for the component to have set cookies if it were going to. - await waitFor(500); + // Wait enough time for the component to have set cookies if it were going to. + await waitFor(500); - // Verify: NO advertising cookie should exist while consent is pending. - expect(getAdvertisingCookie()).toBeNull(); + // Verify: NO advertising cookie should exist while consent is pending. + expect(getAdvertisingCookie()).toBeNull(); + }); cleanupAll(); }); @@ -144,55 +139,59 @@ describe("Advertising - Consent gate", () => { worker.use(sendEventHandler, setConsentAcceptHandler); // Set URL params for click-through - const url = new URL(window.location.href); - url.searchParams.set("s_kwcid", "AL!test_kwcid_789"); - url.searchParams.set("ef_id", "test_efid_012"); - window.history.replaceState({}, "", url.toString()); - - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - defaultConsent: "pending", - }); + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.searchParams.set("s_kwcid", "AL!test_kwcid_789"); + url.searchParams.set("ef_id", "test_efid_012"); + applyUrl(url); + + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + defaultConsent: "pending", + }); - // Verify no cookies yet - await waitFor(300); - expect(getAdvertisingCookie()).toBeNull(); + // Verify no cookies yet + await waitFor(300); + expect(getAdvertisingCookie()).toBeNull(); - // Grant consent — the mock returns a state:store handle that sets the - // consent cookie, which makes the SDK transition consent to "in". - await alloy("setConsent", { - consent: [ - { - standard: "Adobe", - version: "1.0", - value: { general: "in" }, - }, - ], - }); + // Grant consent — the mock returns a state:store handle that sets the + // consent cookie, which makes the SDK transition consent to "in". + await alloy("setConsent", { + consent: [ + { + standard: "Adobe", + version: "1.0", + value: { general: "in" }, + }, + ], + }); - // Wait for the fire-and-forget conversion flow to complete. - // After consent transitions to "in", sendAdConversion resumes, - // writes cookies, and sends the conversion network call. - await waitFor(3000); + // Wait for the fire-and-forget conversion flow to complete. + // After consent transitions to "in", sendAdConversion resumes, + // writes cookies, and sends the conversion network call. + await waitFor(3000); - // Verify: advertising cookie should now exist (written during click-through) - const cookieValue = getAdvertisingCookie(); - expect(cookieValue).not.toBeNull(); + // Verify: advertising cookie should now exist (written during click-through) + const cookieValue = getAdvertisingCookie(); + expect(cookieValue).not.toBeNull(); - // Verify a click-through conversion call was made - const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/, { - retries: 10, - delayMs: 200, - }); - const conversionCall = calls.find((call) => { - const body = - typeof call.request.body === "string" - ? JSON.parse(call.request.body) - : call.request.body; - return body?.events?.[0]?.xdm?.eventType === "advertising.enrichment_ct"; + // Verify a click-through conversion call was made + const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/, { + retries: 10, + delayMs: 200, + }); + const conversionCall = calls.find((call) => { + const body = + typeof call.request.body === "string" + ? JSON.parse(call.request.body) + : call.request.body; + return ( + body?.events?.[0]?.xdm?.eventType === "advertising.enrichment_ct" + ); + }); + expect(conversionCall).toBeTruthy(); }); - expect(conversionCall).toBeTruthy(); cleanupAll(); }); @@ -249,51 +248,55 @@ describe("Advertising - Consent gate", () => { worker.use(sendEventHandler, setConsentDeclineHandler); // Set URL params for click-through - const url = new URL(window.location.href); - url.searchParams.set("s_kwcid", "AL!test_kwcid_reject"); - url.searchParams.set("ef_id", "test_efid_reject"); - window.history.replaceState({}, "", url.toString()); + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.searchParams.set("s_kwcid", "AL!test_kwcid_reject"); + url.searchParams.set("ef_id", "test_efid_reject"); + applyUrl(url); + + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + defaultConsent: "pending", + }); - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - defaultConsent: "pending", - }); + // Verify no cookies yet + await waitFor(300); + expect(getAdvertisingCookie()).toBeNull(); - // Verify no cookies yet - await waitFor(300); - expect(getAdvertisingCookie()).toBeNull(); + // Decline consent + await alloy("setConsent", { + consent: [ + { + standard: "Adobe", + version: "1.0", + value: { general: "out" }, + }, + ], + }); - // Decline consent - await alloy("setConsent", { - consent: [ - { - standard: "Adobe", - version: "1.0", - value: { general: "out" }, - }, - ], - }); + // Wait to ensure nothing fires after decline + await waitFor(500); - // Wait to ensure nothing fires after decline - await waitFor(500); + // Verify: NO advertising cookie should exist after decline + expect(getAdvertisingCookie()).toBeNull(); - // Verify: NO advertising cookie should exist after decline - expect(getAdvertisingCookie()).toBeNull(); - - // Verify no conversion calls were made (only consent call should exist) - const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/, { - retries: 3, - delayMs: 100, - }); - const conversionCall = calls.find((call) => { - const body = - typeof call.request.body === "string" - ? JSON.parse(call.request.body) - : call.request.body; - return body?.events?.[0]?.xdm?.eventType === "advertising.enrichment_ct"; + // Verify no conversion calls were made (only consent call should exist) + const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/, { + retries: 3, + delayMs: 100, + }); + const conversionCall = calls.find((call) => { + const body = + typeof call.request.body === "string" + ? JSON.parse(call.request.body) + : call.request.body; + return ( + body?.events?.[0]?.xdm?.eventType === "advertising.enrichment_ct" + ); + }); + expect(conversionCall).toBeFalsy(); }); - expect(conversionCall).toBeFalsy(); cleanupAll(); window.removeEventListener("unhandledrejection", suppressDeclined); From 1d4c798eca291e8725a73af8a9c1f294b0a1f933 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Tue, 3 Mar 2026 14:03:50 -0700 Subject: [PATCH 11/23] Rewrite adConversionHandler tests --- .../createAdConversionHandler.spec.js | 239 +++--------------- 1 file changed, 30 insertions(+), 209 deletions(-) diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js index b293aeaf0..bbe8a2526 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/createAdConversionHandler.spec.js @@ -13,40 +13,15 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; import createAdConversionHandler from "../../../../../../src/components/Advertising/handlers/createAdConversionHandler.js"; -// FIXME: Module-level global mutation leaks across files. -// Mock network operations to prevent real network calls -vi.mock("fetch", () => vi.fn()); -Object.defineProperty(globalThis, "fetch", { - value: vi.fn(() => - Promise.resolve({ - ok: true, - json: () => Promise.resolve({ success: true }), - }), - ), - writable: true, -}); - -// FIXME: Module mocks are leaky; use dependency injection instead. -// Mock dependencies -vi.mock( - "../../../../../../src/utils/request/createDataCollectionRequestPayload.js", -); -vi.mock("../../../../../../src/utils/request/createDataCollectionRequest.js"); - describe("Advertising::createAdConversionHandler", () => { - let eventManager; let sendEdgeNetworkRequest; let consent; let logger; let handler; - let createDataCollectionRequestPayload; - let createDataCollectionRequest; - - beforeEach(async () => { - eventManager = { - createEvent: vi.fn(), - }; + let event; + let request; + beforeEach(() => { sendEdgeNetworkRequest = vi.fn(); consent = { @@ -54,212 +29,58 @@ describe("Advertising::createAdConversionHandler", () => { }; logger = { - info: vi.fn(), error: vi.fn(), - debug: vi.fn(), - warn: vi.fn(), }; - // Mock the request creation functions - const mockCreateDataCollectionRequestPayload = await import( - "../../../../../../src/utils/request/createDataCollectionRequestPayload.js" - ); - const mockCreateDataCollectionRequest = await import( - "../../../../../../src/utils/request/createDataCollectionRequest.js" - ); + event = { + finalize: vi.fn(), + }; - createDataCollectionRequestPayload = - mockCreateDataCollectionRequestPayload.default; - createDataCollectionRequest = mockCreateDataCollectionRequest.default; + request = { id: "request" }; - createDataCollectionRequestPayload.mockReset(); - createDataCollectionRequest.mockReset(); + const createPayload = vi.fn(() => ({ + addEvent: vi.fn(), + })); + const createRequest = vi.fn(() => { + return request; + }); handler = createAdConversionHandler({ sendEdgeNetworkRequest, consent, - createDataCollectionRequest, - createDataCollectionRequestPayload, + createDataCollectionRequest: createRequest, + createDataCollectionRequestPayload: createPayload, logger, }); }); describe("trackAdConversion", () => { - it("should create and send conversion request", async () => { - const mockEvent = { - finalize: vi.fn(), - }; - - const mockPayload = { - addEvent: vi.fn(), - }; - - const mockRequest = { - body: { events: [] }, - getUseIdThirdPartyDomain: vi.fn().mockReturnValue(false), - }; - - createDataCollectionRequestPayload.mockReturnValue(mockPayload); - createDataCollectionRequest.mockReturnValue(mockRequest); + it("returns success and coordinates collaborators, with consent", async () => { sendEdgeNetworkRequest.mockResolvedValue({ status: "success" }); - const result = await handler.trackAdConversion({ event: mockEvent }); + const result = await handler.trackAdConversion({ event }); - expect(createDataCollectionRequestPayload).toHaveBeenCalled(); - expect(mockPayload.addEvent).toHaveBeenCalledWith(mockEvent); - expect(mockEvent.finalize).toHaveBeenCalled(); - expect(createDataCollectionRequest).toHaveBeenCalledWith({ - payload: mockPayload, - }); + expect(event.finalize).toHaveBeenCalledTimes(1); expect(consent.awaitConsent).toHaveBeenCalled(); - expect(sendEdgeNetworkRequest).toHaveBeenCalledWith({ - request: mockRequest, - }); - expect(result).toEqual({ success: true }); - }); - - it("should handle missing event parameter", () => { - const mockPayload = { - addEvent: vi.fn(), - }; - createDataCollectionRequestPayload.mockReturnValue(mockPayload); - - expect(() => { - handler.trackAdConversion({ event: undefined }); - }).toThrow("Cannot read properties of undefined (reading 'finalize')"); - }); - - it("should handle consent rejection", async () => { - const mockEvent = { - finalize: vi.fn(), - }; - - const consentError = new Error("Consent denied"); - consent.awaitConsent.mockRejectedValue(consentError); - - await expect( - handler.trackAdConversion({ event: mockEvent }), - ).rejects.toThrow("Consent denied"); - }); - - it("should handle network request failure", async () => { - const mockEvent = { - finalize: vi.fn(), - }; - - const mockPayload = { - addEvent: vi.fn(), - }; - - const mockRequest = { - body: { events: [] }, - getUseIdThirdPartyDomain: vi.fn().mockReturnValue(false), - }; - - createDataCollectionRequestPayload.mockReturnValue(mockPayload); - createDataCollectionRequest.mockReturnValue(mockRequest); - - const networkError = new Error("Network failed"); - sendEdgeNetworkRequest.mockRejectedValue(networkError); + expect(sendEdgeNetworkRequest).toHaveBeenCalledWith({ request }); - await expect( - handler.trackAdConversion({ event: mockEvent }), - ).rejects.toThrow("Network failed"); - }); - - it("should handle options parameter", async () => { - const mockEvent = { - finalize: vi.fn(), - }; - - const mockPayload = { - addEvent: vi.fn(), - }; - - const mockRequest = { - body: { events: [] }, - getUseIdThirdPartyDomain: vi.fn().mockReturnValue(false), - }; - - createDataCollectionRequestPayload.mockReturnValue(mockPayload); - createDataCollectionRequest.mockReturnValue(mockRequest); - sendEdgeNetworkRequest.mockResolvedValue({ status: "success" }); - - const options = { customOption: "value" }; - await handler.trackAdConversion({ event: mockEvent, options }); - - expect(createDataCollectionRequestPayload).toHaveBeenCalled(); - }); - }); - - describe("error handling", () => { - it("should handle payload creation errors", () => { - const mockEvent = { - finalize: vi.fn(), - }; - - createDataCollectionRequestPayload.mockImplementation(() => { - throw new Error("Payload creation failed"); - }); - - expect(() => { - handler.trackAdConversion({ event: mockEvent }); - }).toThrow("Payload creation failed"); - }); - - it("should handle request creation errors", () => { - const mockEvent = { - finalize: vi.fn(), - }; - - const mockPayload = { - addEvent: vi.fn(), - }; - - createDataCollectionRequestPayload.mockReturnValue(mockPayload); - createDataCollectionRequest.mockImplementation(() => { - throw new Error("Request creation failed"); - }); - - expect(() => { - handler.trackAdConversion({ event: mockEvent }); - }).toThrow("Request creation failed"); + expect(result).toEqual({ success: true }); }); - it("should handle event finalization errors", () => { - const mockEvent = { - finalize: vi.fn().mockImplementation(() => { - throw new Error("Finalization failed"); - }), - }; - - const mockPayload = { - addEvent: vi.fn(), - }; - - createDataCollectionRequestPayload.mockReturnValue(mockPayload); + it("does not send when consent is rejected", async () => { + const error = new Error("Consent denied"); + consent.awaitConsent.mockRejectedValueOnce(error); - expect(() => { - handler.trackAdConversion({ event: mockEvent }); - }).toThrow("Finalization failed"); + await expect(handler.trackAdConversion({ event })).rejects.toThrow(); + expect(sendEdgeNetworkRequest).not.toHaveBeenCalled(); }); - }); - describe("handler structure", () => { - it("should return object with trackAdConversion method", () => { - expect(handler).toHaveProperty("trackAdConversion"); - expect(typeof handler.trackAdConversion).toBe("function"); - }); + it("logs and rethrows network failures", async () => { + const error = new Error("Network failed"); + sendEdgeNetworkRequest.mockRejectedValueOnce(error); - it("should create handler with all required dependencies", () => { - expect(() => { - createAdConversionHandler({ - eventManager, - sendEdgeNetworkRequest, - consent, - logger, - }); - }).not.toThrow(); + await expect(handler.trackAdConversion({ event })).rejects.toThrow(); + expect(logger.error).toHaveBeenCalled(); }); }); }); From dfe4194cfda88dd5ac40b37e9e55dee3be4c4279 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Tue, 3 Mar 2026 14:56:47 -0700 Subject: [PATCH 12/23] Remove dead mocks in advertising specs --- .../handlers/clickThroughHandler.spec.js | 90 ------------ .../handlers/viewThroughHandler.spec.js | 133 +----------------- 2 files changed, 6 insertions(+), 217 deletions(-) diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js index 11d92ecb9..9563a379d 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/clickThroughHandler.spec.js @@ -17,88 +17,6 @@ import { LAST_CONVERSION_TIME_KEY, } from "../../../../../../src/components/Advertising/constants/index.js"; -// FIXME: Module-level global mutation leaks across files. -// Mock network operations to prevent real network calls -vi.mock("fetch", () => vi.fn()); - -// FIXME: Overwrites runtime globals without guaranteed restoration. -// Mock globalThis fetch and other network APIs -Object.defineProperty(globalThis, "fetch", { - value: vi.fn(() => - Promise.resolve({ - ok: true, - json: () => Promise.resolve({ success: true }), - }), - ), - writable: true, -}); - -// FIXME: Overwrites runtime globals without guaranteed restoration. -// Mock XMLHttpRequest -Object.defineProperty(globalThis, "XMLHttpRequest", { - value: class MockXMLHttpRequest { - open() { - this.readyState = 4; - } - - send() { - this.status = 200; - } - - setRequestHeader() { - this.headers = {}; - } - }, - writable: true, -}); - -// FIXME: Mutates document/window globals at module scope; leaks into unrelated specs. -// Mock DOM operations to prevent network calls from script loading -if (typeof globalThis.document !== "undefined") { - globalThis.document.createElement = vi.fn(() => ({ - src: "", - onload: null, - onerror: null, - addEventListener: vi.fn(), - removeEventListener: vi.fn(), - })); - if (globalThis.document.head) { - globalThis.document.head.appendChild = vi.fn(); - } - if (globalThis.document.body) { - globalThis.document.body.appendChild = vi.fn(); - } -} - -if (typeof globalThis.window !== "undefined") { - globalThis.window.addEventListener = vi.fn(); - globalThis.window.removeEventListener = vi.fn(); -} - -// FIXME: Module mocks are leaky; use dependency injection instead. -// Mock helpers with all functions that might make network calls -vi.mock( - "../../../../../../src/components/Advertising/utils/helpers.js", - () => ({ - normalizeAdvertiser: vi.fn((advertiserSettings) => { - if (!advertiserSettings || !Array.isArray(advertiserSettings)) { - return "UNKNOWN"; - } - - return advertiserSettings - .filter((item) => item && item.enabled === true && item.advertiserId) - .map((item) => item.advertiserId) - .join(", "); - }), - getUrlParams: vi.fn(() => ({ skwcid: null, efid: null })), - appendAdvertisingIdQueryToEvent: vi.fn(), - isAnyIdUnused: vi.fn(() => true), - markIdsAsConverted: vi.fn(), - isThrottled: vi.fn(() => false), - shouldThrottle: vi.fn(() => false), - }), -); - describe("Advertising::clickThroughHandler", () => { let eventManager; let cookieManager; @@ -122,14 +40,6 @@ describe("Advertising::clickThroughHandler", () => { info: vi.fn(), error: vi.fn(), }; - - // FIXME: Date.now spy is never restored in this file. - const fixedTs = Date.UTC(2024, 0, 1, 0, 0, 0); - const mockNow = { - valueOf: () => fixedTs, - toISOString: () => new Date(fixedTs).toISOString(), - }; - vi.spyOn(Date, "now").mockReturnValue(mockNow); }); it("should handle click-through with skwcid", async () => { diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js index 51a8ba801..f1001536f 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js @@ -12,122 +12,15 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; import handleViewThrough from "../../../../../../src/components/Advertising/handlers/viewThroughHandler.js"; +import collectAllIdentities from "../../../../../../src/components/Advertising/identities/collectAllIdentities.js"; import flushPromiseChains from "../../../../helpers/flushPromiseChains.js"; -// Mock network operations to prevent real network calls -vi.mock("fetch", () => vi.fn()); - -// Mock globalThis fetch and other network APIs -Object.defineProperty(globalThis, "fetch", { - value: vi.fn(() => - Promise.resolve({ - ok: true, - json: () => Promise.resolve({ success: true }), - }), - ), - writable: true, -}); - -// FIXME: Overwrites runtime globals without guaranteed restoration. -// Mock XMLHttpRequest -Object.defineProperty(globalThis, "XMLHttpRequest", { - value: class MockXMLHttpRequest { - open() { - this.readyState = 4; - } - - send() { - this.status = 200; - } - - setRequestHeader() { - this.headers = {}; - } - }, - writable: true, -}); - -// FIXME: Mutates document/window globals at module scope; leaks into unrelated specs. -// Mock DOM operations to prevent network calls from script loading -if (typeof globalThis.document !== "undefined") { - globalThis.document.createElement = vi.fn(() => ({ - src: "", - height: 0, - width: 0, - frameBorder: 0, - style: { display: "none" }, - addEventListener: vi.fn(), - onerror: vi.fn(), - })); - if (globalThis.document.body) { - globalThis.document.body.appendChild = vi.fn(); - } - if (globalThis.document.head) { - globalThis.document.head.appendChild = vi.fn(); - } -} - -if (typeof globalThis.window !== "undefined") { - // FIXME: Mutates document/window globals at module scope; leaks into unrelated specs. - globalThis.window.addEventListener = vi.fn(); - globalThis.window.removeEventListener = vi.fn(); - globalThis.window.attachEvent = vi.fn(); - globalThis.window.detachEvent = vi.fn(); - globalThis.window.ats = undefined; - globalThis.window.ID5 = undefined; -} - // FIXME: Module mocks are leaky; use dependency injection instead. // Mock dependencies vi.mock( - "../../../../../../src/components/Advertising/identities/collectAllIdentities.js", -); - -// FIXME: Module mocks are leaky; use dependency injection instead. -// Mock helpers to prevent network calls -vi.mock( - "../../../../../../src/components/Advertising/utils/helpers.js", - () => ({ - appendAdvertisingIdQueryToEvent: vi.fn( - (availableIds, event, cookieManager, componentConfig, addEventType) => { - const query = { - advertising: { - stitchIds: { - ...(availableIds.surferId && { - surferId: availableIds.surferId, - }), - ...(availableIds.id5Id && { id5: availableIds.id5Id }), - ...(availableIds.rampId && { - rampIdEnv: availableIds.rampId, - }), - ipAddress: "DUMMY_IP_ADDRESS", - }, - advIds: "", - ...(addEventType && { eventType: "advertising.enrichment" }), - }, - }; - - event.mergeQuery(query); - return event; - }, - ), - normalizeAdvertiser: vi.fn((advertiserSettings) => { - if (!advertiserSettings || !Array.isArray(advertiserSettings)) { - return ""; - } - - return advertiserSettings - .filter((item) => item && item.enabled === true && item.advertiserId) - .map((item) => item.advertiserId) - .join(", "); - }), - getUrlParams: vi.fn(() => ({ skwcid: null, efid: null })), - isAnyIdUnused: vi.fn(() => true), - markIdsAsConverted: vi.fn(), - isThrottled: vi.fn(() => false), - shouldThrottle: vi.fn(() => false), - createConversionEvent: vi.fn(), - }), + import( + "../../../../../../src/components/Advertising/identities/collectAllIdentities.js" + ), ); describe("Advertising::viewThroughHandler", () => { @@ -136,10 +29,9 @@ describe("Advertising::viewThroughHandler", () => { let logger; let componentConfig; let adConversionHandler; - let collectAllIdentities; let getBrowser; - beforeEach(async () => { + beforeEach(() => { eventManager = { createEvent: vi.fn(() => ({ mergeQuery: vi.fn(), @@ -172,20 +64,7 @@ describe("Advertising::viewThroughHandler", () => { getBrowser = vi.fn(); - // FIXME: Date.now spy is never restored in this file. - const fixedTs = Date.UTC(2024, 0, 1, 0, 0, 0); - const mockNow = { - valueOf: () => fixedTs, - toISOString: () => new Date(fixedTs).toISOString(), - }; - vi.spyOn(Date, "now").mockReturnValue(mockNow); - - // Mock collectAllIdentities - const { default: mockCollectAllIdentities } = await import( - "../../../../../../src/components/Advertising/identities/collectAllIdentities.js" - ); - collectAllIdentities = mockCollectAllIdentities; - collectAllIdentities.mockReset(); + vi.mocked(collectAllIdentities).mockReset(); }); it("should handle empty identity promises", async () => { From 7151ba057b82e3c2f6ce1812e6ee125c57abda00 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Tue, 3 Mar 2026 19:04:43 -0700 Subject: [PATCH 13/23] Make all module mocks docs compliant --- .../handlers/viewThroughHandler.spec.js | 2 -- .../Advertising/utils/helpers.spec.js | 20 ------------------- .../createSendPushSubscriptionPayload.spec.js | 3 --- .../makeSendPushSubscriptionRequest.spec.js | 3 --- .../core/identity/createIdentity.spec.js | 3 --- 5 files changed, 31 deletions(-) diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js index f1001536f..006d7ca0e 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js @@ -15,8 +15,6 @@ import handleViewThrough from "../../../../../../src/components/Advertising/hand import collectAllIdentities from "../../../../../../src/components/Advertising/identities/collectAllIdentities.js"; import flushPromiseChains from "../../../../helpers/flushPromiseChains.js"; -// FIXME: Module mocks are leaky; use dependency injection instead. -// Mock dependencies vi.mock( import( "../../../../../../src/components/Advertising/identities/collectAllIdentities.js" diff --git a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js index d312840d2..9df246558 100644 --- a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js @@ -28,28 +28,12 @@ import { AD_CONVERSION_VIEW_EVENT_TYPE, } from "../../../../../../src/components/Advertising/constants/index.js"; -// FIXME: Module mocks are leaky; use dependency injection instead. -// Mock DOM utilities -vi.mock("../../../../../../src/utils/dom/index.js", () => ({ - awaitSelector: vi.fn(), - loadScript: vi.fn().mockResolvedValue(), - createNode: vi.fn(), - appendNode: vi.fn(), - matchesSelector: vi.fn(), - querySelectorAll: vi.fn(), - removeNode: vi.fn(), - selectNodes: vi.fn(), - selectNodesWithShadow: vi.fn(), -})); - describe("Advertising::helpers", () => { let mockEvent; let mockCookieManager; let mockLogger; beforeEach(() => { - // Reset modules to clear any cached state - vi.resetModules(); // Mock event object mockEvent = { mergeQuery: vi.fn(), @@ -67,14 +51,10 @@ describe("Advertising::helpers", () => { warn: vi.fn(), error: vi.fn(), }; - - // Reset all mocks - vi.clearAllMocks(); }); afterEach(() => { vi.restoreAllMocks(); - vi.unstubAllGlobals(); }); describe("getUrlParams", () => { diff --git a/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js b/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js index dd843bf1c..d524565fc 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/request/createSendPushSubscriptionPayload.spec.js @@ -12,7 +12,6 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; -// FIXME: Module mocks are leaky; use dependency injection instead. vi.mock( "../../../../../../src/utils/request/createDataCollectionRequestPayload.js", ); @@ -29,8 +28,6 @@ describe("createSendPushSubscriptionPayload", () => { let appId; beforeEach(() => { - vi.clearAllMocks(); - ecid = "12345678901234567890123456789012345678"; appId = "my-app-id"; serializedPushSubscriptionDetails = JSON.stringify({ diff --git a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js index f1fde62ab..3600cb87a 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/request/makeSendPushSubscriptionRequest.spec.js @@ -12,7 +12,6 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; -// FIXME: Module mocks are leaky; use dependency injection instead. vi.mock( "../../../../../../src/components/PushNotifications/helpers/getPushSubscriptionDetails.js", ); @@ -27,8 +26,6 @@ describe("makeSendPushSubscriptionRequest", () => { let mockSetUserData; beforeEach(() => { - vi.clearAllMocks(); - mockStorage = { cache: {}, // eslint-disable-next-line func-names diff --git a/packages/core/test/unit/specs/core/identity/createIdentity.spec.js b/packages/core/test/unit/specs/core/identity/createIdentity.spec.js index b5bf0f4c3..c632a2937 100644 --- a/packages/core/test/unit/specs/core/identity/createIdentity.spec.js +++ b/packages/core/test/unit/specs/core/identity/createIdentity.spec.js @@ -12,7 +12,6 @@ governing permissions and limitations under the License. import { describe, it, expect, vi, beforeEach } from "vitest"; -// FIXME: Module mocks are leaky; use dependency injection instead. vi.mock("../../../../../src/utils/createDecodeKndctrCookie.js", () => ({ default: vi.fn(), })); @@ -27,8 +26,6 @@ describe("createIdentity", () => { let mockDecodeKndctrCookie; beforeEach(() => { - vi.clearAllMocks(); - mockLogger = {}; mockLoggingCookieJar = {}; From dfc7de735ebae3dad841db79fcdb51b5e5f35aa4 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Tue, 3 Mar 2026 19:15:14 -0700 Subject: [PATCH 14/23] Use safer withTemporaryUrl function in some unit tests --- .../core/test/unit/helpers/utils/location.js | 12 +++++ .../Advertising/utils/helpers.spec.js | 44 +++++++++++-------- 2 files changed, 38 insertions(+), 18 deletions(-) create mode 100644 packages/core/test/unit/helpers/utils/location.js diff --git a/packages/core/test/unit/helpers/utils/location.js b/packages/core/test/unit/helpers/utils/location.js new file mode 100644 index 000000000..920cd6be9 --- /dev/null +++ b/packages/core/test/unit/helpers/utils/location.js @@ -0,0 +1,12 @@ +/* +Copyright 2026 Adobe. All rights reserved. +This file is licensed to you under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. You may obtain a copy +of the License at http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software distributed under +the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS +OF ANY KIND, either express or implied. See the License for the specific language +governing permissions and limitations under the License. +*/ +export { withTemporaryUrl } from "../../../integration/helpers/utils/location.js"; diff --git a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js index 9df246558..d9e5b27e6 100644 --- a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js @@ -27,6 +27,7 @@ import { DISPLAY_CLICK_COOKIE_KEY_EXPIRES, AD_CONVERSION_VIEW_EVENT_TYPE, } from "../../../../../../src/components/Advertising/constants/index.js"; +import { withTemporaryUrl } from "../../../../helpers/utils/location.js"; describe("Advertising::helpers", () => { let mockEvent; @@ -58,38 +59,45 @@ describe("Advertising::helpers", () => { }); describe("getUrlParams", () => { - afterEach(() => { - // FIXME: Mutates shared URL state; this must stay paired with robust cleanup. - window.history.pushState({}, "", "/"); - }); - it("should return URL parameters when present", () => { - window.history.pushState({}, "", "?s_kwcid=test_kwcid&ef_id=test_efid"); + return withTemporaryUrl(({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.search = "?s_kwcid=test_kwcid&ef_id=test_efid"; + applyUrl(url); - const result = getUrlParams(); + const result = getUrlParams(); - expect(result).toEqual({ - skwcid: "test_kwcid", - efid: "test_efid", + expect(result).toEqual({ + skwcid: "test_kwcid", + efid: "test_efid", + }); }); }); it("should return undefined values when parameters are not present", () => { - window.history.pushState({}, "", "?foo=bar"); + return withTemporaryUrl(({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.search = "?foo=bar"; + applyUrl(url); - const result = getUrlParams(); + const result = getUrlParams(); - expect(result.skwcid).toBeUndefined(); - expect(result.efid).toBeUndefined(); + expect(result.skwcid).toBeUndefined(); + expect(result.efid).toBeUndefined(); + }); }); it("should handle empty search string", () => { - window.history.pushState({}, "", ""); + return withTemporaryUrl(({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.search = ""; + applyUrl(url); - const result = getUrlParams(); + const result = getUrlParams(); - expect(result.skwcid).toBeUndefined(); - expect(result.efid).toBeUndefined(); + expect(result.skwcid).toBeUndefined(); + expect(result.efid).toBeUndefined(); + }); }); }); From c92fa6447eac77651367342a7cd85bd6e537d123 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 09:22:57 -0700 Subject: [PATCH 15/23] Harden the mediaEvents integration tests --- .../specs/StreamingMedia/mediaEvents.spec.js | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/core/test/integration/specs/StreamingMedia/mediaEvents.spec.js b/packages/core/test/integration/specs/StreamingMedia/mediaEvents.spec.js index 31c451203..c22ea47c3 100644 --- a/packages/core/test/integration/specs/StreamingMedia/mediaEvents.spec.js +++ b/packages/core/test/integration/specs/StreamingMedia/mediaEvents.spec.js @@ -24,6 +24,12 @@ const streamingMediaConfig = { }, }; +const getEventFromCall = (call, eventType) => { + return call.request.body.events.find((event) => { + return event.xdm?.eventType === eventType; + }); +}; + describe("Streaming Media Events", () => { test("media events include timestamp in xdm", async ({ alloy, @@ -56,12 +62,19 @@ describe("Streaming Media Events", () => { }), }); - const sessionCall = await networkRecorder.findCall(/edge\.adobedc\.net/); + const sessionCalls = await networkRecorder.findCalls(/edge\.adobedc\.net/); + expect(sessionCalls.length).toBeGreaterThan(0); + const sessionCall = sessionCalls.find((call) => { + return getEventFromCall(call, "media.sessionStart"); + }); expect(sessionCall).toBeDefined(); - expect(sessionCall.request.body.events[0].xdm.timestamp).toBeDefined(); - expect(sessionCall.request.body.events[0].xdm.eventType).toBe( + const sessionStartEvent = getEventFromCall( + sessionCall, "media.sessionStart", ); + expect(sessionStartEvent).toBeDefined(); + expect(sessionStartEvent.xdm.timestamp).toBeDefined(); + expect(sessionStartEvent.xdm.eventType).toBe("media.sessionStart"); await alloy("sendMediaEvent", { playerId: "player1", @@ -73,8 +86,20 @@ describe("Streaming Media Events", () => { const mediaEventCalls = await networkRecorder.findCalls(/\/va\//); expect(mediaEventCalls.length).toBeGreaterThan(0); - const playEvent = mediaEventCalls[0]; - expect(playEvent.request.body.events[0].xdm.timestamp).toBeDefined(); - expect(playEvent.request.body.events[0].xdm.eventType).toBe("media.play"); + const playCall = mediaEventCalls.find((call) => { + return getEventFromCall(call, "media.play"); + }); + expect(playCall).toBeDefined(); + const playEvent = getEventFromCall(playCall, "media.play"); + expect(playEvent).toBeDefined(); + expect(playEvent.xdm.timestamp).toBeDefined(); + expect(playEvent.xdm.eventType).toBe("media.play"); + + await alloy("sendMediaEvent", { + playerId: "player1", + xdm: { + eventType: "media.sessionComplete", + }, + }); }); }); From e4e63295b8c7270786590dd3c46ed621e0f7ea7c Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 11:04:25 -0700 Subject: [PATCH 16/23] Fix viewThroughHandler mocks --- .../Advertising/handlers/viewThroughHandler.spec.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js index 006d7ca0e..3dd99ae7e 100644 --- a/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js @@ -19,6 +19,9 @@ vi.mock( import( "../../../../../../src/components/Advertising/identities/collectAllIdentities.js" ), + () => ({ + default: vi.fn(), + }), ); describe("Advertising::viewThroughHandler", () => { @@ -61,8 +64,6 @@ describe("Advertising::viewThroughHandler", () => { }; getBrowser = vi.fn(); - - vi.mocked(collectAllIdentities).mockReset(); }); it("should handle empty identity promises", async () => { From 79ba414ab1462236ad6c9beafe279f3c48364d84 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 11:05:14 -0700 Subject: [PATCH 17/23] Reset dom action nonces better in tests --- .../dom-actions/addNonceToInlineStyleElements.spec.js | 8 ++++++-- .../Personalization/dom-actions/dom/getNonce.spec.js | 8 ++++++-- .../actions/displayIframeContent.spec.js | 2 ++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/packages/core/test/unit/specs/components/Personalization/dom-actions/addNonceToInlineStyleElements.spec.js b/packages/core/test/unit/specs/components/Personalization/dom-actions/addNonceToInlineStyleElements.spec.js index 5ff331e5b..661b529cf 100644 --- a/packages/core/test/unit/specs/components/Personalization/dom-actions/addNonceToInlineStyleElements.spec.js +++ b/packages/core/test/unit/specs/components/Personalization/dom-actions/addNonceToInlineStyleElements.spec.js @@ -10,7 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import { afterEach, describe, it, expect } from "vitest"; +import { beforeEach, afterEach, describe, it, expect } from "vitest"; import addNonceToInlineStyleElements from "../../../../../../src/components/Personalization/dom-actions/addNonceToInlineStyleElements.js"; import { testResetCachedNonce } from "../../../../../../src/components/Personalization/dom-actions/dom/getNonce.js"; import { createFragment } from "../../../../../../src/components/Personalization/dom-actions/dom/index.js"; @@ -23,11 +23,15 @@ import { } from "../../../../../../src/utils/dom/index.js"; describe("Personalization::dom-actions::addNonceToInlineStyleElements", () => { + beforeEach(() => { + testResetCachedNonce(); + }); + afterEach(() => { + testResetCachedNonce(); selectNodes("#fooById").forEach(removeNode); }); it("should add nonce to inline style elements if available", () => { - testResetCachedNonce(); // Make sure a nonce is available to alloy appendNode( document.head, diff --git a/packages/core/test/unit/specs/components/Personalization/dom-actions/dom/getNonce.spec.js b/packages/core/test/unit/specs/components/Personalization/dom-actions/dom/getNonce.spec.js index 0fb947a33..f2fc88640 100644 --- a/packages/core/test/unit/specs/components/Personalization/dom-actions/dom/getNonce.spec.js +++ b/packages/core/test/unit/specs/components/Personalization/dom-actions/dom/getNonce.spec.js @@ -10,7 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import { afterEach, describe, it, expect } from "vitest"; +import { beforeEach, afterEach, describe, it, expect } from "vitest"; import { testResetCachedNonce } from "../../../../../../../src/components/Personalization/dom-actions/dom/getNonce.js"; import { selectNodes, @@ -21,11 +21,15 @@ import { import { getNonce } from "../../../../../../../src/components/Personalization/dom-actions/dom/index.js"; describe("Personalization::DOM::getNonce", () => { + beforeEach(() => { + testResetCachedNonce(); + }); + afterEach(() => { + testResetCachedNonce(); selectNodes("#fooById").forEach(removeNode); }); it("should return the nonce if defined", () => { - testResetCachedNonce(); appendNode( document.head, createNode("script", { diff --git a/packages/core/test/unit/specs/components/Personalization/in-app-message-actions/actions/displayIframeContent.spec.js b/packages/core/test/unit/specs/components/Personalization/in-app-message-actions/actions/displayIframeContent.spec.js index 9adb2ccd4..e81a812d6 100644 --- a/packages/core/test/unit/specs/components/Personalization/in-app-message-actions/actions/displayIframeContent.spec.js +++ b/packages/core/test/unit/specs/components/Personalization/in-app-message-actions/actions/displayIframeContent.spec.js @@ -23,11 +23,13 @@ import { TEXT_HTML } from "../../../../../../../src/constants/contentType.js"; describe("DOM Actions on Iframe", () => { beforeEach(() => { + testResetCachedNonce(); cleanUpDomChanges("alloy-messaging-container"); cleanUpDomChanges("alloy-overlay-container"); cleanUpDomChanges("alloy-content-iframe"); }); afterEach(() => { + testResetCachedNonce(); cleanUpDomChanges("alloy-messaging-container"); cleanUpDomChanges("alloy-overlay-container"); cleanUpDomChanges("alloy-content-iframe"); From de720ae9e5bfe0a46d230c7536dad25a063e424b Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 14:21:11 -0700 Subject: [PATCH 18/23] Remove location changing from advertising helper specs --- .../core/test/unit/helpers/utils/location.js | 12 ----- .../Advertising/utils/helpers.spec.js | 45 ++++++++----------- 2 files changed, 19 insertions(+), 38 deletions(-) delete mode 100644 packages/core/test/unit/helpers/utils/location.js diff --git a/packages/core/test/unit/helpers/utils/location.js b/packages/core/test/unit/helpers/utils/location.js deleted file mode 100644 index 920cd6be9..000000000 --- a/packages/core/test/unit/helpers/utils/location.js +++ /dev/null @@ -1,12 +0,0 @@ -/* -Copyright 2026 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ -export { withTemporaryUrl } from "../../../integration/helpers/utils/location.js"; diff --git a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js index d9e5b27e6..8ea38dfb0 100644 --- a/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js +++ b/packages/core/test/unit/specs/components/Advertising/utils/helpers.spec.js @@ -27,7 +27,7 @@ import { DISPLAY_CLICK_COOKIE_KEY_EXPIRES, AD_CONVERSION_VIEW_EVENT_TYPE, } from "../../../../../../src/components/Advertising/constants/index.js"; -import { withTemporaryUrl } from "../../../../helpers/utils/location.js"; +import { queryString } from "../../../../../../src/utils/index.js"; describe("Advertising::helpers", () => { let mockEvent; @@ -60,44 +60,37 @@ describe("Advertising::helpers", () => { describe("getUrlParams", () => { it("should return URL parameters when present", () => { - return withTemporaryUrl(({ currentHref, applyUrl }) => { - const url = new URL(currentHref); - url.search = "?s_kwcid=test_kwcid&ef_id=test_efid"; - applyUrl(url); + vi.spyOn(queryString, "parse").mockReturnValue({ + s_kwcid: "test_kwcid", + ef_id: "test_efid", + }); - const result = getUrlParams(); + const result = getUrlParams(); - expect(result).toEqual({ - skwcid: "test_kwcid", - efid: "test_efid", - }); + expect(result).toEqual({ + skwcid: "test_kwcid", + efid: "test_efid", }); }); it("should return undefined values when parameters are not present", () => { - return withTemporaryUrl(({ currentHref, applyUrl }) => { - const url = new URL(currentHref); - url.search = "?foo=bar"; - applyUrl(url); + vi.spyOn(queryString, "parse").mockReturnValue({ + foo: "bar", + }); - const result = getUrlParams(); + const result = getUrlParams(); - expect(result.skwcid).toBeUndefined(); - expect(result.efid).toBeUndefined(); - }); + expect(result.skwcid).toBeUndefined(); + expect(result.efid).toBeUndefined(); }); it("should handle empty search string", () => { - return withTemporaryUrl(({ currentHref, applyUrl }) => { - const url = new URL(currentHref); - url.search = ""; - applyUrl(url); + vi.spyOn(queryString, "parse").mockReturnValue({}); - const result = getUrlParams(); + const result = getUrlParams(); - expect(result.skwcid).toBeUndefined(); - expect(result.efid).toBeUndefined(); - }); + expect(result.skwcid).toBeUndefined(); + expect(result.efid).toBeUndefined(); }); }); From 7e2f82bc11f06f4b9650ced23d1b1076afe3c2a4 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 14:34:09 -0700 Subject: [PATCH 19/23] Make serviceWorkerNotificationClickListener a default export --- .../helpers/serviceWorkerNotificationClickListener.js | 6 +----- packages/core/src/serviceWorker.js | 2 +- .../helpers/serviceWorkerNotificationClickListener.spec.js | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js b/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js index 5139d2748..679bc5001 100644 --- a/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js +++ b/packages/core/src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js @@ -27,11 +27,7 @@ const canHandleUrl = (type) => ["DEEPLINK", "WEBURL"].includes(type); * @param {ServiceWorkerGlobalScope} dependencies.sw * @param {ServiceWorkerLogger} dependencies.logger */ -export const createServiceWorkerNotificationClickListener = ({ - makeSendServiceWorkerTrackingData, - sw, - logger, -}) => { +export default ({ makeSendServiceWorkerTrackingData, sw, logger }) => { /** * @function * diff --git a/packages/core/src/serviceWorker.js b/packages/core/src/serviceWorker.js index a0c2fb2ec..e014699dc 100644 --- a/packages/core/src/serviceWorker.js +++ b/packages/core/src/serviceWorker.js @@ -12,7 +12,7 @@ governing permissions and limitations under the License. /** @import { ServiceWorkerLogger } from './components/PushNotifications/types.js' */ -import { createServiceWorkerNotificationClickListener } from "./components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; +import createServiceWorkerNotificationClickListener from "./components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; import serviceWorkerPushListener from "./components/PushNotifications/helpers/serviceWorkerPushListener.js"; import { createMakeSendServiceWorkerTrackingData } from "./components/PushNotifications/request/makeSendServiceWorkerTrackingData.js"; import readFromIndexedDb from "./components/PushNotifications/helpers/readFromIndexedDb.js"; diff --git a/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js b/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js index 3a223e3f3..eb64ae8a3 100644 --- a/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js +++ b/packages/core/test/unit/specs/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.spec.js @@ -14,7 +14,7 @@ governing permissions and limitations under the License. import { vi, beforeEach, describe, it, expect } from "vitest"; -import { createServiceWorkerNotificationClickListener } from "../../../../../../src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; +import createServiceWorkerNotificationClickListener from "../../../../../../src/components/PushNotifications/helpers/serviceWorkerNotificationClickListener.js"; describe("serviceWorkerNotificationClickListener", () => { let mockEvent; From 71fecb942cb17605a387912735a0f0139fc5ae88 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 15:04:31 -0700 Subject: [PATCH 20/23] Pass missing params into serviceWorkerPushListener --- packages/core/src/serviceWorker.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/serviceWorker.js b/packages/core/src/serviceWorker.js index e014699dc..dc95de4db 100644 --- a/packages/core/src/serviceWorker.js +++ b/packages/core/src/serviceWorker.js @@ -72,7 +72,9 @@ sw.addEventListener("activate", (event) => { * @param {PushEvent} event * @returns {Promise} */ -sw.addEventListener("push", (event) => serviceWorkerPushListener({ event })); +sw.addEventListener("push", (event) => + serviceWorkerPushListener({ event, sw, logger }), +); /** * @listens notificationclick From 1147d3501ac60a6768673e46efd6fdda23d09c00 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 16:30:33 -0700 Subject: [PATCH 21/23] Fix jsdoc comment --- .../request/makeSendServiceWorkerTrackingData.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js b/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js index 100e8632c..f8e1b54d5 100644 --- a/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js +++ b/packages/core/src/components/PushNotifications/request/makeSendServiceWorkerTrackingData.js @@ -21,7 +21,7 @@ governing permissions and limitations under the License. * @param {() => string} dependencies.uuidv4 * @param {ServiceWorkerLogger} dependencies.logger * @param {(url: string, options: object) => Promise} dependencies.fetch - * @returns {(options: { xdm: Object, actionLabel?: string, applicationLaunches?: number }, utils: { logger: ServiceWorkerLogger, fetch: (url: string, options: object) => Promise }) => Promise} + * @returns {(options: { xdm: Object, actionLabel?: string, applicationLaunches?: number }) => Promise} */ export const createMakeSendServiceWorkerTrackingData = ({ readFromIndexedDb, From e0cd65396c480477f9fea50f7d33b2c2ea4e91f9 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 17:03:07 -0700 Subject: [PATCH 22/23] Clean up event listeners --- .../specs/Advertising/consent_gate.spec.js | 194 +++++++++--------- 1 file changed, 98 insertions(+), 96 deletions(-) diff --git a/packages/core/test/integration/specs/Advertising/consent_gate.spec.js b/packages/core/test/integration/specs/Advertising/consent_gate.spec.js index c2c6ab264..0d7401c3b 100644 --- a/packages/core/test/integration/specs/Advertising/consent_gate.spec.js +++ b/packages/core/test/integration/specs/Advertising/consent_gate.spec.js @@ -11,7 +11,13 @@ governing permissions and limitations under the License. */ import { http, HttpResponse } from "msw"; -import { test, describe, expect } from "../../helpers/testsSetup/extend.js"; +import { + test, + describe, + expect, + beforeEach, + afterEach, +} from "../../helpers/testsSetup/extend.js"; import { sendEventHandler } from "../../helpers/mswjs/handlers.js"; import alloyConfig from "../../helpers/alloy/config.js"; import { createAdvertisingConfig } from "../../helpers/advertising.js"; @@ -37,7 +43,6 @@ const clearCookie = (key) => { }; const cleanupAll = () => { - // FIXME: Cleanup is manually invoked in tests (not afterEach); failures can leak shared state. clearCookie("advertising"); clearCookie("consent"); }; @@ -75,11 +80,18 @@ const setConsentAcceptHandler = http.post( ); describe("Advertising - Consent gate", () => { + beforeEach(() => { + cleanupAll(); + }); + + afterEach(() => { + cleanupAll(); + }); + test("should not write advertising cookies while consent is pending (click-through)", async ({ alloy, worker, }) => { - cleanupAll(); worker.use(sendEventHandler, setConsentAcceptHandler); // Set URL params for click-through BEFORE configure @@ -103,15 +115,12 @@ describe("Advertising - Consent gate", () => { // Verify: NO advertising cookie should exist while consent is pending. expect(getAdvertisingCookie()).toBeNull(); }); - - cleanupAll(); }); test("should not write advertising cookies while consent is pending (view-through)", async ({ alloy, worker, }) => { - cleanupAll(); worker.use(sendEventHandler, setConsentAcceptHandler); // Configure with consent pending and advertiser settings for view-through @@ -126,8 +135,6 @@ describe("Advertising - Consent gate", () => { // Verify: NO advertising cookie while consent is pending expect(getAdvertisingCookie()).toBeNull(); - - cleanupAll(); }); test("should write cookies and send conversion after consent is accepted (click-through)", async ({ @@ -135,7 +142,6 @@ describe("Advertising - Consent gate", () => { worker, networkRecorder, }) => { - cleanupAll(); worker.use(sendEventHandler, setConsentAcceptHandler); // Set URL params for click-through @@ -192,8 +198,6 @@ describe("Advertising - Consent gate", () => { }); expect(conversionCall).toBeTruthy(); }); - - cleanupAll(); }); // This test is last because declining consent triggers internal SDK @@ -212,93 +216,91 @@ describe("Advertising - Consent gate", () => { event.preventDefault(); } }; - // FIXME: Listener cleanup is not in finally; assertion failures can leak this handler. window.addEventListener("unhandledrejection", suppressDeclined); - cleanupAll(); - - const setConsentDeclineHandler = http.post( - /https:\/\/edge\.adobedc\.net\/ee\/(?:[^/]+\/)?v1\/privacy\/set-consent/, - async (req) => { - const url = new URL(req.request.url); - const configId = url.searchParams.get("configId"); - - if (configId === "bc1a10e0-aee4-4e0e-ac5b-cdbb9abbec83") { - return HttpResponse.json({ - requestId: "consent-request-id", - handle: [ - { - type: "state:store", - payload: [ - { - key: getNamespacedCookieName("consent"), - value: "general=out", - maxAge: 15552000, - }, - ], - }, - ], - }); - } - - throw new Error("Handler not configured properly"); - }, - ); - - worker.use(sendEventHandler, setConsentDeclineHandler); - - // Set URL params for click-through - await withTemporaryUrl(async ({ currentHref, applyUrl }) => { - const url = new URL(currentHref); - url.searchParams.set("s_kwcid", "AL!test_kwcid_reject"); - url.searchParams.set("ef_id", "test_efid_reject"); - applyUrl(url); - - await alloy("configure", { - ...alloyConfig, - ...createAdvertisingConfig(), - defaultConsent: "pending", - }); - - // Verify no cookies yet - await waitFor(300); - expect(getAdvertisingCookie()).toBeNull(); - - // Decline consent - await alloy("setConsent", { - consent: [ - { - standard: "Adobe", - version: "1.0", - value: { general: "out" }, - }, - ], + try { + const setConsentDeclineHandler = http.post( + /https:\/\/edge\.adobedc\.net\/ee\/(?:[^/]+\/)?v1\/privacy\/set-consent/, + async (req) => { + const url = new URL(req.request.url); + const configId = url.searchParams.get("configId"); + + if (configId === "bc1a10e0-aee4-4e0e-ac5b-cdbb9abbec83") { + return HttpResponse.json({ + requestId: "consent-request-id", + handle: [ + { + type: "state:store", + payload: [ + { + key: getNamespacedCookieName("consent"), + value: "general=out", + maxAge: 15552000, + }, + ], + }, + ], + }); + } + + throw new Error("Handler not configured properly"); + }, + ); + + worker.use(sendEventHandler, setConsentDeclineHandler); + + // Set URL params for click-through + await withTemporaryUrl(async ({ currentHref, applyUrl }) => { + const url = new URL(currentHref); + url.searchParams.set("s_kwcid", "AL!test_kwcid_reject"); + url.searchParams.set("ef_id", "test_efid_reject"); + applyUrl(url); + + await alloy("configure", { + ...alloyConfig, + ...createAdvertisingConfig(), + defaultConsent: "pending", + }); + + // Verify no cookies yet + await waitFor(300); + expect(getAdvertisingCookie()).toBeNull(); + + // Decline consent + await alloy("setConsent", { + consent: [ + { + standard: "Adobe", + version: "1.0", + value: { general: "out" }, + }, + ], + }); + + // Wait to ensure nothing fires after decline + await waitFor(500); + + // Verify: NO advertising cookie should exist after decline + expect(getAdvertisingCookie()).toBeNull(); + + // Verify no conversion calls were made (only consent call should exist) + const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/, { + retries: 3, + delayMs: 100, + }); + const conversionCall = calls.find((call) => { + const body = + typeof call.request.body === "string" + ? JSON.parse(call.request.body) + : call.request.body; + return ( + body?.events?.[0]?.xdm?.eventType === "advertising.enrichment_ct" + ); + }); + expect(conversionCall).toBeFalsy(); }); - - // Wait to ensure nothing fires after decline - await waitFor(500); - - // Verify: NO advertising cookie should exist after decline - expect(getAdvertisingCookie()).toBeNull(); - - // Verify no conversion calls were made (only consent call should exist) - const calls = await networkRecorder.findCalls(/edge\.adobedc\.net/, { - retries: 3, - delayMs: 100, - }); - const conversionCall = calls.find((call) => { - const body = - typeof call.request.body === "string" - ? JSON.parse(call.request.body) - : call.request.body; - return ( - body?.events?.[0]?.xdm?.eventType === "advertising.enrichment_ct" - ); - }); - expect(conversionCall).toBeFalsy(); - }); - - cleanupAll(); - window.removeEventListener("unhandledrejection", suppressDeclined); + } finally { + window.removeEventListener("unhandledrejection", suppressDeclined); + } }); }); From d831b023b359feda93477fa724dbc8e2dbe78c03 Mon Sep 17 00:00:00 2001 From: Carter McBride <18412686+carterworks@users.noreply.github.com> Date: Wed, 4 Mar 2026 17:12:28 -0700 Subject: [PATCH 23/23] Resolve other FIXME comments --- .../specs/Advertising/options_modes.spec.js | 22 +++++- .../specs/Target/deduplication_spa.spec.js | 79 +++++++++---------- .../components/BrandConcierge/index.spec.js | 10 ++- .../visitorService/awaitVisitorOptIn.spec.js | 10 ++- .../injectGetEcidFromVisitor.spec.js | 10 ++- .../components/RulesEngine/index.spec.js | 9 ++- 6 files changed, 85 insertions(+), 55 deletions(-) diff --git a/packages/core/test/integration/specs/Advertising/options_modes.spec.js b/packages/core/test/integration/specs/Advertising/options_modes.spec.js index 7631d8b19..133d25159 100644 --- a/packages/core/test/integration/specs/Advertising/options_modes.spec.js +++ b/packages/core/test/integration/specs/Advertising/options_modes.spec.js @@ -2,7 +2,13 @@ Copyright 2025 Adobe. All rights reserved. This file is licensed to you under the Apache License, Version 2.0 (the "License"); */ -import { test, describe, expect } from "../../helpers/testsSetup/extend.js"; +import { + test, + describe, + expect, + beforeEach, + afterEach, +} from "../../helpers/testsSetup/extend.js"; import { sendEventHandler } from "../../helpers/mswjs/handlers.js"; import alloyConfig from "../../helpers/alloy/config.js"; import { @@ -17,11 +23,23 @@ const getNamespacedAdvertisingCookieName = () => { const setAdvertisingCookie = (value) => { const name = getNamespacedAdvertisingCookieName(); - // FIXME: Writes process-global cookie state and this file never clears it between tests. document.cookie = `${name}=${value}; path=/`; }; +const clearAdvertisingCookie = () => { + const name = getNamespacedAdvertisingCookieName(); + document.cookie = `${name}=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/`; +}; + describe("Advertising - Modes (auto, wait, disabled)", () => { + beforeEach(() => { + clearAdvertisingCookie(); + }); + + afterEach(() => { + clearAdvertisingCookie(); + }); + test("auto mode should append cookie data immediately", async ({ alloy, worker, diff --git a/packages/core/test/integration/specs/Target/deduplication_spa.spec.js b/packages/core/test/integration/specs/Target/deduplication_spa.spec.js index b7ec4f2cb..db756457e 100644 --- a/packages/core/test/integration/specs/Target/deduplication_spa.spec.js +++ b/packages/core/test/integration/specs/Target/deduplication_spa.spec.js @@ -22,50 +22,49 @@ describe("Target Custom Code SPA", () => { alloy, worker, }) => { - // Counter to track custom code executions - // FIXME: Mutates shared window state; cleanup is manual and not protected by finally. - window.customCodeExecutionCount = 0; - - // Use the custom code handler - worker.use(customCodeHandler); - - // Configure Alloy - await alloy("configure", alloyConfig); - - // First sendEvent call - await alloy("sendEvent", { - renderDecisions: true, - xdm: { - web: { - webPageDetails: { - viewName: "home", + try { + // Counter to track custom code executions + window.customCodeExecutionCount = 0; + + // Use the custom code handler + worker.use(customCodeHandler); + + // Configure Alloy + await alloy("configure", alloyConfig); + + // First sendEvent call + await alloy("sendEvent", { + renderDecisions: true, + xdm: { + web: { + webPageDetails: { + viewName: "home", + }, }, }, - }, - }); - - // Verify custom code ran once after first call - expect(window.customCodeExecutionCount).toBe(1); - - // Second sendEvent call (simulating SPA navigation back to same view) - await alloy("sendEvent", { - renderDecisions: true, - xdm: { - web: { - webPageDetails: { - viewName: "home", + }); + + // Verify custom code ran once after first call + expect(window.customCodeExecutionCount).toBe(1); + + // Second sendEvent call (simulating SPA navigation back to same view) + await alloy("sendEvent", { + renderDecisions: true, + xdm: { + web: { + webPageDetails: { + viewName: "home", + }, }, }, - }, - }); - - // Verify custom code ran twice - once from first response, - // and once from cached/persisted offers on second call - expect(window.customCodeExecutionCount).toBe(2); - - // FIXME: Cleanup runs only on happy path; failures can leak shared window state. - // Clean up - delete window.customCodeExecutionCount; + }); + + // Verify custom code ran twice - once from first response, + // and once from cached/persisted offers on second call + expect(window.customCodeExecutionCount).toBe(2); + } finally { + delete window.customCodeExecutionCount; + } }); test("prependHtml action only renders once when sendEvent is called twice with renderDecisions and viewName", async ({ diff --git a/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js b/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js index ca0b63a56..81d14360a 100644 --- a/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js +++ b/packages/core/test/unit/specs/components/BrandConcierge/index.spec.js @@ -9,16 +9,16 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -import { vi, beforeEach, describe, it, expect } from "vitest"; +import { vi, beforeEach, afterEach, describe, it, expect } from "vitest"; import createConciergeComponent from "../../../../../src/components/BrandConcierge/index.js"; import testConfigValidators from "../../../helpers/testConfigValidators.js"; describe("BrandConcierge", () => { let mockDependencies; + let originalFetch; beforeEach(() => { - // FIXME: Mutates global fetch and never restores original value in this file. - // Mock window.fetch + originalFetch = window.fetch; window.fetch = vi.fn(); mockDependencies = { @@ -63,6 +63,10 @@ describe("BrandConcierge", () => { }; }); + afterEach(() => { + window.fetch = originalFetch; + }); + it("creates a brand concierge component", () => { const component = createConciergeComponent(mockDependencies); diff --git a/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js b/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js index 7001384ae..9343dd90f 100644 --- a/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js +++ b/packages/core/test/unit/specs/components/Identity/visitorService/awaitVisitorOptIn.spec.js @@ -10,19 +10,21 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import { vi, beforeEach, afterAll, describe, it, expect } from "vitest"; +import { vi, beforeEach, afterEach, describe, it, expect } from "vitest"; import awaitVisitorOptIn from "../../../../../../src/components/Identity/visitorService/awaitVisitorOptIn.js"; const logger = { info: vi.fn(), }; describe("awaitVisitorOptIn", () => { + let originalAdobe; + beforeEach(() => { - // FIXME: Mutates global window.adobe state; keep cleanup robust to avoid cross-spec leaks. + originalAdobe = window.adobe; window.adobe = undefined; }); - afterAll(() => { - window.adobe = undefined; + afterEach(() => { + window.adobe = originalAdobe; }); describe("No legacy opt in object is present", () => { it("should return promise resolved with undefined", () => { diff --git a/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js b/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js index 3075a03cd..d7e61d971 100644 --- a/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js +++ b/packages/core/test/unit/specs/components/Identity/visitorService/injectGetEcidFromVisitor.spec.js @@ -10,7 +10,7 @@ OF ANY KIND, either express or implied. See the License for the specific languag governing permissions and limitations under the License. */ -import { vi, beforeEach, afterAll, describe, it, expect } from "vitest"; +import { vi, beforeEach, afterEach, describe, it, expect } from "vitest"; import injectGetEcidFromVisitor from "../../../../../../src/components/Identity/visitorService/injectGetEcidFromVisitor.js"; const logger = { @@ -28,12 +28,14 @@ Visitor.getInstance = () => { }; const orgId = "456org"; describe("getEcidFromVisitor", () => { + let originalVisitor; + beforeEach(() => { - // FIXME: Mutates global window.Visitor state; keep cleanup robust to avoid cross-spec leaks. + originalVisitor = window.Visitor; window.Visitor = undefined; }); - afterAll(() => { - window.Visitor = undefined; + afterEach(() => { + window.Visitor = originalVisitor; }); describe("Visitor does not exist", () => { it("should return promise resolved with undefined", () => { diff --git a/packages/core/test/unit/specs/components/RulesEngine/index.spec.js b/packages/core/test/unit/specs/components/RulesEngine/index.spec.js index af7be7211..407f1e3c0 100644 --- a/packages/core/test/unit/specs/components/RulesEngine/index.spec.js +++ b/packages/core/test/unit/specs/components/RulesEngine/index.spec.js @@ -9,7 +9,7 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -import { vi, beforeEach, describe, it, expect } from "vitest"; +import { vi, beforeEach, afterEach, describe, it, expect } from "vitest"; import createRulesEngine from "../../../../../src/components/RulesEngine/index.js"; import { defer } from "../../../../../src/utils/index.js"; import { @@ -26,6 +26,7 @@ describe("createRulesEngine:commands:evaluateRulesets", () => { let getBrowser; let persistentStorage; let createNamespacedStorage; + let originalReferrer; beforeEach(() => { mergeData = vi.fn(); @@ -34,7 +35,7 @@ describe("createRulesEngine:commands:evaluateRulesets", () => { awaitConsent: vi.fn().mockReturnValue(awaitConsentDeferred.promise), }; getBrowser = vi.fn().mockReturnValue("foo"); - // FIXME: Mutates global window.referrer and never restores original value in this file. + originalReferrer = window.referrer; window.referrer = "https://www.google.com/search?q=adobe+journey+optimizer&oq=adobe+journey+optimizer"; @@ -56,6 +57,10 @@ describe("createRulesEngine:commands:evaluateRulesets", () => { }; }); + afterEach(() => { + window.referrer = originalReferrer; + }); + const setUpDecisionEngine = ({ personalizationStorageEnabled }) => { const config = { orgId: "exampleOrgId",