Skip to content
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

Complete SearchRoomCommand #90

Merged

Conversation

chiamyunqing
Copy link

@chiamyunqing chiamyunqing commented Oct 14, 2020

This PR is quite huge (sorry about that because there's a lot of new PRs being merged this week and I had to keep resolving merge conflicts) so here's a summary of what's done:

  1. Patient class -> updated such that patients with same name are not allowed
  2. DeletePatientCommand class & its test cases-> utilise Name class instead of String (Name class has been updated such that we should not be doing nameInString.trim().toLowerCase() kind of comparisons outside)
  3. Complete EditRoomCommandTest + bug fix for editRoomCommand
  4. Complete ListRoomCommandTest
  5. Complete SearchRoomCommand (the original implementation of searching by room number, searching by patient is done in v1.3) + Test cases
  6. Rooms UI fixes
  7. Refactor missed out CovigentApp() previously

chiamyunqing and others added 11 commits October 12, 2020 16:47
…stead of string, class does not interact with roomlist directly
… testing in EditRoomCommandTest), Add EditRoomCommandTest
…into editroomcommandtest

* 'master' of https://github.com/AY2021S1-CS2103T-W12-1/tp:
  Remove redundant code
  Make changes to according to comments given by reviewers.
  Make changes to pass checkstyle.
  Make changes due to problems during merging.
  Fix checkstyle issues
  Update to save task list when editing room
  Update UserGuide.md

# Conflicts:
#	src/main/java/seedu/address/logic/commands/room/EditRoomCommand.java
@chiamyunqing chiamyunqing added Priority.Medium Type.BugFix Patches or fixes for a bug Type.Documentation Improvements or additions to documentation Type.Feature New feature or request labels Oct 14, 2020
@chiamyunqing chiamyunqing linked an issue Oct 14, 2020 that may be closed by this pull request
@chiamyunqing chiamyunqing added this to the v1.2 milestone Oct 14, 2020
Copy link
Collaborator

@itssodium itssodium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just minor changes that will affect the UI for Room.

Copy link
Collaborator

@LeeMingDe LeeMingDe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! good reorganization of the codebase!


*/
return null;
model.updateFilteredRoomList(room -> room.getRoomNumber() == roomNumber);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe can include the predicate in a different method like:
private Predicate getPredicateForSearchRoom(int roomNumberExpected) {
Predicate predicateForSearchRoom = room1 -> room1.getRoomNumber() == roomNumberExpected;
return predicateForSearchRoom;
}
That is wat I meant.

assertCommandFailure(searchRoomCommand, model, expectedMessage);
}

//TODO success test cases once we finalise how rooms UI works
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that it has been finalised, maybe you want to implement it?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's look into resolving the TODOs in v1.3?

@chiamyunqing chiamyunqing merged commit b0fe21b into AY2021S1-CS2103T-W12-1:master Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority.Medium Type.BugFix Patches or fixes for a bug Type.Documentation Improvements or additions to documentation Type.Feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search for a Room: searchRoom
4 participants