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

Jump to bottom

Tracks event prop representing the number of exported transactions is inaccurate #10172

Open
Jinksi opened this issue Jan 17, 2025 · 2 comments · May be fixed by #10411
Open

Tracks event prop representing the number of exported transactions is inaccurate #10172

Jinksi opened this issue Jan 17, 2025 · 2 comments · May be fixed by #10411
Assignees
@nagpai
Labels
focus: reporting priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability type: bug The issue is a confirmed bug.
Milestone
WooPayments 9.0.0

Comments

@Jinksi
Copy link
Member

Jinksi commented Jan 17, 2025 •
edited
Loading

Describe the bug

The tracks event wcpay_transactions_download_csv_click → exported_transactions property does not accurately represent the number of exported transactions when the async, server-based export method is used.

The inaccuracy of this data has made it more difficult to understand how many transactions are being exported by merchants, which is important to help inform us how this flow is being utilised.

The tracks event property description (found on the wcpay_transactions_download_csv_click tracks event listing) describes it as the "Number of transactions included in the downloaded report". Instead, what is provided is the number of transactions currently visible in the UI (rows.length).

woocommerce-payments/client/transactions/list/index.tsx

Lines 703 to 708 in a904341

recordEvent( 'wcpay_transactions_download_csv_click', {
location: props.depositId ? 'deposit_details' : 'transactions',
download_type: downloadType,
exported_transactions: rows.length,
total_transactions: transactionsSummary.count,
} );

This correct value to use here is provided by the /transactions/download POST response exported_transactions value.

To Reproduce

  1. Go to Payments → Transactions
  2. Ensure you have multiple pages (use the query param &per_page=1 to force this)
  3. Using devtools, observe the network requests to t.gif, representing tracks events
  4. Click Download
  5. Notice in the t.gif network with the _en query param wcadmin_wcpay_transactions_download_csv_click. The exported_transactions query param incorrectly provides the current page value (default 25), rather than the total number of transactions that will be exported.

Expected behavior

The tracks event wcpay_transactions_download_csv_click → exported_transactions property should accurately represent the "Number of transactions included in the downloaded report" when the async, server-based export method is used. This correct value to use is provided by the /transactions/download POST response exported_transactions value.

Additional information

  • Relevant to the Simplify CSV reports project: paJDYF-g5h-p2
@Jinksi Jinksi added focus: reporting type: bug The issue is a confirmed bug. labels Jan 17, 2025
@haszari haszari added the priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability label Jan 23, 2025
@haszari
Copy link
Contributor

haszari commented Jan 23, 2025

Prioritising as medium, assuming it's easy and this data will be useful. Could pull this into the current CSV project or handle as maintenance.

@haszari haszari self-assigned this Feb 14, 2025
@haszari
Copy link
Contributor

haszari commented Feb 14, 2025 •
edited
Loading

Looks like the fix is trivial, use totalRows which is already available when the event fires. Actually it already is there – in total_transactions!

Image

So to fix we can just remove exported_transactions prop (etc for all CSVs) – it's not useful, will always be page size.

🤔 Though maybe this reflects any filtering, e.g. the idea is that when exporting, tracks event has a record of total row count and filtered row count. Investigating…

Digression – idea for more uniform event for all CSVs

While we're here, I propose that we tweak the event name so it's consistent "schema" for all similar events, e.g.

  • wcpay_transactions_download_csv_click
    • exported_row_count or filtered_row_count (was exported_transactions)
    • total_row_count (was total_transactions)
  • wcpay_disputes_download_csv_click
    • row_count
  • etc

We could even rename the events to align. Previously export was hinted at by the row count, but now the button is Export, the event could be too.

So here's a proposal for that:

  • wcpay_list_export_csv_click
    • row_type string e.g. transaction | dispute | payout
    • filtered_row_count number e.g. 10947, 44
    • total_row_count number e.g. 349587

FYI @nagpai @shendy-a8c since you are probably in this code right now 😁

@nagpai nagpai linked a pull request Feb 19, 2025 that will close this issue
Open
6 tasks
@nagpai nagpai assigned nagpai and unassigned haszari Feb 20, 2025
@nagpai nagpai added this to the WooPayments 9.0.0 milestone Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nagpai nagpai

Labels
focus: reporting priority: medium The issue/PR is medium priority—non-critical functionality loss, minimal effect on usability type: bug The issue is a confirmed bug.
Projects
None yet
Milestone
WooPayments 9.0.0
Development

Successfully merging a pull request may close this issue.

Update track events for CSV exports Automattic/woocommerce-payments
3 participants
@Jinksi @nagpai @haszari