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

#1023 - Fix all structural properties ignored when using $expand without $select and camel cased property names #1024

Conversation

orty
Copy link
Contributor

@orty orty commented Aug 18, 2023

This fixes the issue #1023 where all structural properties are removed from the generated SQL query when using $expand without $select in a builder.EnableLowerCamelCase() scenario.

Probably fixes the same issue when using aliases in an EdmModel with the [DataMember(Name=...)] attribute.

@orty orty changed the title #1023 - Fix all structural properties ignored when using $expand without $select #1023 - Fix all structural properties ignored when using $expand without $select and camel cased property names Aug 18, 2023
@ElizabethOkerio
Copy link
Contributor

@orty Thanks for the contribution. Please add some tests.

@@ -13961,7 +13961,6 @@
<member name="P:Microsoft.AspNetCore.OData.Routing.ODataRouteOptions.Order">
<summary>
Gets or sets the route order in conventional routing.
By default, move the conventional routing later as much as possible.
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just rebuilt the solution to validate my fix, and the comment must have been removed in a previous commit without committing the regeneration of the documentation XML. I can revert it if, but it might come back the next time someone rebuilds the solution.

@orty
Copy link
Contributor Author

orty commented Aug 21, 2023

@orty Thanks for the contribution. Please add some tests.

@ElizabethOkerio done, I added a new lower cased model and all its context, plus a copy of your previous test method in this context, checking in both scenario that the top-level properties are not null or default

@ElizabethOkerio
Copy link
Contributor

ElizabethOkerio commented Aug 22, 2023

The build is not running. Let me see whether I can run it manually

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.

3 participants