Skip to content
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

Reporting: Track events for View report links on Payment activity widget #8687

Merged
merged 18 commits into from
May 1, 2024
Merged

Reporting: Track events for View report links on Payment activity widget #8687

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ad3fc2c
Add link click handler to tile component
Apr 22, 2024
7394807
Add track events to each link
Apr 22, 2024
9293aff
Update event types
Apr 22, 2024
4c050df
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
Apr 22, 2024
4358ef9
Add changelog
Apr 22, 2024
21d36a0
Amend event names
Apr 23, 2024
5454bc2
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
Apr 23, 2024
4a346a7
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
jessy-p Apr 24, 2024
922feb8
Tweak comment
Apr 24, 2024
bf77f83
Update changelog to a comment
Apr 24, 2024
ed37a77
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
haszari Apr 26, 2024
33317bc
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
Apr 26, 2024
5069046
Refactor track event handler
Apr 26, 2024
3d5a2d1
Update event type
Apr 26, 2024
019f8cd
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
Apr 29, 2024
5c5da5b
Use an explicit prop for tracksSource
Apr 29, 2024
fd6b1c8
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
Apr 29, 2024
83a690e
Merge branch 'develop' into add/8686-track-events-payment-activity-wi…
shendy-a8c Apr 29, 2024
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
4 changes: 4 additions & 0 deletions changelog/add-8686-track-events-payment-activity-widget
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: add

nagpai marked this conversation as resolved.
Show resolved Hide resolved
Comment: Behind a feature flag. The PR adds track events for clicks on `View report` links on Payment activity widget
nagpai marked this conversation as resolved.
Show resolved Hide resolved
34 changes: 34 additions & 0 deletions client/components/payment-activity/payment-activity-data.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { ClickTooltip } from '../tooltip';
import { usePaymentActivityData } from 'wcpay/data';
import { getAdminUrl } from 'wcpay/utils';
import type { DateRange } from './types';
import { recordEvent } from 'wcpay/tracks';
import './style.scss';

/**
Expand Down Expand Up @@ -83,6 +84,15 @@ const PaymentActivityData: React.FC = () => {
/>
}
amount={ totalPaymentVolume }
handleReportLinkClick={ () => {
recordEvent(
'wcpay_overview_payment_activity_total_payment_volume_click',
{
source:
'total_payment_volume_tile_view_report_link',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added an explicit tracksSource prop here - 5c5da5b . Thanks!

}
);
} }
reportLink={ getAdminUrl( {
page: 'wc-admin',
path: '/payments/transactions',
Expand Down Expand Up @@ -121,6 +131,14 @@ const PaymentActivityData: React.FC = () => {
/>
}
amount={ charges }
handleReportLinkClick={ () => {
recordEvent(
'wcpay_overview_payment_activity_charges_click',
{
source: 'charges_tile_view_report_link',
}
);
} }
reportLink={ getAdminUrl( {
page: 'wc-admin',
path: '/payments/transactions',
Expand All @@ -134,6 +152,14 @@ const PaymentActivityData: React.FC = () => {
label={ __( 'Refunds', 'woocommerce-payments' ) }
currencyCode={ storeCurrency }
amount={ refunds }
handleReportLinkClick={ () => {
recordEvent(
'wcpay_overview_payment_activity_refunds_click',
{
source: 'refunds_tile_view_report_link',
}
);
} }
reportLink={ getAdminUrl( {
page: 'wc-admin',
path: '/payments/transactions',
Expand All @@ -153,6 +179,14 @@ const PaymentActivityData: React.FC = () => {
label={ __( 'Disputes', 'woocommerce-payments' ) }
currencyCode={ storeCurrency }
amount={ disputes }
handleReportLinkClick={ () => {
recordEvent(
'wcpay_overview_payment_activity_disputes_click',
{
source: 'disputes_tile_view_report_link',
}
);
} }
reportLink={ getAdminUrl( {
page: 'wc-admin',
path: '/payments/disputes',
Expand Down
8 changes: 7 additions & 1 deletion client/components/payment-activity/payment-data-tile.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { Link } from '@woocommerce/components';
import { formatCurrency } from 'wcpay/utils/currency';
import Loadable from '../loadable';
import './style.scss';

interface PaymentDataTileProps {
/**
* The id for the tile, can be used for CSS styling.
Expand Down Expand Up @@ -41,6 +42,10 @@ interface PaymentDataTileProps {
* Optional hover link to view report.
*/
reportLink?: string;
/**
* Optional click handler for the tile.
*/
handleReportLinkClick?: () => void;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explained in the earlier comment.

#8687 (comment)

Thanks!

}

const PaymentDataTile: React.FC< PaymentDataTileProps > = ( {
Expand All @@ -51,6 +56,7 @@ const PaymentDataTile: React.FC< PaymentDataTileProps > = ( {
amount = 0,
isLoading = false,
reportLink,
handleReportLinkClick,
} ) => {
return (
<div id={ id } className="wcpay-payment-data-highlights__item">
Expand All @@ -71,7 +77,7 @@ const PaymentDataTile: React.FC< PaymentDataTileProps > = ( {
/>
</p>
{ reportLink && (
<Link href={ reportLink }>
<Link href={ reportLink } onClick={ handleReportLinkClick }>
{ __( 'View report', 'woocommerce_payments' ) }
</Link>
) }
Expand Down
4 changes: 4 additions & 0 deletions client/tracks/event.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ export type Event =
| 'wcpay_overview_deposits_view_history_click'
| 'wcpay_overview_deposits_change_schedule_click'
| 'wcpay_overview_task_click'
| 'wcpay_overview_payment_activity_total_payment_volume_click'
| 'wcpay_overview_payment_activity_charges_click'
| 'wcpay_overview_payment_activity_refunds_click'
| 'wcpay_overview_payment_activity_disputes_click'
| 'wcpay_view_submitted_evidence_clicked'
| 'wcpay_settings_deposits_manage_in_stripe_click'
| 'wcpay_merchant_settings_file_upload_started'
Expand Down
Loading