Fix multiple issues (#188)

* Fix multiple issues 1. Error when `charge` does nothing. 2. Error when `cca_delete` tries to delete a CCA that does not exist. 3. Correct `ChargeCommand.MESSAGE_USAGE`. 4. Correct `DeleteCcaCommand.MESSAGE_USAGE`. 5. Added details to `FilterCommand.MESSAGE_USAGE`. 6. Correct `OweCommand.MESSAGE_USAGE`. 7. Fix parsing for OweCommand where multiple `m/` were allowed but not handled. * Improve message for DeleteCcaCommand * Fix wrong parsing of AddCommand and EditCommand
AY2324S2-CS2103T-W11-2 · Apr 14, 2024 · df3367f · df3367f
1 parent f4b6045
commit df3367f
Show file tree

Hide file tree

Showing 11 changed files with 54 additions and 38 deletions.
diff --git a/src/main/java/seedu/address/logic/commands/AddCommand.java b/src/main/java/seedu/address/logic/commands/AddCommand.java
@@ -30,7 +30,7 @@ public class AddCommand extends Command {
             + PREFIX_ADDRESS + "ADDRESS "
             + "[" + PREFIX_ROLE + "ROLE]... "
             + "[" + PREFIX_CCA + "CCA]... "
-            + "[" + PREFIX_METADATA + "METADATA]...\n"
+            + "[" + PREFIX_METADATA + "METADATA]\n"
             + "Example: " + COMMAND_WORD + " "
             + PREFIX_NAME + "John Doe "
             + PREFIX_PHONE + "98765432 "

diff --git a/src/main/java/seedu/address/logic/commands/ChargeCommand.java b/src/main/java/seedu/address/logic/commands/ChargeCommand.java
@@ -8,6 +8,8 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_ROLE;
 
 import java.util.List;
+import java.util.stream.Collector;
+import java.util.stream.Collectors;
 
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
@@ -26,15 +28,18 @@ public class ChargeCommand extends Command {
     public static final String MESSAGE_USAGE = COMMAND_WORD
             + ": Adds a certain amount to how much all matching CCA + optional role members owe. "
             + "Parameters: "
-            + "[" + PREFIX_AMOUNT + "AMOUNT]...\n"
-            + "[" + PREFIX_CCA + "AMOUNT] (more than one allowed)...\n"
-            + "[" + PREFIX_ROLE + "AMOUNT] (more than one allowed)...\n"
+            + PREFIX_AMOUNT + "AMOUNT "
+            + "[" + PREFIX_CCA + "CCA]... "
+            + "[" + PREFIX_ROLE + "ROLE]...\n"
             + "Example: " + COMMAND_WORD + " "
             + PREFIX_AMOUNT + "10.00 "
             + PREFIX_CCA + "NUS Cycling "
             + PREFIX_ROLE + "friends";
 
     public static final String MESSAGE_NO_AMOUNT = "An amount should be provided.";
+    public static final String MESSAGE_DO_NOTHING =
+        "Nobody was charged as nobody is in any of the following CCAs: [%s] "
+        + "AND in any of the following roles: [%s]";
     private final Amount amount;
     private final CcaContainsKeywordPredicate ccas;
 
@@ -64,8 +69,16 @@ public boolean equals(Object other) {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
+        List<Person> oldList = model.getFilteredPersonList().stream().collect(Collectors.toList());
         model.updateFilteredPersonList(this.ccas);
         List<Person> lastShownList = model.getFilteredPersonList();
+        if (lastShownList.isEmpty()) {
+            model.updateFilteredPersonList(oldList::contains);
+            Collector<CharSequence, ?, String> join = Collectors.joining(", ");
+            throw new CommandException(String.format(MESSAGE_DO_NOTHING,
+                ccas.getCcas().stream().map(c -> c.ccaName).collect(join),
+                ccas.getRoles().map(rs -> rs.stream().map(r -> r.roleName).collect(join)).orElse("")));
+        }
 
         StringBuilder result = new StringBuilder();
         for (Person personToOwe : lastShownList) {

diff --git a/src/main/java/seedu/address/logic/commands/DeleteCcaCommand.java b/src/main/java/seedu/address/logic/commands/DeleteCcaCommand.java
@@ -5,7 +5,6 @@
 import static java.util.Objects.requireNonNull;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_CCA;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -27,11 +26,13 @@ public class DeleteCcaCommand extends Command {
     public static final String MESSAGE_USAGE = COMMAND_WORD
             + ": Deletes a CCA and all its associated members from your contacts. "
             + "Parameters: "
-            + "[" + PREFIX_CCA + "CCA]...\n"
+            + PREFIX_CCA + "CCA\n"
             + "Example: " + COMMAND_WORD + " "
             + PREFIX_CCA + "NUS Cycling ";
 
     public static final String MESSAGE_NO_CCA = "A CCA should be provided.";
+    public static final String MESSAGE_NONEXISTANT_CCA = "No CCA was deleted because "
+        + "the CCA `%s` does not exists.";
     private final CcaContainsKeywordPredicate ccas;
 
     /**
@@ -53,9 +54,21 @@ public boolean equals(Object other) {
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
+
+        // Check if any CCA does not exists
+        List<Cca> ccalist = model.getAddressBook().getCcaList();
+        List<Cca> nonexistantCcas = this.ccas.getCcas().stream()
+            .filter(c1 -> !ccalist.stream().anyMatch(c1::isSameCcaName))
+            .collect(Collectors.toList());
+        if (!nonexistantCcas.isEmpty()) {
+            throw new CommandException(String.format(MESSAGE_NONEXISTANT_CCA,
+                nonexistantCcas.stream().map(c -> c.ccaName).collect(Collectors.joining(", "))));
+        }
+
         model.updateFilteredPersonList(this.ccas);
         StringBuilder result = new StringBuilder();
-        result.append(String.format("Deleting CCA(s) %s tags from all its members:\n", this.ccas));
+        result.append(String.format("Deleting CCA(s) %s tags from all its members:\n",
+            this.ccas.getCcas().stream().map(c -> c.ccaName).collect(Collectors.joining(", "))));
 
         // We have to essentially clone the list
         // because as `model.setPerson` is called,
@@ -67,19 +80,12 @@ public CommandResult execute(Model model) throws CommandException {
             .collect(Collectors.toList());
 
         // Delete their roles
-        ArrayList<Cca> removedCcas = new ArrayList<>();
         affectedPeople
             .forEach(affectedPerson -> {
                 Set<Cca> updatedCca = affectedPerson
                     .getCcas()
                     .stream()
-                    .filter(c -> {
-                        boolean isToDelete = ccas.contains(c);
-                        if (isToDelete) {
-                            removedCcas.add(c);
-                        }
-                        return !isToDelete;
-                    })
+                    .filter(c -> !ccas.contains(c))
                     .collect(Collectors.toSet());
                 model.setPerson(affectedPerson, affectedPerson.replaceCca(updatedCca));
                 result.append(String.format("Person affected: %s\n", affectedPerson.getName()));

diff --git a/src/main/java/seedu/address/logic/commands/EditCommand.java b/src/main/java/seedu/address/logic/commands/EditCommand.java
@@ -51,9 +51,9 @@ public class EditCommand extends Command {
             + "[" + PREFIX_PHONE + "PHONE] "
             + "[" + PREFIX_EMAIL + "EMAIL] "
             + "[" + PREFIX_ADDRESS + "ADDRESS] "
-            + "[" + PREFIX_ROLE + "ROLE] "
-            + "[" + PREFIX_CCA + "CCA] "
-            + "[" + PREFIX_METADATA + "Meta-data]...\n"
+            + "[" + PREFIX_ROLE + "ROLE]... "
+            + "[" + PREFIX_CCA + "CCA]... "
+            + "[" + PREFIX_METADATA + "METADATA]\n"
             + "Example: " + COMMAND_WORD + " 1 "
             + PREFIX_PHONE + "91234567 "
             + PREFIX_EMAIL + "[email protected]\n\n"

diff --git a/src/main/java/seedu/address/logic/commands/FilterCommand.java b/src/main/java/seedu/address/logic/commands/FilterCommand.java
@@ -3,6 +3,8 @@
 package seedu.address.logic.commands;
 
 import static java.util.Objects.requireNonNull;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_CCA;
+import static seedu.address.logic.parser.CliSyntax.PREFIX_ROLE;
 
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.Messages;
@@ -17,10 +19,14 @@ public class FilterCommand extends Command {
 
     public static final String COMMAND_WORD = "filter";
 
-    public static final String MESSAGE_USAGE = COMMAND_WORD + ": Filters a person by their CCA "
-            + "(case-sensitive) and optionally their roles."
+    public static final String MESSAGE_USAGE = COMMAND_WORD
+            + ": Filters a person by their CCA "
+            + "(case-sensitive) and optionally their roles. "
+            + "Exact word matches are required. "
             + "Displays them as a list with index numbers.\n"
-            + "Parameters: c/CCA c/[MORE CCAs]... r/ROLE /r[MORE ROLES]\n"
+            + "Parameters: "
+            + "[" + PREFIX_CCA + "CCA]... "
+            + "[" + PREFIX_ROLE + "ROLE]...\n"
             + "Example: " + COMMAND_WORD + " c/NUS Cycling r/Member";
 
     public static final String MESSAGE_NOT_FILTER_CCA = "A CCA should be provided.";

diff --git a/src/main/java/seedu/address/logic/commands/OweCommand.java b/src/main/java/seedu/address/logic/commands/OweCommand.java
@@ -36,7 +36,7 @@ public class OweCommand extends Command {
             + "by the index number used in the displayed person list. "
             + "Existing values will be overwritten by the input values.\n"
             + "Parameters: INDEX (must be a positive integer) "
-            + "[" + PREFIX_AMOUNT + "AMOUNT]...\n"
+            + PREFIX_AMOUNT + "AMOUNT\n"
             + "Example: " + COMMAND_WORD + " 2 "
             + PREFIX_AMOUNT + "10.00 ";
 

diff --git a/src/main/java/seedu/address/logic/parser/AddCommandParser.java b/src/main/java/seedu/address/logic/parser/AddCommandParser.java
@@ -47,7 +47,8 @@ public AddCommand parse(String args) throws ParseException {
             throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE));
         }
 
-        argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS);
+        argMultimap.verifyNoDuplicatePrefixesFor(
+            PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_METADATA);
         Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get());
         Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get());
         Email email = ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get());

diff --git a/src/main/java/seedu/address/logic/parser/ChargeCommandParser.java b/src/main/java/seedu/address/logic/parser/ChargeCommandParser.java
@@ -48,13 +48,6 @@ public ChargeCommand parse(String args) throws ParseException {
             );
         }
 
-        // Make sure all roles are not empty
-        if (argumentMultimap.getAllValues(PREFIX_ROLE).stream().anyMatch(String::isEmpty)) {
-            throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, FilterCommand.MESSAGE_ROLE_EMPTY)
-            + "\n" + ChargeCommand.MESSAGE_USAGE);
-        }
-
-
         Set<Cca> ccas = new HashSet<>(ParserUtil.parseCcas(argumentMultimap.getAllValues(PREFIX_CCA)));
 
         Optional<Set<Role>> roles = argumentMultimap.getAllValues(PREFIX_ROLE).isEmpty()

diff --git a/src/main/java/seedu/address/logic/parser/DeleteCcaCommandParser.java b/src/main/java/seedu/address/logic/parser/DeleteCcaCommandParser.java
@@ -38,13 +38,7 @@ public DeleteCcaCommand parse(String args) throws ParseException {
             );
         }
 
-        if (argumentMultimap.getAllValues(PREFIX_CCA).size() > 1) {
-            throw new ParseException(
-                    String.format(
-                            MESSAGE_INVALID_COMMAND_FORMAT, "Cannot delete more than one CCA."
-                    )
-            );
-        }
+        argumentMultimap.verifyNoDuplicatePrefixesFor(PREFIX_CCA);
 
         Set<Cca> ccas = new HashSet<>(ParserUtil.parseCcas(argumentMultimap.getAllValues(PREFIX_CCA)));
 

diff --git a/src/main/java/seedu/address/logic/parser/EditCommandParser.java b/src/main/java/seedu/address/logic/parser/EditCommandParser.java
@@ -48,7 +48,8 @@ public EditCommand parse(String args) throws ParseException {
                 + "\n" + EditCommand.MESSAGE_USAGE, pe);
         }
 
-        argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS);
+        argMultimap.verifyNoDuplicatePrefixesFor(
+            PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_METADATA);
 
         EditPersonDescriptor editPersonDescriptor = new EditPersonDescriptor();
 

diff --git a/src/main/java/seedu/address/logic/parser/OweCommandParser.java b/src/main/java/seedu/address/logic/parser/OweCommandParser.java
@@ -34,6 +34,8 @@ public OweCommand parse(String args) throws ParseException {
                 + "\n" + OweCommand.MESSAGE_USAGE, pe);
         }
 
+        argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_AMOUNT);
+
         if (argMultimap.getValue(PREFIX_AMOUNT).isEmpty()) {
             throw new ParseException(OweCommand.MESSAGE_NOT_OWE);
         }