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

MSTeams threading #1240

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

matthieu-foucault
Copy link

Would love to get some early feedback on this.

TODO:

  • Polling for replies is a bit slow, I'm wondering if there is some info somewhere as to whether a message has replies
  • Add some documentation indicating that PreserveThreading is now an option for MSteams.

@codeclimate
Copy link

codeclimate bot commented Sep 17, 2020

Code Climate has analyzed commit 89f3abe and detected 0 issues on this pull request.

View more on Code Climate.

if replyerr != nil {
return nil, replyerr
}
rct = append(rct, replyct...)
Copy link
Author

Choose a reason for hiding this comment

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

Just starting to discover Go, so this might not be the way to do this

@42wim
Copy link
Owner

42wim commented Sep 26, 2020

Thanks for taking a look at this! 👍

Polling for replies is a bit slow, I'm wondering if there is some info somewhere as to whether a message has replies

Yes, that's why I didn't include it in my PR because with thousands of messages this becomes very very slow.

According to the most recent graph changelog: https://docs.microsoft.com/en-us/graph/changelog#teamwork they've fixed my issue https://github.com/microsoftgraph/microsoft-graph-docs-contrib/issues/6097 wrt to lastModifiedDateTime

Changed lastModifiedDateTime property in the chatMessage resource to represent the time the entity was last touched. It will always be set and never have a null value

So for the replies you can now keep state and see if this property has changed, if so there's a new reply.

@matthieu-foucault
Copy link
Author

@42wim thanks for the tip, I'll look into using lastModifiedDateTime at some point this week

@matthieu-foucault
Copy link
Author

@42wim sounds like the Delta query would be very useful, but that doesn't seem to be available in the library we're using. Sounds like the code generator there needs to be fixed too (yaegashi/msgraph.go#22). Any suggestion for the best way to move forward?

@42wim
Copy link
Owner

42wim commented Oct 1, 2020

Well, I've also looked into delta when implementing support for msteams. (I've added the methods myself then)
Delta didn't work, see https://github.com/microsoftgraph/microsoft-graph-docs-contrib/issues/6235 and #967

So I'd go with my previous suggestion (or you can try to add delta in a forked library, maybe it's fixed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants