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

Allow edit command to delete optional fields #154

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion docs/UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,12 @@ Format: `edit INDEX [n/NAME] [p/PHONE] [e/EMAIL] [a/ADDRESS] [t/TAG]…​ [d/DA
* When editing tags, the existing tags of the person will be removed i.e adding of tags is not cumulative.
* You can remove all the person’s tags by typing `t/` without
specifying any tags after it.
* For optional fields (email, emergency contact, address, date of last visit) you can delete them by entering the prefix without specifying any value after.

Examples:
* `edit 1 p/91234567 e/[email protected]` Edits the phone number and email address of the 1st person to be `91234567` and `[email protected]` respectively.
* `edit 2 n/Betsy Crower t/` Edits the name of the 2nd person to be `Betsy Crower` and clears all existing tags.
* `edit 2 n/Betsy Crower t/ e/` Edits the name of the 2nd person to be `Betsy Crower`, clears all existing tags and deletes the stored email.


### Locating persons by name: `find`

Expand Down
12 changes: 8 additions & 4 deletions src/main/java/seedu/address/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ public static Phone parsePhone(String phone) throws ParseException {
public static Optional<Address> parseAddress(Optional<String> address) throws ParseException {
requireNonNull(address);

if (address.isEmpty()) {
if (address.isEmpty() || address.get().trim().isEmpty()) {
// if an address prefix was never entered by the user
// or an empty string ("") was entered indicating no address
return Optional.empty();
}

Expand All @@ -97,8 +98,9 @@ public static Optional<Address> parseAddress(Optional<String> address) throws Pa
public static Optional<Email> parseEmail(Optional<String> email) throws ParseException {
requireNonNull(email);

if (email.isEmpty()) {
if (email.isEmpty() || email.get().trim().isEmpty()) {
// if an email prefix was never entered by the user
// or an empty string ("") was entered indicating no email
return Optional.empty();
}
String trimmedEmail = email.get().trim();
Expand Down Expand Up @@ -144,8 +146,9 @@ public static Optional<DateOfLastVisit> parseDateOfLastVisit(Optional<String> da
throws ParseException {
requireNonNull(dateOfLastVisit);

if (dateOfLastVisit.isEmpty()) {
if (dateOfLastVisit.isEmpty() || dateOfLastVisit.get().trim().isEmpty()) {
// if dateOfLastVisit prefix was never entered by the user
// or an empty string ("") was entered indicating no date of last visit
return Optional.empty();
}

Expand All @@ -166,8 +169,9 @@ public static Optional<EmergencyContact> parseEmergencyContact(Optional<String>
throws ParseException {
requireNonNull(emergencyContact);

if (emergencyContact.isEmpty()) {
if (emergencyContact.isEmpty() || emergencyContact.get().trim().isEmpty()) {
// if emergencyContact prefix was never entered by the user
// or an empty string ("") was entered indicating no emergency contact
return Optional.empty();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ public class CommandTestUtil {
public static final String DATEOFLASTVISIT_DESC_BOB = " " + PREFIX_DATEOFLASTVISIT
+ VALID_DATEOFLASTVISIT_BOB;

public static final String ADDRESS_DELETION = " " + PREFIX_ADDRESS;
public static final String EMAIL_DELETION = " " + PREFIX_EMAIL;
public static final String EMERGENCY_CONTACT_DELETION = " " + PREFIX_EMERGENCY_CONTACT;
public static final String DATEOFLASTVISIT_DELETION = " " + PREFIX_DATEOFLASTVISIT;

// whitespace-only names are invalid
public static final String INVALID_NAME_DESC = " " + PREFIX_NAME + " ";
// 'a' not allowed in phones, emergency contacts
Expand Down
20 changes: 13 additions & 7 deletions src/test/java/seedu/address/logic/parser/AddCommandParserTest.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package seedu.address.logic.parser;

import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.DATEOFLASTVISIT_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.DATEOFLASTVISIT_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.DATEOFLASTVISIT_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.EMERGENCY_CONTACT_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.EMERGENCY_CONTACT_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.EMERGENCY_CONTACT_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_ADDRESS_DESC;
Expand Down Expand Up @@ -46,7 +50,6 @@

import seedu.address.logic.Messages;
import seedu.address.logic.commands.AddCommand;
import seedu.address.model.person.Address;
import seedu.address.model.person.DateOfLastVisit;
import seedu.address.model.person.Email;
import seedu.address.model.person.EmergencyContact;
Expand Down Expand Up @@ -226,6 +229,15 @@ public void parse_optionalFieldsMissing_success() {
.withTags().withEmergencyContact().build();
assertParseSuccess(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + DATEOFLASTVISIT_DESC_BOB,
new AddCommand(expectedPersonWithoutAddressOrEmergencyContact));

// Providing empty ("") email, address, dateoflastvisit, emergency contact
Person expectedPersonWithoutAnyOptionalFields = new PersonBuilder(BOB).withEmail().withAddress()
.withDateOfLastVisit().withEmergencyContact().withTags().build();
assertParseSuccess(
//Note DELETION refers to the empty string ex. EMAIL DELETION = "e/"
parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DELETION + EMERGENCY_CONTACT_DELETION
+ ADDRESS_DELETION + DATEOFLASTVISIT_DELETION,
new AddCommand(expectedPersonWithoutAnyOptionalFields));
}

@Test
Expand Down Expand Up @@ -274,12 +286,6 @@ public void parse_invalidValue_failure() {
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND + DATEOFLASTVISIT_DESC_BOB,
Email.MESSAGE_CONSTRAINTS);

// invalid address
assertParseFailure(
parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + INVALID_ADDRESS_DESC
+ TAG_DESC_HUSBAND + TAG_DESC_FRIEND + DATEOFLASTVISIT_DESC_BOB,
Address.MESSAGE_CONSTRAINTS);

// invalid tag
assertParseFailure(parser, NAME_DESC_BOB + PHONE_DESC_BOB + EMAIL_DESC_BOB + ADDRESS_DESC_BOB
+ INVALID_TAG_DESC + VALID_TAG_FRIEND + DATEOFLASTVISIT_DESC_BOB, Tag.MESSAGE_CONSTRAINTS);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package seedu.address.logic.parser;

import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.ADDRESS_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.DATEOFLASTVISIT_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.DATEOFLASTVISIT_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.DATEOFLASTVISIT_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.EMAIL_DESC_BOB;
import static seedu.address.logic.commands.CommandTestUtil.EMERGENCY_CONTACT_DELETION;
import static seedu.address.logic.commands.CommandTestUtil.EMERGENCY_CONTACT_DESC_AMY;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_ADDRESS_DESC;
import static seedu.address.logic.commands.CommandTestUtil.INVALID_DATEOFLASTVISIT_DESC;
Expand Down Expand Up @@ -41,13 +45,14 @@
import static seedu.address.testutil.TypicalIndexes.INDEX_SECOND_PERSON;
import static seedu.address.testutil.TypicalIndexes.INDEX_THIRD_PERSON;

import java.util.Optional;

import org.junit.jupiter.api.Test;

import seedu.address.commons.core.index.Index;
import seedu.address.logic.Messages;
import seedu.address.logic.commands.EditCommand;
import seedu.address.logic.commands.EditCommand.EditPersonDescriptor;
import seedu.address.model.person.Address;
import seedu.address.model.person.DateOfLastVisit;
import seedu.address.model.person.Email;
import seedu.address.model.person.EmergencyContact;
Expand Down Expand Up @@ -100,8 +105,6 @@ public void parse_invalidValue_failure() {
assertParseFailure(parser, "1" + INVALID_PHONE_DESC, Phone.MESSAGE_CONSTRAINTS);
// invalid email
assertParseFailure(parser, "1" + INVALID_EMAIL_DESC, Email.MESSAGE_CONSTRAINTS);
// invalid address
assertParseFailure(parser, "1" + INVALID_ADDRESS_DESC, Address.MESSAGE_CONSTRAINTS);
// invalid tag
assertParseFailure(parser, "1" + INVALID_TAG_DESC, Tag.MESSAGE_CONSTRAINTS);
// invalid dateOfLastVisit
Expand Down Expand Up @@ -204,6 +207,21 @@ public void parse_oneFieldSpecified_success() {
assertParseSuccess(parser, userInput, expectedCommand);
}

@Test
public void parse_optionalFieldDeletion_success() {
//One test case to cover all valid deletions
Index targetIndex = INDEX_THIRD_PERSON;
String userInput = targetIndex.getOneBased() + ADDRESS_DELETION
+ EMAIL_DELETION + DATEOFLASTVISIT_DELETION + EMERGENCY_CONTACT_DELETION;
EditPersonDescriptor descriptor = new EditPersonDescriptor();
descriptor.setAddress(Optional.empty());
descriptor.setEmail(Optional.empty());
descriptor.setDateOfLastVisit(Optional.empty());
descriptor.setEmergencyContact(Optional.empty());
EditCommand expectedCommand = new EditCommand(targetIndex, descriptor);
assertParseSuccess(parser, userInput, expectedCommand);
}

@Test
public void parse_multipleRepeatedFields_failure() {
// More extensive testing of duplicate parameter detections is done in
Expand Down
52 changes: 46 additions & 6 deletions src/test/java/seedu/address/logic/parser/ParserUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,19 @@
import seedu.address.model.tag.Tag;

public class ParserUtilTest {
private static final String EMPTY_ENTRY = " ";

private static final String INVALID_NAME = " \t \n ";
private static final String INVALID_PHONE = "+651234";
private static final String INVALID_SHORT_PHONE = "999";
private static final String INVALID_LONG_PHONE = "123456789";
private static final String INVALID_ADDRESS = " ";
private static final String INVALID_EMAIL = "example.com";
private static final String INVALID_EMERGENCY_CONTACT = INVALID_PHONE;
private static final String INVALID_TAG = "#friend";
private static final String INVALID_DATEOFLASTVISIT = "13/13/2024";

private static final String NO_ENTRY = "";

private static final String VALID_NAME = "Rachel Walker-Runner";
private static final String VALID_PHONE = "12345678";
private static final String VALID_ADDRESS = "123 Main Street #0505";
Expand Down Expand Up @@ -114,13 +117,18 @@ public void parsePhone_validValueWithWhitespace_returnsTrimmedPhone() throws Exc
}

@Test
public void parseAddress_null_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseAddress(Optional.ofNullable((String) null)));
public void parseAddress_emptyValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseAddress(Optional.of(EMPTY_ENTRY)));
}

@Test
public void parseAddress_invalidValue_throwsParseException() {
assertThrows(ParseException.class, () -> ParserUtil.parseAddress(Optional.of(INVALID_ADDRESS)));
public void parseAddress_noValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseAddress(Optional.of(NO_ENTRY)));
}

@Test
public void parseAddress_null_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseAddress(Optional.ofNullable((String) null)));
}

@Test
Expand All @@ -137,6 +145,17 @@ public void parseAddress_validValueWithWhitespace_returnsTrimmedAddress() throws
ParserUtil.parseAddress(Optional.of(addressWithWhitespace)));
}

@Test
public void parseEmail_emptyValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseEmail(Optional.of(EMPTY_ENTRY)));
}

@Test
public void parseEmail_noValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseEmail(Optional.of(NO_ENTRY)));
}


@Test
public void parseEmail_null_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseEmail(Optional.ofNullable((String) null)));
Expand Down Expand Up @@ -207,7 +226,17 @@ public void parseTags_collectionWithValidTags_returnsTagSet() throws Exception {
}

@Test
public void parseDateOfLastVisit_null_throwsNullPointerException() throws Exception {
public void parseDateOfLastVisit_emptyValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseDateOfLastVisit(Optional.of(EMPTY_ENTRY)));
}

@Test
public void parseDateOfLastVisit_noValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseDateOfLastVisit(Optional.of(NO_ENTRY)));
}

@Test
public void parseDateOfLastVisit_null_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseDateOfLastVisit(Optional.ofNullable((String) null)));
}

Expand All @@ -233,6 +262,17 @@ public void parseDateOfLastVisit_validValueWithWhitespace_returnsTrimmedDateOfLa
ParserUtil.parseDateOfLastVisit(Optional.of(dateOfLastVisitWithWhitespace)));
}

@Test
public void parseEmergencyContact_emptyValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseEmergencyContact(Optional.of(EMPTY_ENTRY)));
}

@Test
public void parseEmergencyContact_noValue_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseEmergencyContact(Optional.of(NO_ENTRY)));
}


@Test
public void parseEmergencyContact_null_optionalEmpty() throws Exception {
assertEquals(Optional.empty(), ParserUtil.parseEmergencyContact(Optional.ofNullable((String) null)));
Expand Down