-
Notifications
You must be signed in to change notification settings - Fork 647
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
[FIX] Migrate TransactionDao calls to TransactionRepository part - 1 #3228
[FIX] Migrate TransactionDao calls to TransactionRepository part - 1 #3228
Conversation
Hey @ILIYANGERMANOV , some more changes are there including test, but i wanted your comments, whether my procedures are correct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks like a step in the right direction. Left some comments that must be addressed
screen/edit-transaction/src/main/java/com/ivy/transaction/EditTransactionViewModel.kt
Outdated
Show resolved
Hide resolved
screen/loans/src/main/java/com/ivy/loans/loandetails/LoanDetailsViewModel.kt
Outdated
Show resolved
Hide resolved
screen/edit-transaction/src/main/java/com/ivy/transaction/EditTransactionViewModel.kt
Outdated
Show resolved
Hide resolved
screen/transactions/src/main/java/com/ivy/transactions/TransactionsViewModel.kt
Outdated
Show resolved
Hide resolved
shared/data/core/src/main/java/com/ivy/data/repository/TransactionRepository.kt
Outdated
Show resolved
Hide resolved
shared/data/core/src/main/java/com/ivy/data/repository/TransactionRepository.kt
Outdated
Show resolved
Hide resolved
shared/data/core/src/main/java/com/ivy/data/repository/TransactionRepository.kt
Outdated
Show resolved
Hide resolved
Hi @ILIYANGERMANOV , i stuck up with one issue, in BackupUseCaseTest , in that json needs to import 117 data, but it is importing 0 and that making the unitTest failed. |
screen/edit-transaction/src/main/java/com/ivy/transaction/EditTransactionViewModel.kt
Outdated
Show resolved
Hide resolved
shared/data/core/src/main/java/com/ivy/data/repository/fake/FakeTransactionRepository.kt
Outdated
Show resolved
Hide resolved
shared/data/core/src/main/java/com/ivy/data/repository/impl/TransactionRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
temp/legacy-code/src/main/java/com/ivy/legacy/datamodel/temp/TransactionExt.kt
Outdated
Show resolved
Hide resolved
screen/import-data/src/main/java/com/ivy/importdata/csv/domain/CSVImporterV2.kt
Show resolved
Hide resolved
@ILIYANGERMANOV when we are using flagDelete, basically we are only changing isDeleted reference in db, |
Yup, exactly 💯 We no longer have cloud sync to keep the record in DB. So let's just remove it |
Cool then, i have already done the changes for this. So we can consider this as a final commit for transaction Migration, if any changes required ping me. |
Thanks @ILIYANGERMANOV , got some good explanation on why exactly we need to do some implementation in a specific way😁. |
Pull Request (PR) Checklist
Please check if your pull request fulfills the following requirements:
main
branch.What's changed?
Describe with a few bullets what's new:
Risk Factors
What may go wrong if we merge your PR?
In what cases your code won't work?
Does this PR closes any GitHub Issues?
Check Ivy Wallet Issues.
TransactionDao
calls toTransactionRepository
#3132Troubleshooting CI failures ❌
GitHub Actions failing? Read our CI Troubleshooting guide.