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.

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

[W5][W13-4]Chua Eng Soon #148

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
20 changes: 20 additions & 0 deletions docs/UserGuide.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,26 @@ Examples:
* `add John Doe p/98765432 e/[email protected] a/John street, block 123, #01-01`
* `add Betsy Crowe pp/1234567 e/[email protected] pa/Newgate Prison t/criminal t/friend`

== Updates a person's info: `update`

Updates a person in the address book. +
Format: `update NAME [p]p/PHONE_NUMBER [p]e/EMAIL [p]a/ADDRESS [t/TAG]...`

****
Words in `UPPER_CASE` are the parameters, items in `SQUARE_BRACKETS` are optional,
items with `...` after them can have multiple instances. Order of parameters are fixed.

Put a `p` before the phone / email / address prefixes to mark it as `private`. `private` details can only
be seen using the `viewall` command.

Persons can have any number of tags (including 0).
****

Examples:

* `update John Doe p/98765432 e/[email protected] a/John street, block 123, #01-01`
* `update Betsy Crowe pp/1234567 e/[email protected] pa/Newgate Prison t/criminal t/friend`

== Listing all persons : `list`

Shows a list of all persons, along with their non-private details, in the address book. +
Expand Down
1 change: 1 addition & 0 deletions src/seedu/addressbook/commands/HelpCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class HelpCommand extends Command {
public CommandResult execute() {
return new CommandResult(
AddCommand.MESSAGE_USAGE
+ "\n" + UpdateCommand.MESSAGE_USAGE
+ "\n" + DeleteCommand.MESSAGE_USAGE
+ "\n" + ClearCommand.MESSAGE_USAGE
+ "\n" + FindCommand.MESSAGE_USAGE
Expand Down
78 changes: 78 additions & 0 deletions src/seedu/addressbook/commands/UpdateCommand.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package seedu.addressbook.commands;

import java.util.HashSet;
import java.util.Set;

import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.Address;
import seedu.addressbook.data.person.Email;
import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.Phone;
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.data.person.UniquePersonList;
import seedu.addressbook.data.tag.Tag;

/**
* Updates existing person to the address book.
*/
public class UpdateCommand extends Command {

public static final String COMMAND_WORD = "update";

public static final String MESSAGE_USAGE = COMMAND_WORD + ": Updates existing person to the address book. "
+ "Contact details can be marked private by prepending 'p' to the prefix.\n"
+ "Parameters: NAME [p]p/PHONE [p]e/EMAIL [p]a/ADDRESS [t/TAG]...\n"
+ "Example: " + COMMAND_WORD
+ " John Doe p/98765432 e/[email protected] a/311, Clementi Ave 2, #02-25 t/friends t/owesMoney";

public static final String MESSAGE_SUCCESS = "Person updated: %1$s";
public static final String MESSAGE_NO_PERSON = "This person does not exist in the address book";

private final Person toUpdate;
private final String NAME;

/**
* Convenience constructor using raw values.
*
* @throws IllegalValueException if any of the raw values are invalid
*/
public UpdateCommand(String name,
String phone, boolean isPhonePrivate,
String email, boolean isEmailPrivate,
String address, boolean isAddressPrivate,
Set<String> tags) throws IllegalValueException {

Choose a reason for hiding this comment

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

Indentation is a bit strange here - either align the parameters vertically or keep everything to the left one indent in from the start of the declaration, maybe.

final Set<Tag> tagSet = new HashSet<>();
for (String tagName : tags) {
tagSet.add(new Tag(tagName));
}
this.NAME = name;
this.toUpdate = new Person(
new Name(name),
new Phone(phone, isPhonePrivate),
new Email(email, isEmailPrivate),
new Address(address, isAddressPrivate),
tagSet

Choose a reason for hiding this comment

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

Indent 4 spaces each time?

);
}

public UpdateCommand(Person toUpdate) {
this.toUpdate = toUpdate;
this.NAME = toUpdate.getName().toString();
}

public ReadOnlyPerson getPerson() {
return toUpdate;
}

@Override
public CommandResult execute() {
try {
addressBook.updatePerson(toUpdate);
return new CommandResult(String.format(MESSAGE_SUCCESS, toUpdate));
} catch (UniquePersonList.PersonNotFoundException npe) {
return new CommandResult(MESSAGE_NO_PERSON);
}
}

}
9 changes: 9 additions & 0 deletions src/seedu/addressbook/data/AddressBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ public void addPerson(Person toAdd) throws DuplicatePersonException {
allPersons.add(toAdd);
}

/**
* Updates a person to the address book.
*
* @throws PersonNotFoundException if the person does not exist.
*/
public void updatePerson(Person toUpdate) throws PersonNotFoundException {
allPersons.update(toUpdate);
}

/**
* Returns true if an equivalent person exists in the address book.
*/
Expand Down
18 changes: 18 additions & 0 deletions src/seedu/addressbook/data/person/UniquePersonList.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,24 @@ public void add(Person toAdd) throws DuplicatePersonException {
internalList.add(toAdd);
}

/**
* Updates a person in the list.
*
* @throws PersonNotFoundException if the person to update does not exist in the list.
*/
public void update(Person toUpdate) throws PersonNotFoundException {
if (contains(toUpdate) == false) {
throw new PersonNotFoundException();
} else {
for (Person p : internalList) {
if (p.isSamePerson(toUpdate)) {
internalList.remove(p);
}
}
internalList.add(toUpdate);
}
}

/**
* Removes the equivalent person from the list.
*
Expand Down
35 changes: 35 additions & 0 deletions src/seedu/addressbook/parser/Parser.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import java.util.regex.Pattern;

import seedu.addressbook.commands.AddCommand;
import seedu.addressbook.commands.UpdateCommand;
import seedu.addressbook.commands.ClearCommand;
import seedu.addressbook.commands.Command;
import seedu.addressbook.commands.DeleteCommand;
Expand All @@ -22,6 +23,7 @@
import seedu.addressbook.commands.ListCommand;
import seedu.addressbook.commands.ViewAllCommand;
import seedu.addressbook.commands.ViewCommand;

import seedu.addressbook.data.exception.IllegalValueException;

/**
Expand Down Expand Up @@ -76,6 +78,9 @@ public Command parseCommand(String userInput) {
case AddCommand.COMMAND_WORD:
return prepareAdd(arguments);

case UpdateCommand.COMMAND_WORD:
return prepareUpdate(arguments);

case DeleteCommand.COMMAND_WORD:
return prepareDelete(arguments);

Expand Down Expand Up @@ -134,7 +139,37 @@ private Command prepareAdd(String args) {
return new IncorrectCommand(ive.getMessage());
}
}
/**
* Parses arguments in the context of the update person command.
*
* @param args full command args string
* @return the prepared command
*/
private Command prepareUpdate(String args) {
final Matcher matcher = PERSON_DATA_ARGS_FORMAT.matcher(args.trim());
// Validate arg string format
if (!matcher.matches()) {
return new IncorrectCommand(String.format(MESSAGE_INVALID_COMMAND_FORMAT, UpdateCommand.MESSAGE_USAGE));
}
try {
return new UpdateCommand(
matcher.group("name"),

matcher.group("phone"),
isPrivatePrefixPresent(matcher.group("isPhonePrivate")),

matcher.group("email"),
isPrivatePrefixPresent(matcher.group("isEmailPrivate")),

matcher.group("address"),
isPrivatePrefixPresent(matcher.group("isAddressPrivate")),

getTagsFromArgs(matcher.group("tagArguments"))
);
} catch (IllegalValueException ive) {
return new IncorrectCommand(ive.getMessage());
}
}
/**
* Returns true if the private prefix is present for a contact detail in the add command's arguments string.
*/
Expand Down
116 changes: 116 additions & 0 deletions test/java/seedu/addressbook/commands/UpdateCommandTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package seedu.addressbook.commands;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import org.junit.Test;

import seedu.addressbook.data.AddressBook;
import seedu.addressbook.data.exception.IllegalValueException;
import seedu.addressbook.data.person.Address;
import seedu.addressbook.data.person.Email;
import seedu.addressbook.data.person.Name;
import seedu.addressbook.data.person.Person;
import seedu.addressbook.data.person.Phone;
import seedu.addressbook.data.person.ReadOnlyPerson;
import seedu.addressbook.data.person.UniquePersonList;
import seedu.addressbook.util.TestUtil;

public class UpdateCommandTest {
private static final List<ReadOnlyPerson> EMPTY_PERSON_LIST = Collections.emptyList();
private static final Set<String> EMPTY_STRING_SET = Collections.emptySet();

@Test
public void updateCommand_invalidName_throwsException() {
final String[] invalidNames = {"", " ", "[]\\[;]"};
for (String name : invalidNames) {
assertConstructingInvalidUpdateCmdThrowsException(name, Phone.EXAMPLE, true, Email.EXAMPLE, false,
Address.EXAMPLE, true, EMPTY_STRING_SET);
}
}

@Test
public void updateCommand_invalidPhone_throwsException() {
final String[] invalidNumbers = {"", " ", "1234-5678", "[]\\[;]", "abc", "a123", "+651234"};
for (String number : invalidNumbers) {
assertConstructingInvalidUpdateCmdThrowsException(Name.EXAMPLE, number, false, Email.EXAMPLE, true,
Address.EXAMPLE, false, EMPTY_STRING_SET);
}
}

@Test
public void updateCommand_invalidEmail_throwsException() {
final String[] invalidEmails = {"", " ", "def.com", "@", "@def", "@def.com", "abc@",
"@invalid@email", "invalid@email!", "!invalid@email"};
for (String email : invalidEmails) {
assertConstructingInvalidUpdateCmdThrowsException(Name.EXAMPLE, Phone.EXAMPLE, false, email, false,
Address.EXAMPLE, false, EMPTY_STRING_SET);
}
}

@Test
public void updateCommand_invalidAddress_throwsException() {
final String[] invalidAddresses = {"", " "};
for (String address : invalidAddresses) {
assertConstructingInvalidUpdateCmdThrowsException(Name.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE,
true, address, true, EMPTY_STRING_SET);
}
}

@Test
public void updateCommand_invalidTags_throwsException() {
final String[][] invalidTags = {{""}, {" "}, {"'"}, {"[]\\[;]"}, {"validTag", ""},
{"", " "}};
for (String[] tags : invalidTags) {
Set<String> tagsToAdd = new HashSet<>(Arrays.asList(tags));
assertConstructingInvalidUpdateCmdThrowsException(Name.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE,
true, Address.EXAMPLE, false, tagsToAdd);
}
}

/**
* Asserts that attempting to construct an update command with the supplied
* invalid data throws an IllegalValueException
*/
private void assertConstructingInvalidUpdateCmdThrowsException(String name, String phone,
boolean isPhonePrivate, String email, boolean isEmailPrivate, String address,
boolean isAddressPrivate, Set<String> tags) {
try {
new AddCommand(name, phone, isPhonePrivate, email, isEmailPrivate, address, isAddressPrivate,
tags);
} catch (IllegalValueException e) {
return;
}
String error = String.format(
"An add command was successfully constructed with invalid input: %s %s %s %s %s %s %s %s",
name, phone, isPhonePrivate, email, isEmailPrivate, address, isAddressPrivate, tags);
fail(error);
}

@Test
public void updateCommand_validData_correctlyConstructed() throws Exception {
UpdateCommand command = new UpdateCommand(Name.EXAMPLE, Phone.EXAMPLE, true, Email.EXAMPLE, false,
Address.EXAMPLE, true, EMPTY_STRING_SET);
ReadOnlyPerson p = command.getPerson();

// TODO: add comparison of tags to person.equals and equality methods to
// individual fields that compare privacy to simplify this
assertEquals(Name.EXAMPLE, p.getName().fullName);
assertEquals(Phone.EXAMPLE, p.getPhone().value);
assertTrue(p.getPhone().isPrivate());
assertEquals(Email.EXAMPLE, p.getEmail().value);
assertFalse(p.getEmail().isPrivate());
assertEquals(Address.EXAMPLE, p.getAddress().value);
assertTrue(p.getAddress().isPrivate());
boolean isTagListEmpty = !p.getTags().iterator().hasNext();
assertTrue(isTagListEmpty);
}
}