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

Bugfix/js/deep nested arrays #13

Merged
merged 7 commits into from
Aug 24, 2020

Conversation

J3m5
Copy link
Contributor

@J3m5 J3m5 commented Aug 22, 2020

It modify the getField function in order to works with deeply nested arrays of objects and output a flat array of items.

This also helps interpreters to validate items without having to loop through nested arrays.

It removes the PROJECTED_FIELD const and its export as it's not needed anymore.

And it refactor the within interpreter by using testValueOrArray, the size one by removing the projected_field check and the all one by reordering the array check.

It also adds tests to be sure that getting and checking deeply nested arrays of objects works.

@stalniy
Copy link
Owner

stalniy commented Aug 23, 2020

Looks very good at first sight! Thank you very much!

I'll do a closer look tomorrow morning, today is a bit busy

@J3m5
Copy link
Contributor Author

J3m5 commented Aug 24, 2020

This PR introduce a bug with the size interpreter that is not caught with the current tests.
The reduce will concat arrays even if the arrays are the actual items that interpreters operates on.
Example:

    it('checks that at least one item from projected array satisfies condition', () => {
      const condition = new Field('size', 'items.a', 2)

      expect(interpret(condition, { items: [{ a: [2, 3] }, { a: [] }, {}] })).to.be.true
      expect(interpret(condition, { items: [5, 4] })).to.be.false
      expect(interpret(condition, { items: { a: [2, 3] } })).to.be.true

      expect(interpret(condition, { items: [{ a: [2, 3] }, { a: [4] }, {}] })).to.be.true
     // --> return false because the value is [2,3,4] and not [[2,3], [4]]
    })

@stalniy
Copy link
Owner

stalniy commented Aug 24, 2020

I’ll check how MongoDB works in case of checking size for deeply nested array props

@stalniy
Copy link
Owner

stalniy commented Aug 24, 2020

I fixed the issue. Thanks for pointing out!

@stalniy stalniy merged commit 2efeb91 into stalniy:master Aug 24, 2020
@stalniy
Copy link
Owner

stalniy commented Aug 24, 2020

available in @ucast/js@2.2.1

@J3m5 J3m5 deleted the bugfix/js/deep-nested-arrays branch August 24, 2020 15:30
@J3m5
Copy link
Contributor Author

J3m5 commented Aug 24, 2020

Great!

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