search

jeffrey-jian / pe

0 stars 0 forks source link

Adding a valid course with a slash breaks the app #8

Open jeffrey-jian opened 12 months ago

jeffrey-jian commented 12 months ago

Steps to reproduce:

Screenshot 2023-11-17 at 4.43.42 PM.png

  1. Edited the courses.json file, and renamed CS2103T to CS2103/T.

Screenshot 2023-11-17 at 4.44.46 PM.png

  1. Start the app. No data is displayed. Unable to add/edit any data now.

Justification

I labelled this issue as High for severity because there does exist courses in NUS that have the / within their course codes. Within the UG, it is not mentioned that there are certain constraints on what the course codes can be, and it is also mentioned that COURSE_CODE is String in valid NUS course code format.

In the DG, I acknowledge that it does mention that a planned enhancement is to include a Add Course Command, which I believe would have these strict requirements set in place to not allow users to include /. However, it is important to let users know the current workaround (which is to edit the courses.json file directly), and the limitations of this workaround.

soc-pe-bot commented 12 months ago

Team's Response

While designing this feature, we have researched what is a valid course code. Link to NUS website

However, CS2103/T is not a valid course code according to our documentation. Our app has the expected behaviour of clearing all data when the json file is invalid, which in this case has correct behaviour.

Course codes should start with 2-3 alphabets, followed by 4 numbers, and optionally end with an alphabet. is the error message that is given when a user tries to add CS2103/T.

This should be added as CS2103T.

Using the / character will cause issues will cause problems with the current implementation of AB3. We can look into how we can handle the / character in a future iteration, therefore it is currently not in scope.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: [replace this with your explanation]