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

Make output set of $apply transformations available for subsequent transformations or other query operators #987

Conversation

ElizabethOkerio
Copy link
Contributor

@ElizabethOkerio ElizabethOkerio commented Jul 12, 2023

This PR fixes #945, #984

This fix makes this work:

http://localhost:5000/odata/Orders?$apply=groupby((Customer/Name), aggregate($count as OrderCount, TotalAmount with sum as Total))&$orderby=Total desc

Like you can use the output of $apply transformations, Total, in this case to perform subsequent transformations, $orderby in this case.

This is how the ApplyClause property looks like:

        public ApplyClause ApplyClause
        {
            get
            {
                if (_applyClause == null)
                {
                    _applyClause = _queryOptionParser.ParseApply();
                }

                return _applyClause;
            }
        }

So what happens when this is called: if (options.Apply?.ApplyClause != null). if ApplyClause is null,which will be null at this point, then the ParseApply gets called which parses the apply clause and binds the dynamic properties introduced in the $apply transformation. This makes these properties recognizable in subsequent transformations.

Before this change, the parsing of the apply clause was not happening before the $orderby gets called.

@ElizabethOkerio ElizabethOkerio changed the title apply with orderby using dynamic props Output set of $apply transformations available for subsequent transformations or other query operators Jul 12, 2023
@ElizabethOkerio ElizabethOkerio changed the title Output set of $apply transformations available for subsequent transformations or other query operators Make output set of $apply transformations available for subsequent transformations or other query operators Jul 12, 2023
@gathogojr
Copy link
Contributor

@ElizabethOkerio For my understanding, how did this regression come about and how does the seemingly innocuous code change fix the issue? Can you add an explanation in the pull request description?

Copy link
Contributor

@gathogojr gathogojr left a comment

Choose a reason for hiding this comment

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

:shipit:

@gathogojr
Copy link
Contributor

@ElizabethOkerio Does this pull request fix this issue too? #984
If it does, please link it to this PR such that the issue will be closed when the pull request is merged.

@ElizabethOkerio ElizabethOkerio merged commit c2b19f3 into OData:main Jul 13, 2023
2 checks passed
ElizabethOkerio added a commit to ElizabethOkerio/AspNetCoreOData that referenced this pull request Aug 8, 2023
…ansformations or other query operators (OData#987)

* apply with orderby using dynamic props

* u[pdate based on review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants