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

Fail to handle string[] #5

Closed
Steffan-Ennis opened this issue Jul 26, 2017 · 7 comments
Closed

Fail to handle string[] #5

Steffan-Ennis opened this issue Jul 26, 2017 · 7 comments
Assignees

Comments

@Steffan-Ennis
Copy link
Contributor

I'm not sure if this is intentional behaviour. If it is it would be a good feature to be able to compare arrays and not just their items

Found a bad condition check on line 7 of conditionsMeet.js
line 7-8
if (!isObject(conditions) || !isObject(formData)) { toError(Rule ${conditions} with ${formData} can't be processed); }

line 21
return refVal.some(val => conditionsMeet(refCondition, val));

can call with bad arguments

Example condition, targets is a string[]
"conditions": { "targets": "_empty" }

screen shot 2017-07-26 at 12 54 39 pm

screen shot 2017-07-26 at 12 55 19 pm

@Steffan-Ennis
Copy link
Contributor Author

ps I really like the library. Good work

@mavarazy mavarazy self-assigned this Aug 1, 2017
@mavarazy
Copy link
Contributor

mavarazy commented Aug 1, 2017

Thanks, a lot, sorry for getting back late to this.
Can you clarify something

  1. Condition
    "conditions": { "targets": "_empty" }
    _empty is not part of predicate js library, that is used.
    It should display appropriate error when you try to use it this way

Try with this one
"conditions": { "targets": "empty" }

  1. By referencing line 7 and related checks, do you mean you want to feed something other, than object to the rules engine?

I've opened an issue - #7, which should allow you to specify any conditional logic you want.

@Steffan-Ennis
Copy link
Contributor Author

Steffan-Ennis commented Aug 7, 2017

Hey, thanks for getting back to me.

I like your solution for extending the predicate logic. I took a very similar approach by extending the import predicate object.

I referenced line 23 because it would be nice to have the empty predicate ran against the array and its elements.

something like this.
if(Array.isArray(refVal)) { return refVal.some(val => conditionsMeet(refCondition, val)) || checkField(refVal, refCondition) }
and in checkField line 9
return fieldVal.some(val => checkField(val, rule)) || predicate[rule](fieldVal)

The feature could be very useful for associating rules against json-schema-form arrays. We have a use case for hiding an entire section of the form until one element has been added to the array.

@mavarazy
Copy link
Contributor

mavarazy commented Aug 8, 2017

@Steffan-Ennis Can you give me an example? So I would see how to better address this?

@mavarazy
Copy link
Contributor

mavarazy commented Aug 9, 2017

@Steffan-Ennis merged your request

@mavarazy mavarazy closed this as completed Aug 9, 2017
@Steffan-Ennis
Copy link
Contributor Author

Awesome, thanks.

mavarazy added a commit that referenced this issue Aug 9, 2017
mavarazy added a commit that referenced this issue Aug 9, 2017
@mavarazy
Copy link
Contributor

mavarazy commented Aug 9, 2017

@Steffan-Ennis I've updated your fix, should still work

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

No branches or pull requests

2 participants