mobile-wallet
mobile-wallet copied to clipboard
Fix #729 - Added option to filter payment history
Fixes #729
-
[x] Apply the
AndroidStyle.xml
style template to your code in Android Studio. -
[x] Run the unit tests with
./gradlew check
to make sure you didn't break anything -
[x] If you have multiple commits please combine them into one commit by squashing them.
Updated
@naman14 could you review this?
Hey @dubeyaayush07 thanks for this PR. Seems like a good functionality to add. I had 1 comment regarding the filter logic (it should be a part of usecase so that it's not bound to the UI). Why? This is where clean architecture helps us. This allows us to plug filters dynamically later on. Maybe we start sending filterlist in api instead of filtering locally after we have already fetched all transactions. Maybe we need to save the currently selected filters in which case when users comes next time, we should only be fetching the selected filters. We also make sure that other modules are also able to use filtering if its a part of core module.
Also, maybe we can also add it at the api level. You may need to check the api docs if it supports filtering there. (Can be a separate PR) but make sure to make this a part of core usecase.
Let me know if you need more clarity.
@naman14 So instead of filtering in the presenter I should modify fetchTransactions(maybe overload the method ) to use filterList. Am I understanding it correctly?
Yes, pass filterList to fetchTransactions (can overload) and then pass to usecase where filtering will take place. (through request values of FetchAccountTransactions)
@naman14 I modified the code such that the filtering now takes place in the TransactionMapper
. I was nervous about changing code other than that of HistoryFragment
so I overloaded the fetchTransactionsHistory
method. The credit, debit constants were not accessible from the the core module so for now I have hardcoded string literals.
@dubeyaayush07 any updates on this?
@shiv07tiwari I am busy with my exams right now, I will try to make the necessary updates as soon as I get time
@shiv07tiwari @devansh-299 @naman14 In this pull request I was using a checkbox menu for the filter list but since my last commit chips were added for selecting transaction type. Should I use the menu or the chips?