g-tejas / pe

0 stars 0 forks source link

JSON File with multiple duplicate field names is not detected #4

Open g-tejas opened 4 months ago

g-tejas commented 4 months ago

Description

As sseen in the screenshot, if the Json file is modified to have multiple fields with "name", there is no error thrown, even though this is invalid state, and the last field with "name" is taken instead. This might cause some issues as the person might want the earlier name to be used instead, but this errs silently. This should be raised to the user, or can be listed in the UG.

Screenshots

CleanShot 2024-04-19 at 16.36.27@2x.png

CleanShot 2024-04-19 at 16.36.52@2x.png

Steps to reproduce

  1. As shown in the vim screen shot above, edit one of the persons to have two "name" fields within the same person.
  2. Boot up the application.
  3. Notice that the person uses the last instance of "name"

Expected vs Actual Behaviour

The application boots up as normal, and the last instance of the "name" field is used, and the first "name" is dropped. This should return as an error instead, and let the user know that the JSON file is invalid state.

Suggestion

This seems to be a flaw within the JSON parsing part, as it silently ignores that there are two fields w the same name. In this part of ur UG, perhaps u can. mention that not only does the name need to be unique across persons, but it also needs to be only one field with "name"' inside the person object.

CleanShot 2024-04-19 at 16.38.57@2x.png

nus-se-script commented 4 months ago

Team's Response

Thanks for raising this. This is a very rare occurrence.

https://github.com/nus-cs2103-AY2324S2/forum/issues/917

Based on the issues, we have indicated that editing the JSON file does not guarantee whether it will work as intended.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: [replace this with your explanation]


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]