Jo3LW / pe

0 stars 0 forks source link

The json data is deleted #8

Open Jo3LW opened 1 year ago

Jo3LW commented 1 year ago

Steps:

1) Type add n/John Doe p/98765432 e/johnd@example.com a/311, Clementi Ave 2, #02-25 c/3.50/4.00 g/male u/Nanyang Polytechnic gd/05-2024 m/Computer Science ji/173296 jt/Software Engineer Intern t/rejected t/KIV into command box 2) Type export 3) Edit json file by removing the p/ 4) Type checkout (FILE_NAME) into command box

Expected: The app should throw a error that the field of p/ is missing or wrong. Actual: The app deletes all the data in that Json file and replaces with sample data.

This is an issue as a user might have accidently edited a field in the Json file, and all the data is removed without informing the user, or giving the user chance to edit the file again. This will affect the user experience greatly if the user already has a lot of data stored. Thus an error thrown would be better.

Screenshot 2022-11-11 at 4.59.32 PM.png

Screenshot 2022-11-11 at 5.08.24 PM.png

soc-pe-bot commented 1 year ago

Team's Response

Thank you for the report. As mentioned in 3.10 of the UG, we already have a warning that if the file contains invalid data or has invalid format, the content will be erased and an empty addressbook will be loaded. This is a warning for the user before the user tries to edit the file directly.

image.png

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: I strongly feel that this implementation is flawed as in the case where the user actually modifies the Json file and may not be familiar to Json files, they might accidentally edit a field wrong or add additional characters. This will cause the entire file to be erased. If the file contains a up to 1000 applicants as required in the NFR, it would be a huge inconvenience to users. Screenshot 2022-11-17 at 8.55.09 PM.png

Thus a warning would be much better to ask the reader to confirm that they want to load the file as there is invalid input in the Json file.

I feel that this should be feature flaw and not a functionality bug as well.