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

NO-ISSUE: Add range and enum DMN Runner validation for the time, date, date-time and duration types of the new Type Constraint property #2344

Merged
merged 4 commits into from
May 21, 2024

Conversation

ljmotta
Copy link
Contributor

@ljmotta ljmotta commented May 17, 2024

The new DMN editor uses the new type constraints property to set the Item Definition/Item Components constraint. This new property is mapped to the JSON Schema x-dmn-type-constraints keyword. This PR adds the property to the AJV validator. This PR also break the file constants into multiple specific ones.

Before:
image

After:
image

@ljmotta ljmotta added the DMN label May 17, 2024
@ljmotta ljmotta self-assigned this May 17, 2024
@jomarko jomarko self-requested a review May 20, 2024 10:04
@jomarko
Copy link
Contributor

jomarko commented May 20, 2024

Looking

Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this PR. For me it seems working for most of the data types. However I see some issue for date and time

This attached model is simple id function, it takes the input (date and time) and return the same value.

dateandtime.dmn.txt

If the dmn built in date and time is used, it works well, see:
Screenshot 2024-05-20 162933

However if the input data node data type is changed to tDateAndTime, it crashes.
Screenshot 2024-05-20 162903

Please notice the extended servies needs to be running.

@ljmotta
Copy link
Contributor Author

ljmotta commented May 20, 2024

Apparently, the string is converted to a Date class after a new edit, and I'm not sure why. I've added a new line to handle this case. @jomarko Thanks for spotting this one.

Copy link
Contributor

@jomarko jomarko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the updates, the scenario with the date and time seems to work fine now!

@tiagobento tiagobento merged commit 2af12bf into apache:main May 21, 2024
14 checks passed
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

3 participants