Merge pull request #69 from jeffreygohkw/week10

[V1.3][Week 10][W09-B2][Logic] Change Privacy Command

docs/DeveloperGuide.adoc

@@ -512,7 +512,6 @@ Note that the execution of the sort methods result in the actual person list bei
 **Pros:** Faster for the user to key in one word rather than a command with many words +
 **Cons:** Reduces choice for experienced users who may want to save to another location or use a different save name
 
-// tag::Privacy
 === Privacy of fields
 
 The `Name`, `Phone`, `Email` and `Address` of a `Person` contains a boolean `isPrivate`, which will determine if the field belonging to that person is private or not.
@@ -565,8 +564,6 @@ In addition, if `isPrivate` is `true` for `Name`, clicking on a person's card in
 
 Instead, a `NewResultAvailableEvent` will be raised by `BrowserPanel` to inform the user that they are not allowed to search for a person possessing a private `Name`.
 
-`ChangePrivacyCommand` facilitates the setting of an existing person's field's privacy. Depending on the user's input, `ChangePrivacyCommand` will use each field's `setPrivate()` method to set the value of `isPrivate`.
-
 To keep track of whether each field is private or not, `XmlAdaptedPerson` will have to store the `isPrivate` value for each field of `Person`.
 This is done by adding the following
 
@@ -604,7 +601,6 @@ If an old save file without privacy data is loaded, then the `toModelType()` met
 If a private field is to be edited by `EditCommand`, `createEditedPerson()` in `EditCommand` will not modify the data of that field, even though `EditCommand` will create a new `CommandResult` with a success message. +
 Hence, a private field will remain private and the value stored by that field will remain the same as it originally was.
 
-
 ==== Design Considerations
 
 **Aspect:** Implementation of `isPrivate` +
@@ -624,7 +620,39 @@ Hence, a private field will remain private and the value stored by that field wi
 **Pros:** Easier implementation, do not have to modify `AddCommand`. +
 **Cons:** Requires 2 command lines to create a `Person` with private fields, which takes more time and is more inconvenient for users.
 
-// end::Privacy
+=== Changing of a Person's Privacy
+
+`ChangePrivacyCommand` facilitates the setting of an existing person's field's privacy. Depending on the user's input, `ChangePrivacyCommand` will use each field's `setPrivate()` method to set the value of `isPrivate`.
+
+The sequence diagram for `ChangePrivacyCommand` is illustrated below.
+
+image::ChangePrivacySequenceDiagram.png[width="800"]
+
+Upon receiving a String containing the arguments from `AddressBookParser`, `ChangePrivacyCommandParser` will create a `PersonPrivacySettings` object.
+
+Depending on the input, the `ChangePrivacyCommandParser` will set the privacy values, represented by Booleans, in the `PersonPrivacySettings` object to be `true` or `false`.
+
+`ChangePrivacyCommandParser` will then create a new `ChangePrivacyCommand` using the input `Index` and the `PersonPrivacySettings`
+
+When `ChangePrivacyCommand` starts executing, it will create a new `Person` based on the data of the `Person` at the specified `Index`.
+It will then adjust the privacy values based on the input `PersonPrivacySettings`.
+
+Once it is done, it will update the original `Person` with the newly created `Person` in `Model`.
+
+[NOTE]
+If there are missing fields in the input string, the getter methods in `PersonPrivacySettings` will return `false`, but the actual value stored will remain as `null`.
+This allows `isAnyFieldNonNull` to check if the user has input any field at all.
+
+==== Design Considerations
+
+**Aspect:** How to implement changing of a person's privacy. +
+**Alternative 1 (current choice):** Create a separate command to do so. +
+**Pros:** It is clear to users and developers that `changeprivacy` is to modify a person's privacy while `edit` changes the actual data if the relevant field is not private.  +
+**Cons:** Additional command, parser and tests must be created.  +
+**Alternative 2:** Enhance the functionality of EditCommand. +
+**Pros:** Can make use of existing code to aid the implementation. +
+**Cons:** Increases the complexity of EditCommand for both users and developers. More ambiguous as to how editing a private field will affect the data.
+
 
 === Add/Delete Tag mechanism
 

docs/UserGuide.adoc

@@ -193,8 +193,8 @@ Changes the tag called `friends` to one called `enemies` instead. All existing c
 
 === Changing the Privacy of a person's details : `changeprivacy` `cp` (Since V1.3)
 
