Skip to content

Fix date formatting for API requests when using a non-standard numerals system #11596

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

Merged
merged 1 commit into from
May 28, 2024

Conversation

hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented May 28, 2024

Description

While testing how the app's behave when using Arabic language, I found a very serious bug, multiple of our API requests fail, because they send the date formatted using Arabic-Indic numerals, and thus the API calls fails, this impacts the stats card on the dashboard, all of the Analytics Hub cards, and the Products/variations sale dates. This impacts Arabic language, but could also impact other languages.

I updated the logic to use the Locale.ROOT by default when formatting those dates, which fixes the issues:

Before After
Screenshot 2024-05-28 at 13 10 29 Screenshot 2024-05-28 at 13 13 05

Since this is a serious bug, I'm targeting 18.8 with the fix, but on the other hand, this has been a bug for a very long time, let me know if it's better to update the PR to target 18.9

Testing instructions

  1. Set your phone to Arabic-Egypt language.
  2. Open the app.
  3. Pull-to-refresh the dashboard.
  4. Confirm the stats card is working correctly.
  5. Optionally: check the Analytics Hub, and the products sale feature (this might not be very straightforward when not knowing Arabic 😄)
  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

@hichamboushaba hichamboushaba added type: bug A confirmed bug. impact: high Impacts a lot of users as reported by our Happiness Engineers. labels May 28, 2024
@hichamboushaba hichamboushaba changed the title Use Locale.ROOT by default for formatting dates intended for API Fix date formatting for API requests when using a non-standard numerals system May 28, 2024
@hichamboushaba hichamboushaba marked this pull request as ready for review May 28, 2024 12:20
@dangermattic
Copy link
Collaborator

dangermattic commented May 28, 2024

1 Warning
⚠️ This PR is assigned to the milestone 18.8 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.

Generated by 🚫 Danger

@hichamboushaba hichamboushaba added this to the 18.8 ❄️ milestone May 28, 2024
@hichamboushaba hichamboushaba changed the base branch from trunk to release/18.8 May 28, 2024 12:25
@wpmobilebot
Copy link
Collaborator

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
FlavorJalapeno
Build TypeDebug
Commit31d8f99
Direct Downloadwoocommerce-prototype-build-pr11596-31d8f99.apk

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.33%. Comparing base (70f75dc) to head (31d8f99).

Additional details and impacted files
@@               Coverage Diff               @@
##             release/18.8   #11596   +/-   ##
===============================================
  Coverage           40.33%   40.33%           
  Complexity           5201     5201           
===============================================
  Files                1087     1087           
  Lines               63226    63227    +1     
  Branches             8666     8666           
===============================================
+ Hits                25501    25502    +1     
  Misses              35431    35431           
  Partials             2294     2294           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@0nko 0nko self-assigned this May 28, 2024
Copy link
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

Nice fix! I've confirmed the stats card and the analytics hub are working without issues 👍

@0nko 0nko merged commit fe9505c into release/18.8 May 28, 2024
20 of 22 checks passed
@0nko 0nko deleted the fix-date-formatting-for-api-requests branch May 28, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact: high Impacts a lot of users as reported by our Happiness Engineers. type: bug A confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants