soc-se-bot
commented
2 years ago Team's Response
This is the same issue!
The 'Original' Bug
[The team marked this bug as a duplicate of the following bug]
Manually editing studentbook.json and lessonbook.json with unalloyed inputs wipes both files
How to replicate the issue:
Manually edit any field within either file studentbook.json or lessonbook.json, with an invalid input. For example, I manually edited durationHours here to an unaccepted input "&&". Next, start the application again, and then exit the application.
After performing the above, the lessonbook is completely wiped.
Users may accidentally perform editing of the json files and as such there should be some safeguard to prevent all user data from being lost here.
[original: nus-cs2103-AY2122S2/pe-interim#1256] [original labels: type.FunctionalityBug severity.High]
Their Response to the 'Original' Bug
[This is the team's response to the above 'original' bug]
Good catch! But at this stage, wiping both files is an expected behaviour for the program, as mentioned in the UG's FAQ section (reproduced below):
Reason for current implementation
Wiping both files makes sense as these two lists are related to one another; ie the student assigned to a lesson in the LessonList is stored in StudentList.
Thus, if some student in the StudentList is corrupted (ie some fields were modified so that they are now invalid), the entry should be removed. Also, all the lessons that the student was assigned to should un-assign this student too (as he/she would no longer exist).
Other scenarios are also possible (eg the same case as above, but some lesson was corrupted instead).
To implement this logic would take a considerable amount of time, and in view of time constraints, our team decided to err on the safe side by wiping both lists to ensure that users would not be misled by inconsistencies caused by corrupted data lingering in the system.
Reason for downgrade
Additionally, we feel that this is a flaw that is unlikely to affect normal operations of the product (only happens when a user explicitly attempts to modify the JSON files). Hence, we would downgrade the severity to
low.Comments
Although you do raise a good point: ie wiping both files seems to be pretty harsh and some smarter way of detecting and fixing corrupted data would definitely make the user experience better, for the reasons mentioned above we would be tagging this report as
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)
- [x] I disagree
Reason for disagreement: Both bugs emphasize two distinct issues and that is:
- In the 'original' bug report, the main issue the report is trying to emphasize is that the user's data is lost and is irretrievable if the dev team chooses to completely delete the data from the JSON file.
- In this bug report, the main issue the report is trying to emphasize is that a sudden blank app during start-up might be shocking for the users as they might not know that their data file is corrupted and they would expect the data from previous sessions to be completely loaded into the app.
Although both issues have the same cause (reading from corrupted JSON files), they lead to two different problems where solving one might not necessarily solve the other.
For instance, to solve the issue from this bug report, the dev team can display an error message, explicitly notifying the user that a corrupted file is read by the app. This provided solution would not solve the issue from the 'original' bug.
On the other hand, to solve the issue from the 'original' bug report, a backup file can be created so that the data is not irretrievable even though the data in the JSON file is completely deleted. Similarly, this solution would not solve the issue from this bug report.
No matter the solution that the dev team has come up with to solve the bug stated in the 'original' bug report, it is reasonable to expect some form of notification/message to notify the user that a corrupted entry is detected in the JSON files.
Based on the definition of duplicated bugs shown on the CS2103T website (image shown below), a bug is only considered a duplicate if fixing one bug automatically fixes the other and I have shown that solving one bug might not necessarily solve the other.
Hence, both bugs are not duplicates.
:question: Issue response
Team chose [response.NotInScope]
- [x] I disagree
Reason for disagreement: For something to be classified as NotInScope, it has to:
- Be specified in the UG that the missing feature is not supported.
- Prevent user from using the feature or when the user does so, fails gracefully with a suitable error message.
This is the definition of NotInScope as shown in the image retrieved from the course website.
However, both of the conditions are not fulfilled in this case.
Point (1) is not applicable because the reported bug is not regarding a missing feature but a flaw in the existing feature. Moreover, the dev team did not specify any intentions to fix the issue in later iterations.
When the app reads the corrupted JSON file, a blank app with empty data entries would be loaded. Since the users would expect their data from previous sessions to be loaded into the app and they might not know that their JSON file is corrupted, a blank app during start-up would be shocking for the users. It is reasonable to expect some form of error message or notification, notifying the users that the app has read a corrupted data file. Without this simple notification, the user would be frantically trying to find the cause of the "blank app" and this would lead to a poor user experience. Since the dev team did not ensure that the app fails gracefully (no error message or notification), they are in violation of point (2).
Hence, the issue raised from this bug cannot be classified as NotInScope.
:question: Issue type
Team chose [type.FunctionalityBug]
Originally [type.FeatureFlaw]
- [ ] I disagree
Reason for disagreement: [replace this with your explanation]
Steps to reproduce:
Save the file.
Open the app and this is shown:
Expected: Dummy data to be shown or uncorrupted entries to be shown since only 1 student data is corrupted.
Actual: A completely empty app with no data.