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.

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

Apply User-Defined Date Formatting Settings to WP Admin React Components #9635

Conversation

mgascam
Copy link
Contributor

@mgascam mgascam commented Oct 28, 2024

Fixes #6567

This pull request includes changes to ensure that user-defined date and time formatting settings are respected across various React components in the WooCommerce Payments plugin. The most important changes include the replacement of dateI18n with custom date formatting functions, the addition of a new DateFormatNotice component, and updates to test files to accommodate these changes.

Improvements to date and time formatting:

Addition of DateFormatNotice component:

Test updates:

  • Updated test files to mock date formats and include the new DateFormatNotice component in snapshots. [1] [2] [3] [4] [5] [6] [7] [8] [9]

Testing instructions

General Date and Time Display Testing
  • Navigate to WP Admin > Settings > General and set your preferred Date and Time Formats and Timezone.
  • Verify that the date format matches the user-defined settings in WordPress when you navigate to the WooPayments plugin pages.
  • Verify that the dates and times are correctly converted from UTC to your preferred timezone.
Component Specific Testing

Transactions

  • Navigate to WP Admin > Payments > Transactions.
  • Verify the values in Date / Time and Deposit Date columns.
Screenshot 2024-10-29 at 17 12 32 Screenshot 2024-10-29 at 17 12 37

Capital Loans

This section is hidden if your user does not have capital loans. Follow the steps in this guide to create one offer for your user. Once the offer is created, your user will get an email to confirm it. Visit the link in the email, but make sure to use your local store URL, not the server one.

  • Navigate to WP Admin > Payments > Capital loans
  • Verify that the Active loan overview dates are formatted according to the user settings.
  • Verify that the All Loans table dates are formatted according to the user settings. There are two columns that contain dates: Disbursed and First Paydown.
Screenshot 2024-11-04 at 12 17 37

Deposits

  • Navigate to WP Admin > Payments > Deposits
  • Verify that the dates in the Deposit history table are formatted according to the user settings
Screenshot 2024-11-04 at 12 23 38
  • Click on a row and verify that the date and times in the Date /Time column of the deposit details are formatted correctly.
Screenshot 2024-11-04 at 12 24 13

Disputes
You can use test cards to create disputed orders.

  • Navigate to WP Admin > Payments > Disputes
  • Verify the date and time in the Disputed on column (you may need to reveal it)
Screenshot 2024-11-04 at 12 40 28 - Click one row to navigate to the details view and verify the dates Screenshot 2024-11-04 at 12 41 16
  • Click on Challenge Dispute and verify the dates in the form are correct
Screenshot 2024-11-04 at 12 45 21

Documents

In the server, you can use the cli to add a document to your account wp wcpay add_document_for_account 236870460 vat_invoice --period_from="2024-10-01 00:00:00" --period_to="2024-10-31 23:59:59"

  • Navigate to WP Admin > Payments > Documents
  • Verify that the dates in the Date and Description columns in the Documents list are correctly formatted
Screenshot 2024-11-04 at 13 07 17

Date and time format change notice

  • Navigate to any page under the WP Admin > Payments menu i.e. Transactions.
  • Check that the following notice is displayed.
Screenshot 2024-12-18 at 13 18 33 - Click the `settings link` and check that you are redirected to the `Settings > General` page. - Navigate back to the previous page. - Check that the notice is no longer displayed. - Delete the `wcpay_date_format_notice_dismissed` option. - Check the notice is displayed on any page under `WP Admin > Payments` menu. - Dismiss the notice by clicking on the icon. - Check that the notice is no longer displayed.
  • Run npm run changelog to add a changelog file, choose patch to 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.
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply)

Post merge

@mgascam mgascam linked an issue Oct 28, 2024 that may be closed by this pull request
@mgascam mgascam removed the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Dec 13, 2024
@mgascam
Copy link
Contributor Author

mgascam commented Dec 13, 2024

Thank you, everyone, for your helpful comments!

Ship the change with a release note, clarify in docs etc as above AND implement some kind of notice in the admin UI. Any kind of notice, whatever works best / we can ship without a hassle.

@haszari I also prefer this option, and it seems we are all aligned on it. I've made the necessary changes to use the BannerNotice component in this commit.

For context, I completely agree that this is a bug. However, my main concern has been avoiding unintended negative impacts from fixing it, especially given the low number of complaints. The suggestion to show a notice for merchants resonated with me as a practical solution. Additionally, I appreciated @rogermattic's idea to place the notice within the table context, as it would have been more targeted.

