support multiple validation errors

[basz]

Prevents get errors() to return an array-in-array with messages.

This results in all validation messages to be rendered even when showMultipleErrors is false...

ps. I've tried a unit test, but failed... :-/

[basz]

does this seem legit to you?

[jelhan]

ps. I've tried a unit test, but failed... :-/

I guess you meant a component integration test? Could you share what you have tried? I can not recall when I have seen validation to be an array of validation messages. It would be helpful if you can provide an example for that one.

[jelhan]

If I understand the documentation of validated-changeset correctly, model.error.${this.property}.validation could be either a string or an array: https://github.com/validated-changeset/validated-changeset/#error

Ember Bootstrap expects errors property of FormElement to be an array of validation messages: https://www.ember-bootstrap.com/api/classes/Components.FormElement.html#property_errors

Of course model.error.[this.property] could also be undefined if the property is valid. So it seems like we need to handle three cases here:

1. model.error.${this.property}.validation is undefined -> return empty array
2. model.error.${this.property}.validation is a string -> wrap that string into an array and return that one
3. model.error.${this.property}.validation is an array -> return that array

Can you please update the code accordingly? Some in-source comments about the different states may also be helpful for future readers.

[basz]

will do...

you are correct;

arrays are returned when multiple (an array of) validators have been defined.
strings when a single validator (I assumed ember-bootstrap already handled those)
and undefined for none.

[basz]

as for testing. I think i need to do this

ember generate component-test bs-form/element --unit

to create a unit test for the component. However it (empty) fails to run with 'TypeError: Cannot convert undefined or null to object'

[jelhan]

as for testing. I think i need to do this

ember generate component-test bs-form/element --unit

to create a unit test for the component. However it (empty) fails to run with 'TypeError: Cannot convert undefined or null to object'

I think unit tests are not supported for @glimmer/component. But I also don't think we want a unit test. As the consumer will only use the addon through the template, all tests should be component integration tests, which actually renders the component. I think it should be just an additional test in ember-bootstrap-changeset-validations/tests/integration/components/bs-form-element-test.js.

[basz]

hmmm... ok. how do we test undefined then. Simply no rendered message?

[jelhan]

hmmm... ok. how do we test undefined then. Simply no rendered message?

Yes. undefined means that there isn't a validation error. So it should not show a validation message. But I think for your use case the multiple validation messages is more important.

[basz]

there added a test for each case

[jelhan]

This looks great. Thanks a lot.

If you have some time left an additional test case that multiple validation errors are actually shown if showMultipleErrors argument of <BsForm:: Element> is true would be great. But I totally understand if you don't have any more bandwidth to work on this topic. Would merge as-is in that case.

[basz]

Nope, wanted to but forgot in 34 degrees. Will do tomorrow

…

On 12 Aug 2020, at 19:31, Jeldrik Hanschke ***@***.***> wrote:  @jelhan approved this pull request. This looks great. Thanks a lot. If you have some time left an additional test case that multiple validation errors are actually shown if showMultipleErrors argument of <BsForm:: Element> is true would be great. But I totally understand if you don't have any more bandwidth to work on this topic. Would merge as-is in that case. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[basz]

will you do a release or are you waiting on other stuff?

[jelhan]

Will try to do a release later today.

[jelhan]

Released v3.1.2 containing this fix just right now.