Skip to content

Add a failWhenUndefined option to the SchemaBasedCondition #2095

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

Merged
merged 1 commit into from
Sep 25, 2023
Merged

Add a failWhenUndefined option to the SchemaBasedCondition #2095

merged 1 commit into from
Sep 25, 2023

Conversation

clemens-msupply
Copy link
Contributor

This PR adds an option to mark the scope of a rule condition as required.

The original proposed solution was to add:
whenUndefined: 'fail' | 'success'
but after understanding the problem a bit better I think the main problem is that you can't define the condition scope as required and adding this option keeps it a bit closer to JSONSchema.

I added a comment about my interpretation about whats happening in the code. Let me know what you think, I might be wrong about it and AJV just does something weird... I also still like whenUndefined since it might actually be more user friendly.

Original discussion:
https://jsonforms.discourse.group/t/no-rule-validation-for-array-contains-value-when-array-undefined/1308/7

@netlify
Copy link

netlify bot commented Feb 16, 2023

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit 8fe5fca
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/650dae700959530008ac33bc
😎 Deploy Preview https://deploy-preview-2095--jsonforms-examples.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sdirix
Copy link
Member

sdirix commented Feb 17, 2023

Thanks for the contribution ❤️

I'll talk with the team in regards whether we prefer whenUndefined or scopeRequired 👍.

What I don't understand is your comment in the discussion board where the reverse effect and condition did not work for you, i.e.

am pretty sure I tried the reverse rule and it didn’t work for me

Looking at the code here and the suggested improvements of scopeRequired only applying in the true case, this seems for me to indicate that reversing the effect/rule for the undefined case should also have worked. Can you verify what's going on?

@coveralls
Copy link

coveralls commented Feb 17, 2023

Coverage Status

coverage: 84.469%. remained the same when pulling 8fe5fca on clemens-msupply:rules-required-scope into 29e42b7 on eclipsesource:master.

sdirix

This comment was marked as off-topic.

@sdirix sdirix dismissed their stale review February 17, 2023 15:31

Accidentally approved the wrong PR

@clemens-msupply
Copy link
Contributor Author

I created some examples for the "range" rule and the "array contains" rule:

https://github.com/clemens-msupply/jsonforms-react-seed/tree/rule-condition-with-undefined-values

I reversed the rule by using "not" but it both behaves the same (dependent controls are showing on initial load). Is this what you meant?

@sdirix
Copy link
Member

sdirix commented Mar 7, 2023

Heads up: We'll take a look at this again soon

Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
@sdirix sdirix changed the title Add a scopeRequired option to the SchemaBasedCondition Add a failWhenUndefined option to the SchemaBasedCondition Sep 22, 2023
@sdirix sdirix merged commit 8fc90d3 into eclipsesource:master Sep 25, 2023
@sdirix sdirix added this to the 3.2 milestone Sep 25, 2023
@sdirix sdirix added the core label Sep 25, 2023
@FedeTommi
Copy link

I created some examples for the "range" rule and the "array contains" rule:

https://github.com/clemens-msupply/jsonforms-react-seed/tree/rule-condition-with-undefined-values

I reversed the rule by using "not" but it both behaves the same (dependent controls are showing on initial load). Is this what you meant?

Hi there, I experienced the same issue here https://jsonforms.discourse.group/t/default-hide-for-array-contains-value-when-array-undefined/2152. Are there any updates on this?

@sdirix
Copy link
Member

sdirix commented Jun 6, 2024

I answered in the community thread 👍

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

Successfully merging this pull request may close these issues.

None yet

5 participants