Skip to content

fixes #834, add DefaultQueryConfigurations class - #835

Merged
xuzhg merged 3 commits into
mainfrom
issue834
Feb 7, 2023
Merged

fixes #834, add DefaultQueryConfigurations class#835
xuzhg merged 3 commits into
mainfrom
issue834

Conversation

@xuzhg

@xuzhg xuzhg commented Feb 6, 2023

Copy link
Copy Markdown
Member

fixes #834

  1. Add DefaultQueryConfigurations class
  2. Use it in ODataOptions (a public api changes)
  3. change the related test codes.

Benefit:

  1. It's a mistake to use DefaultQuerySetting from ModelBuilder ( I think)
  2. Now, we can add other configuration, for example $compute, $search.

ElizabethOkerio
ElizabethOkerio previously approved these changes Feb 7, 2023
View reviewed changes
@ElizabethOkerio

ElizabethOkerio commented Feb 7, 2023

Copy link
Copy Markdown
Contributor

there are so many tests failing though

gathogojr
gathogojr previously approved these changes Feb 7, 2023
View reviewed changes

@gathogojr gathogojr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Left a few naming suggestions and minor comments. Otherwise LGTM

Comment thread src/Microsoft.AspNetCore.OData/Edm/EdmHelpers.cs Outdated
Comment thread src/Microsoft.AspNetCore.OData/Query/Validator/FilterQueryValidator.cs Outdated
Comment thread src/Microsoft.AspNetCore.OData/Query/DefaultQueryConfigurations.cs Outdated
@xuzhg @gathogojr
Address the comments
1f8c8ee 
Co-authored-by: John Gathogo <john.gathogo@microsoft.com>
@xuzhg
xuzhg merged commit 276ce4c into main Feb 7, 2023
@xuzhg
xuzhg deleted the issue834 branch February 7, 2023 19:02
@AndreaCuneo AndreaCuneo mentioned this pull request Mar 27, 2023
Merged
@commonsensesoftware commonsensesoftware mentioned this pull request Mar 29, 2023
Closed
1 task
This was referenced Mar 29, 2023
Closed
Closed
This was referenced Apr 29, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ElizabethOkerio ElizabethOkerio ElizabethOkerio left review comments
@gathogojr gathogojr gathogojr left review comments
@habbes habbes Awaiting requested review from habbes
@KenitoInc KenitoInc Awaiting requested review from KenitoInc
@lisicase lisicase Awaiting requested review from lisicase

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ASP.NET Core OData should have its own DefaultQuerySettings

3 participants

@xuzhg @ElizabethOkerio @gathogojr