diff --git a/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.spec.ts b/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.spec.ts index cfc367cf..0c7c7d84 100644 --- a/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.spec.ts +++ b/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.spec.ts @@ -137,19 +137,6 @@ describe("formatCheckLib", () => { expect(errors).toEqual([]) }) - - test("returns an error when event's name is a reserved name", () => { - const payload = {app_instance_id: "12345678901234567890123456789012", events: [{name: 'ad_click'}]} - const firebaseAppId = '1:1233455666:android:abcdefgh' - const instanceId = {firebase_app_id: firebaseAppId} - const api_secret = '123' - - let errors = formatCheckLib(payload, instanceId, api_secret, true) - - expect(errors[0].description).toEqual( - "ad_click is a reserved event name" - ) - }) }) describe("returns invalidUserPropertyName errors", () => { @@ -163,19 +150,6 @@ describe("formatCheckLib", () => { expect(errors).toEqual([]) }) - - test("returns an error when event's name is a reserved name", () => { - const payload = {app_instance_id: "12345678901234567890123456789012", user_properties: {'first_open_time': 'test'}} - const firebaseAppId = '1:1233455666:android:abcdefgh' - const instanceId = {firebase_app_id: firebaseAppId} - const api_secret = '123' - - let errors = formatCheckLib(payload, instanceId, api_secret, true) - - expect(errors[0].description).toEqual( - "user_property: 'first_open_time' is a reserved user property name" - ) - }) }) describe("returns invalidCurrencyType errors", () => { diff --git a/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.ts b/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.ts index 1fade516..0ea0cd72 100644 --- a/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.ts +++ b/src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.ts @@ -4,26 +4,12 @@ import sizeof from "object-sizeof" import {eventDefinitions} from "../schemas/eventTypes/eventDefinitions" import {InstanceId} from "../../types" -const RESERVED_EVENT_NAMES = [ - "ad_activeview", "ad_click", "ad_exposure", "ad_query", - "adunit_exposure", "app_clear_data", "app_install", "app_update", - "app_remove", "error", "first_open", "first_visit", "in_app_purchase", - "notification_dismiss", "notification_foreground", "notification_open", - "notification_receive", "os_update", "session_start", "user_engagement" -] -const RESERVED_USER_PROPERTY_NAMES = [ - "first_open_time", "first_visit_time", "last_deep_link_referrer", "user_id", - "first_open_after_install" -] - // formatCheckLib provides additional validations for payload not included in // the schema validations. All checks are consistent with Firebase documentation. export const formatCheckLib = (payload: any, instanceId: InstanceId, api_secret: string, useFirebase: boolean) => { let errors: ValidationMessage[] = [] const appOrClientErrors = isValidAppOrClientId(payload, useFirebase) - const eventNameErrors = isValidEventName(payload) - const userPropertyNameErrors = isValidUserPropertyName(payload) const currencyErrors = isValidCurrencyType(payload) const emptyItemsErrors = isItemsEmpty(payload) const itemsRequiredKeyErrors = itemsHaveRequiredKey(payload) @@ -34,8 +20,6 @@ export const formatCheckLib = (payload: any, instanceId: InstanceId, api_secret: return [ ...errors, ...appOrClientErrors, - ...eventNameErrors, - ...userPropertyNameErrors, ...currencyErrors, ...emptyItemsErrors, ...itemsRequiredKeyErrors, @@ -91,42 +75,6 @@ const isValidAppOrClientId = (payload: any, useFirebase: boolean) => { return errors } - -const isValidEventName = (payload: any) => { - let errors: ValidationMessage[] = [] - - payload.events?.forEach((ev: any) => { - if (RESERVED_EVENT_NAMES.includes(ev.name)) { - errors.push({ - description: `${ev.name} is a reserved event name`, - validationCode: "value_invalid", - fieldPath: "#/events/name" - }) - } - }) - - return errors -} - -const isValidUserPropertyName = (payload: any) => { - const errors: ValidationMessage[] = [] - const userProperties = payload.user_properties - - if (userProperties) { - Object.keys(userProperties).forEach(prop => { - if (RESERVED_USER_PROPERTY_NAMES.includes(prop)) { - errors.push({ - description: `user_property: '${prop}' is a reserved user property name`, - validationCode: "value_invalid", - fieldPath: "user_property" - }) - } - }) - } - - return errors -} - const isValidCurrencyType = (payload: any) => { const errors: ValidationMessage[] = [] diff --git a/src/components/ga4/EventBuilder/index.spec.tsx b/src/components/ga4/EventBuilder/index.spec.tsx index 150e1679..d13857a2 100644 --- a/src/components/ga4/EventBuilder/index.spec.tsx +++ b/src/components/ga4/EventBuilder/index.spec.tsx @@ -50,11 +50,8 @@ describe("Event Builder", () => { const { wrapped } = withProviders(, { isLoggedIn: false }) const { findByLabelText, findByTestId } = renderer.render(wrapped) - await React.act(async () => { - // Choose the second view in the list - const clientToggle = await findByTestId("use firebase") - clientToggle.click() - }) + const clientToggle = await findByTestId("use firebase") + userEvent.click(clientToggle) await findByLabelText(Label.APISecret, { exact: false }) @@ -95,45 +92,49 @@ describe("Event Builder", () => { exact: false, }) - await React.act(async () => { - await userEvent.type(apiSecret, "my_secret", { delay: 1 }) - await userEvent.type(firebaseAppId, "my_firebase_app_id", { - delay: 1, - }) - await userEvent.type(appInstanceId, "my_instance_id", { delay: 1 }) - await userEvent.type(userId, "my_user_id", { delay: 1 }) - - // TODO - I'm pretty unhappy with this, but I'm having a lot of - // trouble testing the Autocomplete component without doing this. - // This test is somewhat likely to break if we add/remove events & - // event categories so if it's broken, it's probably fine to just - // change the expected values. - const ecInput = within(eventCategory).getByRole("combobox") - eventCategory.focus() - renderer.fireEvent.change(ecInput, { target: { value: "All apps" } }) - - const enInput = within(eventName).getByRole("combobox") - eventCategory.focus() - renderer.fireEvent.change(enInput, { target: { value: "select_content" } }) + userEvent.type(apiSecret, "my_secret") + userEvent.type(firebaseAppId, "my_firebase_app_id") + userEvent.type(appInstanceId, "my_instance_id") + userEvent.type(userId, "my_user_id") + + // TODO - I'm pretty unhappy with this, but I'm having a lot of + // trouble testing the Autocomplete component without doing this. + // This test is somewhat likely to break if we add/remove events & + // event categories so if it's broken, it's probably fine to just + // change the expected values. + const ecInput = within(eventCategory).getByRole("combobox") + eventCategory.focus() + renderer.fireEvent.change(ecInput, { + target: { value: "All apps" }, + }) - await userEvent.type(timestampMicros, "1234", { delay: 1 }) - nonPersonalizedAds.click() + const enInput = within(eventName).getByRole("combobox") + eventCategory.focus() + renderer.fireEvent.change(enInput, { + target: { value: "select_content" }, }) + userEvent.type(timestampMicros, "1234") + userEvent.click(nonPersonalizedAds) + const validatePaper = await findByTestId("validate and send") - expect(validatePaper).toHaveTextContent(/api_secret=my_secret/) - expect(validatePaper).toHaveTextContent( - /firebase_app_id=my_firebase_app_id/ - ) + await renderer.waitFor(() => { + expect(validatePaper).toHaveTextContent(/api_secret=my_secret/) + expect(validatePaper).toHaveTextContent( + /firebase_app_id=my_firebase_app_id/ + ) + }) const payload = await findByTestId("payload") - expect(payload).toHaveTextContent( - /"app_instance_id":"my_instance_id"/ - ) - expect(payload).toHaveTextContent(/"user_id":"my_user_id"/) - expect(payload).toHaveTextContent(/"timestamp_micros":1234/) - expect(payload).toHaveTextContent(/"non_personalized_ads":true/) - expect(payload).toHaveTextContent(/"name":"select_content"/) + await renderer.waitFor(() => { + expect(payload).toHaveTextContent( + /"app_instance_id":"my_instance_id"/ + ) + expect(payload).toHaveTextContent(/"user_id":"my_user_id"/) + expect(payload).toHaveTextContent(/"timestamp_micros":1234/) + expect(payload).toHaveTextContent(/"non_personalized_ads":true/) + expect(payload).toHaveTextContent(/"name":"select_content"/) + }) }) }) describe("for gtag switch", () => { @@ -146,11 +147,8 @@ describe("Event Builder", () => { wrapped ) - await React.act(async () => { - // Choose the second view in the list - const clientToggle = await findByTestId("use firebase") - clientToggle.click() - }) + const clientToggle = await findByTestId("use firebase") + userEvent.click(clientToggle) const apiSecret = await find(Label.APISecret, { exact: false }) const measurementId = await find(Label.MeasurementID, { @@ -167,49 +165,47 @@ describe("Event Builder", () => { exact: false, }) - await React.act(async () => { - await userEvent.type(apiSecret, "my_secret", { delay: 1 }) - await userEvent.type(measurementId, "my_measurement_id", { - delay: 1, - }) - await userEvent.type(clientId, "my_client_id", { delay: 1 }) - await userEvent.type(userId, "{selectall}{backspace}my_user_id", { - delay: 1, - }) - - // TODO - I'm pretty unhappy with this, but I'm having a lot of - // trouble testing the Autocomplete component without doing this. - // This test is somewhat likely to break if we add/remove events & - // event categories so if it's broken, it's probably fine to just - // change the expected values. - const ecInput = within(eventCategory).getByRole("combobox") - //eventCategory.focus() - renderer.fireEvent.change(ecInput, { target: { value: "All apps" } }) - - const enInput = within(eventName).getByRole("combobox") - //eventCategory.focus() - renderer.fireEvent.change(enInput, { target: { value: "campaign_details" } }) - - await userEvent.type( - timestampMicros, - "{selectall}{backspace}1234", - { delay: 1 } - ) - nonPersonalizedAds.click() + userEvent.type(apiSecret, "my_secret") + userEvent.type(measurementId, "my_measurement_id") + userEvent.type(clientId, "my_client_id") + userEvent.type(userId, "{selectall}{backspace}my_user_id") + + // TODO - I'm pretty unhappy with this, but I'm having a lot of + // trouble testing the Autocomplete component without doing this. + // This test is somewhat likely to break if we add/remove events & + // event categories so if it's broken, it's probably fine to just + // change the expected values. + const ecInput = within(eventCategory).getByRole("combobox") + //eventCategory.focus() + renderer.fireEvent.change(ecInput, { + target: { value: "All apps" }, }) + const enInput = within(eventName).getByRole("combobox") + //eventCategory.focus() + renderer.fireEvent.change(enInput, { + target: { value: "campaign_details" }, + }) + + userEvent.type(timestampMicros, "{selectall}{backspace}1234") + userEvent.click(nonPersonalizedAds) + const validatePaper = await findByTestId("validate and send") - expect(validatePaper).toHaveTextContent(/api_secret=my_secret/) - expect(validatePaper).toHaveTextContent( - /measurement_id=my_measurement_id/ - ) + await renderer.waitFor(() => { + expect(validatePaper).toHaveTextContent(/api_secret=my_secret/) + expect(validatePaper).toHaveTextContent( + /measurement_id=my_measurement_id/ + ) + }) const payload = await findByTestId("payload") - expect(payload).toHaveTextContent(/"client_id":"my_client_id"/) - expect(payload).toHaveTextContent(/"user_id":"my_user_id"/) - expect(payload).toHaveTextContent(/"timestamp_micros":1234/) - expect(payload).toHaveTextContent(/"non_personalized_ads":true/) - expect(payload).toHaveTextContent(/"name":"campaign_details"/) + await renderer.waitFor(() => { + expect(payload).toHaveTextContent(/"client_id":"my_client_id"/) + expect(payload).toHaveTextContent(/"user_id":"my_user_id"/) + expect(payload).toHaveTextContent(/"timestamp_micros":1234/) + expect(payload).toHaveTextContent(/"non_personalized_ads":true/) + expect(payload).toHaveTextContent(/"name":"campaign_details"/) + }) }) }) }) @@ -219,10 +215,8 @@ describe("Event Builder", () => { const { wrapped } = withProviders(, { isLoggedIn: false }) const { findByTestId } = renderer.render(wrapped) - await React.act(async () => { - const clientToggle = await findByTestId("use firebase") - clientToggle.click() - }) + const clientToggle = await findByTestId("use firebase") + userEvent.click(clientToggle) const eventName = await findByTestId(Label.EventName) const enInput = within(eventName).getByRole("combobox")