Merge pull request #219 from cmHuang777/branch-AddAppointment

Add more test cases and improve code quality
AY2324S1-CS2103T-T09-3 · Nov 11, 2023 · 0abbbc6 · 0abbbc6
2 parents 4fa3adc + f953cd9
commit 0abbbc6
Show file tree

Hide file tree

Showing 21 changed files with 395 additions and 95 deletions.
diff --git a/docs/UserGuide.md b/docs/UserGuide.md
@@ -366,12 +366,11 @@ need to save manually.
 
 ### Editing the data file
 
-MediLink Contacts data are saved automatically as a JSON file `[JAR file location]/data/addressbook.json`. Advanced
-users are welcome to update data directly by editing that data file.
+MediLink Contacts data are saved automatically as a JSON file `[JAR file location]/data/addressbook.json`. Advanced users are welcome to update data directly by editing that data file. However, please ensure that the edits are valid, else it may cause unexpected behaviours when the invalid data is not detected by the system.
 
 <div markdown="span" class="alert alert-warning">
 :exclamation: **Caution:**
-If your changes to the data file makes its format invalid, MediLink Contacts will discard all data and start with an empty data file at the next run. Hence, it is recommended to take a backup of the file before editing it.
+If your changes to the data file makes its format invalid, MediLink Contacts may discard all data and start with an empty data file at the next run. Hence, it is recommended to take a backup of the file before editing it. Some changes may also be invalid, but not detected by the system. In that case, there may be many unexpected behaviours due to those undetected errors.
 </div>
 
 ### Archiving data files `[coming in v2.0]`

diff --git a/src/main/java/seedu/address/logic/commands/AddAppointmentCommand.java b/src/main/java/seedu/address/logic/commands/AddAppointmentCommand.java
@@ -87,10 +87,6 @@ private void checkPatientAndDoctor(Patient chosenPatient, Doctor chosenDoctor) t
         if (chosenDoctor == null) {
             throw new CommandException(MESSAGE_INVALID_DOCTOR);
         }
