Skip to content

Commit f94e627

Browse files
lindsey-voltalindsey-volta
authored
chore(EventBuilder): remove reserved event validation (#2263)
* remove reserved event checks * chore: fix up some async issues in unit tests * remove check for reserved user property names
1 parent 37c3219 commit f94e627

3 files changed

Lines changed: 79 additions & 163 deletions

File tree

‎src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.spec.ts‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -137,19 +137,6 @@ describe("formatCheckLib", () => {
137137

138138
expect(errors).toEqual([])
139139
})
140-
141-
test("returns an error when event's name is a reserved name", () => {
142-
const payload = {app_instance_id: "12345678901234567890123456789012", events: [{name: 'ad_click'}]}
143-
const firebaseAppId = '1:1233455666:android:abcdefgh'
144-
const instanceId = {firebase_app_id: firebaseAppId}
145-
const api_secret = '123'
146-
147-
let errors = formatCheckLib(payload, instanceId, api_secret, true)
148-
149-
expect(errors[0].description).toEqual(
150-
"ad_click is a reserved event name"
151-
)
152-
})
153140
})
154141

155142
describe("returns invalidUserPropertyName errors", () => {
@@ -163,19 +150,6 @@ describe("formatCheckLib", () => {
163150

164151
expect(errors).toEqual([])
165152
})
166-
167-
test("returns an error when event's name is a reserved name", () => {
168-
const payload = {app_instance_id: "12345678901234567890123456789012", user_properties: {'first_open_time': 'test'}}
169-
const firebaseAppId = '1:1233455666:android:abcdefgh'
170-
const instanceId = {firebase_app_id: firebaseAppId}
171-
const api_secret = '123'
172-
173-
let errors = formatCheckLib(payload, instanceId, api_secret, true)
174-
175-
expect(errors[0].description).toEqual(
176-
"user_property: 'first_open_time' is a reserved user property name"
177-
)
178-
})
179153
})
180154

