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

different null validation messages for complex and primitive collections #1793

Conversation

ElizabethOkerio
Copy link
Contributor

@ElizabethOkerio ElizabethOkerio commented May 28, 2020

Issues

This pull request fixes issue #1715.

Description

The error message returned by the null validation of a collection of primitive values and a collection of complex values is different. The expected result is for the error message for the null validation of both collections to be the same.

A null collection of primitive values returns this error message:

A null value was found for the property named 'XXX', 
which has the expected type 'Collection(Edm.String)[Nullable=False]'.
The expected type 'Collection(Edm.String)[Nullable=False]' 
does not allow null values.

while a null collections of complex values returns:

A node of type 'PrimitiveValue' was read from the JSON reader
when trying to read the contents of the property 'XXX';
however, a 'StartArray' node was expected

The expected result is for the error message for both collections to be:

A null value was found for the property named 'XXX', 
which has the expected type 'Collection(XXX)[Nullable=False]'. 
The expected type 'Collection(XXX)[Nullable=False]' does not allow null values

Checklist (Uncheck if it is not completed)

  • Test cases added
  • Build and test with one-click build and test script passed

Additional work necessary

If documentation update is needed, please add "Docs Needed" label to the issue and provide details about the required document change in the issue.

@ElizabethOkerio ElizabethOkerio added the Ready for review Use this label if a pull request is ready to be reviewed label Jun 3, 2020
@ElizabethOkerio ElizabethOkerio force-pushed the fix/different_null_validation_messages_for_complex_and_primitive_collections branch from 16cfdea to db2720e Compare June 3, 2020 05:19
g2mula
g2mula previously requested changes Jun 3, 2020
Copy link
Member

@g2mula g2mula left a comment

Choose a reason for hiding this comment

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

Could you add a test to document/show this?

@ElizabethOkerio ElizabethOkerio requested a review from g2mula June 9, 2020 08:52
@ElizabethOkerio ElizabethOkerio force-pushed the fix/different_null_validation_messages_for_complex_and_primitive_collections branch from b6483e6 to 58876e8 Compare June 9, 2020 11:48
@ElizabethOkerio ElizabethOkerio requested a review from odero June 9, 2020 11:57
@Sreejithpin Sreejithpin added this to the 7.7.0 milestone Jun 11, 2020
Copy link
Contributor

@paulodero paulodero left a comment

Choose a reason for hiding this comment

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

Looks like some tests are failing.

@ElizabethOkerio ElizabethOkerio force-pushed the fix/different_null_validation_messages_for_complex_and_primitive_collections branch 2 times, most recently from c4bc7ae to 01db728 Compare June 15, 2020 16:18
@ElizabethOkerio ElizabethOkerio force-pushed the fix/different_null_validation_messages_for_complex_and_primitive_collections branch from 01db728 to 7e98502 Compare June 25, 2020 06:23
@ElizabethOkerio ElizabethOkerio force-pushed the fix/different_null_validation_messages_for_complex_and_primitive_collections branch 2 times, most recently from c8a37cf to c411429 Compare August 31, 2020 18:35
Copy link
Member

@mikepizzo mikepizzo left a comment

Choose a reason for hiding this comment

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

:shipit:

@ElizabethOkerio ElizabethOkerio force-pushed the fix/different_null_validation_messages_for_complex_and_primitive_collections branch from 58b526f to cc089bd Compare September 1, 2020 06:11
Copy link
Contributor

@KenitoInc KenitoInc left a comment

Choose a reason for hiding this comment

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

LGTM

@odero odero dismissed g2mula’s stale review September 1, 2020 13:49

Already fixed

@ElizabethOkerio ElizabethOkerio force-pushed the fix/different_null_validation_messages_for_complex_and_primitive_collections branch from cc089bd to 2143f74 Compare September 1, 2020 13:58
@ElizabethOkerio ElizabethOkerio merged commit 6e942d1 into OData:master Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready for review Use this label if a pull request is ready to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants