Adjust event tracking for the creating and claim buttons of Google Ads account #2419

eason9487 · 2024-05-30T11:21:28Z

Changes proposed in this Pull Request:

PT: pcTzPl-2iK-p2

Since the context property of most event tracking is currently hardcoded, and there is no proper way to obtain state or data from far away components when sending event tracking, this PR tries to implement it via @wordpress/hooks. The goal is to avoid passing event properties in a prop drilling fashion, e.g. context and step at the <Stepper> layer in this PR need to be sent together in buttons several layers away.

Due to the number of files being changed, to avoid this PR getting too large, I would like to use two tracking events to show how the mechanism works, and then add the remaining event tracking in a subsequent PR.

📌 Mechanism:

Add a React hook useEventPropertiesFilter to handle event properties across multiple layers of components via @wordpress/hooks.

📌 Event tracking:

Add context and step event properties to Extension and Ads onboarding steppers for use by their subcomponents.
Add event tracking to ClaimAccountButton.
Add context and step event properties to the click event tracking in TermsModal.

📌 Extra change:

Reuse ClaimAccountButton to reduce the duplicate implementation.
Adjust jest config and setup so that tests related to event tracking don't have to validate global event properties.

Screenshots:

📷 Event tracking when clicking on button to create Google Ads account in the Extension onboarding

📷 Event tracking when clicking on button to open the claim page of Google Ads account in the Extension onboarding

📷 Event tracking when clicking on button to create Google Ads account in the Ads onboarding

📷 Event tracking when clicking on button to open the claim page of Google Ads account in the Ads onboarding

Detailed test instructions:

Open the Console tab in the browser DevTool, and run localStorage.setItem( 'debug', 'wc-admin:*' ) to enable tracking debugging mode.
Disconnect Google Ads account if there is a connected one.
Go to step 1 of the Extension onboarding.
Create a new Google Ads account.
Check if the gla_ads_account_create_button_click event is sent with context and step
Click the "Claim account" button to open the invitation link in a pop-up window for the newly created Google Ads account.
- It doesn't need to proceed with the claim process here
Check if the gla_ads_account_create_button_click event is sent with context and step.
Disconnect Google Ads account if there is a connected one.
Go to step 1 of the Ads onboarding.
Repeat test steps 4-7

Changelog entry

Tweak - Adjust event tracking for the creating and claim buttons of Google Ads account.

…don't have to validate global event properties.

…s across multiple layers of components via `@wordpress/hooks`.

…ding steppers for use by their subcomponents.

… in `TermsModal`.

codecov · 2024-05-30T11:23:24Z

Codecov Report

Attention: Patch coverage is 98.07692% with 1 line in your changes missing coverage. Please review.

Project coverage is 62.6%. Comparing base (c5fe2d8) to head (1179f5b).

Additional details and impacted files

@@                             Coverage Diff                             @@
##             feature/seamlessly-conversion-tracking   #2419      +/-   ##
===========================================================================
- Coverage                                      64.0%   62.6%    -1.4%     
===========================================================================
  Files                                           778     321     -457     
  Lines                                         21854    5019   -16835     
  Branches                                       1212    1216       +4     
===========================================================================
- Hits                                          13987    3141   -10846     
+ Misses                                         7698    1709    -5989     
  Partials                                        169     169

Flag	Coverage Δ
js-unit-tests	`62.6% <98.1%> (+0.8%)`	⬆️
php-unit-tests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...ts/google-ads-account-card/claim-account-button.js	`100.0% <100.0%> (+50.0%)`	⬆️
...mponents/google-ads-account-card/create-account.js	`65.6% <100.0%> (ø)`
...nents/google-ads-account-card/terms-modal/index.js	`100.0% <100.0%> (+88.9%)`	⬆️
js/src/hooks/useEventPropertiesFilter.js	`100.0% <100.0%> (ø)`
js/src/setup-ads/ads-stepper/index.js	`100.0% <100.0%> (ø)`
.../src/setup-mc/setup-stepper/saved-setup-stepper.js	`87.8% <100.0%> (+0.3%)`	⬆️
...ectComponentToRecordEventWithFilteredProperties.js	`100.0% <100.0%> (ø)`
js/src/utils/tracks.js	`75.8% <100.0%> (+6.5%)`	⬆️
...nts/google-ads-account-card/claim-account-modal.js	`16.7% <50.0%> (+8.3%)`	⬆️