-You can use the `changeprivacy` command to set the privacy settings for each field of an existing `Person` in the address book, which allows you to choose specifically, what information will be displayed. +
-Format: `changeprivacy INDEX [n/NAME] [p/PHONE] [e/EMAIL] [a/ADDRESS] [b/BIRTHDAY] [t/TAG]`
+You can use the `changeprivacy` command to set the privacy settings for each field of an existing `Person` in the address book, which allows you to choose specifically what information will be displayed. +
+Format: `changeprivacy INDEX [n/NAME] [p/PHONE] [e/EMAIL] [a/ADDRESS]`
 Shorthand commands: `cp`
 
 ****
@@ -203,15 +203,16 @@ Shorthand commands: `cp`
 * You can only provide `true` or `false` as inputs after each prefix.
 * If you choose to input `false`, you will set the privacy of that field for that person to be public. The data in that field will be visible in the UI.
 * If you choose to input `true`, you will set the privacy of that field for that person to be private. The data in that field cannot be modified and will not be visible in the UI.
-* Fields that do not originally contain any data will still remain empty after changing their privacy
+* Fields that do not originally contain any data will still remain empty after changing their privacy.
+* If you do not add a prefix for the field in the command, that field will default to be public.
 ****
 
 Examples:
 
 * `changeprivacy 1 p/false e/true` +
 Sets the phone number of the 1st person to be public and their email address to be private. The 1st person's phone number will be displayed, if available, while their email address will be hidden in the UI.
-* `cp 2 a/false n/true t/false` +
-Sets the address and tags of the 2nd person to be public and their name to be private. The 2nd person's address and tags will be displayed, if available, while their name will be hidden in the UI.
+* `cp 2 a/false n/true e/false` +
+Sets the address and email of the 2nd person to be public and their name to be private. The 2nd person's address and email will be displayed, if available, while their name will be hidden in the UI.
 
 === Locating persons by name : `find`
 
@@ -633,10 +634,14 @@ The program will learn your typing habits and give autocomplete suggestions that
 
 * *Add* : `add or a n/NAME p/PHONE_NUMBER e/EMAIL a/ADDRESS b/BIRTHDAY f/FACULTY c/[GOOGLE CALENDAR URL] [t/TAG]...` +
 e.g. `add n/James Ho p/22224444 e/[email protected] a/123, Clementi Rd, 1234665, b/29-02-1996 f/SOC c/www.enteryourcalendarurlhere.com t/friend t/colleague`
+* *Add with private fields* : `add or a pn/NAME pp/PHONE_NUMBER pe/EMAIL pa/ADDRESS b/BIRTHDAY f/FACULTY c/[GOOGLE CALENDAR URL] [t/TAG]...` +
+e.g. `add pn/James Ho pp/22224444 pe/[email protected] pa/123, Clementi Rd, 1234665`
 * *Add task* : `add task n/NAME d/DESCRIPTION [t/DEADLINE] [p/PRIORITY]` +
 e.g. `add task n/Update Documentation d/Update documentations for V1.1 for CS2103T t/30/10/17 p/high`
 * *Add tag* : `addtag INDEX t/[TAG]` +
 e.g. `addtag 1 2 t/friends`
+* *Change a person's details' privacy* : `changeprivacy or cp INDEX [n/TRUE or FALSE] [p/TRUE or FALSE] [e/TRUE or FALSE] [a/TRUE or FALSE]` +
+e.g. `changeprivacy 2 n/true p/false e/true a/false`
 * *Clear person* : `clear`
 * *Clear tasks* : `cleartask`
 * *Delete person* : `delete INDEX` +

src/main/java/seedu/address/logic/commands/AddCommand.java

