Remove "input" from required rule fields, because it's not guaranteed to exist

[VanTanev]

input field will be omitted from rules json when a function is used to generate it:

{
  id: 'test',
  type: 'string',
  input: function(rule, input_name) {
    return `<input class="form-control" type="text" name=${input_name} />`
  }
}

QueryBuilderParser will then do an overzealous check, and drop the rule all together from the query.

It's worth considering that most rule fields are not guaranteed to exist: https://github.com/mistic100/jQuery-QueryBuilder/blob/2a237884b8a665ed9757a4ce08166b8d9e2b24d6/src/public.js#L257-L264

Perhaps id and type do not need to be verified for existence either?

[coveralls]

Coverage remained the same at 100.0% when pulling feccc31 on VanTanev:rule-validation-too-strict into 2bba288 on timgws:master.

[timgws]

Hi @VanTanev

Thanks for your contribution to this repository 👍

I have had a look at it, and it does seem that there is a problem. but it is not exactly as you have pointed out.

It's worth considering that most rule fields are not guaranteed to exist:
https://github.com/mistic100/jQuery-QueryBuilder/blob/2a237884b8a665ed9757a4ce08166b8d9e2b24d6/src/public.js#L257-L264

In the code pointed out here, the ruleData does have the keys specified in this list, however, the items will be set to null when rule.filter is empty/null.

This means that in the Query JSON data that is sent to the server, the keys are still set. They are set to null.

The problem here with QBP, though, is that the specific check (isset) checks if they key exsists, as well as if the value of the key not being null.

Maybe we should consider checking that the property exists, instead? http://php.net/manual/en/function.property-exists.php

Seeing as the QueryBuilder should always send these keys (as per the JS code), I don't think that we should stop checking that the values have actually been set.

If you agree with me, please feel free to create a new commit, where the test is not changed (after all, these are JSON objects that I created from the QueryBuilder example to test specific queries were being generated correctly).

feccc31#diff-d7a109249daf3e1d5f9edcaae4e5dbf8R139

I realise that these keys are never used in the project's code, but as the project is aiming to validate the entire query as being a valid QB ruleset, I believe that we should not skip checking them.

That is, unless ofcourse, we can still use setRules() in QB with missing keys and the queries are still run.

But I'm too lazy to check for that, and would rather write this wall of text instead, haha 🎉

[VanTanev]

Hey @timgws,

As I mentioned in my initial message, jQuery-QueryBuilder will sometimes completely omit the input field. Not set it to null, but instead never even add it to the serialized rules.

Upon further inspection, id is actually required (https://github.com/mistic100/jQuery-QueryBuilder/blob/dev/src/core.js#L16) and while type is not strictly required, I guess it's better to keep it.

input however should be omitted from verification in QueryBuilderParser. Again, jQuery-QueryBuilder will happily generate serialized rules without it when you have a filter definition that uses an { input: function() { ... } } definition. As such, I think it should also be removed from the test.

PS And yes, setRules() works with input missing :)

[timgws]

@VanTanev thanks for the correction 😄

Indeed, JSON.stringify will remove keys that have the value of a function. I will merge this.

[VanTanev]

Appreciate it!
Have a good one :)