... and 460 files with indirect coverage changes

tomalec · 2024-05-30T21:06:47Z

At step 7. on onboardign flow, I didn't get context and step properties

Screencast from 30.05.2024 21:45:13.webm

I don't get it when I click "Claim Account" button in the modal in the main flow just after creating a new account.
Once I reload the page after account creation and click "Calim Account" the properties are set correctly.

…oid different hook entities affecting each other. Address: #2419 (comment)

eason9487 · 2024-05-31T03:12:18Z

Hi @tomalec, thanks for catching this.

At step 7. on onboardign flow, I didn't get context and step properties
I don't get it when I click "Claim Account" button in the modal in the main flow just after creating a new account. Once I reload the page after account creation and click "Calim Account" the properties are set correctly.

Sorry, I tested each event tracking separately but missed the interaction between useEventPropertiesFilter instances. The issue should have been fixed in 2129e87.

Could you help with a new round?

eason9487 · 2024-06-04T04:00:01Z

Added JS tests for the useEventPropertiesFilter hook. 8dd83ab

tomalec

Tested locally and reviewed the code LGTM.

Left some non-blocking comments and questions.

js/src/components/google-ads-account-card/claim-account-button.js

js/src/hooks/useEventPropertiesFilter.js

tomalec · 2024-06-04T19:22:10Z

js/src/components/google-ads-account-card/claim-account-button.test.js