@@ -59,7 +59,6 @@ public class AddCommand extends UndoableCommand {
 
     public static final String MESSAGE_TASK_SUCCESS = "New task added: %1$s";
     public static final String MESSAGE_SUCCESS = "New person added: \n%1$s";
-
     public static final String MESSAGE_DUPLICATE_PERSON = "This person already exists in the address book";
     public static final String MESSAGE_DUPLICATE_TASK = "This task already exists in the address book";
 

src/main/java/seedu/address/logic/commands/ChangePrivacyCommand.java

@@ -5,15 +5,25 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
-import static seedu.address.logic.parser.CliSyntax.PREFIX_TAG;
+import static seedu.address.model.Model.PREDICATE_SHOW_ALL_PERSONS;
 
 import java.util.List;
+import java.util.Set;
 
 import seedu.address.commons.core.Messages;
 import seedu.address.commons.core.index.Index;
 import seedu.address.commons.util.CollectionUtil;
 import seedu.address.logic.commands.exceptions.CommandException;
+import seedu.address.model.person.Address;
+import seedu.address.model.person.Email;
+import seedu.address.model.person.Name;
+import seedu.address.model.person.Person;
+import seedu.address.model.person.Phone;
 import seedu.address.model.person.ReadOnlyPerson;
+import seedu.address.model.person.Remark;
+import seedu.address.model.person.exceptions.DuplicatePersonException;
+import seedu.address.model.person.exceptions.PersonNotFoundException;
+import seedu.address.model.tag.Tag;
 
 /**
  * Changes the privacy setting of a person's details in the address book
@@ -26,19 +36,21 @@ public class ChangePrivacyCommand extends UndoableCommand {
     public static final String FALSE_WORD = "false";
 
     public static final String MESSAGE_USAGE = COMMAND_WORD + ": Changes the privacy of the details of the person"
-            + "identified by the index number used in the last person listing.\n "
+            + " identified by the index number used in the last person listing.\n"
             + "Parameters: INDEX (must be a positive integer) "
             + "[" + PREFIX_NAME + TRUE_WORD + " OR " + FALSE_WORD + "]"
             + "[" + PREFIX_PHONE + TRUE_WORD + " OR " + FALSE_WORD + "]"
             + "[" + PREFIX_EMAIL + TRUE_WORD + " OR " + FALSE_WORD + "]"
-            + "[" + PREFIX_ADDRESS + TRUE_WORD + " OR " + FALSE_WORD + "]"
-            + "[" + PREFIX_TAG + TRUE_WORD + " OR " + FALSE_WORD + "]...\n"
+            + "[" + PREFIX_ADDRESS + TRUE_WORD + " OR " + FALSE_WORD + "]\n"
             + "Example: " + COMMAND_WORD + " 1 "
-            + PREFIX_PHONE + TRUE_WORD
-            + PREFIX_EMAIL + FALSE_WORD;
+            + PREFIX_NAME + TRUE_WORD + " "
+            + PREFIX_PHONE + FALSE_WORD + " "
+            + PREFIX_EMAIL + TRUE_WORD + " "
+            + PREFIX_ADDRESS + FALSE_WORD;
 
     public static final String MESSAGE_CHANGE_PRIVACY_SUCCESS = "Changed the Privacy of the Person: %1$s";
     public static final String MESSAGE_NO_FIELDS = "At least one field to change must be provided.";
+    public static final String MESSAGE_DUPLICATE_PERSON = "This person already exists in the address book.";
 
     private final Index index;
     private final PersonPrivacySettings pps;
@@ -64,31 +76,59 @@ public CommandResult executeUndoableCommand() throws CommandException {
 
         ReadOnlyPerson personToChange = lastShownList.get(index.getZeroBased());
 
-        changePersonPrivacy(personToChange, pps);
+        Person newPerson = createPersonWithChangedPrivacy(personToChange, pps);
 
-        return new CommandResult(String.format(MESSAGE_CHANGE_PRIVACY_SUCCESS, personToChange));
+        try {
+            model.updatePerson(personToChange, newPerson);
+        } catch (DuplicatePersonException dpe) {
+            throw new CommandException(MESSAGE_DUPLICATE_PERSON);
+        } catch (PersonNotFoundException pnfe) {
+            throw new AssertionError("The target person cannot be missing");
+        }
+
+        model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
+        return new CommandResult(String.format(MESSAGE_CHANGE_PRIVACY_SUCCESS, newPerson));
     }
 
     /**
      * Changes a person's fields' privacy
      * @param person the person whose privacy we would like to change
      * @param pps the settings of privacy for each field
      */
-    private static void changePersonPrivacy(ReadOnlyPerson person, PersonPrivacySettings pps) {
-        if (pps.nameIsPrivate() != null) {
-            person.getName().setPrivate(pps.nameIsPrivate());
+    private static Person createPersonWithChangedPrivacy(ReadOnlyPerson person, PersonPrivacySettings pps) {
+        assert person != null;
+
+        Name n = person.getName();
+        Phone p = person.getPhone();
+        Email e = person.getEmail();
+        Address a = person.getAddress();
+        Remark r = person.getRemark();
+        Set<Tag> t = person.getTags();
+
+        if (pps.getNameIsPrivate() != null) {
+            n.setPrivate(pps.getNameIsPrivate());
         }
-        if (pps.phoneIsPrivate() != null) {
-            person.getPhone().setPrivate(pps.phoneIsPrivate());
+        if (pps.getPhoneIsPrivate() != null) {
+            p.setPrivate(pps.getPhoneIsPrivate());
         }
 
-        if (pps.emailIsPrivate() != null) {
-            person.getEmail().setPrivate(pps.emailIsPrivate());
+        if (pps.getEmailIsPrivate() != null) {
+            e.setPrivate(pps.getEmailIsPrivate());
         }
 
-        if (pps.addressIsPrivate() != null) {
-            person.getAddress().setPrivate(pps.addressIsPrivate());
+        if (pps.getAddressIsPrivate() != null) {
+            a.setPrivate(pps.getAddressIsPrivate());
         }
+
+        return new Person(n, p, e, a, r, t);
+    }
+
+    public Index getIndex() {
+        return index;
+    }
+
+    public PersonPrivacySettings getPps() {
+        return pps;
     }
 
     /**
@@ -100,6 +140,15 @@ public static class PersonPrivacySettings {
         private Boolean emailIsPrivate;
         private Boolean addressIsPrivate;
 
+        public PersonPrivacySettings() {}
+
+        public PersonPrivacySettings(PersonPrivacySettings toCopy) {
+            this.nameIsPrivate = toCopy.nameIsPrivate;
+            this.phoneIsPrivate = toCopy.phoneIsPrivate;
+            this.emailIsPrivate = toCopy.emailIsPrivate;
+            this.addressIsPrivate = toCopy.addressIsPrivate;
+        }
+
         /**
          * Returns true if at least one field is not null.
          */
@@ -108,7 +157,14 @@ public boolean isAnyFieldNonNull() {
                     this.emailIsPrivate, this.addressIsPrivate);
         }
 
-        public Boolean nameIsPrivate() {
+        /**
+         * Returns the value of nameIsPrivate, returns false if null
+         * @return the value of nameIsPrivate
+         */
+        public Boolean getNameIsPrivate() {
+            if (nameIsPrivate == null) {
+                return false;
+            }
             return nameIsPrivate;
         }
 
@@ -117,7 +173,14 @@ public void setNameIsPrivate(boolean nameIsPrivate) {
             this.nameIsPrivate = nameIsPrivate;
         }
 
-        public Boolean phoneIsPrivate() {
+        /**
+         * Returns the value of phoneIsPrivate, returns false if null
+         * @return the value of phoneIsPrivate
+         */
+        public Boolean getPhoneIsPrivate() {
+            if (phoneIsPrivate == null) {
+                return false;
+            }
             return phoneIsPrivate;
         }
 
@@ -126,7 +189,14 @@ public void setPhoneIsPrivate(boolean phoneIsPrivate) {
             this.phoneIsPrivate = phoneIsPrivate;
         }
 
-        public Boolean emailIsPrivate() {
+        /**
+         * Returns the value of emailIsPrivate, returns false if null
+         * @return the value of emailIsPrivate
+         */
+        public Boolean getEmailIsPrivate() {
+            if (emailIsPrivate == null) {
+                return false;
+            }
             return emailIsPrivate;
         }
 
@@ -135,7 +205,14 @@ public void setEmailIsPrivate(boolean emailIsPrivate) {
             this.emailIsPrivate = emailIsPrivate;
         }
 
-        public Boolean addressIsPrivate() {
+        /**
+         * Returns the value of addressIsPrivate, returns false if null
+         * @return the value of addressIsPrivate
+         */
+        public Boolean getAddressIsPrivate() {
+            if (addressIsPrivate == null) {
+                return false;
+            }
             return addressIsPrivate;
         }
 

src/main/java/seedu/address/logic/commands/SortCommand.java

@@ -38,11 +38,11 @@ public SortCommand(String field, String order) {
         this.order = order;
     }
 
-    private String getField() {
+    public String getField() {
         return this.field;
     }
 
-    private String getOrder() {
+    public String getOrder() {
         return this.order;
     }
 

src/main/java/seedu/address/logic/parser/AddressBookParser.java

@@ -9,6 +9,7 @@
 import seedu.address.logic.commands.AddCommand;
 import seedu.address.logic.commands.AddTagCommand;
 import seedu.address.logic.commands.BackupCommand;
+import seedu.address.logic.commands.ChangePrivacyCommand;
 import seedu.address.logic.commands.ClearCommand;
 import seedu.address.logic.commands.Command;
 import seedu.address.logic.commands.DeleteCommand;
@@ -61,6 +62,10 @@ public Command parseCommand(String userInput) throws ParseException {
         case AddTagCommand.COMMAND_ALIAS:
             return new AddTagCommandParser().parse(arguments);
 
+        case ChangePrivacyCommand.COMMAND_WORD:
+        case ChangePrivacyCommand.COMMAND_ALIAS:
+            return new ChangePrivacyCommandParser().parse(arguments);
+
         case EditCommand.COMMAND_WORD:
         case EditCommand.COMMAND_ALIAS:
             return new EditCommandParser().parse(arguments);