Initially, I assumed it would be straightforward to implement this change using the new TableCard component from the woocommerce/components repository. However, after gaining more experience, I realized I underestimated the complexity. Specifically, the plugin relies on the woocommerce/components bundle from WooCommerce core, which means we would need to wait for a new WooCommerce core release. This approach would be problematic, as merchants who update WooPayments without updating WooCommerce core would not see the notice.

Given this limitation, I believe moving forward with the BannerNotice is the best option. While not ideal, it ensures we can inform merchants about the change effectively.

I've added the notice to all pages under the Payments navigation menu. Below are some screenshots for your review—please let me know your thoughts.

Lastly, I’ll be on extended AFK starting Dec 23rd, so it would be great to get additional reviews and feedback next week. Thank you!

Screenshot 2024-12-13 at 17 38 52 Screenshot 2024-12-13 at 17 39 09 Screenshot 2024-12-13 at 17 38 52

cc: @AashikP @aheckler

@AashikP
Copy link

AashikP commented Dec 14, 2024

Looks good to me! Thank you!

@Jinksi Jinksi self-requested a review December 17, 2024 23:00
Copy link
Member

@Jinksi Jinksi left a comment

Choose a reason for hiding this comment

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

I’ve conducted a round of testing on this PR and can see that it is looking good! It is so good to see more consistent datetime handling across the UI and in the code. 🙌

I've left a suggestion about using WP option rather than localStorage to persist the banner dismissal state, which should improve UX for merchants who dismiss the notice.

To summarise the only inconsistencies that I’ve found (not blocking merge):

  • Server-generated CSVs datetimes do not use the WP settings datetime format
  • Client-generated CSV datetimes do use the WP settings datetime format, except for Transactions
    • Transaction client-generated CSV export looks to be intentionally matching the server formatting instead
    • We shouldn’t worry about inconsistencies across JS/server CSVs in this PR, since we are fixing those in Reporting: Move all CSV exports to endpoint (server) exports #9969. @Automattic/helix we should consider a consistent, intentional approach to datetime formatting as part of the Simplify CSV project paJDYF-g1S-p2 – either use the WP Settings datetime formatting, or use a standard format, e.g. ISO 8601.
  • The main dates and times of the transaction timeline view don’t use the WP settings datetime format – this may need to be fixed upstream in @woocommerce/components
  • Server-generated VAT invoices use a server-defined format – again, not a worry for this PR

Steps tested

  • Set WP date/time settings to m_d_Y H_i (12_17_2024 23_56)
  • Set WP store TZ to New York
  • Set browser time to Mumbai (Browser Dev Tools → More Tools → Sensors)
  • Payments → Overview
    • New notice displayed and links to the correct page ✅
    • Dismissing the notice works ✅
    • Payout dates are formatted as per WP settings ✅
  • Payout detail view
    • Dates and times formatted as per WP settings ✅
  • Payouts list view
    • Dates and times formatted as per WP settings ✅
    • Client-side CSV export dates and times are formatted as per WP settings ✅
    • Server CSV export dates and times are not formatted as per WP settings ⚠️
  • Transaction detail view
    • Transation summary and dispute dates and times formatted as per WP settings ✅
    • Timeline event details (e.g. payout event) dates and times are formatted as per WP settings ✅
    • Main timeline dates and times are not formatted as per WP settings ⚠️
  • Transactions list view
    • Dates and times formatted as per WP settings ✅
    • Client-side CSV export dates and times are not formatted as per WP settings ⚠️
    • Server CSV export dates and times are not formatted as per WP settings ⚠️
  • Disputes list view
    • Dates and times formatted as per WP settings ✅
    • Client-side CSV export dates and times are formatted as per WP settings ✅
    • Server CSV export dates and times are not formatted as per WP settings ⚠️
  • Dispute challenge view
    • Dates and times formatted as per WP settings ✅
  • Capital Loan list view
    • Dates and times formatted as per WP settings ✅
  • Documents list view
    • Dates and times formatted as per WP settings ✅
  • VAT invoice download
    • Dates and times are formatted as per WP settings ⚠️ (server-generated)

client/components/date-format-notice/index.tsx Outdated Show resolved Hide resolved
@mgascam mgascam added this pull request to the merge queue Dec 20, 2024
Merged via the queue into develop with commit 3794508 Dec 20, 2024
25 checks passed
@mgascam mgascam deleted the fix/6567-user-set-date-and-time-formatting-arent-respected-in-react-components branch December 20, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

User set Date and Time formatting aren't respected in react components
9 participants