leonghongfai / pe

0 stars 0 forks source link

Lack of validation checks for users who edit the text file directly #7

Open leonghongfai opened 2 years ago

leonghongfai commented 2 years ago

Improper number format by editing directly on the file image.png results in nothing being shown image.png No error message provided to tell the user what went wrong when they were editing directly on the file which is intended behaviour for advanced users according to UG

nus-pe-bot commented 2 years ago

Team's Response

Thank you for the suggestion.

On the category

If I may refer you to DonnaFin UG: Editing the Data File, there is a detailed explanation of the situation that occurs when you edit files and the behaviour that occurs:

If your changes to the data file make its format invalid, DonnaFin will discard all data and start with an empty data file on the next run (no clients are shown).

As we can see, this is not a functionality but, and at most a feature flaw as this is entirely expected behaviour.

On the severity

Furthermore, in that section, we took great care to explain how invalid formats can render the entire data file invalid. We used a specific example of missing the '$' symbol in a monetary value field, but it was clear that the example is not exhaustive of the ways the data file can be invalid. We further elaborated on the situation as follows:

However, when this happens, to prevent total loss of your data, we do not delete it right away. DonnaFin will assume that the intended action is to clear your data and restart only when any valid command is run. DonnaFin will then proceed to cleanly wipe donnafin.json and execute your command.

On our response

While the previous points could be grounds for rejecting this issue, I do believe this is a valid feature suggestion. We had actually considered making such a feature, and planned for creating a UI pop up on application start up that lists issues with the file if any, or if none, it would list possible duplicates (e.g. lower case / upper case or double spaces) to make the file editing experience as convenient as the add or edit commands with thorough and helpful error / warning messages. Unfortunately, this would require a lot of work:

At the end of the day when we planned out the work all the way up to v1.4 we deemed this was not a critical feature and elected to treat it as out of scope of this tP.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: There may be a detailed description but there is nothing to inform users of what is actually happening that would then resultingly bring them to that part of the UG to refer for help. No one would refer to that section of the UG unless they actually run into that issue, but how would they know that is the issue when there is nothing indicating so?


:question: Issue type

Team chose [type.FeatureFlaw] Originally [type.FunctionalityBug]

Reason for disagreement: [replace this with your explanation]


:question: Issue severity

Team chose [severity.VeryLow] Originally [severity.High]

Reason for disagreement: Probably should not be high but should not be very low as well as all it takes is for the user to try adding in a new user to test if the app is actually working and not crashing, and all data will be wiped. This also backs up my point that there should be a check or an error message to let users know something is wrong with their data file, and not the app hanging or crashing etc.