yleeyilin / pe

0 stars 0 forks source link

ADD Feature does not account for spacing validation for drug allergies #4

Open yleeyilin opened 2 months ago

yleeyilin commented 2 months ago

Steps to reproduce:

I have also inputted the following command add ic/S9974567D n/John Doe g/M b/10-07-1999 p/98765432 e/johnd@example.com d/Penicillin|Cephalosporins i/Infectious Disease i/Genetic Disorders

Expected:

Actual:

Screenshots:

image.png

This is problematic as a contact card can get messy especially if users can decide what delimiters they want to use. You can consider supporting this for better convenience and usability for your users. One way you can do is by pre-deciding a delimiter, or handling it like how illness is handled.

nus-se-script commented 2 months ago

Team's Response

Duplicate of #5429 as this issue mainly address about validation concerns of drug allergy field.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Special characters recorded as drug allergies in add command

image.png

command: add ic/S9974944F n/John Doe p/91234567 e/johndoe@email.com g/M b/11-11-1990 d/|| | | || i/Infectious Diseases Here the separator characters are actually recorded as drug allergy types in the app, leading to confusion. Should consider imposing some restrictions on the type of information to be placed between separators.


[original: nus-cs2103-AY2324S2/pe-interim#4991] [original labels: severity.Medium type.FeatureFlaw]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

When developing drug allergy functionality, we have decided to allow the user to have the flexibility to input any information related to the drug allergy field. This could include explanation on specific person's reaction for a specific drug allergy, or in rare occasion where clinics has some SOP where they have symbols that represent a respective drug allergy, which in that case they can just enter that symbol in the field. We aim to make it flexible enough for various use cases that various clinics might have. Though, regarding overzealous validation the bug issue can be addressed in by simply adding some warning messages that only symbols are inputted in the future releases. Hence marking this NotInScope

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: While both are covering concerns regarding the validation of input for allergies, I believe that mine is different since it is referring to the lack of handling of spacing for the users themselves, while the other duplicate response is referring to the acceptance of | as a type by itself.


## :question: Issue response Team chose [`response.NotInScope`] - [x] I disagree **Reason for disagreement:** I believe that this bug is an issue that must be fixed since the rest of your functions such as `find` hinges on this bug. For example, there is no warning to the user who may have left out spacing by accident. Since there is no processing of input, it remains stored with no spaces. This causes the find function to fail to work since the find function does not find by substring, but by space separated words in its entirety. Due to the circumstance and consequence of the lack of processing, I believe that it should have been handled not as an enhancement, but as a necessity. ![image.png](https://raw.githubusercontent.com/yleeyilin/pe/main/files/ce17ce7d-0e79-4582-8367-fdb23e0d68e2.png) Without proper input processing, users are left without warnings or corrections for unintentional spacing errors, resulting in stored data lacking necessary spaces between words. Consequently, the find function fails to work as intended, as it relies on space-separated words for accurate searches.