Skip to content

chore(EventBuilder): remove reserved event validation #2263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lindsey-volta merged 3 commits into from
Oct 7, 2025
Merged

chore(EventBuilder): remove reserved event validation #2263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a834e9a
remove reserved event checks
lindsey-volta Oct 3, 2025
254148e
chore: fix up some async issues in unit tests
lindsey-volta Oct 3, 2025
c479118
remove check for reserved user property names
lindsey-volta Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 0 additions & 26 deletions src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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", () => {
Expand Down
52 changes: 0 additions & 52 deletions src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -34,8 +20,6 @@ export const formatCheckLib = (payload: any, instanceId: InstanceId, api_secret:
return [
...errors,
...appOrClientErrors,
...eventNameErrors,
...userPropertyNameErrors,
...currencyErrors,
...emptyItemsErrors,
...itemsRequiredKeyErrors,
Expand Down Expand Up @@ -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[] = []

Expand Down
164 changes: 79 additions & 85 deletions src/components/ga4/EventBuilder/index.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,8 @@ describe("Event Builder", () => {
const { wrapped } = withProviders(<Sut />, { 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 })

Expand Down Expand Up @@ -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", () => {
Expand All @@ -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, {
Expand All @@ -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"/)
})
})
})
})
Expand All @@ -219,10 +215,8 @@ describe("Event Builder", () => {
const { wrapped } = withProviders(<Sut />, { 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")
Expand Down
Loading