Update UG and DG #156

Criss-Wang · 2020-10-26T12:05:36Z

No description provided.

jeffreytjs

Looks good, UML diagrams will be reviewed in minutes

jeffreytjs · 2020-10-26T12:18:30Z

docs/DeveloperGuide.md

+
+#### Path Execution of Delete Command
+
+The workflow of an `Delete` Command when it is executed by a user is shown in the activity diagram below:


Minor typo, should be "workflow of a" instead

Can consider removing "it is" too

jeffreytjs · 2020-10-26T12:24:01Z

docs/_sass/minima/initialize.scss

@@ -14,7 +14,7 @@ $spacing-unit:     30px !default;
 $table-text-align: left !default;

 // Width of the content area
-$content-width:    800px !default;
+$content-width:    1000px !default;


What does this do? Expand the width of md?

jeffreytjs · 2020-10-26T12:25:05Z

src/main/java/seedu/clinic/model/warehouse/Warehouse.java

@@ -83,6 +83,21 @@ public Product getProductByName(Name targetName) throws ProductNotFoundException
        return matchedProduct;
    }

+    /**
+     * Remove a product sold by the Warehouse.


Should be removes

jeffreytjs · 2020-10-26T12:25:15Z

src/main/java/seedu/clinic/model/supplier/Supplier.java

@@ -78,6 +78,21 @@ public Product getProductByName(Name targetName) throws ProductNotFoundException
        return matchedProduct;
    }

+    /**
+     * Remove a product sold by the Supplier.


Should be removes

tohyuting

Just some minor issues/suggestions you may want to consider! :)

tohyuting · 2020-10-26T12:15:44Z

docs/DeveloperGuide.md

+
+#### Path Execution of Delete Command
+
+The workflow of an `Delete` Command when it is executed by a user is shown in the activity diagram below:


I think this should be a Delete command

This is already fixed, thank!

tohyuting · 2020-10-26T12:23:43Z

docs/DeveloperGuide.md

+1. The target to delete is a `supplier`/`warehouse` <br>
+CLI-nic finds the target `supplier`/`warehouse` at the specified `INDEX` of the displayed list, and remove it completely.
+
+1. The `INDEX` specified is invalid (e.g. exceeds the length of the list) <br>
+A **`CommandException`** error message wil be thrown.
+
+1. The target to delete is a `product` in a particular 'supplier' <br>
+CLI-nic finds the target `supplier` at the specified `INDEX` of the displayed supplier list, and retrieve its product list.<br>
+It finds the `product` with specified `PRODUCT_NAME` and remove it from the product list.
+
+1. No `product` has the `PRODUCT_NAME` in the target warehouse/supplier <br>
+A **`CommandException`** error message wil be thrown.


Should the numbering be changed to "1. 2. 3. 4." ?

Nope, this is the markdown standard actually.

Oh okay, I didn't know that, sorry about that!:)

tohyuting · 2020-10-26T12:24:28Z

docs/DeveloperGuide.md

+<center><i>Figure n. Delete Command Class Diagram</i></center>
+<br>
+
+Note that some commonly applied methods (like getter/setter methods for each attribute) are not included to reduce verbosity.


Maybe "such as" instead of like would be better in this sentence :)

tohyuting · 2020-10-26T12:26:06Z

docs/DeveloperGuide.md

+    CLI-nic's `ClinicParser` will parse the user input and if the `delete` command word is present, the parser will try to parse the
+    input into a valid `DeleteCommand` via `DeleteCommandParser`. <br>


Would "parse the input to generate a valid DeleteCommand" be better rather than "parse the input into a valid DeleteCommand"?

That's right! Thanks for the suggestion!

tohyuting · 2020-10-26T12:30:33Z

docs/DeveloperGuide.md

+
+    If multiple entries of `ct/TYPE` or `i/INDEX` are found, the last entry will be used as the argument. <br>
+
+    Afterwards, all the valid arguments (`INDEX` and `TYPE`) will form a `DeleteCommand`, which will be executed.


"form a DeleteCommand" sounds a bit weird, maybe "will create a new DeleteCommand" is better.

tohyuting · 2020-10-26T12:39:44Z

docs/DeveloperGuide.md

+
+    It then locates the warehouse/supplier entry that user wants to delete via the `INDEX` passed in.
+
+    Afterwards, `model#deleteWarehouse`/`model#deleteSupplier` will remove the target entry from the list in the `model`, and the `model` will then update the displayed list.


I think splitting this long sentence up to shorter sentences would be clearer :) Maybe something along like:

Afterwards, model#deleteWarehouse/model#deleteSupplier will remove the target entry from the list in the model. The model will then update the displayed list.

tohyuting · 2020-10-26T12:40:34Z

docs/DeveloperGuide.md

+
+    The parsing workflow is the same except that now an additional field `pd/PRODUCT_NAME` will be checked (with both prefix and argument) and parsed. <br>
+
+    Afterwards, all the valid arguments (`INDEX`, `TYPE` and `PRODUCT_NAME`) will form a `DeleteCommand`, which will be executed.


Same issue as above, I think maybe using create would be more appropriate

tohyuting · 2020-10-26T12:42:20Z

docs/DeveloperGuide.md

+
+    Base on the classification, the model will again retrieve the relevant displayed list of warehouse/supplier via `model#getFilteredWarehouseList()`/`model#getFilteredSupplierList()`. <br>
+
+    It then locates the warehouse/supplier entry from whom the product to delete via the `INDEX` passed in.


This line is a bit unclear to me, maybe "It then locates the respective warehouse/supplier entry at the INDEX passed in for deletion".

Thanks for the suggestion! I guess my original intention is to establish the link between warehouse/supplier and the product to delete

tohyuting · 2020-10-26T12:45:27Z

docs/DeveloperGuide.md

+
+    Next, the product matching the required `PRODUCT_NAME` will be retrieved via `getProductByName`, and an updated target entry with this product removed will be returned through `removeProduct` call.
+
+    Afterwards, `model#setWarehouse`/`model#setSupplier` will replace the old entry with the updated target entry from the list in the `model`, and the `model` will then update the displayed list.


Perhaps breaking this long sentence up would be clearer as well! "replace the old entry with the updated target entry from the list in model . The model will then update the displayed list." would be better :)

tohyuting · 2020-10-26T12:50:15Z

docs/UserGuide.md

@@ -75,6 +75,9 @@ and efficient Graphical User Interface interaction.

 * Parameters can be in any order.<br>
  e.g. if the command specifies `n/NAME p/PHONE`, `p/PHONE n/NAME` is also acceptable.
+
+* If multiple arguments with same prefixes are in the input and all the values are valid, the last value is chosen. <br>
+  e.g. if the command specifies `ct/w ct/s`, because both `w` and `s` are valid, there is no error, and the type __s - supplier__ is chosen.


Perhaps we can consider to split up this long sentence as well. Maybe something like:

"if a user's input specifies ct/w ct/s where both w and s are valid, there will be no error thrown. However, the type s - supplier will be chosen instead.

qlchan24 · 2020-10-26T13:24:08Z

docs/UserGuide.md

-* `delete ct/w i/1` Removes the warehouse at index 1.
-* `delete ct/s i/12` Removes supplier at index 12 from the list of suppliers.
+* `delete ct/w i/1` Removes the warehouse at index 1 of the list of warehouses.
+* `delete ct/s i/12` Removes supplier at index 12 of the list of suppliers.


Perhaps can add "the" like you did in warehouse

qlchan24 · 2020-10-26T13:25:28Z

docs/UserGuide.md

-* The PRODUCT_NAME must be identifiable and starts with alphanumeric character.
+* The TYPE specified should be one of these values: `pw` / `ps`.
+* The INDEX must be a positive integer, not exceeding the total length of the displayed list.
+* The PRODUCT_NAME must be an identifiable full name of the product chosen, and must starts with an alphanumeric character.


"start".
Also "an identifiable full name of the product" sounds a bit off, perhaps can change to "the"

qlchan24 · 2020-10-26T13:27:14Z

docs/UserGuide.md

-* `delete supplier 12 pd/Aspirin` Removes product with the name _Aspirin_
-from the supplier at index 12 of the list of suppliers.
+<div markdown="span" class="alert alert-info">:information_source:
+**Note:** If additional PRODUCT_NAME behind prefix `pd/` is added when the TYPE `ct/` given is "w" or "s", the PRODUCT_NAME will be ignored


perhaps can put "an".

actually I think the word "additional" is unecessary here also

tohyuting

LGTM, will be reading through UG/DG after merging everyone's requests to fix any phrasing/grammatical errors for UG/DG

Update UG and DG

Criss-Wang and others added 6 commits October 3, 2020 15:37

Update DG, fix grammer

348d2e1

Merge remote-tracking branch 'upstream/master' into edit-dg

573d2dc

update DG'

79254a4

Generate PNG images for PlantUML diagrams

4254390

updated DG with UML diagrams

e0dd7cd

Generate PNG images for PlantUML diagrams

7ff08d5

jeffreytjs reviewed Oct 26, 2020

View reviewed changes

Criss-Wang and others added 3 commits October 26, 2020 20:43

Fix some UML diagram issues

c568ea1

Generate PNG images for PlantUML diagrams

601f9da

new use case added to DG

7f390b9

tohyuting reviewed Oct 26, 2020

View reviewed changes

tohyuting added this to the v1.3 milestone Oct 26, 2020

Criss-Wang added 2 commits October 26, 2020 20:56

remove trailing whitespace

2ae2374

Update some syntax and grammar issue

f6943b4

qlchan24 reviewed Oct 26, 2020

View reviewed changes

tohyuting approved these changes Oct 26, 2020

View reviewed changes

tohyuting merged commit 2bc605e into AY2021S1-CS2103-W14-4:master Oct 26, 2020

jeffreytjs pushed a commit to jeffreytjs/CLI-nic that referenced this pull request Oct 27, 2020

Merge pull request AY2021S1-CS2103-W14-4#156 from Criss-Wang/edit-dg

3ada782

Update UG and DG

jeffreytjs pushed a commit to jeffreytjs/CLI-nic that referenced this pull request Oct 27, 2020

Merge pull request AY2021S1-CS2103-W14-4#156 from Criss-Wang/edit-dg

9446461

Update UG and DG

jeffreytjs pushed a commit to jeffreytjs/CLI-nic that referenced this pull request Oct 27, 2020

Merge pull request AY2021S1-CS2103-W14-4#156 from Criss-Wang/edit-dg

cfc242b

Update UG and DG

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update UG and DG #156

Update UG and DG #156

Criss-Wang commented Oct 26, 2020

jeffreytjs left a comment

jeffreytjs Oct 26, 2020

jeffreytjs Oct 26, 2020

jeffreytjs Oct 26, 2020

jeffreytjs Oct 26, 2020

jeffreytjs Oct 26, 2020

tohyuting left a comment

tohyuting Oct 26, 2020

Criss-Wang Oct 26, 2020

tohyuting Oct 26, 2020

Criss-Wang Oct 26, 2020

tohyuting Oct 26, 2020

tohyuting Oct 26, 2020

tohyuting Oct 26, 2020

Criss-Wang Oct 26, 2020

tohyuting Oct 26, 2020

tohyuting Oct 26, 2020

tohyuting Oct 26, 2020

tohyuting Oct 26, 2020

Criss-Wang Oct 26, 2020

tohyuting Oct 26, 2020

Criss-Wang Oct 26, 2020

tohyuting Oct 26, 2020

qlchan24 Oct 26, 2020

qlchan24 Oct 26, 2020

qlchan24 Oct 26, 2020

qlchan24 Oct 26, 2020

tohyuting left a comment


		#### Path Execution of Delete Command

		The workflow of an `Delete` Command when it is executed by a user is shown in the activity diagram below:

		CLI-nic's `ClinicParser` will parse the user input and if the `delete` command word is present, the parser will try to parse the
		input into a valid `DeleteCommand` via `DeleteCommandParser`. <br>


		If multiple entries of `ct/TYPE` or `i/INDEX` are found, the last entry will be used as the argument. <br>

		Afterwards, all the valid arguments (`INDEX` and `TYPE`) will form a `DeleteCommand`, which will be executed.


		It then locates the warehouse/supplier entry that user wants to delete via the `INDEX` passed in.

		Afterwards, `model#deleteWarehouse`/`model#deleteSupplier` will remove the target entry from the list in the `model`, and the `model` will then update the displayed list.


		The parsing workflow is the same except that now an additional field `pd/PRODUCT_NAME` will be checked (with both prefix and argument) and parsed. <br>

		Afterwards, all the valid arguments (`INDEX`, `TYPE` and `PRODUCT_NAME`) will form a `DeleteCommand`, which will be executed.


		Base on the classification, the model will again retrieve the relevant displayed list of warehouse/supplier via `model#getFilteredWarehouseList()`/`model#getFilteredSupplierList()`. <br>

		It then locates the warehouse/supplier entry from whom the product to delete via the `INDEX` passed in.


		Next, the product matching the required `PRODUCT_NAME` will be retrieved via `getProductByName`, and an updated target entry with this product removed will be returned through `removeProduct` call.

		Afterwards, `model#setWarehouse`/`model#setSupplier` will replace the old entry with the updated target entry from the list in the `model`, and the `model` will then update the displayed list.

Update UG and DG #156

Update UG and DG #156

Conversation

Criss-Wang commented Oct 26, 2020

jeffreytjs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tohyuting left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tohyuting left a comment

Choose a reason for hiding this comment