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

[HOLD for payment 2024-01-18] [$250] Dev: IOS - App crashes when opening a transaction report #27392

Closed
1 of 6 tasks
kbecciv opened this issue Sep 13, 2023 · 78 comments
Closed
1 of 6 tasks
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Sep 13, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open the ios app
  2. Open an iou report that has transactions
  3. Click on a transaction to open the transaction report

Expected Result:

App does not crash

Actual Result:

App crashes with console error: Render Error Invalid time value

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: Dev 1.3.69.1
Reproducible in staging?: no
Reproducible in production?: no
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

2023-09-13.21-46-10.mp4

Expensify/Expensify Issue URL:
Issue reported by: @tienifr
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694440628004359

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~010d24045ce9cc8210
  • Upwork Job ID: 1716907956821823488
  • Last Price Increase: 2024-06-27
  • Automatic offers:
    • situchan | Reviewer | 28015222
    • tienifr | Contributor | 28015224
Issue OwnerCurrent Issue Owner: @
@kbecciv kbecciv added the External Added to denote the issue can be worked on by a contributor label Sep 13, 2023
@melvin-bot melvin-bot bot changed the title Dev: IOS - App crashes when opening a transaction report [$500] Dev: IOS - App crashes when opening a transaction report Sep 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 13, 2023

Job added to Upwork: https://www.upwork.com/jobs/~011e5e93003759619c

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 13, 2023

Triggered auto assignment to @NicMendonca (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot melvin-bot bot added the Daily KSv2 label Sep 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 13, 2023

Triggered auto assignment to Contributor-plus team member for initial proposal review - @situchan (External)

@tienifr
Copy link
Contributor

tienifr commented Sep 14, 2023

I reported this bug a long time ago here: https://expensify.slack.com/archives/C049HHMV9SM/p1693349618431379.

@tienifr
Copy link
Contributor

tienifr commented Sep 15, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

iOS - App crashes when opening a report

What is the root cause of that problem?

This is due to our migration from momentjs to date-fns library recently. One of the participants of that report may have a deprecated timezone in momentjs that is not supported in date-fns. In this case it is Etc/GMT-1 and Asia/Saigon. This is explained in momentjs documentation:

Some libraries use the ECMAScript Intl API for locales, time zones, or both.
Some libraries still provide their own locale and time zone files like Moment and Moment-Timezone do.

date-fns belongs to the Intl group.

However, there might be differences in implementing this Intl across browsers/runtimes. Although they use the same Intl interface, some timezones are supported on one device but not the others. For example, I tried this code to validate a timezone:

Intl.DateTimeFormat(undefined, { timeZone: 'Asia/Saigon' })

but Mac Chrome and iOS yielded different results: success and error. This is a known issue of Hermes: facebook/hermes#23. That's why the app only crashed on iOS but not Web. One more thing is that because momentjs supports almost all IANA timezones, the issue has not happend before.

There've been efforts to add polyfills for this Intl.DateTimeFormat interface for native here:

require('@formatjs/intl-datetimeformat');

But that only implemented the core functions of DateTimeFormat, not the actual polyfills to support ECMA-402.

What changes do you think we should make in order to solve the problem?

Add support for all IANA timezones using formatjs (reference) for iOS platform:

require('@formatjs/intl-datetimeformat/polyfill-force');
require('@formatjs/intl-datetimeformat/add-all-tz');
require('@formatjs/intl-datetimeformat/locale-data/en');
require('@formatjs/intl-datetimeformat/locale-data/es');

@melvin-bot melvin-bot bot added the Overdue label Sep 18, 2023
@DylanDylann
Copy link
Contributor

I also see this bug on the IOS App (production version). It seems this bug is serious

@NicMendonca
Copy link
Contributor

@situchan proposal here for you! ^^

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 18, 2023
@NicMendonca
Copy link
Contributor

bump @situchan ^

@tienifr
Copy link
Contributor

tienifr commented Sep 22, 2023

FYI, this is the supported timezones on iOS and former momentjs.

@melvin-bot melvin-bot bot added the Overdue label Sep 25, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 25, 2023

@NicMendonca, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

@situchan
Copy link
Contributor

reviewing today

@melvin-bot
Copy link

melvin-bot bot commented Sep 28, 2023

@NicMendonca, @situchan Whoops! This issue is 2 days overdue. Let's get this updated quick!

@situchan
Copy link
Contributor

@tienifr proposal looks promising but do you have clear repro step?

@melvin-bot melvin-bot bot removed the Overdue label Sep 28, 2023
@tienifr
Copy link
Contributor

tienifr commented Sep 28, 2023

@situchan You could try this:

  1. Change your IP location by a VPN service (UrbanVPN is a great & free one) then switch to one of the affected locations (Vietnam for example)
  2. Open Web app on Chrome
  3. Settings >> Profile >> Timezones >> Turn on Automatically determine your location
  4. Verify the automatic timezone is Asia/Saigon
  5. Open iOS app, go to any chat participated in by the above account

@melvin-bot melvin-bot bot changed the title [$500] Dev: IOS - App crashes when opening a transaction report [HOLD for payment 2024-01-18] [$500] Dev: IOS - App crashes when opening a transaction report Jan 11, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 11, 2024
Copy link

melvin-bot bot commented Jan 11, 2024

Reviewing label has been removed, please complete the "BugZero Checklist".

Copy link

melvin-bot bot commented Jan 11, 2024

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.24-3 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2024-01-18. 🎊

For reference, here are some details about the assignees on this issue:

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 17, 2024
@NicMendonca
Copy link
Contributor

everyone has been paid!

@tienifr
Copy link
Contributor

tienifr commented May 10, 2024

@NicMendonca Hi, sorry for getting here late but it seems that I wasn't paid a reporting bonus ($50) here (the issue was from long ago when the reporting bonus still applies)

Could you help handle this? Thanks!

Copy link

melvin-bot bot commented Jun 26, 2024

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@pecanoro
Copy link
Contributor

@jliexpensify It seems we are missing a reporting bonus here for a while ago. Could you double-check? Nicole is OOO so she won't be able to check for a while

@pecanoro pecanoro reopened this Jun 26, 2024
@jliexpensify
Copy link
Contributor

jliexpensify commented Jun 27, 2024

I can confirm that @tienifr qualifies for a $50 payout as indicated here

@jliexpensify jliexpensify added External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Jun 27, 2024
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2024-01-18] [$500] Dev: IOS - App crashes when opening a transaction report [HOLD for payment 2024-01-18] [$250] Dev: IOS - App crashes when opening a transaction report Jun 27, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 27, 2024
Copy link

melvin-bot bot commented Jun 27, 2024

Current assignee @situchan is eligible for the External assigner, not assigning anyone new.

@jliexpensify jliexpensify removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 27, 2024
@jliexpensify jliexpensify assigned tienifr and unassigned tienifr Jun 27, 2024
@jliexpensify
Copy link
Contributor

Damn, I thought that would create a new Upworks job.

@jliexpensify
Copy link
Contributor

Ah, no need! I can just do a ND payment 😓

@JmillsExpensify
Copy link

$50 approved for @tienifr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

10 participants