+	} );
+
+	it( 'should record click events and be aware of extra event properties from filters', async () => {
+		await expectEventWithPropertiesFilter(


💅
I read expectEventWithPropertiesFilter as I'd read expect(event_with_properties_filter), so something that starts an expectation to which I can attach matches.
But then, in this usage, "I expect the event with properties filter to do what?"

Here, the function already applies matches and actually tests.

WDYT, of renaming the function to expect___To___(). I hope such a notion could help me with the below.
Actually, I had a hard time understanding even after reading the source code, what entity we attach the expectation to: an event, a component, a filter.; and what is the behavior we expect: to be recorded, to be recorded with given properties, to be clicked, to be filtered?

Is it that we "expect the given component, to record an event of given name, with given properties filtered by given filter, once clicked on a target"?
So something like expect( Component ).toRecordEventWithFilteredPropertiesOnceClicked(eventName, properties, filterName, clickTarget )
Or do I miss something?

Is it that we "expect the given component, to record an event of given name, with given properties filtered by given filter, once clicked on a target"?

Yes.

WDYT, of renaming the function to expect___To___(). I hope such a notion could help me with the below.
Actually, I had a hard time understanding even after reading the source code, what entity we attach the expectation to: an event, a component, a filter.; and what is the behavior we expect: to be recorded, to be recorded with given properties, to be clicked, to be filtered?

Thanks for the suggestion! Made some changes in 8348389:

Added JSDoc

Changed the use of the queryElement callback and renamed it to performAction

The action of triggering an event tracking is completely moved from this function to the callers

This should make the use of this function more flexible and the naming does not need to carry the “OnceClicked”

Renamed expectEventWithPropertiesFilter to expectComponentToRecordEventWithFilteredProperties

js/src/hooks/useEventPropertiesFilter.js

…st a callback argument to make its purpose clearer. Address: #2419 (comment)

Address: - #2419 (comment) - #2419 (comment)

eason9487 added 6 commits May 29, 2024 18:23

Adjust jest config and setup so that tests related to event tracking …

6278f3a

…don't have to validate global event properties.

Add a React hook useEventPropertiesFilter to handle event propertie…

5248319

…s across multiple layers of components via `@wordpress/hooks`.

Add context and step event properties to Extension and Ads onboar…

b771bb2

…ding steppers for use by their subcomponents.

Reuse ClaimAccountButton to reduce the duplicate implementation.

de339b0

Add event tracking to ClaimAccountButton.

b415243

Add context and step event properties to the click event tracking…

5930abc

… in `TermsModal`.

eason9487 requested a review from a team May 30, 2024 11:21

eason9487 self-assigned this May 30, 2024

github-actions bot added the changelog: tweak Small change, that isn't actually very important. label May 30, 2024

eason9487 added 2 commits May 31, 2024 10:58

Make each filter namespace in useEventPropertiesFilter unique to av…

2129e87

…oid different hook entities affecting each other. Address: #2419 (comment)

Tweak the JSDoc of the ClaimAccountButton component.

a63df8d

Add JS tests for the useEventPropertiesFilter hook.

8dd83ab

tomalec approved these changes Jun 4, 2024

View reviewed changes

eason9487 added 2 commits June 5, 2024 16:17

Rename expectEventWithPropertiesFilter function, add JSDoc and adju…

8348389

…st a callback argument to make its purpose clearer. Address: #2419 (comment)

Adjust the JSDoc of the useEventPropertiesFilter hook.

1179f5b

Address: - #2419 (comment) - #2419 (comment)

eason9487 merged commit fade6ca into feature/seamlessly-conversion-tracking Jun 5, 2024
7 checks passed

eason9487 deleted the tweak/2215-event-tracking-part-1 branch June 5, 2024 08:43

eason9487 mentioned this pull request Jun 5, 2024

Enable users to seamlessly set up conversion tracking during the onboarding flow #2415

Merged

2 tasks

mikkamp mentioned this pull request Jun 10, 2024

Release 2.7.2 #2426

Merged

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjust event tracking for the creating and claim buttons of Google Ads account #2419

Adjust event tracking for the creating and claim buttons of Google Ads account #2419

eason9487 commented May 30, 2024 •

edited

Loading

codecov bot commented May 30, 2024 •

edited

Loading

tomalec commented May 30, 2024

eason9487 commented May 31, 2024

eason9487 commented Jun 4, 2024

tomalec left a comment

tomalec Jun 4, 2024

tomalec Jun 4, 2024

eason9487 Jun 5, 2024 •

edited

Loading

Adjust event tracking for the creating and claim buttons of Google Ads account #2419

Adjust event tracking for the creating and claim buttons of Google Ads account #2419

Conversation

eason9487 commented May 30, 2024 • edited Loading

Changes proposed in this Pull Request:

📌 Mechanism:

📌 Event tracking:

📌 Extra change:

Screenshots:

📷 Event tracking when clicking on button to create Google Ads account in the Extension onboarding

📷 Event tracking when clicking on button to open the claim page of Google Ads account in the Extension onboarding

📷 Event tracking when clicking on button to create Google Ads account in the Ads onboarding

📷 Event tracking when clicking on button to open the claim page of Google Ads account in the Ads onboarding

Detailed test instructions:

Changelog entry

codecov bot commented May 30, 2024 • edited Loading

Codecov Report

tomalec commented May 30, 2024

eason9487 commented May 31, 2024

eason9487 commented Jun 4, 2024

tomalec left a comment

Choose a reason for hiding this comment

tomalec Jun 4, 2024

Choose a reason for hiding this comment

tomalec Jun 4, 2024

Choose a reason for hiding this comment

eason9487 Jun 5, 2024 • edited Loading

Choose a reason for hiding this comment

eason9487 commented May 30, 2024 •

edited

Loading

codecov bot commented May 30, 2024 •

edited

Loading

eason9487 Jun 5, 2024 •

edited

Loading