Merge pull request nus-cs2103-AY2223S2#66 from pkpaing/v1.3

[v1.3] Updated DG (Import), Fixed Import command bug

Author: SHni99

docs/DeveloperGuide.md

@@ -22,6 +22,7 @@ connections with their peers during their time in University.
 
 System: BookFace
 
+```
 Use case: UC1 - Add Contact
 
 Actor: User
@@ -37,7 +38,9 @@ Extensions:
 2a. BookFace detects incomplete/invalid details entered.
 BookFace displays an example of valid details.
 System repeats Step 1-2 until valid details are entered.
+```
 
+```
 Use case: UC2 - Delete Contact
 
 Actor: User
@@ -53,7 +56,9 @@ Extensions:
 2a. BookFace detects incomplete/invalid index entered.
 BookFace displays an example of a valid index.
 System repeats Step 1-2 until valid information is entered.
+```
 
+```
 Use case: UC3 - Edit Contact
 
 Actor: User
@@ -69,7 +74,9 @@ Extensions:
 2a. BookFace detects incomplete/invalid details entered.
 BookFace displays an example of valid details.
 System repeats Step 1-2 until valid details are entered.
+```
 
+```
 Use case: UC4 - Find Contact
 
 Actor: User
@@ -85,7 +92,9 @@ Extensions:
 2a. BookFace detects no name entered.
 BookFace displays an example of a valid name.
 System repeats Step 1-2 until a name is entered.
+```
 
+```
 Use case: UC5 - List Contacts
 
 Actor: User
@@ -94,7 +103,9 @@ MSS:
 1. User chooses to view all contacts.
 2. BookFace displays all the user's contacts.
    Use case ends.
+```
 
+```
 Use case: UC6 - Get Help on Commands
 
 Actor: User
@@ -103,7 +114,9 @@ MSS:
 1. User chooses to view instructions on how to use BookFace.
 2. BookFace displays a url to its User Guide.
    Use case ends.
+```
 
+```
 Use case: UC7 - Add Image for a Contact
 Actor: User
 MSS:
@@ -117,7 +130,9 @@ Extensions:
 2a. BookFace detects invalid/incomplete filepath or index.
 BookFace displays an example of a valid filepath and index.
 System repeats Step 1-2 until valid information is entered.
+```
 
+```
 Use case: UC8 - Delete Image for a Contact
 
 Actor: User
@@ -136,7 +151,9 @@ System repeats Step 1-2 until valid information is entered.
 
 2b. BookFace detects that the contact does not have an image added.
 Use case ends.
+```
 
+```
 Use case: UC9 - Import Contacts from a Faculty
 
 Actor: User
@@ -152,6 +169,7 @@ Extensions:
 2a. BookFace detects incomplete/invalid details entered.
 BookFace displays an example of valid details.
 System repeats Step 1-2 until valid details are entered.
+```
 
 **Glossary** :
 * Student: A user who belongs to a faculty and attends one or more classes with other students
@@ -209,7 +227,7 @@ The following activity diagram summarizes what happens when a user executes add-
       - Path is easily invalidated (e.g. user moves/deletes/renames the image)
 ## Delete Image Feature
 ### Delete Image Implementation
-The delete-image feature is facilitated by the classes `DeleteImageCommand`, 
+The delete-image feature is facilitated by the classes `DeleteImageCommand`,
 `DeleteImageCommandParser`, `ImageUtil`, and `ParserUtil`.
 The `DeleteImageCommandParser` first parses through the user command to obtain
 the desired index through using `ParserUtil#parseIndex`. Following which an
@@ -225,7 +243,7 @@ which have already had an image added.
 
 Step 2: The user decides that the image given to the contact at index 4 is not
 suitable, and wants to delete it. The user inputs `delete-image 4`.
-`DeleteImageCommandParser#parse` is then called to parse this input for the 
+`DeleteImageCommandParser#parse` is then called to parse this input for the
 desired index.
 
 > **Note**: If the user inputs an index of a contact which currently does not have
@@ -258,10 +276,64 @@ directory.
         - Ensures application does not consume excess storage
     - Cons:
         - Extra complexity in requiring file i/o operations
-    
+
 - **Alternative 2:** Disregard deleting the image file from program directory.
     - Pros:
         - Easier to implement
     - Cons:
         - Application will take up increasingly more unnecessary storage during
           its lifetime of usage
