Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

Commit

Permalink
EditCommand: Store Person to be edited
Browse files Browse the repository at this point in the history
EditCommand only stores the `Index` of the `Person`
in the current `filteredPersonList` to be edited.

This could cause the wrong `Person` to be edited during the
re-execution of the same command during `redo()` due to
different indexing in different `filteredPersonList` views.

Let's update `EditCommand` to store the `Person` to be
edited before its first execution.
  • Loading branch information
yamidark committed Jan 14, 2018
1 parent fbfdef4 commit 449969b
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 7 deletions.
34 changes: 27 additions & 7 deletions src/main/java/seedu/address/logic/commands/EditCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ public class EditCommand extends UndoableCommand {
public static final String MESSAGE_NOT_EDITED = "At least one field to edit must be provided.";
public static final String MESSAGE_DUPLICATE_PERSON = "This person already exists in the address book.";

private final Index index;
private final Index targetIndex;
private final EditPersonDescriptor editPersonDescriptor;

private ReadOnlyPerson personToEdit;
private boolean isFirstExecution;


/**
* @param index of the person in the filtered person list to edit
* @param editPersonDescriptor details to edit the person with
Expand All @@ -61,19 +65,20 @@ public EditCommand(Index index, EditPersonDescriptor editPersonDescriptor) {
requireNonNull(index);
requireNonNull(editPersonDescriptor);

this.index = index;
this.editPersonDescriptor = new EditPersonDescriptor(editPersonDescriptor);
targetIndex = index;
isFirstExecution = true;
personToEdit = null;
}

@Override
public CommandResult executeUndoableCommand() throws CommandException {
List<ReadOnlyPerson> lastShownList = model.getFilteredPersonList();

if (index.getZeroBased() >= lastShownList.size()) {
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
if (isFirstExecution) {
preprocessEditCommand();
isFirstExecution = false;
}

ReadOnlyPerson personToEdit = lastShownList.get(index.getZeroBased());
Person editedPerson = createEditedPerson(personToEdit, editPersonDescriptor);

try {
Expand Down Expand Up @@ -104,6 +109,21 @@ private static Person createEditedPerson(ReadOnlyPerson personToEdit,
return new Person(updatedName, updatedPhone, updatedEmail, updatedAddress, updatedTags);
}

/**
* Preprocess the command before its first execution by finding and storing
* the {@code personToEdit}
*/
private void preprocessEditCommand() throws CommandException {

List<ReadOnlyPerson> lastShownList = model.getFilteredPersonList();

if (targetIndex.getZeroBased() >= lastShownList.size()) {
throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
}

personToEdit = lastShownList.get(targetIndex.getZeroBased());
}

@Override
public boolean equals(Object other) {
// short circuit if same object
Expand All @@ -118,7 +138,7 @@ public boolean equals(Object other) {

// state check
EditCommand e = (EditCommand) other;
return index.equals(e.index)
return targetIndex.equals(e.targetIndex)
&& editPersonDescriptor.equals(e.editPersonDescriptor);
}

Expand Down
12 changes: 12 additions & 0 deletions src/test/java/systemtests/EditCommandSystemTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ public void edit() throws Exception {
/* ------------------ Performing edit operation while a filtered list is being shown ------------------------ */

/* Case: filtered person list, edit index within bounds of address book and person list -> edited */
model = getModel();
showPersonsWithName(KEYWORD_MATCHING_MEIER);
index = INDEX_FIRST_PERSON;
assertTrue(index.getZeroBased() < getModel().getFilteredPersonList().size());
Expand All @@ -109,6 +110,17 @@ public void edit() throws Exception {
editedPerson = new PersonBuilder(personToEdit).withName(VALID_NAME_BOB).build();
assertCommandSuccess(command, index, editedPerson);

/* Case: undo editing the first person in the filtered list -> first person in filtered list restored */
command = UndoCommand.COMMAND_WORD;
expectedResultMessage = UndoCommand.MESSAGE_SUCCESS;
assertCommandSuccess(command, model, expectedResultMessage);

/* Case: redo editing the first person in the filtered list -> first person in filtered list edited again */
command = RedoCommand.COMMAND_WORD;
expectedResultMessage = RedoCommand.MESSAGE_SUCCESS;
model.updatePerson(personToEdit, editedPerson);
assertCommandSuccess(command, model, expectedResultMessage);

/* Case: filtered person list, edit index within bounds of address book but out of bounds of person list
* -> rejected
*/
Expand Down

0 comments on commit 449969b

Please sign in to comment.