kevin-pek / pe

0 stars 0 forks source link

Able to violate unique naming constraint in the application #7

Open kevin-pek opened 1 week ago

kevin-pek commented 1 week ago

Screenshot 2024-11-15 at 5.02.59 PM.png

The application does not trim whitespace when importing the json data file. This means the user is able to effectively get multiple users with the same name in the application, which violates the constraint that duplicate names are not provided.

Screenshot 2024-11-15 at 5.03.46 PM.png

Screenshot 2024-11-15 at 5.05.47 PM.png

An example json file is provided below:

{
  "persons" : [ {
    "name" : "Alex",
    "phone" : "87438807",
    "email" : "alexyeoh@example.com",
    "address" : "Blk 30 Geylang Street 29, #06-40",
    "age" : "23",
    "sex" : "Male",
    "appointments" : [ "11/11/2025 1100" ],
    "tags" : [ "friends" ],
    "note" : {
      "appointments" : [ "12/12/2023 1111", "11/12/2022 1100" ],
      "remark" : [ ],
      "medication" : [ ]
    },
    "starredStatus" : "false"
  }, {
    "name" : "Alex ",
    "phone" : "87438807",
    "email" : "alexyeoh@example.com",
    "address" : "Blk 30 Geylang Street 29, #06-40",
    "age" : "23",
    "sex" : "Male",
    "appointments" : [ "11/11/2025 1100" ],
    "tags" : [ "friends" ],
    "note" : {
      "appointments" : [ "12/12/2023 1111", "11/12/2022 1100" ],
      "remark" : [ ],
      "medication" : [ ]
    },
    "starredStatus" : "false"
  } ]
}
nus-pe-script commented 1 week ago

Team's Response

Thank you for raising this. However, users aren't actually able to add duplicate names using the add command regardless of whitespaces: Screenshot 2024-11-17 at 4.36.34 PM.png

I believe what you're referring to is if users were to manually modify data/addressbook.json. However, we did state in the user guide that certain edits may cause unexpected behaviour and thus users should only edit it if they are confident in doing so: Screenshot 2024-11-17 at 4.37.53 PM.png

For now, this is not a priority as we don't encourage users to modify the data directly and users won't be inconvenienced by this in that case. However, we will consider improving this in future iterations.

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]