+## Import Contacts Feature
+### Import Contacts Implementation
+The import feature is facilitated by the classes `ImportCommand`, `ImportCommandParser`,
+`SocContacts` and `ChsContacts`. The `ImportCommandParser` first parses through the user
+command to obtain the desired faculty to be imported. An instance of
+a `ImportCommand` containing the desired faculty from either `SocContacts` or
+`ChsContacts` is then returned. `ImportCommand#execute` is then called,
+which calls `Model#addPerson` to add the unique contacts into BookFace.
+
+Given below is an example usage scenario for how the import mechanism behaves.
+
+Step 1: User starts up the application and sees their list of contacts.
+
+Step 2: User decides to import contacts from faculty SOC and input
+`import soc`. `ImportCommandParser#parse` is then called to parse
+this input for the desired faculty.
+
+> **Note**: If the user inputs a faculty that does not exist,
+> an error will be returned to the user
+>
+
+Step 3: If the faculty was valid, `ImportCommand#execute` is called
+with the valid faculty that was parsed
+
+Step 4: `Model#addPerson` is called for however
+many non-duplicate contacts are to be added.
+
+
+The following sequence diagram shows how the import command works.
+
+![ImportSequenceDiagram](images/ImportSequenceDiagram.png)
+
+
+The following activity diagram summarizes what happens when a user executes an
+import command:
+
+![ImportActivityDiagram](images/ImportActivityDiagram.png)
+
+### Design Considerations:
+- **Alternative 1 (current choice):** ImportCommand#execute takes in a string to
+determine which faculty of in-built contacts to import
+    - Pros:
+        - User input is easily parsed (as it is just a string), faster to read and
+       execute the command
+    - Cons:
+        - Possibly bad OOP design, ImportCommand#execute should take in a Faculty object
+
+- **Alternative 2:** ImportCommand#execute takes in a Faculty object instead of a string
+to determine which faculty of in-built contacts to import
+    - Pros:
+        - Better OOP design then simply taking in a string
+    - Cons:
+        - Input will take longer to parse as the string input has to be parsed into
+          a faculty object to be used as input to ImportCommand#execute

docs/diagrams/ImportActivityDiagram.puml

@@ -0,0 +1,15 @@
+@startuml
+start
+:User executes import command;
+'Since the beta syntax does not support placing the condition outside the
+'diamond we place it as the true branch instead.
+
+if () then ([faculty is valid])
+    :Faculty contacts are retrieved;
+    :Non-duplicate contacts are added;
+    : Display success message;
+else ([else])
+    : Display error message;
+endif
+stop
+@enduml

docs/diagrams/ImportSequenceDiagram.puml

@@ -0,0 +1,67 @@
+@startuml
+!include style.puml
+
+box Logic LOGIC_COLOR_T1
+participant ":LogicManager" as LogicManager LOGIC_COLOR
+participant ":AddressBookParser" as AddressBookParser LOGIC_COLOR
+participant ":ImportCommandParser" as ImportCommandParser LOGIC_COLOR
+participant "d:ImportCommand" as ImportCommand LOGIC_COLOR
+participant ":CommandResult" as CommandResult LOGIC_COLOR
+end box
+
+box Model MODEL_COLOR_T1
+participant ":Model" as Model MODEL_COLOR
+end box
+
+[-> LogicManager : execute("import soc")
+activate LogicManager
+
+LogicManager -> AddressBookParser : parseCommand("import soc")
+activate AddressBookParser
+
+create ImportCommandParser
+AddressBookParser -> ImportCommandParser
+activate ImportCommandParser
+
+AddressBookParser -> ImportCommandParser : parse("soc")
+activate ImportCommandParser
+
+create ImportCommand
+ImportCommandParser -> ImportCommand
+activate ImportCommand
+
+ImportCommand --> ImportCommandParser : d
+deactivate ImportCommand
+
+ImportCommandParser --> AddressBookParser : d
+deactivate ImportCommandParser
+'Hidden arrow to position the destroy marker below the end of the activation bar.
+ImportCommandParser -[hidden]-> AddressBookParser
+destroy ImportCommandParser
+
+AddressBookParser --> LogicManager : d
+deactivate AddressBookParser
+
+LogicManager -> ImportCommand : execute()
+activate ImportCommand
+
+
+ImportCommand -> Model : addPerson(toAdd)
+activate Model
+
+Model --> ImportCommand
+deactivate Model
+
+create CommandResult
+ImportCommand -> CommandResult
+activate CommandResult
+
+CommandResult --> ImportCommand
+deactivate CommandResult
+
+ImportCommand --> LogicManager : result
+deactivate ImportCommand
+
+[<--LogicManager
+deactivate LogicManager
+@enduml

docs/images/ImportActivityDiagram.png

@@ -43,6 +43,7 @@ public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
         String duplicates = "";
         String successes = "";
+        int duplicateCounter = 0;
         ArrayList<Person> importList = new ArrayList<Person>();
 
         if (faculty.equalsIgnoreCase("soc")) {
@@ -56,14 +57,15 @@ public CommandResult execute(Model model) throws CommandException {
         for (Person toAdd : importList) {
             if (model.hasPerson(toAdd)) {
                 duplicates += toAdd.getName() + MESSAGE_DUPLICATE_IMPORT + "\n";
+                duplicateCounter++;
                 continue;
             } else {
                 duplicates += toAdd.getName() + " was successfully imported\n";
                 successes += "\n" + toAdd.getName();
             }
             model.addPerson(toAdd);
         }
-        if (duplicates.length() > 0) {
+        if (duplicateCounter == importList.size()) {
             throw new CommandException(duplicates);
         }
         return new CommandResult(String.format(MESSAGE_SUCCESS + successes));