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

Missing navigation path items for navigation properties with contains target #123

Closed
baywet opened this issue Nov 10, 2021 · 5 comments · Fixed by #160
Closed

Missing navigation path items for navigation properties with contains target #123

baywet opened this issue Nov 10, 2021 · 5 comments · Fixed by #160
Assignees
Labels
type:bug A broken experience
Milestone

Comments

@baywet
Copy link
Member

baywet commented Nov 10, 2021

The current beta description is missing a path item and corresponding operations for

/teams/{team-id}/channels/{channel-id}/messages/{chatMessage-id}/replies/{reply_id}/hostedContents

Where:

  • Hosted contents is a navigation property on chatMessage (no contains target)
  • Replies is a collection of chatMessages navigation property on chatMessage (contains target true)
  • Messages is a collection of chatMessages navigation property on channel (contains target true)
  • Channels is a collection of channel navigation property on team (contains target true)
  • And Teams is a collection of teams
@baywet baywet added this to the 1.0.10 milestone Nov 18, 2021
@baywet baywet added the type:bug A broken experience label Nov 18, 2021
@irvinesunday
Copy link
Contributor

From the chatMessage Entity type snippet below:

image

hostedContents navigation property should be accessible using the path:

/teams/{team-id}/channels/{channel-id}/messages/{chatMessage-id}/hostedContents

or to retrieve a single entity...

/teams/{team-id}/channels/{channel-id}/messages/{chatMessage-id}/hostedContents/{chatMessageHostedContent-id}

The replies navigation property can be accessible using the path:

/teams/{team-id}/channels/{channel-id}/messages/{chatMessage-id}/replies

or to retrieve a single entity...

/teams/{team-id}/channels/{channel-id}/messages/{chatMessage-id}/replies/{chatMessage-id1}

Expansion beyond the .../replies/{chatMessage-id1} path is not possible despite the containment, because the defining entity type (graph.chatMessage) references the same entity type (chatMessage) that the replies navigation property is contained in, and this will lead to circular references.

@baywet
Copy link
Member Author

baywet commented Nov 18, 2021

Thanks for the additional information.
The expansion logic could detect circular references effectively

  • not expanding /teams/{team-id}/channels/{channel-id}/messages/{chatMessage-id}/replies/{reply_id}/replies, as it's already "been through" the replies navigation property
  • expanding all the way to /teams/{team-id}/channels/{channel-id}/messages/{chatMessage-id}/replies/{reply_id}/hostedContents

Instead of relying on a blanket behavior of not expanding any navigation property beyond a certain point.

@baywet baywet self-assigned this Nov 23, 2021
@darrelmiller
Copy link
Member

The problem is that replies should not be contained. Replies are also messages and therefore their canonical path is via the messages collection. I'll go confirm this with Anand.

@darrelmiller
Copy link
Member

darrelmiller commented Nov 24, 2021

It appears I may be wrong here. It looks like you cannot access any message in a channel via ./channel/messages. Replies are actually contained in the root chat message. Sigh.

Which means that hostedContent under replies should become a path. I just don't know what annotation we need to rely on to know that we need to expand the navigation properties on a reply. We had decided that we shouldn't use ContainsTarget but I am struggling to think of what we should use.

@baywet
Copy link
Member Author

baywet commented Nov 24, 2021

and this appears to be deployed to v1.0 now, so turning around will be difficult...
https://docs.microsoft.com/en-us/graph/api/chatmessagehostedcontent-get?view=graph-rest-1.0&tabs=http

Hence my earlier comment on the expansion logic which should evaluate whether it's already been here on a navigation property by navigation property base and not on an entity base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug A broken experience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants