Merge pull request #104 from Rikko1204/edit-DG

Edit dg

docs/DeveloperGuide.md

@@ -251,6 +251,60 @@ The following activity diagram summarizes what happens when a user executes a ne
 
 _{more aspects and alternatives to be added}_
 
+
+### Schedule feature
+
+#### Proposed Implementation
+
+The schedule mechanism is facilitated by `ScheduleCommand` and it extends `Command`. A schedule is added if s/ and/or r/ parameter is present. Otherwise if s/ parameter is absent, the schedule is removed. Additionally, it implements the following operations:
+
+* `ScheduleCommand#execute()` — Execute the schedule command for the given person.
+* `ScheduleCommand#generateSuccessMessage()` — Generate message for either add schedule or remove schedule.
+
+`ScheduleCommand#execute()` is exposed in the `Logic` interface as `Logic#execute()`.
+
+Given below is an example usage scenario and how the `ScheduleCommand` mechanism behaves at each step.
+
+Step 1. The user launches the application. The `AddressBook` will be initialized with the initial address book state.
+
+Step 2. The user executes `schedule id/E1234567 s/20-12-2022 r/Consultation at 3pm` command to schedule a consultation session with person nusId E1234567 at date 20-12-2022 in the address book. The `schedule` command calls `Model#getFilteredPersonList()` to get a list of person in the `addressbook` and filter to find the relevant person with the given nusId. `Model#setPerson()` is called to update the schedule for that person in `addressbook`.
+
+<box type="info" seamless>
+
+**Note:** If a command fails its execution, it will not call `Model#setPerson()`, so the schedule will not be updated for that person in `addressbook`.
+
+</box>
+
+Step 3. Now the user decides to remove the schedule with that person. The user executes `schedule id/E1234567` to remove the schedule.`schedule` again calls `Model#getFilteredPersonList()` and filter to find the relevant person. `Model#setPerson()` is called to remove the schedule for that person in `addressbook`.
+
+The following sequence diagram shows how a schedule operation goes through the `Logic` component:
+
+<puml src="diagrams/ScheduleSequenceDiagram.puml" alt="ScheduleSequenceDiagram" />
+
+<box type="info" seamless>
+
+**Note:** The lifeline for `ScheduleCommand` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram.
+
+</box>
+
+The following activity diagram summarizes what happens when a user executes a schedule command:
+
+<puml src="diagrams/ScheduleDiagram.puml" width="250" />
+
+#### Design considerations:
+
+**Aspect: How schedule executes:**
+
+* **Alternative 1 (current choice):** Set the schedule for the person by using s/ parameter. Remove schedule by removing s/ parameter.
+    * Pros: Easy to implement.
+    * Cons: Additional checks are required to check if it is an add or remove schedule command.
+
+* **Alternative 2:** Introduce add schedule and remove schedule command as separate commands.
+    * Pros: There is usage of Single Responsibility Principle.
+    * Cons: We must ensure that the implementation of each individual command are correct.
+
+_{more aspects and alternatives to be added}_
+
 ### \[Proposed\] Data archiving
 
 _{Explain here how the data archiving feature will be implemented}_

docs/diagrams/ScheduleDiagram.puml

@@ -0,0 +1,19 @@
+@startuml
+skin rose
+skinparam ActivityFontSize 15
+skinparam ArrowFontSize 12
+start
+:User executes schedule command;
+
+'Since the beta syntax does not support placing the condition outside the
+'diamond we place it as the true branch instead.
+
+if () then ([valid schedule command])
+    :Parse schedule command;
+    :Execute schedule command;
+    :Save modified person
+          AddressBook;
+else ([else])
+endif
+stop
+@enduml

src/main/java/seedu/address/logic/Messages.java

@@ -1,5 +1,6 @@
 package seedu.address.logic;
 
+import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -15,10 +16,10 @@ public class Messages {
     public static final String MESSAGE_UNKNOWN_COMMAND = "Unknown command";
     public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s";
     public static final String MESSAGE_UNKNOWN_NUSID = "The NUSID provided does not exist!";
-    public static final String MESSAGE_NON_EXISTENT_PERSON = "This person does not exist in the address book";
+    public static final String MESSAGE_NON_EXISTENT_PERSON = "The person(s) does not exist in the address book";
     public static final String MESSAGE_PERSONS_LISTED_OVERVIEW = "%1$d persons listed!";
     public static final String MESSAGE_GROUP_PERSON = "Group has been assigned!";
-    public static final String MESSAGE_GROUP_PERSON_INVALID = "This person does not exist in the address book";
+    public static final String MESSAGE_GROUP_PERSON_INVALID = "The person(s) does not exist in the address book";
     public static final String MESSAGE_DUPLICATE_FIELDS =
                 "Multiple values specified for the following single-valued field(s): ";
 
@@ -59,4 +60,15 @@ public static String format(Person person) {
         return builder.toString();
     }
 
+    /**
+     * Formats the {@code persons} for display to the user.
+     */
+    public static String format(List<Person> persons) {
+        final StringBuilder builder = new StringBuilder();
+        persons.stream()
+                .map(person -> format(person))
+                .forEach(builder::append);
+        return builder.toString();
+    }
+
 }

src/main/java/seedu/address/logic/commands/GroupCommand.java

@@ -6,12 +6,15 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_NUSID;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
 
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.Messages;
@@ -27,14 +30,10 @@
 import seedu.address.model.person.Schedule;
 import seedu.address.model.person.Tag;
 
-
-
-
 /**
  * assigns a group to an existing person in the address book.
  */
 public class GroupCommand extends Command {
-
     public static final String MESSAGE_GROUP_PERSON_SUCCESS = "Grouped Person: %1$s";
 
     public static final String COMMAND_WORD = "group";
@@ -48,101 +47,116 @@ public class GroupCommand extends Command {
             + PREFIX_GROUP + "Class T15 "
             + PREFIX_TAG + "TA";
 
-    private final NusId toGroup;
+    private final Set<NusId> toGroup;
     private final GroupPersonDescriptor groupPersonDescriptor;
 
-
-
-
     /**
-     * @param nusid of the person in the filtered person list to group
+     * @param nusIds of the person in the filtered person list to group
      * @param groupPersonDescriptor details to group the person with
      */
-    public GroupCommand(NusId nusid, GroupPersonDescriptor groupPersonDescriptor) {
-        requireNonNull(nusid);
-        toGroup = nusid;
+    public GroupCommand(Set<NusId> nusIds, GroupPersonDescriptor groupPersonDescriptor) {
+        requireNonNull(nusIds);
+        toGroup = nusIds;
         this.groupPersonDescriptor = new GroupPersonDescriptor(groupPersonDescriptor);
-
-
     }
 
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
 
         List<Person> lastShownList = model.getFilteredPersonList();
-        Person personToGroup = lastShownList.stream().filter(person -> person.getNusId().equals(toGroup))
-                        .findFirst().orElse(null);
+        List<Person> personToGroup = lastShownList.stream()
+                .filter(person -> toGroup.contains(person.getNusId()))
+                .collect(Collectors.toList());
+        List<Person> groupedPerson = createGroupedPerson(personToGroup, groupPersonDescriptor);
 
-
-        if (personToGroup == null) {
+        if (personToGroup.isEmpty() || personToGroup.size() != toGroup.size()) {
             throw new CommandException(MESSAGE_GROUP_PERSON_INVALID);
         }
-        Person groupedPerson = createGroupedPerson(personToGroup, groupPersonDescriptor);
-        model.setPerson(personToGroup, groupedPerson);
+
+        IntStream.range(0, personToGroup.size())
+                .forEach(index -> {
+                    Person original = personToGroup.get(index);
+                    Person modified = groupedPerson.get(index);
+                    model.setPerson(original, modified);
+                });
 
         return new CommandResult(String.format(MESSAGE_GROUP_PERSON_SUCCESS, Messages.format(groupedPerson)));
     }
 
-    private static Person createGroupedPerson(Person personToGroup, GroupPersonDescriptor GroupPersonDescriptor) {
-        assert personToGroup != null;
-
-        NusId nusId = personToGroup.getNusId();
-        Name name = personToGroup.getName();
-        Phone phone = personToGroup.getPhone();
-        Email email = personToGroup.getEmail();
-        Tag updatedTag = GroupPersonDescriptor.getTag().orElse(personToGroup.getTag());
-        Set<Group> updatedGroups = GroupPersonDescriptor.getGroups().orElse(personToGroup.getGroups());
-        Schedule schedule = personToGroup.getSchedule();
-        Remark remark = personToGroup.getRemark();
+    /**
+     * Modify {@code personsToGroup} according to {@code groupPersonDescriptor}
+     *
+     * @param personsToGroup List of Person to be modified
+     * @param groupPersonDescriptor Values to be modified to
+     * @return Modified list of Person
+     */
+    public static List<Person> createGroupedPerson(List<Person> personsToGroup,
+                                                   GroupPersonDescriptor groupPersonDescriptor) {
+        List<Person> groupedPersons = new ArrayList<>();
+
+        for (Person personToGroup : personsToGroup) {
+            assert personToGroup != null;
+
+            NusId nusId = personToGroup.getNusId();
+            Name name = personToGroup.getName();
+            Phone phone = personToGroup.getPhone();
+            Email email = personToGroup.getEmail();
+            Tag updatedTag = groupPersonDescriptor.getTag().orElse(personToGroup.getTag());
+            Set<Group> updatedGroups = groupPersonDescriptor.getGroups().orElse(personToGroup.getGroups());
+            Schedule schedule = personToGroup.getSchedule();
+            Remark remark = personToGroup.getRemark();
+
+            groupedPersons.add(new Person(nusId, name, phone, email, updatedTag, updatedGroups, schedule, remark));
+        }
 
-        return new Person(nusId, name, phone, email, updatedTag, updatedGroups, schedule, remark);
+        return groupedPersons;
     }
 
     @Override
     public String toString() {
         return new ToStringBuilder(this)
-                .add("nusid", toGroup)
+                .add("nusIds", toGroup)
                 .add("groupPersonDescriptor", groupPersonDescriptor)
                 .toString();
     }
+
+    @Override
+    public boolean equals(Object other) {
+        if (other == this) {
+            return true;
+        }
+
+        // instanceof handles nulls
+        if (!(other instanceof GroupCommand)) {
+            return false;
+        }
+
+        GroupCommand otherGroupCommand = (GroupCommand) other;
+        return toGroup.size() == otherGroupCommand.toGroup.size()
+                && toGroup.containsAll(otherGroupCommand.toGroup)
+                && groupPersonDescriptor.equals(otherGroupCommand.groupPersonDescriptor);
+    }
+
     /**
      * Sets {@code group} to this object's {@code groups}.
      * A defensive copy of {@code groups} is used internally.
      */
     public static class GroupPersonDescriptor {
-
-        private NusId nusid;
         private Set<Group> groups;
         private Tag tag;
 
-
         public GroupPersonDescriptor() {}
+
         /**
          * Copy constructor.
          * A defensive copy of {@code tags} is used internally.
          */
         public GroupPersonDescriptor(GroupPersonDescriptor toCopy) {
-            //setNusId(toCopy.nusid);
-            this.nusid = toCopy.nusid;
             setGroups(toCopy.groups);
             setTag(toCopy.tag);
         }
 
-
-
-        /**
-         * Returns true if at least one field is edited.
-         */
-
-
-        public void setNusId(NusId nusid) {
-            this.nusid = nusid;
-        }
-
-        public Optional<NusId> getNusId() {
-            return Optional.ofNullable(nusid);
-        }
         public void setTag(Tag tag) {
             this.tag = tag;
         }
@@ -180,15 +194,13 @@ public boolean equals(Object other) {
             }
 
             GroupPersonDescriptor otherGroupPersonDescriptor = (GroupPersonDescriptor) other;
-            return Objects.equals(nusid, otherGroupPersonDescriptor.nusid)
-                    && Objects.equals(groups, otherGroupPersonDescriptor.groups)
+            return Objects.equals(groups, otherGroupPersonDescriptor.groups)
                     && Objects.equals(tag, otherGroupPersonDescriptor.tag);
         }
 
         @Override
         public String toString() {
             return new ToStringBuilder(this)
-                    .add("nusId", nusid)
                     .add("groups", groups)
                     .add("tag", tag)
                     .toString();

src/main/java/seedu/address/logic/parser/AddressBookParser.java

@@ -90,5 +90,4 @@ public Command parseCommand(String userInput) throws ParseException {
             throw new ParseException(MESSAGE_UNKNOWN_COMMAND);
         }
     }
-
 }

src/main/java/seedu/address/logic/parser/GroupCommandParser.java

@@ -38,28 +38,24 @@ public GroupCommand parse(String args) throws ParseException {
             throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, GroupCommand.MESSAGE_USAGE));
         }
 
-        argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NUSID, PREFIX_GROUP, PREFIX_TAG);
-        NusId nusid = ParserUtil.parseNusId(argMultimap.getValue(PREFIX_NUSID).get());
-
+        argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_GROUP, PREFIX_TAG);
 
         GroupCommand.GroupPersonDescriptor groupPersonDescriptor = new GroupCommand.GroupPersonDescriptor();
 
