Skip to content
This repository has been archived by the owner on Nov 5, 2024. It is now read-only.

[BUG] Manual time entry during transaction reverts 6 hours earlier #3451

Closed
1 task done
asadmash opened this issue Aug 31, 2024 · 10 comments
Closed
1 task done

[BUG] Manual time entry during transaction reverts 6 hours earlier #3451

asadmash opened this issue Aug 31, 2024 · 10 comments
Assignees
Labels
approved Approved by the Ivy Wallet team. Ready for dev bug Something isn't working

Comments

@asadmash
Copy link

Please confirm the following

Describe the bug

When manually entering a time while adding a transaction, the time reverts to 6 hours earlier than the intended entry. Automatic time picking works correctly, and the issue is specific to manual time entry for transactions.

To Reproduce

  1. Set the device to UTC (+06:00) BST time zone.
  2. Add a transaction and enter a specific time manually.
  3. Observe that the time shifts 6 hours earlier than the entered time.

Expected behavior

The manually entered time for a transaction should match the intended time without any time shift.

Screenshots

No response

App version

Latest demo APK from TG

Smartphone

Symphony Z40 Android 10

Additional context

Time Zone: UTC (+06:00) BST

@asadmash asadmash added the bug Something isn't working label Aug 31, 2024
@ILIYANGERMANOV ILIYANGERMANOV added the approved Approved by the Ivy Wallet team. Ready for dev label Aug 31, 2024
@ivywallet
Copy link
Collaborator

Thank you @asadmash for raising Issue #3451! 🚀
What's next? Read our Contribution Guidelines 📚.

Tagging @ILIYANGERMANOV for review & approval 👀

@ILIYANGERMANOV
Copy link
Collaborator

I'm on it

@ivywallet
Copy link
Collaborator

Thank you for your interest @ILIYANGERMANOV! 🎉
Issue #3451 is assigned to you. You can work on it! ✅

If you don't want to work on it now, please un-assign yourself so other contributors can take it.

Also, make sure to read our Contribution Guidelines.

@ILIYANGERMANOV
Copy link
Collaborator

Should be fixed in #3453

@jamilxt
Copy link

jamilxt commented Sep 30, 2024

It's not happening in Emulator, rather in real device. I've debugged it, found out, the method updateDateTime() is called, the date is converting it to UTC which is causing the date to set previous date than selcted date.

image

May I know, why are you converting the datetime to UTC?

@ILIYANGERMANOV
Copy link
Collaborator

It's not happening in Emulator, rather in real device. I've debugged it, found out, the method updateDateTime() is called, the date is converting it to UTC which is causing the date to set previous date than selcted date.

image May I know, why are you converting the datetime to UTC?

All records in the DB should be in UTC so they are timezone agnostic. Can you test the latest version? It should fix this issue

@jamilxt
Copy link

jamilxt commented Sep 30, 2024

The screenshot is taken after running the latest code from the "main" branch.
Unable to produce the bug on the emulator. But, it's happening in my physical device.

@jamilxt
Copy link

jamilxt commented Sep 30, 2024

My suggestion will be - to convert dateTime in UTC in Repository layer while saving the data, rather than in viewModel. After retriving from DB, we can convert it to user's timezone. We can utilize Room's TypeConverter feature for that?

@jamilxt
Copy link

jamilxt commented Sep 30, 2024

image

Handling this centrally will improve maintainability, as it eliminates the need to manually implement it everywhere, reducing errors.

@ILIYANGERMANOV
Copy link
Collaborator

@jamilxt yes it should be centralized but we have a lot of legacy code that works with LocalDateTime. A good solution would be:

  • Always work with Instant (UTC)
  • Firmat this Instant to String (timezone) included before displaying it in the UI

Having LocalDateTime in the UI is terrible for a few reasons:

  • It's inefficient: formatting being done in the Compsable and repeated on each recomposition
  • It also makes composables unstable
  • The timezone bugs that we have

However, I don't have the time to migrate all legacy code so tried to hotfix it with back and forth time conversions. My point is that if we make everything Instant there will be no need for conversions

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Approved by the Ivy Wallet team. Ready for dev bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants