Show UTC label in date time column header of transactions CSV #9461
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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: +43 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
shendy-a8c
changed the title
…ocommerce-payments into add/8522-utc-list-page-csv
shendy-a8c
changed the title
shendy-a8c
changed the title
|
As I am working on this PR, I found a bug. Created here: #9471. |
nagpai
approved these changes
Sep 23, 2024
haszari
approved these changes
Sep 24, 2024
There was a problem hiding this comment.
Approving, as this is a surgical small change that will make the CSVs more consistent and correct – the CSVs are always UTC, so should be labelled as such.
I do not add UTC to the other list pages (deposits and disputes) because they are showing dates only (without time).
We'll need follow up here, because dates can also be affected by timezone. e.g. Disputed at column shows date only but the stored value includes a time, and the date can shift in the UI depending on the WordPress (view) timezone.
In the example below, the transaction / dispute date is 24 Sept, but renders as 23 Sept for some timezones.
Bigger picture – we need to audit how we display dates & times
I got a little bit rabbit holed reviewing this PR and trying to understand what the correct behaviour is – i.e. what we designed.
I think we need to review the various screens with dates/times and audit if they are displayed as UTC, or localised using WordPress timezone (or something else).
Then we can either…
- (a) Confirm the intended design and fix any discrepancies.
- OR
- (b) Refine the design so its consistent, and then fix discrepancies.
I don't think we can fix these bugs in isolation without confirming the intended design. Getting clarity on this might also help with the payment activity card on overview screen.
I had been assuming that we could go with (b) and use UTC everywhere, but I see there are lots of places in the UI where we adjust to WordPress timezone. As noted by @souravdebnath1986 ideally we'd show all times/dates in store timezone (this could be interpreted as WordPress site timezone).
Here are the issues & PRs I found in my research:
open issues
- CSV exports have date/time in UTC, not clear to merchant (vs. localised timezone) (all – e.g. Transactions, Disputes, Deposits) #8522
- https://github.com/Automattic/woocommerce-payments-server/issues/2847
- https://github.com/Automattic/woocommerce-payments-server/issues/4845
- The timezone of the times displayed on the transactions list page is not clear #9472
- Transaction detail screen shows transaction time inconsistently – UTC in header, localised/merchant timezone in timeline #9135 - timeline view,
focus: payments acceptance & processing- notreporting
merged PRs / closed issues
nagpai
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8522
Fixes https://github.com/Automattic/woocommerce-payments-server/issues/2847
I had a hard time digesting the problem here so summarizing it here:
2024-09-19 06:11in my local time (UTC+7).2024-09-18 23:11.dateI18n's description. Therefore, I see the transaction time on the Transactions list page as2024-09-19 00:11.Problem:
Date / Timeand it's not clear what the timezone is.Date / Timeand it's not clear what the timezone is. This is not within the scope of this PR. Created a separate issue: The timezone of the times displayed on the transactions list page is not clear #9472.Changes proposed in this Pull Request
(UTC)to the date time column header. Apply the same for blocked transactions. This PR will add UTC for 'browser' download only because CSV from 'endpoint' download already has UTC in the Date / Time header.I do not add UTC to the other list pages (deposits and disputes) because they are showing dates only (without time).
Testing instructions
Set your blog's timezone to non-UTC. Go to wp-admin > Settings > General > Timezone.
npm run build:client.Go to wp-admin > Payments > Transactions. Click download.
Go to wp-admin > Payments > Transactions > Blocked. Click download. I don't have any blocked transactions, so the download button does not appear for me.
Test the 2 types of download: from browser and from server.
To force it using the
browsermethod:To force it using the
endpointmethod:Confirm that the date time header column is
Date / Time (UTC). For CSV coming from server, if no email arrives at your inbox, check the mail logger installed in your local server WordPress.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