Skip to content

feature/java orderby#301

Merged
baywet merged 4 commits intodevfrom
feature/java-orderby
Sep 25, 2020
Merged

feature/java orderby#301
baywet merged 4 commits intodevfrom
feature/java-orderby

Conversation

@baywet
Copy link
Copy Markdown
Member

@baywet baywet commented Sep 25, 2020 •
edited
Loading

Generated code differences

microsoftgraph/msgraph-sdk-java#503

@baywet baywet self-assigned this Sep 25, 2020
zengin
zengin reviewed Sep 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@zengin zengin left a comment

Choose a reason for hiding this comment

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

@baywet Do you have an example where orderBy is used on a non-collection? I looked at couple of OData docs and all the samples I saw were on collections even though they don't say the clause is exclusive to collections, e.g. https://docs.microsoft.com/en-us/odata/concepts/queryoptions-overview#orderby

@baywet
Copy link
Copy Markdown
Member Author

baywet commented Sep 25, 2020

good catch. I think you're referring to these two templates specifically?
https://github.com/microsoftgraph/MSGraph-SDK-Code-Generator/blob/56a5b257af6c0188fa97db6ce08346819f4120aa/Templates/Java/requests_extensions/BaseEntityRequest.java.tt
https://github.com/microsoftgraph/MSGraph-SDK-Code-Generator/blob/56a5b257af6c0188fa97db6ce08346819f4120aa/Templates/Java/requests_extensions/BaseEntityWithReferenceRequest.java.tt

I think what happened is that when I introduced filter, I did a search for expand. And went a bit too far.
And the same thing happened for orderBy but with filter this time. I think both odata query parameters functions should be removed from these two.

Did I miss anything else?

@zengin
Copy link
Copy Markdown
Contributor

zengin commented Sep 25, 2020

I think we should remove orderBy from all these as they are not operating on collections:

Templates/Java/requests_extensions/BaseEntityReferenceRequest.java.tt 
Templates/Java/requests_extensions/BaseEntityRequest.java.tt 
Templates/Java/requests_extensions/BaseEntityWithReferenceRequest.java.tt 
Templates/Java/requests_extensions/BaseMethodBodyRequest.java.tt 
Templates/Java/requests_extensions/BaseMethodRequest.java.tt

@baywet
Copy link
Copy Markdown
Member Author

baywet commented Sep 25, 2020

thanks! updated, sorry about the mess.

@baywet baywet requested a review from zengin September 25, 2020 18:03
@baywet baywet merged commit 4fcd88e into dev Sep 25, 2020
@baywet baywet deleted the feature/java-orderby branch September 25, 2020 18:11
@baywet baywet mentioned this pull request Oct 2, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MIchaelMainer MIchaelMainer Awaiting requested review from MIchaelMainer

@nikithauc nikithauc Awaiting requested review from nikithauc

1 more reviewer

@zengin zengin zengin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@baywet baywet

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@baywet @zengin