-        // check that patient and doctor are not the same person
-        if (chosenPatient.isSamePerson(chosenDoctor)) {
-            throw new CommandException(MESSAGE_SAME_DOCTOR_AND_PATIENT);
-        }
     }
 
     private Patient findPatient(Model model) {

diff --git a/src/main/java/seedu/address/model/AddressBook.java b/src/main/java/seedu/address/model/AddressBook.java
@@ -157,12 +157,6 @@ public void setDoctor(Doctor target, Doctor editedDoctor) {
         doctors.setObject(target, editedDoctor);
     }
 
-    public void setAppointment(Appointment target, Appointment editedAppointment) {
-        requireNonNull(editedAppointment);
-
-        appointments.setObject(target, editedAppointment);
-    }
-
     /**
      * Removes {@code key} from this {@code AddressBook}.
      * {@code key} must exist in the address book.
@@ -232,6 +226,7 @@ public boolean equals(Object other) {
                 && doctors.equals((otherAddressBook.doctors))
                 && appointments.equals((otherAddressBook.appointments));
     }
+
     @Override
     public int hashCode() {
         return Objects.hash(patients, doctors, appointments);

diff --git a/src/main/java/seedu/address/model/Model.java b/src/main/java/seedu/address/model/Model.java
@@ -92,13 +92,6 @@ public interface Model {
 
     void addAppointment(Appointment appointment);
 
-    void setAppointment(Appointment target, Appointment editedAppointment);
-
-    /**
-     * Returns an unmodifiable view of the filtered person list
-     */
-    ObservableList<Person> getFilteredPersonList();
-
     /**
      * Returns an unmodifiable view of the filtered patient list
      */

diff --git a/src/main/java/seedu/address/model/ModelManager.java b/src/main/java/seedu/address/model/ModelManager.java
@@ -133,12 +133,10 @@ public ReadOnlyAddressBook getAddressBook() {
     @Override
     public boolean hasPerson(Person person) {
         requireNonNull(person);
-        if (person instanceof Patient) {
+        if (person.isPatient()) {
             return addressBook.hasPatient((Patient) person);
-        } else if (person instanceof Doctor) {
-            return addressBook.hasDoctor((Doctor) person);
         } else {
-            return false;
+            return addressBook.hasDoctor((Doctor) person);
         }
     }
     @Override
@@ -196,19 +194,6 @@ public void setPerson(Person target, Person editedPerson) {
         }
     }
 
-    @Override
-    public void setAppointment(Appointment target, Appointment editedAppointment) {
-        updateBackup();
-        addressBook.setAppointment(target, editedAppointment);
-    }
-
-    /**
-     * Returns an unmodifiable view of the filtered person list
-     */
-    @Override
-    public ObservableList<Person> getFilteredPersonList() {
-        return null;
-    }
 
     //=========== Filtered Person List Accessors =============================================================
 
@@ -240,6 +225,7 @@ public void updateFilteredPersonList(Predicate<Person> predicate) {
         filteredPatients.setPredicate(predicate);
         filteredDoctors.setPredicate(predicate);
     }
+
     @Override
     public void updateFilteredAppointmentList(Predicate<Appointment> predicate) {
         requireNonNull(predicate);
@@ -261,7 +247,8 @@ public boolean equals(Object other) {
         return addressBook.equals(otherModelManager.addressBook)
                 && userPrefs.equals(otherModelManager.userPrefs)
                 && filteredDoctors.equals(otherModelManager.filteredDoctors)
-                && filteredPatients.equals(otherModelManager.filteredPatients);
+                && filteredPatients.equals(otherModelManager.filteredPatients)
+                && filteredAppointments.equals(otherModelManager.filteredAppointments);
     }
 
 }
diff --git a/src/main/java/seedu/address/model/person/Patient.java b/src/main/java/seedu/address/model/person/Patient.java
@@ -95,7 +95,8 @@ public boolean equals(Object other) {
     @Override
     public int hashCode() {
         // use this method for custom fields hashing instead of implementing your own
-        return Objects.hash(name, phone, email, address, gender, ic, condition, bloodType, appointments, tags);
+        return Objects.hash(name, phone, emergencyContact, email, address, gender, ic, condition, bloodType,
+                appointments, tags);
     }
 
     @Override

diff --git a/src/main/java/seedu/address/storage/JsonAdaptedDoctor.java b/src/main/java/seedu/address/storage/JsonAdaptedDoctor.java
@@ -21,7 +21,6 @@
 import seedu.address.model.tag.Tag;
 
 
-
 /**
  * Jackson-friendly version of {@link Doctor}.
  */
@@ -66,10 +65,7 @@ public Doctor toModelType() throws IllegalValueException {
             personTags.add(tag.toModelDoctorType());
         }
         final Set<Tag> modelTags = new HashSet<>(personTags);
-        final List<Appointment> personAppointments = new ArrayList<>();
-        for (JsonAdaptedAppointment appointment : this.getAppointments()) {
-            personAppointments.add(appointment.toModelType());
-        }
+        final List<Appointment> personAppointments = checkAppointments();
         final Set<Appointment> modelAppointments = new HashSet<>(personAppointments);
         return new Doctor(modelName, modelPhone, modelEmail, modelAddress, modelRemark, modelGender, modelIc,
                 modelAppointments, modelTags);

diff --git a/src/main/java/seedu/address/storage/JsonAdaptedPatient.java b/src/main/java/seedu/address/storage/JsonAdaptedPatient.java
@@ -81,10 +81,7 @@ public Patient toModelType() throws IllegalValueException {
         final Set<Tag> modelTags = new HashSet<>(personTags);
         final Condition modelCondition = checkCondition();
         final BloodType modelBloodType = checkBloodType();
-        final List<Appointment> personAppointments = new ArrayList<>();
-        for (JsonAdaptedAppointment appointment : this.getAppointments()) {
-            personAppointments.add(appointment.toModelType());
-        }
+        final List<Appointment> personAppointments = checkAppointments();
         final Set<Appointment> modelAppointments = new HashSet<>(personAppointments);
         return new Patient(modelName, modelPhone, modelEmergencyContact, modelEmail, modelAddress, modelRemark,
                 modelGender, modelIc, modelCondition, modelBloodType, modelAppointments, modelTags);

diff --git a/src/main/java/seedu/address/storage/JsonAdaptedPerson.java b/src/main/java/seedu/address/storage/JsonAdaptedPerson.java
@@ -25,6 +25,10 @@
 public abstract class JsonAdaptedPerson {
 
     public static final String MISSING_FIELD_MESSAGE_FORMAT = "Person's %s field is missing!";
+    public static final String MESSAGE_DUPLICATE_APPOINTMENT_TIME =
+            "Person has more than 1 appointment at the same timing!";
+    public static final String MESSAGE_INVALID_APPOINTMENT =
+            "Person contains an appointment that does not belong to him!";
 
     private final String name;
     private final String phone;
@@ -92,6 +96,7 @@ public List<JsonAdaptedTag> getTags() {
     public List<JsonAdaptedAppointment> getAppointments() {
         return this.appointments;
     }
+
     /**
      * Checks the name given by storage.
      *
@@ -203,6 +208,8 @@ public Ic checkIc() throws IllegalValueException {
 
     /**
      * Gives the list of appointments of the Person.
+     * Please ensure that this method is called only after the Ic has been added to the person.
+     * This checks for any appointments happening at the same time and any appointments not belonging to this person.
      *
      * @return a List of JsonAdaptedAppointments
      */

diff --git a/src/test/java/seedu/address/logic/commands/AddAppointmentCommandTest.java b/src/test/java/seedu/address/logic/commands/AddAppointmentCommandTest.java
@@ -158,19 +158,6 @@ public void addAppointment(Appointment appointment) {
             throw new AssertionError("This method should not be called.");
         }
 
-        @Override
-        public void setAppointment(Appointment target, Appointment editedAppointment) {
-            throw new AssertionError("This method should not be called.");
-        }
-
-        /**
-         * Returns an unmodifiable view of the filtered person list
-         */
-        @Override
-        public ObservableList<Person> getFilteredPersonList() {
-            return null; // not sure abt this method
-        }
-
         @Override
         public ObservableList<Patient> getFilteredPatientList() {
             throw new AssertionError("This method should not be called.");

diff --git a/src/test/java/seedu/address/logic/commands/AddAppointmentIntegrationTest.java b/src/test/java/seedu/address/logic/commands/AddAppointmentIntegrationTest.java
@@ -4,7 +4,9 @@
 import static seedu.address.logic.commands.CommandTestUtil.assertCommandFailure;
 import static seedu.address.logic.commands.CommandTestUtil.assertCommandSuccess;
 import static seedu.address.testutil.TypicalAddressBook.getTypicalAddressBook;
+import static seedu.address.testutil.TypicalDoctor.CHERYL;
 import static seedu.address.testutil.TypicalDoctor.DEREK;
+import static seedu.address.testutil.TypicalPatient.AMY;
 import static seedu.address.testutil.TypicalPatient.BOB;
 
 import java.util.Set;
@@ -18,7 +20,6 @@
 import seedu.address.model.appointment.Appointment;
 import seedu.address.model.person.Doctor;
 import seedu.address.model.person.Patient;
-import seedu.address.model.person.Person;
 import seedu.address.testutil.AppointmentBuilder;
 import seedu.address.testutil.DoctorBuilder;
 import seedu.address.testutil.PatientBuilder;
@@ -45,23 +46,117 @@ public void execute_newAppointment_success() {
                 new AppointmentBuilder().withDoctorIc(derek.getIc()).withPatientIc(bob.getIc()).build();
 
         Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());
-        //expectedModel.addAppointment(validAppointment);
+        expectedModel.addAppointment(validAppointment);
 
         assertCommandSuccess(new AddAppointmentCommand(validAppointment), model,
                 String.format(AddAppointmentCommand.MESSAGE_SUCCESS, validAppointment),
                 expectedModel);
 
+        // check that the appointments have been added to all patients, doctors and appointment list
         Set<Appointment> patientAppointments = bob.getAppointments();
         Set<Appointment> doctorAppointments = derek.getAppointments();
         assertTrue(patientAppointments.contains(validAppointment));
         assertTrue(doctorAppointments.contains(validAppointment));
+        assertTrue(model.getFilteredAppointmentList().contains(validAppointment));
     }
 
     @Test
-    public void execute_duplicatePerson_throwsCommandException() {
-        Person personInList = model.getAddressBook().getPatientList().get(0);
-        assertCommandFailure(new AddCommand(personInList), model,
-                AddCommand.MESSAGE_DUPLICATE_PERSON);
+    public void execute_appointmentWithPatientNotInModel_throwsCommandException() {
+        Doctor derek = new DoctorBuilder(DEREK).build();
+        Patient bob = new PatientBuilder(BOB).build();
+        // only add doctor
+        model.addPerson(derek);
+        Appointment invalidAppointment =
+                new AppointmentBuilder().withDoctorIc(derek.getIc()).withPatientIc(bob.getIc()).build();
+
+        assertCommandFailure(new AddAppointmentCommand(invalidAppointment), model,
+                AddAppointmentCommand.MESSAGE_INVALID_PATIENT);
+    }
+
+    @Test
+    public void execute_appointmentWithDoctorNotInModel_throwsCommandException() {
+        Doctor derek = new DoctorBuilder(DEREK).build();
+        Patient bob = new PatientBuilder(BOB).build();
+        // only add patient
+        model.addPerson(bob);
+        Appointment invalidAppointment =
+                new AppointmentBuilder().withDoctorIc(derek.getIc()).withPatientIc(bob.getIc()).build();
+
+        assertCommandFailure(new AddAppointmentCommand(invalidAppointment), model,
+                AddAppointmentCommand.MESSAGE_INVALID_DOCTOR);
+    }
+
+    @Test
+    public void execute_appointmentWithDoctorHasAppointmentAtTheSameTime_throwsCommandException() {
+        Doctor derek = new DoctorBuilder(DEREK).build();
+        Patient bob = new PatientBuilder(BOB).build();
+        model.addPerson(derek);
+        model.addPerson(bob);
+        Appointment validAppointment =
+                new AppointmentBuilder().withDoctorIc(derek.getIc()).withPatientIc(bob.getIc()).build();
+
+        Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());
+        expectedModel.addAppointment(validAppointment);
+
+        assertCommandSuccess(new AddAppointmentCommand(validAppointment), model,
+                String.format(AddAppointmentCommand.MESSAGE_SUCCESS, validAppointment),
+                expectedModel);
+
+        Patient amy = new PatientBuilder(AMY).build();
+        model.addPerson(amy);
+        // create another appointment between Amy and Doctor Derek at the same time
+        Appointment invalidAppointment =
+                new AppointmentBuilder().withDoctorIc(derek.getIc()).withPatientIc(amy.getIc()).build();
+
+        assertCommandFailure(new AddAppointmentCommand(invalidAppointment), model,
+                AddAppointmentCommand.MESSAGE_DUPLICATE_APPOINTMENT_DOCTOR);
+    }
+
+    @Test
+    public void execute_appointmentWithPatientHasAppointmentAtTheSameTime_throwsCommandException() {
+        Doctor derek = new DoctorBuilder(DEREK).build();
+        Patient bob = new PatientBuilder(BOB).build();
+        model.addPerson(derek);
+        model.addPerson(bob);
+        Appointment validAppointment =
+                new AppointmentBuilder().withDoctorIc(derek.getIc()).withPatientIc(bob.getIc()).build();
+
+        Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());
+        expectedModel.addAppointment(validAppointment);
+
+        assertCommandSuccess(new AddAppointmentCommand(validAppointment), model,
+                String.format(AddAppointmentCommand.MESSAGE_SUCCESS, validAppointment),
+                expectedModel);
+
+        Doctor cheryl = new DoctorBuilder(CHERYL).build();
+        model.addPerson(cheryl);
+        // create another appointment between patient Bob and Doctor cheryl at the same time
+        Appointment invalidAppointment =
+                new AppointmentBuilder().withDoctorIc(cheryl.getIc()).withPatientIc(bob.getIc()).build();
+
+        assertCommandFailure(new AddAppointmentCommand(invalidAppointment), model,
+                AddAppointmentCommand.MESSAGE_DUPLICATE_APPOINTMENT_PATIENT);
+    }
+
+    @Test
+    public void execute_duplicateAppointment_throwsCommandException() {
+        Doctor derek = new DoctorBuilder(DEREK).build();
+        Patient bob = new PatientBuilder(BOB).build();
+        model.addPerson(derek);
+        model.addPerson(bob);
+        Appointment validAppointment =
+                new AppointmentBuilder().withDoctorIc(derek.getIc()).withPatientIc(bob.getIc()).build();
+
+        Model expectedModel = new ModelManager(model.getAddressBook(), new UserPrefs());
+        expectedModel.addAppointment(validAppointment);
+
+        assertCommandSuccess(new AddAppointmentCommand(validAppointment), model,
+                String.format(AddAppointmentCommand.MESSAGE_SUCCESS, validAppointment),
+                expectedModel);
+
+        // try to add the same appointment again will give an error
+        assertCommandFailure(new AddAppointmentCommand(validAppointment), model,
+                AddAppointmentCommand.MESSAGE_DUPLICATE_APPOINTMENT_PATIENT);
     }
 
 }
diff --git a/src/test/java/seedu/address/logic/commands/AddDoctorCommandTest.java b/src/test/java/seedu/address/logic/commands/AddDoctorCommandTest.java
@@ -164,19 +164,6 @@ public void addAppointment(Appointment appointment) {
             throw new AssertionError("This method should not be called.");
         }
 
-        @Override
-        public void setAppointment(Appointment target, Appointment editedAppointment) {
-            throw new AssertionError("This method should not be called.");
-        }
-
-        /**
-         * Returns an unmodifiable view of the filtered person list
-         */
-        @Override
-        public ObservableList<Person> getFilteredPersonList() {
-            return null; // not sure abt this method
-        }
-
         @Override
         public ObservableList<Patient> getFilteredPatientList() {
             throw new AssertionError("This method should not be called.");

diff --git a/src/test/java/seedu/address/logic/commands/AddPatientCommandTest.java b/src/test/java/seedu/address/logic/commands/AddPatientCommandTest.java
@@ -167,19 +167,6 @@ public void addAppointment(Appointment appointment) {
             throw new AssertionError("This method should not be called.");
         }
 
-        @Override
-        public void setAppointment(Appointment target, Appointment editedAppointment) {
-            throw new AssertionError("This method should not be called.");
-        }
-
-        /**
-         * Returns an unmodifiable view of the filtered person list
-         */
-        @Override
-        public ObservableList<Person> getFilteredPersonList() {
-            return null; // not sure abt this method
-        }
-
         @Override
         public ObservableList<Patient> getFilteredPatientList() {
             throw new AssertionError("This method should not be called.");