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

(feat) O3-3790: Indicate errors in JSON schema #339

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Twiineenock
Copy link
Contributor

@Twiineenock Twiineenock commented Aug 20, 2024

Requirements

  • This PR has a title that briefly describes the work done including the ticket number. If there is a ticket, make sure your PR title includes a conventional commit label. See existing PR titles for inspiration.
  • My work conforms to the OpenMRS 3.0 Styleguide and design documentation.
  • My work includes tests or is validated by existing tests.

Summary

Error Handling in Schema Validation

This PR addresses the two types of errors we encounter when working with schemas:

1. Syntax Errors in the JSON Schema:

These errors occur when attempting to parse an invalid JSON string, typically caught using the JSON.parse() method. The error message should clearly guide the user on the nature and location of the issue. Previously, while we caught these errors, we did not display them to users.

2. Invalid Schema According to OpenMRS Form Schema:

These errors are thrown by the Ajv JSON schema validator when validating the JSON against the openmrs.org/form.schema.json. This aspect has been handled correctly in the current implementation.

Enhancements Introduced in This PR:

  1. Displaying Syntax Errors: Syntax errors will now be shown to users via an InlineNotification, providing clear feedback on the issue.

  2. Guided Error Resolution: Users will be prompted to resolve these errors one at a time, ensuring a smoother debugging process. Therefore we can't have this:

json-2

Screenshots

Pr implementation

json

Screancast

This is the implementation in this PR

json-errors.webm

Related Issue

https://openmrs.atlassian.net/browse/O3-3790

Other

Previously, Ajv validation was failing due to errors in the OpenMRS form schema. However, these issues were resolved in a recent commit by @denniskigen.

@Twiineenock
Copy link
Contributor Author

FYI @NethmiRodrigo

@NethmiRodrigo
Copy link
Contributor

Something weird I'm seeing is that a particular validation gets triggered up only after there has been some other error. Notice that the error notification for it doesn't get triggered the first time around as well. I had to click Render Changes again. But in this scenario, it doesn't matter if the boolean was a string because we have parsed it, so even if it doesn't exactly match the reference schema it does work...

Screen.Recording.2024-08-22.at.12.33.07.PM.mov

@NethmiRodrigo
Copy link
Contributor

My bad, seems like this feature already existed in the schema-editor.component, albeit under the validator config. @denniskigen any objections to taking the errors shown in the schema editor out of the validator config, as in show it even if the validator is turned off?

@Twiineenock Twiineenock marked this pull request as draft August 22, 2024 12:55
@Twiineenock
Copy link
Contributor Author

Hi @NethmiRodrigo and @denniskigen,

I have made some commits to address the current issues. Please review the updates at your convenience. You can view a demonstration of the changes here:

Errors.webm

Thank you!

@Twiineenock Twiineenock marked this pull request as ready for review August 30, 2024 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants