Ivy-Apps / ivy-wallet

Ivy Wallet is an open-source money manager app for Android, no longer maintained. You can fork the code or download the final version from Google Play.
https://play.google.com/store/apps/details?id=com.ivy.wallet
GNU General Public License v3.0
2.8k stars 675 forks source link

fix csv export #3444

Closed akashs056 closed 2 months ago

akashs056 commented 2 months ago

Pull request (PR) checklist

Please check if your pull request fulfills the following requirements:

Screen recording of your changes (if applicable):

What's changed?

Describe with a few bullets what's new:

fixed this issue #3422 previously by #3426 but after the time issue if fixed by #3435 its works incorrectly it exports time at UTC and does not converts it to Local time So, with this changes its now working perfectly fine exports in users local time and aslo imports in local time zone without any issue

Before After
{media} {media}
{media} {media}

Risk factors

What may go wrong if we merge your PR?

In what cases won't your code work?

Does this PR close any GitHub issues? (do not delete)

Troubleshooting GitHub Actions (CI) failures ❌

Pull request checks failing? Read our CI Troubleshooting guide.

ILIYANGERMANOV commented 2 months ago

@akashs056 you neee to fix the ExportCsvUseCase unit tests by providing a mocked TimeConverter impl

ILIYANGERMANOV commented 2 months ago

@akashs056 you neee to fix the ExportCsvUseCase unit tests by providing a mocked TimeConverter impl

Or even better and easier, create a TestTimeConverter impl that converts always to UTC. I mean in it you hard-code the current timezone to UTC. Does that makes sense?

akashs056 commented 2 months ago

@ILIYANGERMANOV, I've implemented a TestTimeConverterthat hard-codes the current timezone to UTC as you suggested. Could you please confirm if this is the correct approach? I'm still new to unit testing, so I want to make sure I'm on the right track.

ILIYANGERMANOV commented 2 months ago

@akashs056 you have a build error e: file:///home/runner/work/ivy-wallet/ivy-wallet/shared/base/src/main/java/com/ivy/base/time/impl/TestTimeConverter.kt:9:1 Class 'TestTimeConverter' is not abstract and does not implement abstract member 'toLocalTime'.

The TestTimeConveter will be red

akashs056 commented 2 months ago

@akashs056 you have a build error e: file:///home/runner/work/ivy-wallet/ivy-wallet/shared/base/src/main/java/com/ivy/base/time/impl/TestTimeConverter.kt:9:1 Class 'TestTimeConverter' is not abstract and does not implement abstract member 'toLocalTime'.

The TestTimeConveter will be red

I've resolved the initial error with the TestTimeConverter class in my local build. However, now I'm encountering a UncompletedCoroutinesError in the test property - num of row and columns matches the format. It seems to be related to the coroutine not completing within the expected time

ILIYANGERMANOV commented 2 months ago

@akashs056 push it so I can have a look - the CI will also provide feedback

akashs056 commented 2 months ago

@ILIYANGERMANOV getting the same error again

akashs056 commented 2 months ago

sometimes when i run locally with the same code it does not gives any error image

ILIYANGERMANOV commented 2 months ago

@akashs056 fix build errors, update branch and tests should pass on the CI 🤞 I don't see anything incorrect

akashs056 commented 2 months ago

@ILIYANGERMANOV Gradle error indicating that the toLocalTimemethod is not implemented, but I can’t find this method in the TimeConverterinterface. The error has been persisting since yesterday, and I’m having trouble resolving it. Could you please help me understand what might be going wrong?

ILIYANGERMANOV commented 2 months ago

@ILIYANGERMANOV Gradle error indicating that the toLocalTimemethod is not implemented, but I can’t find this method in the TimeConverterinterface. The error has been persisting since yesterday, and I’m having trouble resolving it. Could you please help me understand what might be going wrong?

It's there, look into StandardTimeConveter. It was merged yesterday - update branch, git pull and open the implementation

ILIYANGERMANOV commented 2 months ago

@akashs056 now git pull and implement TestTimeConverter and everything should be ✅

akashs056 commented 2 months ago

@ILIYANGERMANOV Its ready to merge