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

[CS2103T-T12-2] DeliveryMANS #59

Open
wants to merge 832 commits into
base: master
Choose a base branch
from

Conversation

charliechoong
Copy link

Copy link

@shaoyi1997 shaoyi1997 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the best!

Comment on lines 302 to 304
|`* * *` |Administrator |check on all of/ sort the deliverymen at once |better manage the deliverymen

|`* * *` |Administrator |help customers add/edit/remove orders |better manage customers' orders

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to separate the different commands into different use cases.
eg. "help customers add orders" is one user story


|`* * *` |Administrator |help customers add/edit/remove orders |better manage customers' orders

|`* * *` |Administrator |generate statistics |see which restaurants use our app the most

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to specify what statistics the app is generating.

@charliechoong charliechoong changed the title [CS2103T-T12-2] DeliveryMANs [CS2103T-T12-2] DeliveryMANS Oct 6, 2019
Copy link

@shaoyi1997 shaoyi1997 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good luck for the last few weeks!!

=== Undo/redo feature

The undo/redo feature lets users undo and redo changes to the data stored in the app, which were
effected by commands they have executed.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be "affected" instead of "effected"?


The following sequence diagram shows how the undo operation works:

image::UndoSequenceDiagram1.png[]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The instance name for UndoCommand could be more descriptive; calling it undoCommand might be more informative.


*insert picture of the ui at startup*

Step 2: The user wants to switch to the customer context for customer-focused commands with the command ‘customer’, however is unsure how to spell the command and types in ‘cus’. The ‘KeyListener’ will pass the characters that have been typed to the trie. The trie will then search for relevant commands and pass them as a list to the ‘Ui’, which will display the relevant results in a dropdown box below the user input command box.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phrasing for the first sentence could be clearer; it was quite confusing to understand at first glance.
"customer" can be highlighted using the "insert code" markup

The following activity diagram summarizes what happens when a user types in letters into the user input command box:

*insert picture of activity diagram*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DG should focus more on the technical implementation of the feature instead of describing how the feature is used.

MackyMaguire and others added 23 commits November 6, 2019 06:13
Minor changes to Restaurant side
Added exception handling for edge case in EditOrderCommand
Finalized autocomplete feature
Adding orderName into Orders.
# Conflicts:
#	src/main/java/seedu/deliverymans/commons/core/Messages.java
# Conflicts:
#	src/main/java/seedu/deliverymans/commons/core/Messages.java
Implement AssignOrderCommand for unassigned orders
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants