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.

Sign up for GitHub

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

Standardize EditClientCommand error msgs #318

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Standardize EditClientCommand error msgs #318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/main/java/seedu/address/logic/commands/EditClientCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ public class EditClientCommand extends Command {
+ PREFIX_EMAIL + "johndoe@example.com";

public static final String MESSAGE_EDIT_CLIENT_SUCCESS = "Edited Client: %1$s";
public static final String MESSAGE_NOT_EDITED = "At least one field to edit must be provided.";
public static final String MESSAGE_NOT_EDITED =
"At least one field to edit must be provided and the value should not be the same as before.";
public static final String MESSAGE_DUPLICATE_CLIENT = "This client already exists in MyInsuRec.";

private final Index index;
Expand Down Expand Up @@ -82,7 +83,9 @@ public CommandResult execute(Model model) throws CommandException {
Client clientToEdit = lastShownList.get(index.getZeroBased());
Client editedClient = createEditedClient(clientToEdit, editClientDescriptor);

if (!clientToEdit.isSameClient(editedClient) && model.hasClient(editedClient)) {
if (clientToEdit.equals(editedClient)) {
Copy link
Member

Choose a reason for hiding this comment

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

good implementation of differentiating changes or no changes

throw new CommandException(MESSAGE_NOT_EDITED);
} else if (model.hasClient(editedClient)) {
throw new CommandException(MESSAGE_DUPLICATE_CLIENT);
}

Expand Down
17 changes: 5 additions & 12 deletions src/main/java/seedu/address/logic/parser/EditClientCommandParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import java.time.LocalDate;
import java.util.Collection;
import java.util.Collections;
import java.util.NoSuchElementException;
import java.util.Optional;
import java.util.Set;

Expand All @@ -40,14 +39,13 @@ public EditClientCommand parse(String args) throws ParseException {
ArgumentTokenizer.tokenize(args, PREFIX_INDEX, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL,
PREFIX_ADDRESS, PREFIX_BIRTHDAY, PREFIX_PRODUCT);

Index index;

try {
index = ParserUtil.parseIndex(argMultimap.getValue(PREFIX_INDEX).get());
} catch (ParseException | NoSuchElementException e) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditClientCommand.MESSAGE_USAGE), e);
if (!ParserUtil.arePrefixesPresent(argMultimap, PREFIX_INDEX)
|| !argMultimap.getPreamble().isEmpty()) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditClientCommand.MESSAGE_USAGE));
}

Index index = ParserUtil.parseIndex(argMultimap.getValue(PREFIX_INDEX).get());

EditClientDescriptor editClientDescriptor = new EditClientDescriptor();
if (argMultimap.getValue(PREFIX_NAME).isPresent()) {
editClientDescriptor.setName(ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()));
Expand All @@ -71,11 +69,6 @@ public EditClientCommand parse(String args) throws ParseException {

parseProductsForEdit(argMultimap.getAllValues(PREFIX_PRODUCT)).ifPresent(editClientDescriptor::setProducts);


if (!editClientDescriptor.isAnyFieldEdited()) {
throw new ParseException(EditClientCommand.MESSAGE_NOT_EDITED);
}

return new EditClientCommand(index, editClientDescriptor);
}

Expand Down
6 changes: 2 additions & 4 deletions src/test/java/seedu/address/logic/commands/EditClientCommandTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,9 @@ public void execute_noFieldSpecifiedUnfilteredList_success() {
EditClientCommand editCommand = new EditClientCommand(INDEX_FIRST_ELEMENT, new EditClientDescriptor());
Client editedClient = model.getFilteredClientList().get(INDEX_FIRST_ELEMENT.getZeroBased());

String expectedMessage = String.format(EditClientCommand.MESSAGE_EDIT_CLIENT_SUCCESS, editedClient);

Model expectedModel = new ModelManager(new MyInsuRec(model.getMyInsuRec()), new UserPrefs());
String expectedMessage = String.format(EditClientCommand.MESSAGE_NOT_EDITED, editedClient);

assertCommandSuccess(editCommand, model, expectedMessage, expectedModel);
assertCommandFailure(editCommand, model, expectedMessage);
}

@Test
Expand Down
22 changes: 9 additions & 13 deletions ...s/logic/parser/EditCommandParserTest.java → ...c/parser/EditClientCommandParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static seedu.address.logic.parser.CliSyntax.PREFIX_PRODUCT;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseFailure;
import static seedu.address.logic.parser.CommandParserTestUtil.assertParseSuccess;
import static seedu.address.logic.parser.ParserUtil.MESSAGE_INVALID_INDEX;
import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_ELEMENT;
import static seedu.address.testutil.TypicalIndexes.INDEX_SECOND_ELEMENT;
import static seedu.address.testutil.TypicalIndexes.INDEX_THIRD_ELEMENT;
Expand All @@ -43,40 +44,35 @@
import seedu.address.model.product.Product;
import seedu.address.testutil.EditClientDescriptorBuilder;

public class EditCommandParserTest {
public class EditClientCommandParserTest {

private static final String PRODUCT_EMPTY = " " + PREFIX_PRODUCT;

private static final String MESSAGE_INVALID_FORMAT =
String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditClientCommand.MESSAGE_USAGE);

private EditClientCommandParser parser = new EditClientCommandParser();

@Test
public void parse_missingParts_failure() {
// no index specified
assertParseFailure(parser, VALID_NAME_AMY, MESSAGE_INVALID_FORMAT);

// no field specified
assertParseFailure(parser, " i/1", EditClientCommand.MESSAGE_NOT_EDITED);
String expectedMessage = String.format(MESSAGE_INVALID_COMMAND_FORMAT, EditClientCommand.MESSAGE_USAGE);
assertParseFailure(parser, VALID_NAME_AMY, expectedMessage);

// no index and no field specified
Copy link
Member

Choose a reason for hiding this comment

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

just a sidenote, in keeping with the heuristic to test one at a time, you might want to separate this

Copy link
Author

Choose a reason for hiding this comment

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

okk

assertParseFailure(parser, "", MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, "", expectedMessage);
}

@Test
public void parse_invalidPreamble_failure() {
// negative index
assertParseFailure(parser, " i/-5" + NAME_DESC_AMY, MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, " i/-5" + NAME_DESC_AMY, MESSAGE_INVALID_INDEX);

// zero index
assertParseFailure(parser, " i/0" + NAME_DESC_AMY, MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, " i/0" + NAME_DESC_AMY, MESSAGE_INVALID_INDEX);

// invalid arguments being parsed as preamble
assertParseFailure(parser, " i/1 some random string", MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, " i/1 some random string", MESSAGE_INVALID_INDEX);

// invalid prefix being parsed as preamble
assertParseFailure(parser, " i/1 i/ string", MESSAGE_INVALID_FORMAT);
assertParseFailure(parser, " i/1 i/ string", MESSAGE_INVALID_INDEX);
}

@Test
Expand Down