⚠️ No Changeset found

Latest commit: b3f4d3d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Some errors occurred when validating the changesets config:
The package or glob expression "@highlight-run/rrdom" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "@highlight-run/rrdom-nodejs" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "@highlight-run/rrweb" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "@highlight-run/rrweb-player" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "@highlight-run/rrweb-snapshot" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "@highlight-run/rrweb-types" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "@highlight-run/rrweb-web-extension" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.
The package or glob expression "rrvideo" is specified in the `ignore` option but it is not found in the project. You may have misspelled the package name or provided an invalid glob expression. Note that glob expressions must be defined according to https://www.npmjs.com/package/micromatch.

I've introduced support for fetching the ms teams channels directly from the microsoft graph api.

thanks @jemiluv8 , testing this today

@jemiluv8 just made a couple more changes - I stopped saving the groups with the workspace and instead just save tenant id, then use the /teams endpoint to load all teams. Tested a concurrent / non-concurrent version on a teams instance with 6 teams, took 1.6s for the concurrent version and 3.5s for the non-concurrent one so decided to keep it concurrent (changed it to use the errgroup library). For permissions, I think it just needs these:

Should be good to merge in now, I'll make sure it gets merged in / passes checks. Thanks for all your work here!

Just to clarify

The changes look good to me.

One issue with fetching all teams(under the tenant) and then all channels is that we cannot send to a channel until the bot is installed on the team. At the moment, we don't "default install" the bot on all teams under the tenant.

This means

So we can setup an alert to be sent to a teams channel that will not receive the message if the bot is not installed on the "team"

Conclusion

That was the reason I was storing the list of teams the bot was installed on in the workspace. So I only fetch the channels for these teams. Also, when a bot is uninstalled from a team, we remove the team from the list of teams. That fixes the above issue to some extent.

Options

One thing we could do is to "auto install" the teams bot on all teams in the channel.

@jemiluv8 thanks for the additional context, I just added another check to verify that our app is installed in the team by using this api. Not great because it's a 2n+1 query, but in testing this took about 2s which should be fine as it's only when loading the alerts page for users with many team. We can look at optimizing in the future if this becomes a user pain point, but it keeps all the state of which teams our bot is installed on on the MS Teams side so there's no issue of keeping our state in sync. Will merge this in today

Your update works just fine and fixes the issues I outlined in the clarification.
I think I finally understand our differing “points of view” about “state”

Your preference for not having some state to keep in sync helps avoid a lot of problems
with managing state even though it may not be the most performant.

At the same time too doing all this exxtra work to sync state over time can lead to bugs that we may have a hard time
keeping track of. The trade-off in this case makes sense because performance is not the priority right now. And the expected
number of users using this feature is “expected to be low”.

I’ll keep this in mind for future work on integrations. Its been cool collaborating on this.

/tip @jemiluv8 200

/tip $200 @jemiluv8

🎉🎈 @jemiluv8 has been awarded $200! 🎈🎊

• Share on socials
• Give feedback