-        if (argMultimap.getValue(PREFIX_NUSID).isPresent()) {
-            groupPersonDescriptor.setNusId(ParserUtil.parseNusId(argMultimap.getValue(PREFIX_NUSID).get()));
-        }
-
+        Set<NusId> nusIds = parseNusIdsForGroup(argMultimap.getAllValues(PREFIX_NUSID)).get();
 
         if (argMultimap.getValue(PREFIX_TAG).isPresent()) {
             groupPersonDescriptor.setTag(ParserUtil.parseTag(argMultimap.getValue(PREFIX_TAG).get()));
         }
 
         parseGroupsForGroup(argMultimap.getAllValues(PREFIX_GROUP)).ifPresent(groupPersonDescriptor::setGroups);
-        return new GroupCommand(nusid, groupPersonDescriptor);
+        return new GroupCommand(nusIds, groupPersonDescriptor);
     }
 
     private static boolean arePrefixesPresent(ArgumentMultimap argumentMultimap, Prefix... prefixes) {
         return Stream.of(prefixes).allMatch(prefix -> argumentMultimap.getValue(prefix).isPresent());
     }
+
     /**
      * Parses {@code Collection<String> groups} into a {@code Set<Group>} if {@code groups} is non-empty.
      * If {@code groups} contain only one element which is an empty string, it will be parsed into a
@@ -74,4 +70,20 @@ private Optional<Set<Group>> parseGroupsForGroup(Collection<String> groups) thro
         Collection<String> groupSet = groups.size() == 1 && groups.contains("") ? Collections.emptySet() : groups;
         return Optional.of(ParserUtil.parseGroups(groupSet));
     }
+
+    /**
+     * Parses {@code Collection<String> nusIds} into a {@code Set<NusId>} if {@code nusIds} is non-empty.
+     * If {@code nusIds} contain only one element which is an empty string, it will be parsed into a
+     * {@code Set<NusId>} containing zero nusId.
+     */
+    private Optional<Set<NusId>> parseNusIdsForGroup(Collection<String> nusIds) throws ParseException {
+        assert nusIds != null;
+
+        if (nusIds.isEmpty()) {
+            return Optional.empty();
+        }
+
+        Collection<String> nusIdSet = nusIds.size() == 1 && nusIds.contains("") ? Collections.emptySet() : nusIds;
+        return Optional.of(ParserUtil.parseNusIds(nusIdSet));
+    }
 }