Skip to content

Conversation

@jdmeyer3
Copy link

This PR deals with #710 where if a form has the datatype "array of objects" it would never validate it. It also deals with the string mutation of an array of objects, causing any entered array of objects to be mutated to [object Object].

@jdmeyer3
Copy link
Author

I would like to give a special thanks to @humoroushorse for helping me find the exact issue.

@jdmeyer3 jdmeyer3 changed the title issue-710 fix array of objects not validating issue-710 fixed array of objects not validating May 15, 2019
@Kami
Copy link
Member

Kami commented May 15, 2019

Thanks for the contribution.

@bigmstone / @VineeshJain Any chance you will have some time to review this?

@jdmeyer3
Copy link
Author

@bigmstone / @VineeshJain Have you guys had any chance to review this PR?

Copy link
Contributor

@VineeshJain VineeshJain left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for your contribution

@VineeshJain
Copy link
Contributor

@jdmeyer3 Reverted the code. This fix breaks the basic array functionality of a simple array type field. Any of the actions with type:array failed to load. Some of the sample actions that were breaking are:

./examples/actions/orquesta-with-items-concurrency.yaml
./examples/actions/orquesta-test-with-items.yaml
./examples/actions/mistral-repeat-with-items.yaml
./examples/actions/orquesta-with-items.yaml
./examples/actions/mistral-jinja-repeat-with-items.yaml

Can you please take a relook at this? From the quick look at things, I found that the above actions don't have the spec.items field defined and thus using spec.items.type.type in the if condition causes errors.

return v;
}

if (Array.isArray(v) && this.props.spec.items.type === 'object') {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the offending line in the scenario where actions may not above spec.items defined

Copy link
Author

Choose a reason for hiding this comment

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

You guys are right, I did not test with the yaml definition not having the type of item. Let me take another look on how I can distinguish what type of items are in the array.

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.

4 participants