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-F12-2] VISIT #64

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

Conversation

gachia
Copy link

@gachia gachia commented Sep 19, 2019

Made change so that it's possible to do a PR to original repo

Copy link

@jon-chua jon-chua left a comment

Choose a reason for hiding this comment

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

Some comments on your Developer Guide were highlighted below! 😸

Comment on lines 302 to 314
|`* * *` |user |delete a person |remove entries that I no longer need

|`* * *` |user |find a person by name |locate details of persons without having to go through the entire list
|`* * *` |user |view the full profile of a patient by searching for his/her name |see all details regarding a patient easily

|`* * *` |user |record down details of each <<visitation,visitation>> |keep track of the patients situation

|`* * *` |user |have daily reminders of important deadlines |keep track of appointments and visitations easily

|`* * *` |user |set follow-up reminders |check in on my patients after some time or when their medication runs out

|`* * *` |user |have <<user-defined-macros,user-defined macros>> |streamline my diagnosis documentation

|`* *` |user |hide <<private-contact-detail,private contact details>> by default |minimize chance of someone else seeing them by accident
Copy link

Choose a reason for hiding this comment

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

Some comments on your User Stories:

  1. The actors of your use cases are pretty general (user), maybe we could change them to something more descriptive and specific? (e.g. doctor)
  2. Try to add more user stories! 😄
  3. Great that you linked specific definitions to your glossary! 👍

** 2a. AddressBook shows no reminder message.
+
Use case ends.

_{More to be added}_

[appendix]
Copy link

Choose a reason for hiding this comment

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

Some comments on your NFRs:

  1. Try to update them soon! For example, we could rename persons to patients to make it more specific 😄
  2. Think of more specific NFRs tied to your application.

Copy link

@Raikonen Raikonen left a comment

Choose a reason for hiding this comment

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

Overall, the Use Cases were well done and can be improved as more details regarding the application is discussed and decided upon!

+
Use case ends.
[none]
* 3. The given name/index is invalid.
Copy link

Choose a reason for hiding this comment

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

Numbering should be 3a instead of 3

* 3. The given name/index is invalid.
+
[none]
** 3a. AddressBook shows an error message.
Copy link

Choose a reason for hiding this comment

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

Numbering should be 3a1 instead of 3a

Use case resumes at step 2.

[none]
* 4. The profile is empty.
Copy link

Choose a reason for hiding this comment

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

Numbering should be 4a instead of 4


1. User requests to list persons
2. AddressBook shows a list of persons
3. User requests to add new visitation record for a specific patient in the list
Copy link

Choose a reason for hiding this comment

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

Maybe it will be good to clarify whether the record added to a specific patient will be a custom one everytime, or just an empty one after it is requested by the user.


Use case resumes at step 1.

* 1b. There are multiple records with the same name and date
Copy link

Choose a reason for hiding this comment

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

Will records with the same date and time but not same name be allowed?

* 1b. There are multiple records with the same name and date
+
[none]
** 1b1. AddressBook shows list of records and prompts user to re-enter last command with index of record to delete.
Copy link

Choose a reason for hiding this comment

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

If records with the same name or datetime are not allowed in the records, would this still be an issue?

Use case resumes at step 1.

[discrete]
=== Use case: Save user-defined macros
Copy link

Choose a reason for hiding this comment

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

Might want to consider adding the definition of macro into the glossary

*Extensions*

[none]
* 2. No reminders are set to show
Copy link

Choose a reason for hiding this comment

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

Extensions should start with 1 instead of 2

*Extensions*

[none]
* 2. No reminders are scheduled for the day
Copy link

Choose a reason for hiding this comment

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

Extensions should start with 1 instead of 2

Use case ends.

[discrete]
=== Use case: View follow-up reminders
Copy link

Choose a reason for hiding this comment

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

Might want to consider adding the definition of follow-up reminders, and its difference with ordinary reminders into the glossary too!

Q-gabe and others added 25 commits October 23, 2019 14:48
chore: sync update-UGDG branch with team repo
docs: clean-up docs for documentation integrity
docs: added GUI redesign part in Implementation section of DevGuide
docs: added Ui Redesign Class Diagram PlantUML file and image
cleanup: Rename ProfilePanel to ProfileWindow to follow naming structure
cleanup: added fxid to ReminderPanel for future use
Q-gabe and others added 30 commits November 10, 2019 00:11
fix: fixed bug where editvisit 1 i/-1 will not throw error
fix: fixed the same bug for deletevisit
refactor!: refactor of `Profile` Component and Tests
docs:updated UG of new behavior for deletevisit and editvisit command
docs: add PPP, update UG and DG
fix: adjust PPP for page limit
* Final Updates to UGDG

docs: fix @Wingedevil link to PPP in About Us
docs: Added Manual Testing Appendix to DG
docs: Updated Ui image to reflect latest build
docs: Standardised image sizes in UG
small fix: made label focus untraversable in CommandBox
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.

10 participants