Add view all loans command #81
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently results in infinite recursion due to storage.
Loans are now sorted by return date. Loans with earlier return dates are shown higher.
marcus-ny
requested changes
Mar 29, 2024
| public Loan(int id, float value, Date startDate, Date returnDate) { | ||
| requireAllNonNull(id, value, startDate, returnDate); | ||
| public Loan(int id, float value, Date startDate, Date returnDate, Person assignee) { | ||
| requireAllNonNull(id, value, startDate, returnDate, assignee); |
| @@ -44,7 +43,8 @@ public ModelManager(ReadOnlyAddressBook addressBook, ReadOnlyUserPrefs userPrefs | |||
| this.addressBook = new AddressBook(addressBook); | |||
| this.userPrefs = new UserPrefs(userPrefs); | |||
| filteredPersons = new FilteredList<>(this.addressBook.getPersonList()); | |||
| currLoanList = FXCollections.observableList(new ArrayList<>()); | |||
| filteredLoans = new FilteredList<>(this.addressBook.getLoanList()); | |||
| sortedLoans = new SortedList<>(filteredLoans, Loan::compareTo); | |||
There was a problem hiding this comment.
Making use of the behaviour of SortedList is really nice here. Removes the need for implicit sorting through functions at every change. 👍
| public CommandResult execute(Model model) { | ||
| requireNonNull(model); | ||
| model.updateFilteredLoanList(PREDICATE_SHOW_ALL_ACTIVE_LOANS); | ||
| return new CommandResult(MESSAGE_SUCCESS, false, false, true); |
There was a problem hiding this comment.
Suggested change
| return new CommandResult(MESSAGE_SUCCESS, false, false, true); | |
| model.updateFilteredPersonList(unused -> false); | |
| return new CommandResult(MESSAGE_SUCCESS, false, false, true); |
There was a problem hiding this comment.
This is just for cosmetic purposes and not really important. But when we call viewloans targeting all active loans, we should not show any particular contact's name at the top.
The UI will show a blank space at the top after this change. We will leave it to be fixed as an issue later on as this task is more suitable for another issue and PR on its own.
There was a problem hiding this comment.
I agree. In the "view all loans" GUI, the name of the loanee should also be shown for each loan,
marcus-ny
approved these changes
Mar 30, 2024
marcus-ny
merged commit 5c1b43b
into
AY2324S2-CS2103T-W13-1:master
3 of 5 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #55 and #32.
Refactor
LoanRecordsasUniqueLoanListUniquePersonListas a model in implementing methods.Extract individual loan lists to global loan lists
UniqueLoanListis now a field ofAddressBookas opposed individual persons.Loannow holds the assigneePersonas a field.Refactor the display of loans
Class changes:
ModelManagernow holds asortedLoanListof loans wrapped around afilteredLoanList.filteredLoanList, in turn, wraps around theuniqueLoanListof the AddressBook, analogous tofilteredPersonList.Implementation changes:
filteredLoanListcontrols which loans are shown. Currently there are two options: show all active loans or show active loans attached to a person.sortedLoanListcontrols the order of loans. Currently it is hardcoded that loans due earlier are placed higher.Add new command
viewloanslist. It displays all active loansRefactor
viewloancommandLoanRecords, this command now simply updates the predicate onfilteredLoanList, similar tofiltercommand.Refactor
deleteloanandmarkloancommanddeleteloan/markloan INDEXwhereINDEXis the index of the loan in the last shown loan list.Refactor storage
Change generic data
addressbook.jsondoes not exist is changed to reflect the above changes.Bugs and issues
The following are essential changes that are not covered in this PR:
deleteloanandmarkloanto reflect the change of parameters.deleteloanandmarkloanmethods.Person,LoanandLoanRecords.Analyticsin this new implementation.EditCommandrelated objects to reflect the deletion ofLoanRecordsfromPerson.