From 2ee69e529d53dd866845d399980d764af30015f5 Mon Sep 17 00:00:00 2001 From: ZShunRen Date: Fri, 1 Nov 2024 03:08:31 +0800 Subject: [PATCH] Add cumulative invalid parameter errors When a user uses a command with multiple flags wrong, namely `add` and `edit`, the error message only displays an error for the first invalid flag. Now the error message is cumulative, allowing users to correct multiple wrong flag values with 1 error message. --- .../logic/commands/CommandCommons.java | 38 ++++++++++++ .../logic/parser/AddCommandParser.java | 43 ++++++++----- .../logic/parser/EditCommandParser.java | 45 ++++++++++---- .../address/logic/parser/ParserUtil.java | 1 - .../logic/commands/CommandTestUtil.java | 3 +- .../logic/parser/AddCommandParserTest.java | 60 +++++++++++++------ .../logic/parser/EditCommandParserTest.java | 50 ++++++++++------ .../JsonSerializableAgentAssistTest.java | 1 - .../seedu/address/testutil/PersonUtil.java | 3 +- 9 files changed, 177 insertions(+), 67 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/CommandCommons.java b/src/main/java/seedu/address/logic/commands/CommandCommons.java index ad3eab11bb5..59b7c55b417 100644 --- a/src/main/java/seedu/address/logic/commands/CommandCommons.java +++ b/src/main/java/seedu/address/logic/commands/CommandCommons.java @@ -1,5 +1,9 @@ package seedu.address.logic.commands; +import java.util.Set; +import java.util.function.Supplier; + +import seedu.address.logic.parser.exceptions.ParseException; import seedu.address.model.person.Person; /** @@ -11,4 +15,38 @@ public final class CommandCommons { public static final String DEFAULT_STATUS = "NONE"; public static final Person EMPTY_PERSON = null; + /** + * A functional interface similar to {@link Supplier}, but allows for a checked exception, {@code ParseException} + * to be thrown. + * @param The return type of the method supplied by this supplier. + */ + @FunctionalInterface + public interface ThrowingSupplier { + T get() throws ParseException; + } + + /** + * Used to parse a field in a command, and if there are problems with the field, add the resulting error message + * to a {@link Set}, to be displayed to the user + * @param The attribute type to be parsed + * @param parserMethod A lambda expression that contains a method, that when run can either return the parsed value + * or throws a {@code ParseException} + * @param errors A {@link Set} that contains the error messages + * @return The parsed value or null + */ + public static T parseField(ThrowingSupplier parserMethod, Set errors) { + try { + return parserMethod.get(); + } catch (ParseException e) { + errors.add(e.getMessage()); + return null; + } + } + + public static String getErrorMessage(Set errors) { + StringBuilder accumulatedErrors = new StringBuilder(); + errors.forEach(error -> accumulatedErrors.append(error + "\n")); + return accumulatedErrors.toString().trim(); + } + } diff --git a/src/main/java/seedu/address/logic/parser/AddCommandParser.java b/src/main/java/seedu/address/logic/parser/AddCommandParser.java index 84bfe028b5c..b5a551fdd62 100644 --- a/src/main/java/seedu/address/logic/parser/AddCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/AddCommandParser.java @@ -1,17 +1,21 @@ package seedu.address.logic.parser; import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; +import static seedu.address.logic.commands.CommandCommons.getErrorMessage; +import static seedu.address.logic.commands.CommandCommons.parseField; import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS; import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL; import static seedu.address.logic.parser.CliSyntax.PREFIX_INCOME; import static seedu.address.logic.parser.CliSyntax.PREFIX_JOB; import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME; -import static seedu.address.logic.parser.CliSyntax.PREFIX_NEW_REMARK; import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE; +import static seedu.address.logic.parser.CliSyntax.PREFIX_REMARK; import static seedu.address.logic.parser.CliSyntax.PREFIX_STATUS; import static seedu.address.logic.parser.CliSyntax.PREFIX_TIER; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -35,6 +39,7 @@ */ public class AddCommandParser implements Parser { + /** * Parses the given {@code String} of arguments in the context of the AddCommand * and returns an AddCommand object for execution. @@ -44,7 +49,7 @@ public class AddCommandParser implements Parser { public AddCommand parse(String args) throws ParseException { ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_JOB, - PREFIX_INCOME, PREFIX_TIER, PREFIX_NEW_REMARK, PREFIX_STATUS); + PREFIX_INCOME, PREFIX_TIER, PREFIX_REMARK, PREFIX_STATUS); PrefixCheckResult prefixCheckResult = arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_ADDRESS, PREFIX_JOB, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_INCOME); if (!prefixCheckResult.isAllPrefixPresent()) { @@ -56,22 +61,32 @@ 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, PREFIX_JOB, - PREFIX_INCOME, PREFIX_TIER, PREFIX_NEW_REMARK, PREFIX_STATUS); - 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()); - Address address = ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get()); - Job job = ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get()); - Income income = ParserUtil.parseIncome(argMultimap.getValue(PREFIX_INCOME).get()); - Tier tier = ParserUtil.parseTier(argMultimap.getValue(PREFIX_TIER).orElse(CommandCommons.DEFAULT_TIER)); - Remark remark = ParserUtil.parseNewRemark(argMultimap.getValue(PREFIX_NEW_REMARK) - .orElse(CommandCommons.DEFAULT_REMARK)); - Status status = - ParserUtil.parseStatus(argMultimap.getValue(PREFIX_STATUS).orElse(CommandCommons.DEFAULT_STATUS)); + PREFIX_INCOME, PREFIX_TIER, PREFIX_REMARK, PREFIX_STATUS); + Set errors = new LinkedHashSet<>(); + Name name = parseField(() -> ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()), errors); + Phone phone = parseField(() -> ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get()), errors); + Email email = parseField(() -> ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get()), errors); + Address address = parseField(() -> ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get()), + errors); + Job job = parseField(() -> ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get()), errors); + Income income = parseField(() -> ParserUtil.parseIncome(argMultimap.getValue(PREFIX_INCOME).get()), + errors); + Tier tier = parseField(() -> ParserUtil.parseTier(argMultimap.getValue(PREFIX_TIER) + .orElse(CommandCommons.DEFAULT_TIER)), errors); + Remark remark = parseField(() -> ParserUtil.parseRemark(argMultimap.getValue(PREFIX_REMARK) + .orElse(CommandCommons.DEFAULT_REMARK)), errors); + Status status = parseField(() -> ParserUtil.parseStatus(argMultimap.getValue(PREFIX_STATUS) + .orElse(CommandCommons.DEFAULT_STATUS)), errors); + if (!errors.isEmpty()) { + throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, + getErrorMessage(errors) + "\n" + AddCommand.MESSAGE_USAGE)); + } Person person = new Person(name, phone, email, address, job, income, tier, remark, status); return new AddCommand(person); } + + /** * Returns {@code PrefixCheckResult} if none of the prefixes contains empty {@code Optional} values in the given * {@code ArgumentMultimap}. diff --git a/src/main/java/seedu/address/logic/parser/EditCommandParser.java b/src/main/java/seedu/address/logic/parser/EditCommandParser.java index ce89e81767c..9c9a4dbfc48 100644 --- a/src/main/java/seedu/address/logic/parser/EditCommandParser.java +++ b/src/main/java/seedu/address/logic/parser/EditCommandParser.java @@ -2,6 +2,8 @@ import static java.util.Objects.requireNonNull; import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; +import static seedu.address.logic.commands.CommandCommons.getErrorMessage; +import static seedu.address.logic.commands.CommandCommons.parseField; import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS; import static seedu.address.logic.parser.CliSyntax.PREFIX_APPEND_REMARK; import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL; @@ -13,6 +15,9 @@ import static seedu.address.logic.parser.CliSyntax.PREFIX_STATUS; import static seedu.address.logic.parser.CliSyntax.PREFIX_TIER; +import java.util.LinkedHashSet; +import java.util.Set; + import seedu.address.commons.core.index.Index; import seedu.address.logic.Messages; import seedu.address.logic.commands.EditCommand; @@ -53,38 +58,52 @@ public EditCommand parse(String args) throws ParseException { PREFIX_INCOME, PREFIX_JOB, PREFIX_TIER, PREFIX_NEW_REMARK, PREFIX_APPEND_REMARK, PREFIX_STATUS); EditPersonDescriptor editPersonDescriptor = new EditPersonDescriptor(); - + Set errors = new LinkedHashSet<>(); if (argMultimap.getValue(PREFIX_NAME).isPresent()) { - editPersonDescriptor.setName(ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get())); + editPersonDescriptor.setName( + parseField(() -> ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()), errors)); } if (argMultimap.getValue(PREFIX_PHONE).isPresent()) { - editPersonDescriptor.setPhone(ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get())); + editPersonDescriptor.setPhone( + parseField(() -> ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get()), errors)); } if (argMultimap.getValue(PREFIX_EMAIL).isPresent()) { - editPersonDescriptor.setEmail(ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get())); + editPersonDescriptor.setEmail( + parseField(() -> ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get()), errors)); } if (argMultimap.getValue(PREFIX_ADDRESS).isPresent()) { - editPersonDescriptor.setAddress(ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get())); + editPersonDescriptor.setAddress( + parseField(() -> ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get()), + errors)); } if (argMultimap.getValue(PREFIX_JOB).isPresent()) { - editPersonDescriptor.setJob(ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get())); + editPersonDescriptor.setJob(parseField(() -> ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get()), + errors)); } if (argMultimap.getValue(PREFIX_INCOME).isPresent()) { - editPersonDescriptor.setIncome(ParserUtil.parseIncome(argMultimap.getValue(PREFIX_INCOME).get())); + editPersonDescriptor.setIncome(parseField(() -> ParserUtil.parseIncome( + argMultimap.getValue(PREFIX_INCOME).get()), errors)); } if (argMultimap.getValue(PREFIX_TIER).isPresent()) { - editPersonDescriptor.setTier(ParserUtil.parseTier(argMultimap.getValue(PREFIX_TIER).get())); + editPersonDescriptor.setTier(parseField(() -> ParserUtil.parseTier( + argMultimap.getValue(PREFIX_TIER).get()), errors)); } if (argMultimap.getValue(PREFIX_NEW_REMARK).isPresent()) { - editPersonDescriptor.setNewRemark(ParserUtil.parseNewRemark(argMultimap.getValue(PREFIX_NEW_REMARK).get())); + editPersonDescriptor.setNewRemark(parseField(() -> ParserUtil.parseNewRemark( + argMultimap.getValue(PREFIX_NEW_REMARK).get()), errors)); } if (argMultimap.getValue(PREFIX_APPEND_REMARK).isPresent()) { - editPersonDescriptor.setAppendedRemark(ParserUtil.parseNewRemark( - argMultimap.getValue(PREFIX_APPEND_REMARK).get())); + editPersonDescriptor.setAppendedRemark(parseField(() -> ParserUtil.parseNewRemark( + argMultimap.getValue(PREFIX_APPEND_REMARK).get()), errors)); } if (argMultimap.getValue(PREFIX_STATUS).isPresent()) { - editPersonDescriptor.setStatus((ParserUtil.parseStatus( - argMultimap.getValue(PREFIX_STATUS).get()))); + editPersonDescriptor.setStatus(parseField(() -> ParserUtil.parseStatus( + argMultimap.getValue(PREFIX_STATUS).get()), errors)); + } + + if (!errors.isEmpty()) { + throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, + getErrorMessage(errors))); } if (!editPersonDescriptor.isAnyFieldEdited()) { diff --git a/src/main/java/seedu/address/logic/parser/ParserUtil.java b/src/main/java/seedu/address/logic/parser/ParserUtil.java index fe4eea9526b..f013e329ef4 100644 --- a/src/main/java/seedu/address/logic/parser/ParserUtil.java +++ b/src/main/java/seedu/address/logic/parser/ParserUtil.java @@ -200,7 +200,6 @@ public static IncomeComparisonOperator parseIncomeComparisonOperator(String oper requireNonNull(operator); String trimmedOperator = operator.trim(); if (!IncomeComparisonOperator.isValidComparisonOperator(trimmedOperator)) { - System.out.println(("HERE")); throw new ParseException(IncomeComparisonOperator.MESSAGE_CONSTRAINTS); } return new IncomeComparisonOperator(trimmedOperator); diff --git a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java index 0259d5b6e83..69b73149720 100644 --- a/src/test/java/seedu/address/logic/commands/CommandTestUtil.java +++ b/src/test/java/seedu/address/logic/commands/CommandTestUtil.java @@ -64,6 +64,7 @@ public class CommandTestUtil { public static final String VALID_REMARK_BOB = "Favourite pastime: Eating"; public static final String TIER_DESC_REJECT = " " + PREFIX_TIER + VALID_TIER_REJECT; public static final String TIER_DESC_GOLD = " " + PREFIX_TIER + VALID_TIER_GOLD; + public static final String REMARK_DESC_BOB = " " + PREFIX_REMARK + VALID_REMARK_BOB; public static final String NEW_REMARK_DESC_BOB = " " + PREFIX_NEW_REMARK + VALID_REMARK_BOB; public static final String APPEND_REMARK_DESC_BOB = " " + PREFIX_APPEND_REMARK + VALID_REMARK_BOB; public static final String VALID_STATUS_NON_URGENT = "non_urgent"; @@ -79,7 +80,7 @@ public class CommandTestUtil { public static final String INVALID_INCOME_DESC = " " + PREFIX_INCOME + "-999"; // negative numbers should not be // allowed public static final String INVALID_TIER_DESC = " " + PREFIX_TIER + "platinum"; // not one of the 4 TierEnums - public static final String INVALID_REMARK_DESC = " " + PREFIX_REMARK; + public static final String INVALID_REMARK_DESC = " " + PREFIX_REMARK + "漢"; public static final String INVALID_NEW_REMARK_DESC = " " + PREFIX_NEW_REMARK; public static final String INVALID_APPEND_REMARK_DESC = " " + PREFIX_APPEND_REMARK; public static final String INVALID_STATUS_DESC = " " + PREFIX_STATUS + "null"; diff --git a/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java b/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java index c1f840b7cbb..d1beda76911 100644 --- a/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/AddCommandParserTest.java @@ -20,11 +20,11 @@ import static seedu.address.logic.commands.CommandTestUtil.JOB_DESC_BOB; import static seedu.address.logic.commands.CommandTestUtil.NAME_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.NAME_DESC_BOB; -import static seedu.address.logic.commands.CommandTestUtil.NEW_REMARK_DESC_BOB; import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_AMY; import static seedu.address.logic.commands.CommandTestUtil.PHONE_DESC_BOB; import static seedu.address.logic.commands.CommandTestUtil.PREAMBLE_NON_EMPTY; import static seedu.address.logic.commands.CommandTestUtil.PREAMBLE_WHITESPACE; +import static seedu.address.logic.commands.CommandTestUtil.REMARK_DESC_BOB; import static seedu.address.logic.commands.CommandTestUtil.STATUS_DESC_BOB; import static seedu.address.logic.commands.CommandTestUtil.TIER_DESC_GOLD; import static seedu.address.logic.commands.CommandTestUtil.TIER_DESC_REJECT; @@ -41,8 +41,8 @@ import static seedu.address.logic.parser.CliSyntax.PREFIX_INCOME; import static seedu.address.logic.parser.CliSyntax.PREFIX_JOB; import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME; -import static seedu.address.logic.parser.CliSyntax.PREFIX_NEW_REMARK; import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE; +import static seedu.address.logic.parser.CliSyntax.PREFIX_REMARK; import static seedu.address.logic.parser.CliSyntax.PREFIX_STATUS; import static seedu.address.logic.parser.CliSyntax.PREFIX_TIER; import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure; @@ -65,6 +65,7 @@ import seedu.address.model.person.Name; import seedu.address.model.person.Person; import seedu.address.model.person.Phone; +import seedu.address.model.person.Remark; import seedu.address.model.status.Status; import seedu.address.model.tier.Tier; import seedu.address.testutil.PersonBuilder; @@ -78,15 +79,15 @@ public void parse_allFieldsPresent_success() { // whitespace only preamble assertParseSuccess(parser, PREAMBLE_WHITESPACE + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB - + ADDRESS_DESC_BOB + JOB_DESC_BOB + INCOME_DESC_BOB + TIER_DESC_GOLD + NEW_REMARK_DESC_BOB, - new AddCommand(expectedPerson)); + + ADDRESS_DESC_BOB + JOB_DESC_BOB + INCOME_DESC_BOB + TIER_DESC_GOLD + REMARK_DESC_BOB + + STATUS_DESC_BOB, new AddCommand(expectedPerson)); } @Test public void parse_repeatedNonTierValue_failure() { String validExpectedPersonString = NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB + JOB_DESC_BOB + INCOME_DESC_BOB + TIER_DESC_GOLD - + NEW_REMARK_DESC_BOB + STATUS_DESC_BOB; + + REMARK_DESC_BOB + STATUS_DESC_BOB; // multiple names assertParseFailure(parser, NAME_DESC_AMY + validExpectedPersonString, @@ -121,8 +122,8 @@ public void parse_repeatedNonTierValue_failure() { Messages.getErrorMessageForDuplicatePrefixes(PREFIX_TIER)); // multiple remarks - assertParseFailure(parser, NEW_REMARK_DESC_BOB + validExpectedPersonString, - Messages.getErrorMessageForDuplicatePrefixes(PREFIX_NEW_REMARK)); + assertParseFailure(parser, REMARK_DESC_BOB + validExpectedPersonString, + Messages.getErrorMessageForDuplicatePrefixes(PREFIX_REMARK)); // multiple status assertParseFailure(parser, STATUS_DESC_BOB + validExpectedPersonString, @@ -132,7 +133,7 @@ public void parse_repeatedNonTierValue_failure() { assertParseFailure(parser, validExpectedPersonString + validExpectedPersonString + VALID_STATUS_NONE, Messages.getErrorMessageForDuplicatePrefixes(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, - PREFIX_JOB, PREFIX_INCOME, PREFIX_TIER, PREFIX_NEW_REMARK, PREFIX_STATUS)); + PREFIX_JOB, PREFIX_INCOME, PREFIX_TIER, PREFIX_REMARK, PREFIX_STATUS)); // invalid name assertParseFailure(parser, INVALID_NAME_DESC + validExpectedPersonString, @@ -223,43 +224,64 @@ public void parse_compulsoryFieldMissing_failure() { public void parse_invalidValue_failure() { // invalid name assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, Name.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Name.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid phone assertParseFailure(parser, NAME_DESC_BOB + INVALID_PHONE_DESC + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, Phone.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Phone.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid email assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + INVALID_EMAIL_DESC + ADDRESS_DESC_BOB - + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, Email.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Email.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid address assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC - + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, Address.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Address.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid income assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INVALID_INCOME_DESC + JOB_DESC_BOB + TIER_DESC_GOLD, Income.MESSAGE_CONSTRAINTS); + + INVALID_INCOME_DESC + JOB_DESC_BOB + TIER_DESC_GOLD, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Income.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid job assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INCOME_DESC_BOB + INVALID_JOB_DESC + TIER_DESC_GOLD, Job.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + INVALID_JOB_DESC + TIER_DESC_GOLD, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Job.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid tier assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INCOME_DESC_BOB + JOB_DESC_BOB + INVALID_TIER_DESC + VALID_TIER_GOLD, Tier.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB + INVALID_TIER_DESC + VALID_TIER_GOLD, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Tier.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid remark assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD + INVALID_REMARK_DESC, Tier.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD + INVALID_REMARK_DESC, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Remark.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // invalid status assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB - + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD + INVALID_STATUS_DESC, Status.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB + TIER_DESC_GOLD + INVALID_STATUS_DESC, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Status.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); - // two invalid values, only first invalid value reported + // two invalid values, both are shown assertParseFailure(parser, INVALID_NAME_DESC + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC - + INCOME_DESC_BOB + JOB_DESC_BOB, Name.MESSAGE_CONSTRAINTS); + + INCOME_DESC_BOB + JOB_DESC_BOB, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, + Name.MESSAGE_CONSTRAINTS + "\n" + Address.MESSAGE_CONSTRAINTS + "\n" + + AddCommand.MESSAGE_USAGE)); // non-empty preamble assertParseFailure(parser, PREAMBLE_NON_EMPTY + NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB diff --git a/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java b/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java index e533e704ff3..5a61ada881a 100644 --- a/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java +++ b/src/test/java/seedu/address/logic/parser/EditCommandParserTest.java @@ -99,34 +99,50 @@ public void parse_invalidPreamble_failure() { @Test public void parse_invalidValue_failure() { - assertParseFailure(parser, "1" + INVALID_NAME_DESC, Name.MESSAGE_CONSTRAINTS); // invalid name - assertParseFailure(parser, "1" + INVALID_PHONE_DESC, Phone.MESSAGE_CONSTRAINTS); // invalid phone - assertParseFailure(parser, "1" + INVALID_EMAIL_DESC, Email.MESSAGE_CONSTRAINTS); // invalid email - assertParseFailure(parser, "1" + INVALID_ADDRESS_DESC, Address.MESSAGE_CONSTRAINTS); // invalid address - assertParseFailure(parser, "1" + INVALID_INCOME_DESC, Income.MESSAGE_CONSTRAINTS); - assertParseFailure(parser, "1" + INVALID_TIER_DESC, Tier.MESSAGE_CONSTRAINTS); + assertParseFailure(parser, "1" + INVALID_NAME_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Name.MESSAGE_CONSTRAINTS)); // invalid name + assertParseFailure(parser, "1" + INVALID_PHONE_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Phone.MESSAGE_CONSTRAINTS)); // invalid phone + assertParseFailure(parser, "1" + INVALID_EMAIL_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Email.MESSAGE_CONSTRAINTS)); // invalid email + assertParseFailure(parser, "1" + INVALID_ADDRESS_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Address.MESSAGE_CONSTRAINTS)); // invalid address + assertParseFailure(parser, "1" + INVALID_INCOME_DESC, + String.format(MESSAGE_INVALID_COMMAND_FORMAT, Income.MESSAGE_CONSTRAINTS)); + assertParseFailure(parser, "1" + INVALID_TIER_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Tier.MESSAGE_CONSTRAINTS)); // invalid new remark which is empty - assertParseFailure(parser, "1" + INVALID_NEW_REMARK_DESC, Remark.MESSAGE_CONSTRAINTS); + assertParseFailure(parser, "1" + INVALID_NEW_REMARK_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Remark.MESSAGE_CONSTRAINTS)); // invalid appended remark which is empty - assertParseFailure(parser, "1" + INVALID_APPEND_REMARK_DESC, Remark.MESSAGE_CONSTRAINTS); - assertParseFailure(parser, "1" + INVALID_STATUS_DESC, Status.MESSAGE_CONSTRAINTS); + assertParseFailure(parser, "1" + INVALID_APPEND_REMARK_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Remark.MESSAGE_CONSTRAINTS)); + assertParseFailure(parser, "1" + INVALID_STATUS_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Status.MESSAGE_CONSTRAINTS)); - // multiple invalid values, but only the first invalid value is captured + // multiple invalid values, all are expected assertParseFailure(parser, "1" + INVALID_NAME_DESC + INVALID_EMAIL_DESC + VALID_ADDRESS_AMY - + VALID_PHONE_AMY, Name.MESSAGE_CONSTRAINTS); - assertParseFailure(parser, "1" + INVALID_TIER_DESC, Tier.MESSAGE_CONSTRAINTS); // invalid tier + + VALID_PHONE_AMY, String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, + Name.MESSAGE_CONSTRAINTS) + "\n" + Email.MESSAGE_CONSTRAINTS); + + assertParseFailure(parser, "1" + INVALID_TIER_DESC, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Tier.MESSAGE_CONSTRAINTS)); // invalid phone followed by valid email - assertParseFailure(parser, "1" + INVALID_PHONE_DESC + EMAIL_DESC_AMY, Phone.MESSAGE_CONSTRAINTS); + assertParseFailure(parser, "1" + INVALID_PHONE_DESC + EMAIL_DESC_AMY, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Phone.MESSAGE_CONSTRAINTS)); - // multiple invalid values, but only the first invalid value is captured + // multiple invalid values, all are captured assertParseFailure(parser, "1" + INVALID_NAME_DESC + INVALID_EMAIL_DESC - + VALID_ADDRESS_AMY + VALID_PHONE_AMY, Name.MESSAGE_CONSTRAINTS); + + VALID_ADDRESS_AMY + VALID_PHONE_AMY, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Name.MESSAGE_CONSTRAINTS) + + "\n" + Email.MESSAGE_CONSTRAINTS); // multiple invalid values but with purely optional fields, only the first in order of logical parsing is - // captured assertParseFailure(parser, "1" + INVALID_STATUS_DESC + INVALID_TIER_DESC - + VALID_ADDRESS_AMY + VALID_PHONE_AMY, Tier.MESSAGE_CONSTRAINTS); + + VALID_ADDRESS_AMY + VALID_PHONE_AMY, + String.format(Messages.MESSAGE_INVALID_COMMAND_FORMAT, Tier.MESSAGE_CONSTRAINTS) + + "\n" + Status.MESSAGE_CONSTRAINTS); } @Test diff --git a/src/test/java/seedu/address/storage/JsonSerializableAgentAssistTest.java b/src/test/java/seedu/address/storage/JsonSerializableAgentAssistTest.java index d8700c2688f..453225adf77 100644 --- a/src/test/java/seedu/address/storage/JsonSerializableAgentAssistTest.java +++ b/src/test/java/seedu/address/storage/JsonSerializableAgentAssistTest.java @@ -26,7 +26,6 @@ public void toModelType_typicalPersonsFile_success() throws Exception { JsonSerializableAgentAssist.class).get(); AgentAssist agentAssistFromFile = dataFromFile.toModelType(); AgentAssist typicalPersonsAgentAssist = TypicalPersons.getTypicalAgentAssist(); - System.out.println("Agent Assist: " + agentAssistFromFile); assertEquals(agentAssistFromFile, typicalPersonsAgentAssist); } diff --git a/src/test/java/seedu/address/testutil/PersonUtil.java b/src/test/java/seedu/address/testutil/PersonUtil.java index 08c6b4244c9..891abc8db61 100644 --- a/src/test/java/seedu/address/testutil/PersonUtil.java +++ b/src/test/java/seedu/address/testutil/PersonUtil.java @@ -7,6 +7,7 @@ import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME; import static seedu.address.logic.parser.CliSyntax.PREFIX_NEW_REMARK; import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE; +import static seedu.address.logic.parser.CliSyntax.PREFIX_REMARK; import static seedu.address.logic.parser.CliSyntax.PREFIX_TIER; import seedu.address.logic.commands.AddCommand; @@ -37,7 +38,7 @@ public static String getPersonDetails(Person person) { sb.append(PREFIX_JOB + person.getJob().value + " "); sb.append(PREFIX_INCOME + (String.valueOf(person.getIncome().value)) + " "); sb.append(PREFIX_TIER + person.getTier().toParsableString() + " "); - sb.append(PREFIX_NEW_REMARK + person.getRemark().value + " "); + sb.append(PREFIX_REMARK + person.getRemark().value + " "); return sb.toString(); }