[Data Layer] Migrate the code to use the new the `AccountRepository`

[ILIYANGERMANOV]

Please confirm the following

• [X] I checked the current issues for duplicate issues.

What would you like to improve?

• Use the AccountRepository to read/write data
• Replace AccountDao and WriteAccountDao usages with the repository

Because...

• Move away from our legacy data layer
• Consistent architecture
• Safe and easy to extend

Description

No response

Success Criteria

• Unit tests
• All the legacy code works as it was before
• The app isn't broken and is working successfully with the new data layer.

[ILIYANGERMANOV]

Hey @Rick-AB wanna try this one? It's kinda challenging but you already did a great job with the data layer 💯

[ivywallet]

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

Tagging @ILIYANGERMANOV for review & approval 👀

[Rick-AB]

Hey @Rick-AB wanna try this one? It's kinda challenging but you already did a great job with the data layer 💯

sure, let's give it a bash!

[Rick-AB]

I'm on it

[ivywallet]

Thank you for your interest @Rick-AB! 🎉 Issue #2961 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.

[Rick-AB]

@ILIYANGERMANOV migrating to the repository would also mean the ui layer will now use the new account model, correct?

[ILIYANGERMANOV]

@ILIYANGERMANOV migrating to the repository would also mean the ui layer will now use the new account model, correct?

@Rick-AB yup correct 💯

[Rick-AB]

@ILIYANGERMANOV I discovered this class in the legacy module is used in the ui layer and uses the deprecated account model. to me it doesn't make sense to use the new account model here instead since we're carrying out a migration. let me know what you think

[ILIYANGERMANOV]

@ILIYANGERMANOV I discovered this class in the legacy module is used in the ui layer and uses the deprecated account model. to me it doesn't make sense to use the new account model here instead since we're carrying out a migration. let me know what you think

Yes, it makes sense. Ideally in the UI layer we should have a different AccountViewState model that has only the things that are to be displayed and must consist only of primitives and @Immutable so Compose can be smooth. See #2965

[Rick-AB]

Yes, it makes sense. Ideally in the UI layer we should have a different AccountViewState model that has only the things that are to be displayed and must consist only of primitives and @Immutable so Compose can be smooth. See #2965

if I understand you correctly, you're saying the legacy Account model in the AccountData class CAN be replaced with the new model???

[ILIYANGERMANOV]

We should have:

• Domain models
• Ui/ViewState models optimized for Compose

See our Architecture guidelines. But that's a bigger change so feel free to implement it as you see fit

[Rick-AB]

@ILIYANGERMANOV I've migrated the accounts tab, but I want you to take a look so we can decide if we should proceed with it or scrape it : ) there are some "not so elegant solutions" trying to patch some classes like CalcWalletBalanceAct, CalcAccIncomeExpenseAct etc pending when other migrations become active.

should I create a draft pr?

[ILIYANGERMANOV]

@Rick-AB yeah, please create a draft PR and I'll have a look when I have time

[Rick-AB]

@ILIYANGERMANOV can we reopen this issue to continue the migration or will a new issue be created?

[Rick-AB]

@ILIYANGERMANOV I noticed the there is no flagDeleted in AccountRepository, did we miss that out or is there a reason?

[ILIYANGERMANOV]

@ILIYANGERMANOV I noticed the there is no flagDeleted in AccountRepository, did we miss that out or is there a reason?

We no longer need that operation but feel free to add it for backwards compatibility

[Rick-AB]

@ILIYANGERMANOV while migrating TransactionsScreen to use new Account model, I noticed some common composables used across many screen take in the legacy model in some way so I'm unable to migrate this screen without affecting the others. we need some kind of workaround in other not to change all these files at once, I'm thinking a temporary extension mapper that maps between these two models?

what's your opinion on this?

[ILIYANGERMANOV]

@ILIYANGERMANOV while migrating TransactionsScreen to use new Account model, I noticed some common composables used across many screen take in the legacy model in some way so I'm unable to migrate this screen without affecting the others. we need some kind of workaround in other not to change all these files at once, I'm thinking a temporary extension mapper that maps between these two models?

what's your opinion on this?

@Rick-AB Ideally, the UI layer should not depend on the domain models. We should create AccountViewState (only with primitives in :shared:ui) and recreate all common composables. That's the right way.

For a workaround, I have no good ideas atm 😄 If you can figure out sth that won't butcher the performance it would be cool!

[Rick-AB]

@ILIYANGERMANOV I'm unassigning myself from this issue as I currently don't have the time bandwidth to continue working on this for now