From 120e4ce76028420169623670c1674b85d8e90659 Mon Sep 17 00:00:00 2001 From: Tang Yetong Date: Mon, 25 Mar 2024 00:52:34 +0800 Subject: [PATCH 1/7] Add Copy Command Currently does not support copy to clipboard functionality Copy to clipboard functionality added alongside relevant tests --- .../java/seedu/address/logic/Messages.java | 2 + .../address/logic/commands/CommandType.java | 6 + .../address/logic/commands/CopyCommand.java | 180 ++++++++++++++++++ .../java/seedu/address/ui/MainWindow.java | 20 +- .../logic/commands/CopyCommandTest.java | 121 ++++++++++++ 5 files changed, 327 insertions(+), 2 deletions(-) create mode 100644 src/main/java/seedu/address/logic/commands/CopyCommand.java create mode 100644 src/test/java/seedu/address/logic/commands/CopyCommandTest.java diff --git a/src/main/java/seedu/address/logic/Messages.java b/src/main/java/seedu/address/logic/Messages.java index 7cfb254650a..638569c684d 100644 --- a/src/main/java/seedu/address/logic/Messages.java +++ b/src/main/java/seedu/address/logic/Messages.java @@ -21,6 +21,8 @@ public class Messages { public static final String MESSAGE_SHOWING_HELP = "Opened help window."; public static final String MESSAGE_EXITING = "Exiting Address Book as requested ..."; + public static final String MESSAGE_COPIED = "Copied details to clipboard: %1$s"; + public static final int MESSAGE_COPIED_LEN = "Copied details to clipboard:".length(); /** * Returns an error message indicating the duplicate prefixes. diff --git a/src/main/java/seedu/address/logic/commands/CommandType.java b/src/main/java/seedu/address/logic/commands/CommandType.java index e52e6955dec..ce6b34d650d 100644 --- a/src/main/java/seedu/address/logic/commands/CommandType.java +++ b/src/main/java/seedu/address/logic/commands/CommandType.java @@ -11,6 +11,12 @@ public Command createCommand(String arguments) throws IllegalArgumentException { return AddCommand.of(arguments); } }, + COPY { + @Override + public Command createCommand(String arguments) throws IllegalArgumentException { + return CopyCommand.of(arguments); + } + }, EDIT { @Override public Command createCommand(String arguments) throws IllegalArgumentException { diff --git a/src/main/java/seedu/address/logic/commands/CopyCommand.java b/src/main/java/seedu/address/logic/commands/CopyCommand.java new file mode 100644 index 00000000000..ff4e16cea21 --- /dev/null +++ b/src/main/java/seedu/address/logic/commands/CopyCommand.java @@ -0,0 +1,180 @@ +package seedu.address.logic.commands; + +import static java.util.Objects.requireNonNull; +import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; +import static seedu.address.model.person.fields.Address.PREFIX_ADDRESS; +import static seedu.address.model.person.fields.Assets.PREFIX_ASSET; +import static seedu.address.model.person.fields.Email.PREFIX_EMAIL; +import static seedu.address.model.person.fields.Name.PREFIX_NAME; +import static seedu.address.model.person.fields.Phone.PREFIX_PHONE; +import static seedu.address.model.person.fields.Tags.PREFIX_TAG; + +import java.util.List; + +import seedu.address.commons.core.index.Index; +import seedu.address.commons.util.ToStringBuilder; +import seedu.address.logic.Messages; +import seedu.address.logic.commands.exceptions.CommandException; +import seedu.address.logic.util.ArgumentMultimap; +import seedu.address.logic.util.ArgumentTokenizer; +import seedu.address.logic.util.ParserUtil; +import seedu.address.model.Model; +import seedu.address.model.person.Person; + +/** + * Copies the details of a person in the address book. + */ +public class CopyCommand extends Command { + + public static final String COMMAND_WORD = "copy"; + + public static final String MESSAGE_USAGE = COMMAND_WORD + ": Copies the details of the prefix identified\n" + + "Parameters: " + + "[" + PREFIX_NAME + "] " + + "[" + PREFIX_PHONE + "] " + + "[" + PREFIX_EMAIL + "] " + + "[" + PREFIX_ADDRESS + "] " + + "[" + PREFIX_TAG + "] " + + "[" + PREFIX_ASSET + "]\n" + + "Example: " + COMMAND_WORD + " 1 " + + PREFIX_NAME + " " + + PREFIX_PHONE + " " + + PREFIX_TAG + " " + + PREFIX_ASSET + " "; + + public static final String MESSAGE_NO_PARAM = "At least one field to copy must be provided."; + + private final Index index; + private final boolean[] info; + + /** + * @param index of the person in the filtered person list to copy + * @param info list of details to copy to clipboard + */ + public CopyCommand(Index index, boolean[] info) { + requireNonNull(index); + + this.index = index; + this.info = info; + } + + @Override + public String execute(Model model) throws CommandException { + requireNonNull(model); + List lastShownList = model.getFilteredPersonList(); + + if (index.getZeroBased() >= lastShownList.size()) { + throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); + } + + Person personToCopy = lastShownList.get(index.getZeroBased()); + String personDetails = copyToClipboard(personToCopy, info); + + return String.format(Messages.MESSAGE_COPIED, personDetails); + } + + /** + * Copies the details of {@code personToCopy} to clipboard + */ + private static String copyToClipboard(Person personToCopy, boolean[] info) { + assert personToCopy != null; + + StringBuilder copiedMsg = new StringBuilder(); + + if (info[0]) { + copiedMsg.append("Name: ").append(personToCopy.getName()).append("; "); + } + if (info[1]) { + copiedMsg.append("Phone: ").append(personToCopy.getPhone()).append("; "); + } + if (info[2]) { + copiedMsg.append("Email: ").append(personToCopy.getEmail()).append("; "); + } + if (info[3]) { + copiedMsg.append("Address: ").append(personToCopy.getAddress()).append("; "); + } + if (info[4]) { + copiedMsg.append("Tags: ").append(personToCopy.getTags()).append("; "); + } + if (info[5]) { + copiedMsg.append("Assets: ").append(personToCopy.getAssets()).append("; "); + } + + return copiedMsg.substring(0, copiedMsg.length() - 2); + } + + /** + * Parses the given {@code String} of arguments in the context of the CopyCommand + * and returns an CopyCommand object for execution. + * @throws IllegalArgumentException if the user input does not conform the expected format + */ + public static CopyCommand of(String args) throws IllegalArgumentException { + System.out.println(args); + requireNonNull(args); + ArgumentMultimap argMultimap = + ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, + PREFIX_ADDRESS, PREFIX_TAG, PREFIX_ASSET); + + Index index; + + try { + index = ParserUtil.parseIndex(argMultimap.getPreamble()); + } catch (IllegalArgumentException ie) { + throw new IllegalArgumentException( + String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE), ie); + } + + argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS); + + boolean[] info = new boolean[6]; + + if (argMultimap.getValue(PREFIX_NAME).isPresent()) { + info[0] = true; + } + if (argMultimap.getValue(PREFIX_PHONE).isPresent()) { + info[1] = true; + } + if (argMultimap.getValue(PREFIX_EMAIL).isPresent()) { + info[2] = true; + } + if (argMultimap.getValue(PREFIX_ADDRESS).isPresent()) { + info[3] = true; + } + if (argMultimap.getValue(PREFIX_TAG).isPresent()) { + info[4] = true; + } + if (argMultimap.getValue(PREFIX_ASSET).isPresent()) { + info[5] = true; + } + + for (boolean b: info) { + if (b) { + return new CopyCommand(index, info); + } + } + + throw new IllegalArgumentException(MESSAGE_NO_PARAM); + } + + @Override + public boolean equals(Object other) { + if (other == this) { + return true; + } + + // instanceof handles nulls + if (!(other instanceof CopyCommand)) { + return false; + } + + CopyCommand otherCopyCommand = (CopyCommand) other; + return index.equals(otherCopyCommand.index); + } + + @Override + public String toString() { + return new ToStringBuilder(this) + .add("index", index) + .toString(); + } +} diff --git a/src/main/java/seedu/address/ui/MainWindow.java b/src/main/java/seedu/address/ui/MainWindow.java index 5bfa9f2e54b..b4e62635029 100644 --- a/src/main/java/seedu/address/ui/MainWindow.java +++ b/src/main/java/seedu/address/ui/MainWindow.java @@ -6,6 +6,8 @@ import javafx.fxml.FXML; import javafx.scene.control.MenuItem; import javafx.scene.control.TextInputControl; +import javafx.scene.input.Clipboard; +import javafx.scene.input.ClipboardContent; import javafx.scene.input.KeyCombination; import javafx.scene.input.KeyEvent; import javafx.scene.layout.StackPane; @@ -164,6 +166,17 @@ private void handleExit() { primaryStage.hide(); } + /** + * Copies string to the clipboard + */ + @FXML + private void handleCopy(String detailsToCopy) { + Clipboard clipboard = Clipboard.getSystemClipboard(); + ClipboardContent content = new ClipboardContent(); + content.putString(detailsToCopy); + clipboard.setContent(content); + } + public PersonListPanel getPersonListPanel() { return personListPanel; } @@ -185,12 +198,15 @@ private String executeCommand(String commandText) throws CommandException, Parse throw e; } - if (commandResult == Messages.MESSAGE_SHOWING_HELP) { + if (commandResult.equals(Messages.MESSAGE_SHOWING_HELP)) { handleHelp(); } - if (commandResult == Messages.MESSAGE_EXITING) { + if (commandResult.equals(Messages.MESSAGE_EXITING)) { handleExit(); } + if (commandResult.startsWith(Messages.MESSAGE_COPIED.substring(0, Messages.MESSAGE_COPIED_LEN + 1))) { + handleCopy(commandResult.substring(Messages.MESSAGE_COPIED_LEN).trim()); + } return commandResult; } } diff --git a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java new file mode 100644 index 00000000000..c56bf44f756 --- /dev/null +++ b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java @@ -0,0 +1,121 @@ +package seedu.address.logic.commands; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; +import static seedu.address.logic.commands.CommandTestUtil.assertParseFailure; +import static seedu.address.logic.commands.CopyCommand.MESSAGE_USAGE; +import static seedu.address.testutil.Assert.assertThrows; + +import org.junit.jupiter.api.Test; + +import seedu.address.commons.core.index.Index; +import seedu.address.logic.Messages; +import seedu.address.logic.commands.exceptions.CommandException; +import seedu.address.model.Model; +import seedu.address.model.ModelManager; +import seedu.address.model.person.Person; +import seedu.address.testutil.PersonBuilder; + +/** + * Contains integration tests (interaction with the Model) and unit tests for CopyCommand. + */ +public class CopyCommandTest { + + private static final String VALID_INDEX = "1"; + private static final String INVALID_INDEX = "a"; + private final Model model = new ModelManager(); + + @Test + public void execute_validCopy1_success() throws CommandException { + Person personToCopy = new PersonBuilder().withTags("Bob").withAssets("Aircon").build(); + model.addPerson(personToCopy); + Index index = Index.fromOneBased(1); + boolean[] info = {true, true, true, true, true, true}; + CopyCommand copyCommand = new CopyCommand(index, info); + + String copiedDetails = "Name: Amy Bee; " + + "Phone: 85355255; " + + "Email: amy@gmail.com; " + + "Address: 123, Jurong West Ave 6, #08-111; " + + "Tags: [[Bob]]; " + + "Assets: [[Aircon]]"; + + String expectedMessage = String.format(Messages.MESSAGE_COPIED, copiedDetails); + assertEquals(expectedMessage, copyCommand.execute(model)); + } + + @Test + public void execute_validCopy2_success() throws CommandException { + Person personToCopy = new PersonBuilder().withTags("Bob").withAssets("Aircon").build(); + model.addPerson(personToCopy); + Index index = Index.fromOneBased(1); + boolean[] info = {false, true, false, true, true, true}; + CopyCommand copyCommand = new CopyCommand(index, info); + + String copiedDetails = "Phone: 85355255; " + + "Address: 123, Jurong West Ave 6, #08-111; " + + "Tags: [[Bob]]; " + + "Assets: [[Aircon]]"; + + String expectedMessage = String.format(Messages.MESSAGE_COPIED, copiedDetails); + assertEquals(expectedMessage, copyCommand.execute(model)); + } + + @Test + public void execute_invalidIndex_throwsCommandException() { + Index index = Index.fromOneBased(2); + boolean[] info = {true, true, true, true, true, true}; + CopyCommand copyCommand = new CopyCommand(index, info); + assertThrows(CommandException.class, () -> copyCommand.execute(model)); + } + + @Test + public void execute_noParam_throwsIllegalArgumentException() { + Index index = Index.fromOneBased(1); + boolean[] info = {false, false, false, false, false, false}; + CopyCommand copyCommand = new CopyCommand(index, info); + assertThrows(CommandException.class, () -> copyCommand.execute(model)); + } + + @Test + public void of_invalidInput_failure() { + assertParseFailure(CopyCommand::of, "Invalid", String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE)); + assertParseFailure(CopyCommand::of, "", String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE)); + assertThrows(IllegalArgumentException.class, () -> CopyCommand.of("1")); + assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(INVALID_INDEX)); + } + + @Test + public void of_validInput_success() { + assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(VALID_INDEX + " ")); + } + + @Test + public void equals_sameValues_returnsTrue() { + Index index = Index.fromOneBased(1); + boolean[] info = {true, true, true, true, true, true}; + CopyCommand copyCommand1 = new CopyCommand(index, info); + CopyCommand copyCommand2 = new CopyCommand(index, info); + assertEquals(copyCommand1, copyCommand2); + } + + @Test + public void equals_differentValues_returnsFalse() { + Index index1 = Index.fromOneBased(1); + Index index2 = Index.fromOneBased(2); + boolean[] info1 = {true, true, true, true, true, true}; + boolean[] info2 = {true, true, true, true, true, false}; + CopyCommand copyCommand1 = new CopyCommand(index1, info1); + CopyCommand copyCommand2 = new CopyCommand(index2, info2); + assertNotEquals(copyCommand1, copyCommand2); + } + + @Test + public void equals_differentObject_returnsFalse() { + Index index = Index.fromOneBased(1); + boolean[] info = {true, true, true, true, true, true}; + CopyCommand copyCommand = new CopyCommand(index, info); + assertNotEquals(copyCommand, new Object()); + } +} From 2aa6c7f97c43b15dc2117948e9ed7ea6659a0bdf Mon Sep 17 00:00:00 2001 From: Tang Yetong Date: Mon, 25 Mar 2024 01:07:06 +0800 Subject: [PATCH 2/7] Add few more tests to commandutil --- .../seedu/address/logic/util/AddressBookParserTest.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/java/seedu/address/logic/util/AddressBookParserTest.java b/src/test/java/seedu/address/logic/util/AddressBookParserTest.java index 195aafe9893..78de30f61da 100644 --- a/src/test/java/seedu/address/logic/util/AddressBookParserTest.java +++ b/src/test/java/seedu/address/logic/util/AddressBookParserTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; import static seedu.address.logic.Messages.MESSAGE_UNKNOWN_COMMAND; +import static seedu.address.model.person.fields.Name.PREFIX_NAME; import static seedu.address.testutil.Assert.assertThrows; import static seedu.address.testutil.TypicalIndexes.INDEX_FIRST_PERSON; @@ -15,6 +16,7 @@ import seedu.address.logic.commands.AddCommand; import seedu.address.logic.commands.ClearCommand; +import seedu.address.logic.commands.CopyCommand; import seedu.address.logic.commands.DeleteCommand; import seedu.address.logic.commands.EditCommand; import seedu.address.logic.commands.EditCommand.EditPersonDescriptor; @@ -86,6 +88,12 @@ public void parseCommand_list() throws Exception { assertTrue(AddressBookParser.parseCommand(ListCommand.COMMAND_WORD + " 3") instanceof ListCommand); } + @Test + public void parseCommand_copy() throws Exception { + assertTrue(AddressBookParser.parseCommand(CopyCommand.COMMAND_WORD + " 1 " + + PREFIX_NAME) instanceof CopyCommand); + } + @Test public void parseCommand_unrecognisedInput_throwsParseException() { assertThrows(ParseException.class, String.format(MESSAGE_INVALID_COMMAND_FORMAT, HelpCommand.MESSAGE_USAGE), () From 08172ca76f2e7c9ef811799da88613a85b6386b3 Mon Sep 17 00:00:00 2001 From: Tang Yetong Date: Tue, 26 Mar 2024 19:13:02 +0800 Subject: [PATCH 3/7] Update code to review Code currently does not conform to certain conventions required Unused print statements removed and defensiveness of code maintained in new logic added --- .../address/logic/commands/CopyCommand.java | 5 ++- .../java/seedu/address/ui/MainWindow.java | 6 ++-- .../logic/commands/CopyCommandTest.java | 33 ++++++++++++++++++- 3 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/CopyCommand.java b/src/main/java/seedu/address/logic/commands/CopyCommand.java index ff4e16cea21..62077c2870b 100644 --- a/src/main/java/seedu/address/logic/commands/CopyCommand.java +++ b/src/main/java/seedu/address/logic/commands/CopyCommand.java @@ -77,7 +77,7 @@ public String execute(Model model) throws CommandException { * Copies the details of {@code personToCopy} to clipboard */ private static String copyToClipboard(Person personToCopy, boolean[] info) { - assert personToCopy != null; + requireNonNull(personToCopy); StringBuilder copiedMsg = new StringBuilder(); @@ -109,10 +109,9 @@ private static String copyToClipboard(Person personToCopy, boolean[] info) { * @throws IllegalArgumentException if the user input does not conform the expected format */ public static CopyCommand of(String args) throws IllegalArgumentException { - System.out.println(args); requireNonNull(args); ArgumentMultimap argMultimap = - ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, + ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_TAG, PREFIX_ASSET); Index index; diff --git a/src/main/java/seedu/address/ui/MainWindow.java b/src/main/java/seedu/address/ui/MainWindow.java index b4e62635029..bd29856e1d9 100644 --- a/src/main/java/seedu/address/ui/MainWindow.java +++ b/src/main/java/seedu/address/ui/MainWindow.java @@ -198,10 +198,12 @@ private String executeCommand(String commandText) throws CommandException, Parse throw e; } - if (commandResult.equals(Messages.MESSAGE_SHOWING_HELP)) { + // == used to prevent an edge case where a command may somehow return this exact string, + // but is not actually a help or exit command. + if (commandResult == Messages.MESSAGE_SHOWING_HELP) { handleHelp(); } - if (commandResult.equals(Messages.MESSAGE_EXITING)) { + if (commandResult == Messages.MESSAGE_EXITING) { handleExit(); } if (commandResult.startsWith(Messages.MESSAGE_COPIED.substring(0, Messages.MESSAGE_COPIED_LEN + 1))) { diff --git a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java index c56bf44f756..6324c5a2e9b 100644 --- a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java @@ -1,5 +1,6 @@ package seedu.address.logic.commands; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT; @@ -84,11 +85,41 @@ public void of_invalidInput_failure() { assertParseFailure(CopyCommand::of, "", String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE)); assertThrows(IllegalArgumentException.class, () -> CopyCommand.of("1")); assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(INVALID_INDEX)); + assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(VALID_INDEX + " ")); } @Test public void of_validInput_success() { - assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(VALID_INDEX + " ")); + assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " n/")); + assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " t/ A/")); + assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " p/ e/ t/ a/ A/")); + } + + @Test + public void equals_sameObject_returnsTrue() { + Index index = Index.fromOneBased(1); + boolean[] info = {true, true, true, true, true, true}; + CopyCommand copyCommand = new CopyCommand(index, info); + + assertEquals(copyCommand, copyCommand); + } + + @Test + public void equals_nullObject_returnsFalse() { + Index index = Index.fromOneBased(1); + boolean[] info = {true, true, true, true, true, true}; + CopyCommand copyCommand = new CopyCommand(index, info); + + assertNotEquals(copyCommand, null); + } + + @Test + public void equals_differentClass_returnsFalse() { + Index index = Index.fromOneBased(1); + boolean[] info = {true, true, true, true, true, true}; + CopyCommand copyCommand = new CopyCommand(index, info); + + assertNotEquals(copyCommand, "Not a CopyCommand object"); } @Test From dc94d9e346f55582e6622915ce193197244d1546 Mon Sep 17 00:00:00 2001 From: Yetong Date: Wed, 27 Mar 2024 12:20:42 +0800 Subject: [PATCH 4/7] Refactor to use default values CopyCommandTest currently does not use default values in personbuilder Default values in PersonBuilder used in test, and refactor for new line spacing --- .../address/logic/commands/CopyCommand.java | 3 +- .../logic/commands/CopyCommandTest.java | 32 ++++++++++++------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/CopyCommand.java b/src/main/java/seedu/address/logic/commands/CopyCommand.java index 62077c2870b..5edc4561734 100644 --- a/src/main/java/seedu/address/logic/commands/CopyCommand.java +++ b/src/main/java/seedu/address/logic/commands/CopyCommand.java @@ -119,8 +119,7 @@ public static CopyCommand of(String args) throws IllegalArgumentException { try { index = ParserUtil.parseIndex(argMultimap.getPreamble()); } catch (IllegalArgumentException ie) { - throw new IllegalArgumentException( - String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE), ie); + throw new IllegalArgumentException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE), ie); } argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS); diff --git a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java index 6324c5a2e9b..20408c2e9c8 100644 --- a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java @@ -7,6 +7,12 @@ import static seedu.address.logic.commands.CommandTestUtil.assertParseFailure; import static seedu.address.logic.commands.CopyCommand.MESSAGE_USAGE; import static seedu.address.testutil.Assert.assertThrows; +import static seedu.address.testutil.PersonBuilder.DEFAULT_ADDRESS; +import static seedu.address.testutil.PersonBuilder.DEFAULT_ASSETS; +import static seedu.address.testutil.PersonBuilder.DEFAULT_EMAIL; +import static seedu.address.testutil.PersonBuilder.DEFAULT_NAME; +import static seedu.address.testutil.PersonBuilder.DEFAULT_PHONE; +import static seedu.address.testutil.PersonBuilder.DEFAULT_TAGS; import org.junit.jupiter.api.Test; @@ -16,6 +22,8 @@ import seedu.address.model.Model; import seedu.address.model.ModelManager; import seedu.address.model.person.Person; +import seedu.address.model.person.fields.Assets; +import seedu.address.model.person.fields.Tags; import seedu.address.testutil.PersonBuilder; /** @@ -29,18 +37,18 @@ public class CopyCommandTest { @Test public void execute_validCopy1_success() throws CommandException { - Person personToCopy = new PersonBuilder().withTags("Bob").withAssets("Aircon").build(); + Person personToCopy = new PersonBuilder().build(); model.addPerson(personToCopy); Index index = Index.fromOneBased(1); boolean[] info = {true, true, true, true, true, true}; CopyCommand copyCommand = new CopyCommand(index, info); - String copiedDetails = "Name: Amy Bee; " - + "Phone: 85355255; " - + "Email: amy@gmail.com; " - + "Address: 123, Jurong West Ave 6, #08-111; " - + "Tags: [[Bob]]; " - + "Assets: [[Aircon]]"; + String copiedDetails = "Name: " + DEFAULT_NAME + "; " + + "Phone: " + DEFAULT_PHONE + "; " + + "Email: " + DEFAULT_EMAIL + "; " + + "Address: " + DEFAULT_ADDRESS + "; " + + "Tags: " + new Tags(DEFAULT_TAGS) + "; " + + "Assets: " + new Assets(DEFAULT_ASSETS); String expectedMessage = String.format(Messages.MESSAGE_COPIED, copiedDetails); assertEquals(expectedMessage, copyCommand.execute(model)); @@ -48,16 +56,16 @@ public void execute_validCopy1_success() throws CommandException { @Test public void execute_validCopy2_success() throws CommandException { - Person personToCopy = new PersonBuilder().withTags("Bob").withAssets("Aircon").build(); + Person personToCopy = new PersonBuilder().build(); model.addPerson(personToCopy); Index index = Index.fromOneBased(1); boolean[] info = {false, true, false, true, true, true}; CopyCommand copyCommand = new CopyCommand(index, info); - String copiedDetails = "Phone: 85355255; " - + "Address: 123, Jurong West Ave 6, #08-111; " - + "Tags: [[Bob]]; " - + "Assets: [[Aircon]]"; + String copiedDetails = "Phone: " + DEFAULT_PHONE + "; " + + "Address: " + DEFAULT_ADDRESS + "; " + + "Tags: " + new Tags(DEFAULT_TAGS) + "; " + + "Assets: " + new Assets(DEFAULT_ASSETS); String expectedMessage = String.format(Messages.MESSAGE_COPIED, copiedDetails); assertEquals(expectedMessage, copyCommand.execute(model)); From e772889c7beca40f9e92b9eced7427525258634b Mon Sep 17 00:00:00 2001 From: Tang Yetong Date: Thu, 28 Mar 2024 15:57:54 +0800 Subject: [PATCH 5/7] Update CopyCommand to use 1 field CopyCommand returns multiple fields and field names Update to return only field and no field name --- .../address/logic/commands/CopyCommand.java | 43 ++++++++++--------- .../logic/commands/CopyCommandTest.java | 38 +++++++--------- 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/CopyCommand.java b/src/main/java/seedu/address/logic/commands/CopyCommand.java index 5edc4561734..0191ec682e0 100644 --- a/src/main/java/seedu/address/logic/commands/CopyCommand.java +++ b/src/main/java/seedu/address/logic/commands/CopyCommand.java @@ -28,7 +28,7 @@ public class CopyCommand extends Command { public static final String COMMAND_WORD = "copy"; - public static final String MESSAGE_USAGE = COMMAND_WORD + ": Copies the details of the prefix identified\n" + public static final String MESSAGE_USAGE = COMMAND_WORD + ": Copies one detail of the prefix identified\n" + "Parameters: " + "[" + PREFIX_NAME + "] " + "[" + PREFIX_PHONE + "] " @@ -37,12 +37,10 @@ public class CopyCommand extends Command { + "[" + PREFIX_TAG + "] " + "[" + PREFIX_ASSET + "]\n" + "Example: " + COMMAND_WORD + " 1 " - + PREFIX_NAME + " " - + PREFIX_PHONE + " " - + PREFIX_TAG + " " - + PREFIX_ASSET + " "; + + PREFIX_NAME + " "; - public static final String MESSAGE_NO_PARAM = "At least one field to copy must be provided."; + public static final String MESSAGE_NO_PARAM = "One field to copy must be provided."; + public static final String MESSAGE_EXTRA_PARAM = "Only one field can be copied."; private final Index index; private final boolean[] info; @@ -79,28 +77,26 @@ public String execute(Model model) throws CommandException { private static String copyToClipboard(Person personToCopy, boolean[] info) { requireNonNull(personToCopy); - StringBuilder copiedMsg = new StringBuilder(); - if (info[0]) { - copiedMsg.append("Name: ").append(personToCopy.getName()).append("; "); + return personToCopy.getName().toString(); } if (info[1]) { - copiedMsg.append("Phone: ").append(personToCopy.getPhone()).append("; "); + return personToCopy.getPhone().toString(); } if (info[2]) { - copiedMsg.append("Email: ").append(personToCopy.getEmail()).append("; "); + return personToCopy.getEmail().toString(); } if (info[3]) { - copiedMsg.append("Address: ").append(personToCopy.getAddress()).append("; "); + return personToCopy.getAddress().toString(); } if (info[4]) { - copiedMsg.append("Tags: ").append(personToCopy.getTags()).append("; "); + return personToCopy.getTags().toString(); } if (info[5]) { - copiedMsg.append("Assets: ").append(personToCopy.getAssets()).append("; "); + return personToCopy.getAssets().toString(); } - return copiedMsg.substring(0, copiedMsg.length() - 2); + throw new IllegalArgumentException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE)); } /** @@ -125,33 +121,40 @@ public static CopyCommand of(String args) throws IllegalArgumentException { argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS); boolean[] info = new boolean[6]; + int totalParams = 0; if (argMultimap.getValue(PREFIX_NAME).isPresent()) { info[0] = true; + totalParams++; } if (argMultimap.getValue(PREFIX_PHONE).isPresent()) { info[1] = true; + totalParams++; } if (argMultimap.getValue(PREFIX_EMAIL).isPresent()) { info[2] = true; + totalParams++; } if (argMultimap.getValue(PREFIX_ADDRESS).isPresent()) { info[3] = true; + totalParams++; } if (argMultimap.getValue(PREFIX_TAG).isPresent()) { info[4] = true; + totalParams++; } if (argMultimap.getValue(PREFIX_ASSET).isPresent()) { info[5] = true; + totalParams++; } - for (boolean b: info) { - if (b) { - return new CopyCommand(index, info); - } + if (totalParams == 0) { + throw new IllegalArgumentException(MESSAGE_NO_PARAM); + } else if (totalParams > 1) { + throw new IllegalArgumentException(MESSAGE_EXTRA_PARAM); } - throw new IllegalArgumentException(MESSAGE_NO_PARAM); + return new CopyCommand(index, info); } @Override diff --git a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java index 20408c2e9c8..09869490672 100644 --- a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java @@ -7,12 +7,8 @@ import static seedu.address.logic.commands.CommandTestUtil.assertParseFailure; import static seedu.address.logic.commands.CopyCommand.MESSAGE_USAGE; import static seedu.address.testutil.Assert.assertThrows; -import static seedu.address.testutil.PersonBuilder.DEFAULT_ADDRESS; import static seedu.address.testutil.PersonBuilder.DEFAULT_ASSETS; -import static seedu.address.testutil.PersonBuilder.DEFAULT_EMAIL; import static seedu.address.testutil.PersonBuilder.DEFAULT_NAME; -import static seedu.address.testutil.PersonBuilder.DEFAULT_PHONE; -import static seedu.address.testutil.PersonBuilder.DEFAULT_TAGS; import org.junit.jupiter.api.Test; @@ -23,7 +19,6 @@ import seedu.address.model.ModelManager; import seedu.address.model.person.Person; import seedu.address.model.person.fields.Assets; -import seedu.address.model.person.fields.Tags; import seedu.address.testutil.PersonBuilder; /** @@ -40,17 +35,10 @@ public void execute_validCopy1_success() throws CommandException { Person personToCopy = new PersonBuilder().build(); model.addPerson(personToCopy); Index index = Index.fromOneBased(1); - boolean[] info = {true, true, true, true, true, true}; + boolean[] info = {true, false, false, false, false, false}; CopyCommand copyCommand = new CopyCommand(index, info); - String copiedDetails = "Name: " + DEFAULT_NAME + "; " - + "Phone: " + DEFAULT_PHONE + "; " - + "Email: " + DEFAULT_EMAIL + "; " - + "Address: " + DEFAULT_ADDRESS + "; " - + "Tags: " + new Tags(DEFAULT_TAGS) + "; " - + "Assets: " + new Assets(DEFAULT_ASSETS); - - String expectedMessage = String.format(Messages.MESSAGE_COPIED, copiedDetails); + String expectedMessage = String.format(Messages.MESSAGE_COPIED, DEFAULT_NAME); assertEquals(expectedMessage, copyCommand.execute(model)); } @@ -59,15 +47,11 @@ public void execute_validCopy2_success() throws CommandException { Person personToCopy = new PersonBuilder().build(); model.addPerson(personToCopy); Index index = Index.fromOneBased(1); - boolean[] info = {false, true, false, true, true, true}; + boolean[] info = {false, false, false, false, false, true}; CopyCommand copyCommand = new CopyCommand(index, info); - String copiedDetails = "Phone: " + DEFAULT_PHONE + "; " - + "Address: " + DEFAULT_ADDRESS + "; " - + "Tags: " + new Tags(DEFAULT_TAGS) + "; " - + "Assets: " + new Assets(DEFAULT_ASSETS); - String expectedMessage = String.format(Messages.MESSAGE_COPIED, copiedDetails); + String expectedMessage = String.format(Messages.MESSAGE_COPIED, new Assets(DEFAULT_ASSETS)); assertEquals(expectedMessage, copyCommand.execute(model)); } @@ -87,6 +71,14 @@ public void execute_noParam_throwsIllegalArgumentException() { assertThrows(CommandException.class, () -> copyCommand.execute(model)); } + @Test + public void execute_extraParam_throwsIllegalArgumentException() { + Index index = Index.fromOneBased(1); + boolean[] info = {false, true, true, false, false, false}; + CopyCommand copyCommand = new CopyCommand(index, info); + assertThrows(CommandException.class, () -> copyCommand.execute(model)); + } + @Test public void of_invalidInput_failure() { assertParseFailure(CopyCommand::of, "Invalid", String.format(MESSAGE_INVALID_COMMAND_FORMAT, MESSAGE_USAGE)); @@ -94,13 +86,15 @@ public void of_invalidInput_failure() { assertThrows(IllegalArgumentException.class, () -> CopyCommand.of("1")); assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(INVALID_INDEX)); assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(VALID_INDEX + " ")); + assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(INVALID_INDEX + " n/ a/")); + assertThrows(IllegalArgumentException.class, () -> CopyCommand.of(VALID_INDEX + " A/ t/ p/")); } @Test public void of_validInput_success() { assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " n/")); - assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " t/ A/")); - assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " p/ e/ t/ a/ A/")); + assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " A/")); + assertDoesNotThrow(() -> CopyCommand.of(VALID_INDEX + " a/")); } @Test From cccd38dad381e0d2346502062d0b4778a9c78a5c Mon Sep 17 00:00:00 2001 From: Tang Yetong Date: Fri, 29 Mar 2024 11:28:27 +0800 Subject: [PATCH 6/7] Refactor spacing in CopyCommand --- src/main/java/seedu/address/logic/commands/CopyCommand.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/seedu/address/logic/commands/CopyCommand.java b/src/main/java/seedu/address/logic/commands/CopyCommand.java index 0191ec682e0..5f19656b41e 100644 --- a/src/main/java/seedu/address/logic/commands/CopyCommand.java +++ b/src/main/java/seedu/address/logic/commands/CopyCommand.java @@ -106,8 +106,7 @@ private static String copyToClipboard(Person personToCopy, boolean[] info) { */ public static CopyCommand of(String args) throws IllegalArgumentException { requireNonNull(args); - ArgumentMultimap argMultimap = - ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, + ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_TAG, PREFIX_ASSET); Index index; From 4a2813ca1e79b6c32abdf1070c5e3bd2b1d80351 Mon Sep 17 00:00:00 2001 From: Tang Yetong Date: Fri, 29 Mar 2024 12:09:29 +0800 Subject: [PATCH 7/7] Increase test coverage for copy command Test coverage currently too little Added more tests --- .../logic/commands/CopyCommandTest.java | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java index 09869490672..651d2d3274c 100644 --- a/src/test/java/seedu/address/logic/commands/CopyCommandTest.java +++ b/src/test/java/seedu/address/logic/commands/CopyCommandTest.java @@ -7,8 +7,12 @@ import static seedu.address.logic.commands.CommandTestUtil.assertParseFailure; import static seedu.address.logic.commands.CopyCommand.MESSAGE_USAGE; import static seedu.address.testutil.Assert.assertThrows; +import static seedu.address.testutil.PersonBuilder.DEFAULT_ADDRESS; import static seedu.address.testutil.PersonBuilder.DEFAULT_ASSETS; +import static seedu.address.testutil.PersonBuilder.DEFAULT_EMAIL; import static seedu.address.testutil.PersonBuilder.DEFAULT_NAME; +import static seedu.address.testutil.PersonBuilder.DEFAULT_PHONE; +import static seedu.address.testutil.PersonBuilder.DEFAULT_TAGS; import org.junit.jupiter.api.Test; @@ -19,6 +23,7 @@ import seedu.address.model.ModelManager; import seedu.address.model.person.Person; import seedu.address.model.person.fields.Assets; +import seedu.address.model.person.fields.Tags; import seedu.address.testutil.PersonBuilder; /** @@ -55,6 +60,58 @@ public void execute_validCopy2_success() throws CommandException { assertEquals(expectedMessage, copyCommand.execute(model)); } + @Test + public void execute_validCopy3_success() throws CommandException { + Person personToCopy = new PersonBuilder().build(); + model.addPerson(personToCopy); + Index index = Index.fromOneBased(1); + boolean[] info = {false, true, false, false, false, false}; + CopyCommand copyCommand = new CopyCommand(index, info); + + + String expectedMessage = String.format(Messages.MESSAGE_COPIED, DEFAULT_PHONE); + assertEquals(expectedMessage, copyCommand.execute(model)); + } + + @Test + public void execute_validCopy4_success() throws CommandException { + Person personToCopy = new PersonBuilder().build(); + model.addPerson(personToCopy); + Index index = Index.fromOneBased(1); + boolean[] info = {false, false, true, false, false, false}; + CopyCommand copyCommand = new CopyCommand(index, info); + + + String expectedMessage = String.format(Messages.MESSAGE_COPIED, DEFAULT_EMAIL); + assertEquals(expectedMessage, copyCommand.execute(model)); + } + + @Test + public void execute_validCopy5_success() throws CommandException { + Person personToCopy = new PersonBuilder().build(); + model.addPerson(personToCopy); + Index index = Index.fromOneBased(1); + boolean[] info = {false, false, false, true, false, false}; + CopyCommand copyCommand = new CopyCommand(index, info); + + + String expectedMessage = String.format(Messages.MESSAGE_COPIED, DEFAULT_ADDRESS); + assertEquals(expectedMessage, copyCommand.execute(model)); + } + + @Test + public void execute_validCopy6_success() throws CommandException { + Person personToCopy = new PersonBuilder().build(); + model.addPerson(personToCopy); + Index index = Index.fromOneBased(1); + boolean[] info = {false, false, false, false, true, false}; + CopyCommand copyCommand = new CopyCommand(index, info); + + + String expectedMessage = String.format(Messages.MESSAGE_COPIED, new Tags(DEFAULT_TAGS)); + assertEquals(expectedMessage, copyCommand.execute(model)); + } + @Test public void execute_invalidIndex_throwsCommandException() { Index index = Index.fromOneBased(2);