search

nus-cs2113-AY2425S1 / pe-dev-response

0 stars 0 forks source link

No attempt of recovery or warning for corrupted JSON #378

Open nus-pe-bot opened 1 day ago

nus-pe-bot commented 1 day ago

Description:

When JSON file has been tampered with and not in the correct format, there is no attempt at recovery or a warning to the user that the file is corrupted. This could cause users to lose precious exercise data or confusion when they do prog list.

Steps to Reproduce:

  1. Add exercises into the program
  2. Exit Program
  3. delete 1 line that will cause formatting issues in JSON
  4. Reenter Program
  5. Do prog list

Expected Outcome: Some warning to be shown like: Error: JSON file is corrupted, would you like to exit program to fix it or delete the current file

Actual Outcome: Nothing shown as warning, users get shock when seeing empty program list.

Video:


video:https://raw.githubusercontent.com/nus-cs2113-AY2425S1/pe/master/files/72c17cf9-9fea-4118-bb97-acb0019fded0.mp4

Photo:

Step 1 and 2:

image.png

Step 3: 1 line deleted

image.png

Step 4 and 5: No warnings shown

image.png

[original: nus-cs2113-AY2425S1/pe-interim#609] [original labels: severity.Medium type.FeatureFlaw]

TVageesan commented 1 day ago

Team's Response

This bug seems to be split into two bugs:

Functionality Bug - Corrupted Data was not handled properly

The functionality bug in this case is rejected as the app currently behaves as we have described in the UG:

image.png

Since the user corrupted the ProgrammeList, it was quietly reinitialized as empty and the programme continued as per normal, following the behavior described in our UG.

Therefore, we will treat this as a feature flaw.

Feature Flaw - Corrupt Data should be handled through an interactive process instead of "queitly" re-intializing it to empty

We believe the tester makes a very good point that handling the corrupted exercise through the use of a interactive process would be more appropriate as it would allow the user to recover their data instead of just wiping it clean. The decision to stick to the "quiet reinitialize" is due to the complexity of validating the JSON file. In fact, we already have a high severity functionality bug regarding the JSON parsing/saving due to the complexity of our data ( :( ) that we were trying to solve during this iteration. We would definitely love to add this feauture on to a future iteration as it would improve the functionality of the saving.

However, we believe our current solution (as well as properly warning the user in the UG) is a valid solution for the 2.1 iteration of BuffBuddy. The reasoning for this is that directly editing the file is a edge case behaviour and users who perform so should be well aware that unintended behaviour can arise from it. We believe our current solution is sufficient to handle the corrupted data given our limited development time, where we needed to prioritize more critical features for users who would likely not intend to directly manipulate the data file at all.

Duplicate status (if any):

--