181155
describe("returns invalidCurrencyType errors", () => {

‎src/components/ga4/EventBuilder/ValidateEvent/handlers/formatCheckLib.ts‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -4,26 +4,12 @@ import sizeof from "object-sizeof"
44
import {eventDefinitions} from "../schemas/eventTypes/eventDefinitions"
55
import {InstanceId} from "../../types"
66

7-
const RESERVED_EVENT_NAMES = [
8-
"ad_activeview", "ad_click", "ad_exposure", "ad_query",
9-
"adunit_exposure", "app_clear_data", "app_install", "app_update",
10-
"app_remove", "error", "first_open", "first_visit", "in_app_purchase",
11-
"notification_dismiss", "notification_foreground", "notification_open",
12-
"notification_receive", "os_update", "session_start", "user_engagement"
13-
]
14-
const RESERVED_USER_PROPERTY_NAMES = [
15-
"first_open_time", "first_visit_time", "last_deep_link_referrer", "user_id",
16-
"first_open_after_install"
17-
]
18-
197
// formatCheckLib provides additional validations for payload not included in
208
// the schema validations. All checks are consistent with Firebase documentation.
219
export const formatCheckLib = (payload: any, instanceId: InstanceId, api_secret: string, useFirebase: boolean) => {
2210
let errors: ValidationMessage[] = []
2311

2412
const appOrClientErrors = isValidAppOrClientId(payload, useFirebase)
25-
const eventNameErrors = isValidEventName(payload)
26-
const userPropertyNameErrors = isValidUserPropertyName(payload)
2713
const currencyErrors = isValidCurrencyType(payload)
2814
const emptyItemsErrors = isItemsEmpty(payload)
2915
const itemsRequiredKeyErrors = itemsHaveRequiredKey(payload)
@@ -34,8 +20,6 @@ export const formatCheckLib = (payload: any, instanceId: InstanceId, api_secret:
3420
return [
3521
...errors,
3622
...appOrClientErrors,
37-
...eventNameErrors,
38-
...userPropertyNameErrors,
3923
...currencyErrors,
4024
...emptyItemsErrors,
4125
...itemsRequiredKeyErrors,
@@ -91,42 +75,6 @@ const isValidAppOrClientId = (payload: any, useFirebase: boolean) => {
9175
return errors
9276
}
9377

94-
95-
const isValidEventName = (payload: any) => {
96-
let errors: ValidationMessage[] = []
97-
98-
payload.events?.forEach((ev: any) => {
99-
if (RESERVED_EVENT_NAMES.includes(ev.name)) {
100-
errors.push({
101-
description: `${ev.name} is a reserved event name`,
102-
validationCode: "value_invalid",
103-
fieldPath: "#/events/name"
104-
})
105-
}
106-
})
107-
108-
return errors
109-
}
110-
111-
const isValidUserPropertyName = (payload: any) => {
112-
const errors: ValidationMessage[] = []
113-
const userProperties = payload.user_properties
114-
115-
if (userProperties) {
116-
Object.keys(userProperties).forEach(prop => {
117-
if (RESERVED_USER_PROPERTY_NAMES.includes(prop)) {
118-
errors.push({
119-
description: `user_property: '${prop}' is a reserved user property name`,
120-
validationCode: "value_invalid",
121-
fieldPath: "user_property"
122-
})
123-
}
124-
})
125-
}
126-
127-
return errors
128-
}
129-
13078
const isValidCurrencyType = (payload: any) => {
13179
const errors: ValidationMessage[] = []
13280

‎src/components/ga4/EventBuilder/index.spec.tsx‎

Lines changed: 79 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,8 @@ describe("Event Builder", () => {
5050
const { wrapped } = withProviders(<Sut />, { isLoggedIn: false })
5151
const { findByLabelText, findByTestId } = renderer.render(wrapped)
5252

53-
await React.act(async () => {
54-
// Choose the second view in the list
55-
const clientToggle = await findByTestId("use firebase")
56-
clientToggle.click()
57-
})
53+
const clientToggle = await findByTestId("use firebase")
54+
userEvent.click(clientToggle)
5855

5956
await findByLabelText(Label.APISecret, { exact: false })
6057

@@ -95,45 +92,49 @@ describe("Event Builder", () => {
9592
exact: false,
9693
})
9794

98-
await React.act(async () => {
99-
await userEvent.type(apiSecret, "my_secret", { delay: 1 })
100-
await userEvent.type(firebaseAppId, "my_firebase_app_id", {
101-
delay: 1,
102-
})
103-
await userEvent.type(appInstanceId, "my_instance_id", { delay: 1 })
104-
await userEvent.type(userId, "my_user_id", { delay: 1 })
105-
106-
// TODO - I'm pretty unhappy with this, but I'm having a lot of
107-
// trouble testing the Autocomplete component without doing this.
108-
// This test is somewhat likely to break if we add/remove events &
109-
// event categories so if it's broken, it's probably fine to just
110-
// change the expected values.
111-
const ecInput = within(eventCategory).getByRole("combobox")
112-
eventCategory.focus()
113-
renderer.fireEvent.change(ecInput, { target: { value: "All apps" } })
114-
115-
const enInput = within(eventName).getByRole("combobox")
116-
eventCategory.focus()
117-
renderer.fireEvent.change(enInput, { target: { value: "select_content" } })
95+
userEvent.type(apiSecret, "my_secret")
96+
userEvent.type(firebaseAppId, "my_firebase_app_id")
97+
userEvent.type(appInstanceId, "my_instance_id")
98+
userEvent.type(userId, "my_user_id")
99+
100+
// TODO - I'm pretty unhappy with this, but I'm having a lot of
101+
// trouble testing the Autocomplete component without doing this.
102+
// This test is somewhat likely to break if we add/remove events &
103+
// event categories so if it's broken, it's probably fine to just
104+
// change the expected values.
105+
const ecInput = within(eventCategory).getByRole("combobox")
106+
eventCategory.focus()
107+
renderer.fireEvent.change(ecInput, {
108+
target: { value: "All apps" },
109+
})
118110

119-
await userEvent.type(timestampMicros, "1234", { delay: 1 })
120-
nonPersonalizedAds.click()
111+
const enInput = within(eventName).getByRole("combobox")
112+
eventCategory.focus()
113+
renderer.fireEvent.change(enInput, {
114+
target: { value: "select_content" },
121115
})
122116

117+
userEvent.type(timestampMicros, "1234")
118+
userEvent.click(nonPersonalizedAds)
119+
123120
const validatePaper = await findByTestId("validate and send")
124-
expect(validatePaper).toHaveTextContent(/api_secret=my_secret/)
125-
expect(validatePaper).toHaveTextContent(
126-
/firebase_app_id=my_firebase_app_id/
127-
)
121+
await renderer.waitFor(() => {
122+
expect(validatePaper).toHaveTextContent(/api_secret=my_secret/)
123+
expect(validatePaper).toHaveTextContent(
124+
/firebase_app_id=my_firebase_app_id/
125+
)
126+
})
128127

129128
const payload = await findByTestId("payload")
130-
expect(payload).toHaveTextContent(
131-
/"app_instance_id":"my_instance_id"/
132-
)
133-
expect(payload).toHaveTextContent(/"user_id":"my_user_id"/)
134-
expect(payload).toHaveTextContent(/"timestamp_micros":1234/)
135-
expect(payload).toHaveTextContent(/"non_personalized_ads":true/)
136-
expect(payload).toHaveTextContent(/"name":"select_content"/)
129+
await renderer.waitFor(() => {
130+
expect(payload).toHaveTextContent(
131+
/"app_instance_id":"my_instance_id"/
132+
)
133+
expect(payload).toHaveTextContent(/"user_id":"my_user_id"/)
134+
expect(payload).toHaveTextContent(/"timestamp_micros":1234/)
135+
expect(payload).toHaveTextContent(/"non_personalized_ads":true/)
136+
expect(payload).toHaveTextContent(/"name":"select_content"/)
137+
})
137138
})
138139
})
139140
describe("for gtag switch", () => {
@@ -146,11 +147,8 @@ describe("Event Builder", () => {
146147
wrapped
147148
)
148149

149-
await React.act(async () => {
150-
// Choose the second view in the list
151-
const clientToggle = await findByTestId("use firebase")
152-
clientToggle.click()
153-
})
150+
const clientToggle = await findByTestId("use firebase")
151+
userEvent.click(clientToggle)
154152

155153
const apiSecret = await find(Label.APISecret, { exact: false })
156154
const measurementId = await find(Label.MeasurementID, {
@@ -167,49 +165,47 @@ describe("Event Builder", () => {
167165
exact: false,
168166
})
169167

170-
await React.act(async () => {
171-
await userEvent.type(apiSecret, "my_secret", { delay: 1 })
172-
await userEvent.type(measurementId, "my_measurement_id", {
173-
delay: 1,
174-
})
175-
await userEvent.type(clientId, "my_client_id", { delay: 1 })
176-
await userEvent.type(userId, "{selectall}{backspace}my_user_id", {
177-
delay: 1,
178-
})
179-
180-
// TODO - I'm pretty unhappy with this, but I'm having a lot of
181-
// trouble testing the Autocomplete component without doing this.
182-
// This test is somewhat likely to break if we add/remove events &
183-
// event categories so if it's broken, it's probably fine to just
184-
// change the expected values.
185-
const ecInput = within(eventCategory).getByRole("combobox")
186-
//eventCategory.focus()
187-
renderer.fireEvent.change(ecInput, { target: { value: "All apps" } })
188-
189-
const enInput = within(eventName).getByRole("combobox")
190-
//eventCategory.focus()
191-
renderer.fireEvent.change(enInput, { target: { value: "campaign_details" } })
192-
193-
await userEvent.type(
194-
timestampMicros,
195-
"{selectall}{backspace}1234",
196-
{ delay: 1 }
197-
)
198-
nonPersonalizedAds.click()
168+
userEvent.type(apiSecret, "my_secret")
169+
userEvent.type(measurementId, "my_measurement_id")
170+
userEvent.type(clientId, "my_client_id")
171+
userEvent.type(userId, "{selectall}{backspace}my_user_id")
172+
173+
// TODO - I'm pretty unhappy with this, but I'm having a lot of
174+
// trouble testing the Autocomplete component without doing this.
175+
// This test is somewhat likely to break if we add/remove events &
176+
// event categories so if it's broken, it's probably fine to just
177+
// change the expected values.
178+
const ecInput = within(eventCategory).getByRole("combobox")
179+
//eventCategory.focus()
180+
renderer.fireEvent.change(ecInput, {
181+
target: { value: "All apps" },
199182
})
200183

184+
const enInput = within(eventName).getByRole("combobox")
185+
//eventCategory.focus()
186+
renderer.fireEvent.change(enInput, {
187+
target: { value: "campaign_details" },
188+
})
189+
190+
userEvent.type(timestampMicros, "{selectall}{backspace}1234")
191+
userEvent.click(nonPersonalizedAds)
192+
201193
const validatePaper = await findByTestId("validate and send")
202-
expect(validatePaper).toHaveTextContent(/api_secret=my_secret/)
203-
expect(validatePaper).toHaveTextContent(
204-
/measurement_id=my_measurement_id/
205-
)
194+
await renderer.waitFor(() => {
195+
expect(validatePaper).toHaveTextContent(/api_secret=my_secret/)
196+
expect(validatePaper).toHaveTextContent(
197+
/measurement_id=my_measurement_id/
198+
)
199+
})
206200

207201
const payload = await findByTestId("payload")
208-
expect(payload).toHaveTextContent(/"client_id":"my_client_id"/)
209-
expect(payload).toHaveTextContent(/"user_id":"my_user_id"/)
210-
expect(payload).toHaveTextContent(/"timestamp_micros":1234/)
211-
expect(payload).toHaveTextContent(/"non_personalized_ads":true/)
212-
expect(payload).toHaveTextContent(/"name":"campaign_details"/)
202+
await renderer.waitFor(() => {
203+
expect(payload).toHaveTextContent(/"client_id":"my_client_id"/)
204+
expect(payload).toHaveTextContent(/"user_id":"my_user_id"/)
205+
expect(payload).toHaveTextContent(/"timestamp_micros":1234/)
206+
expect(payload).toHaveTextContent(/"non_personalized_ads":true/)
207+
expect(payload).toHaveTextContent(/"name":"campaign_details"/)
208+
})
213209
})
214210
})
215211
})
@@ -219,10 +215,8 @@ describe("Event Builder", () => {
219215
const { wrapped } = withProviders(<Sut />, { isLoggedIn: false })
220216
const { findByTestId } = renderer.render(wrapped)
221217

222-
await React.act(async () => {
223-
const clientToggle = await findByTestId("use firebase")
224-
clientToggle.click()
225-
})
218+
const clientToggle = await findByTestId("use firebase")
219+
userEvent.click(clientToggle)
226220

227221
const eventName = await findByTestId(Label.EventName)
228222
const enInput = within(eventName).getByRole("combobox")

0 commit comments

Comments
 (0)