Reporting: Track events for View report links on Payment activity widget
#8687
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +41 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
|
Added note to description – reviewers please test and review these events in context, to ensure they meet the needs of project:
|
| 'wcpay_overview_payment_activity_total_payment_volume_click', | ||
| { | ||
| source: | ||
| 'total_payment_volume_tile_view_report_link', |
There was a problem hiding this comment.
Do we need the source parameter (I'm assuming it's optional)? I think this is used to distinguish generic events by the originating high-level component or container. How do other WooPayments events use source?
In this case we have unique events for each click target, so source is redundant.
An alternative design would be to use a generic event name, and distinguish which box using source:
- en
wcpay_overview_payment_activity_click - source
total_payment_volume
The question is whether we'd want to aggregate any click on the activity widget. If so, a common event is useful. If not, unique event for each thing works well.
There was a problem hiding this comment.
The idea of using a common event for the tile is fantastic, and makes things totally clean! Thanks for opening my eyes to this possibility! Also a great note for me "Is this a good enough reason to pass a callback?" . Clearly no in this case with the alternative you suggested :D .
I have implemented a common click event, and a source that is extrapolated from the existing label prop. ( make it all small case and replace spaces with underscores, where needed)
I hope I have done it appropriately here : 5069046
There was a problem hiding this comment.
Looks good!
I'd suggest using an explicit source value for each tile, rather than generating from label. This is one more prop (tracksSource?), but it's simple and explicit, and allows future tracks investigators to search the code for the source value to work out where it's getting fired.
There was a problem hiding this comment.
I have added an explicit tracksSource prop here - 5c5da5b . Thanks!
| /** | ||
| * Optional click handler for the tile. | ||
| */ | ||
| handleReportLinkClick?: () => void; |
There was a problem hiding this comment.
Is our PaymentDataTile abstraction helping here? If we expose a prop for every detail of it, we might be better off just using Link directly in the outer markup.
Alternatively, if we are ok using the same event name for all tiles e.g. wcpay_overview_payment_tile_click, the component could abstract away the tracking behaviour nicely. It could use an existing prop or expose a tracksEventSource prop, which would avoid complex callback interface.
There was a problem hiding this comment.
|
re-requesting a Helix review as my review was only a quick look/test. |
|
Answering @haszari's question:
Is 'conversion' a click to reports from the Payment Activity widget? If so, yes, with the newly added track event from this PR, we can measure that. If we need more than that, for example, after clicking the link, user will land on the 'report' pages. Views to the transactions list page and disputes list page are already tracked but we can't differentiate the filtering! Looking at the success indicator paJDYF-bWn-p2, the step 'WCPay report views' is looking at 
Viewing the reports from the TPV link, charges link, and refunds link will lead to an increase in I'm struggling to articulate what I mean but I hope it's clear. Otherwise, I can try to explain more. In summary:
|
I think that's fine, we want the user to click the tile and land on report/list page. As I understand it we don't particularly care what state the list view is in (from a tracking/flows POV, we do care of course that the report totals match the widget!). We should clarify this detail with stakeholders / @souravdebnath1986 – so everyone is on the same page about how the events work. |
|
I am merging this PR based on the discussion here - p1714467303200959-slack-C06BBFTF6EM tl;dr, the funnel tool supports adding event properties, e.g. |
|
@haszari
My apologies the PR seems to be related to an older name I was trying, before we moved to single event + different sources per tile. I will have the correct event carefully registered and update here. |
Suggest just adding an issue for the outcome we need, then anyone can pick it up – best to have all tasks logged on the team board so anyone can pick up the next priority, and we have one source of truth for what's left to do. If there's no issue for registering all new / changed events I'd recommend adding one issue for that and mention the specific todos above so they don't get missed. |
@haszari - Thanks for the guidance. I have added an issue to make sure the event is registered before we launch. Automattic/tracks-events-registration#2466 - Added this to the pre-lauch check Epic too. |
Fixes #8686
Changes proposed in this Pull Request
Add track events to track clicks on
View reportlinks on the following tiles:Testing instructions
npm startornpm run watchto build assetsPaymentsand on the overview page, click on theView reportlink that you see overTotal payment volumetile. Click the linkwcpay_overview_total_payment_volume_view_report_clicks. Check the other details shared with the event, including thesourcethat should read astotal_payment_volume_tile_view_reportCharges,RefundsandDisputestile too and see if they generate relevant track events.Reminder: test this change will achieve the desired impact/result!
Reviewers please keep in mind the flow and funnel that we want to measure, and check that this event will allow us to measure the desired funnel conversion rate. i.e. test the whole flow, not just that this event fires.
View report⇒ view relevant report screenSee the project thread on P2 for more details and example funnel.
Notes
Discussion on naming events - p1713786808704209-slack-C06BBFTF6EM
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge