Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Null value in json file which cause application unable to launch #3

Open
kimshitong opened this issue Nov 17, 2023 · 1 comment
Open

Comments

@kimshitong
Copy link
Owner

kimshitong commented Nov 17, 2023

Usage :

  1. There is a null between the classes/data in the json file

Expected Behaviour

  1. Throw Error and Should Run
  2. Ignore the null value (Or even take actions towards the null value)

Actual Behaviour

  1. Throw error and unable to launch

image.png

@nus-se-bot
Copy link

nus-se-bot commented Nov 20, 2023

Team's Response

These are considered to be under the same overarching issue of validating JSON data and throwing an appropriate warning. Since this is simultaneously fixable by simply catching all errors and showing a warning in the UI there, adding support for warnings would also automatically resolve this issue.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

An Invalid Photo Number in JSON could cause Empty Database

Steps

  1. Modify Phone number of a single class data to invalid
  2. Launch jar file

Expected Behaviour

  1. Throw Error/Warning
  2. Proceed run other data

Actual Behaviour

  1. It do throw warning
  2. The rest of the data is not rendered.

image.png


[original: nus-cs2103-AY2324S1/pe-interim#5827] [original labels: type.FunctionalityBug severity.High]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

Under the Editing the data file segment of the user guide, caution was given when edits were made directly to the data file.

This is a screenshot of our team's UG:

image.png

And this is a screenshot of the AB3 UG:

image.png

Our team agrees that appropriate warnings should be captured and shown on the GUI when there is invalid data in the JSON data file for better UX.

However, we do not agree with skipping an invalid value and pretending it doesn't exist, since that may cause the user to not realise their data has gone missing - a better way would be to simply prevent the user from using Jobby until the JSON file itself is rectified.

Below are some considerations we had with the triaging:


Considering that a warning is present in the UG and the feature is intended for advanced users, this issue should be of lower severity. Jobby manages data files by rejecting and refusing to load invalid formats, mirroring AB3's functionality.

For most casual users, direct data file editing isn't common. Instead, they would prefer to edit their data using Jobby's commands, which comes with validations.

For advanced users, they are likely to understand that they have done some misconfigurations when the app fails to load correctly or opens with a blank interface. They typically fix their mistakes promptly, especially if they closely follow the User Guide, likely having backups to revert to. Plus, even if Jobby fails to load your data this way, your data is not immediately lost - the malformed file remains intact unless the user deliberately continues using Jobby commands to edit their data, intentionally ignoring the fact that their data will now be overwritten.

Considering these aspects, we believe this is of a medium severity bug.

A flaw that causes occasional inconvenience to some users but they can continue to use the product.


Showing the appropriate warning in the GUI requires significant implementation effort, since we need to completely revamp the AB3 loading structure to be able to cascade the errors from the Storage component to the UI, let alone skipping parts of the data that are not valid and continuing to parse the remaining data. Yet, it only benefits a tiny fragment of users, so there is way too much cost for little benefit as is.

As mentioned by our prof:

Recovering from improper edits to the data file (or other config files) was not an expectations at v1.4, unless you specifically stated such a feature in the UG. (Source)

Given these considerations, we believe more specialised warnings and the ability to load partial data are out of scope for this v1.4 release.

Items for the Tester to Verify

❓ Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

  • I disagree

Reason for disagreement: [replace this with your explanation]


❓ Issue response

Team chose [response.NotInScope]

  • I disagree

Reason for disagreement: [replace this with your explanation]


❓ Issue severity

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

  • I disagree

Reason for disagreement: [replace this with your explanation]


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants