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

$expand is reported as enum while it is actually string #197

Closed
sherlock1982 opened this issue Mar 11, 2022 · 4 comments · Fixed by #499
Closed

$expand is reported as enum while it is actually string #197

sherlock1982 opened this issue Mar 11, 2022 · 4 comments · Fixed by #499
Assignees
Labels
priority:p0 Blocking issue/ loss of critical functions. An ICM may be filed to communicate urgency. SLA<=48hrs type:bug A broken experience
Milestone

Comments

@sherlock1982
Copy link

Current behavior:
$expand is enum.

Expected behavior:
$expand is string because you can have complex expressions in $expand like for example $expand=Groups($expand=Members)

@darrelmiller
Copy link
Member

darrelmiller commented Aug 18, 2022

If we updated the schema to be like this it would allow us to provide a list of properties that are expandable but also indicate that each expand property could be a more complex syntax.

          schema:
            uniqueItems: true
            type: array
            items:
              anyOf:
                - enum:
                    - '*'
                    - serviceAnnouncement
                - type: string

Thoughts?

@darrelmiller darrelmiller added the status:needs-discussion An issue that requires more discussion internally before resolving label Aug 18, 2022
@baywet
Copy link
Member

baywet commented Feb 29, 2024

adding some more context here due to
microsoft/kiota#4265
microsoftgraph/msgraph-sdk-typescript#512

currently expand, orderby, select are projected as arrays of enums.
In all three cases this is wrong because of nested expands/selects and nested properties ordering

e.g.
$expand=Groups($expand=Members)
$select=manager($select=firstName)
$orderBy=manager.firstName

projecting all possibilities for all sub-properties combinations would lead to huge and hard to read enums
projecting only the first level properties like today is constraining.
projecting only the first level sub-properties with a oneOf with a string (variation of what Darrel proposed) I believe will lead to confusion (is this only what's supported?), won't be useful for validation or code generation purposes.

I propose we simply project those three as arrays of strings instead. Which will have the additional benefit of making Microsoft Graph descriptions quite smaller too.

@baywet baywet added type:bug A broken experience priority:p0 Blocking issue/ loss of critical functions. An ICM may be filed to communicate urgency. SLA<=48hrs and removed status:needs-discussion An issue that requires more discussion internally before resolving labels Feb 29, 2024
@baywet baywet added this to the OData: 1.6 milestone Feb 29, 2024
@baywet
Copy link
Member

baywet commented Feb 29, 2024

We do have a hard dependency on that though, GE relies on this to auto completion.
We need to tie this new behaviour to a conversion setting switch (true by default, false for GE) "EmitExpandSelectAndOrderByAsStrings" (or a better name)

@andrueastman
Copy link
Member

I propose we simply project those three as arrays of strings instead. Which will have the additional benefit of making Microsoft Graph descriptions quite smaller too.

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p0 Blocking issue/ loss of critical functions. An ICM may be filed to communicate urgency. SLA<=48hrs type:bug A broken experience
Projects
None yet
5 participants