Adding bot-conversation-sso-quickstart, bot-ai-meeting-insights and bot-archive-groupchat-messages samples #1737
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
AjayJ12-MSFT
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds three new Python bot samples for Microsoft Teams, expanding the existing functionality with AI insights, SSO authentication, and group chat archiving capabilities.
- Adds bot-meeting-ai-insights sample demonstrating AI-generated meeting insights retrieval via Graph API
- Adds bot-conversation-sso-quickstart sample showcasing Single Sign-On implementation in Teams bots
- Adds bot-archive-groupchat-messages sample for archiving group chat messages and sending them as files
Reviewed Changes
Copilot reviewed 82 out of 108 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| samples/bot-meeting-ai-insights/python/* | Complete Python implementation for retrieving AI meeting insights through Graph API with OAuth authentication |
| samples/bot-conversation-sso-quickstart/python/* | Full Python SSO quickstart sample with Teams authentication and Graph integration |
| samples/bot-archive-groupchat-messages/python/* | Python bot for archiving group chat messages with file consent cards and download functionality |
| README.md | Updated main documentation to include reference to new bot-conversation-sso-quickstart Python sample |
Comments suppressed due to low confidence (1)
samples/bot-archive-groupchat-messages/python/config.py:16
- Inconsistent naming convention: other environment variables use PascalCase (MicrosoftAppId, MicrosoftAppPassword) while this uses camelCase (connectionName). Should be 'ConnectionName' for consistency.
CONNECTION_NAME = os.environ.get("connectionName", "")
| @@ -0,0 +1,131 @@ | |||
| --- | |||
| page_type: sample | |||
| description: This sample app demonstrate iss how to use the Bot Framework support for oauth in your bot. | |||
There was a problem hiding this comment.
There are grammatical errors in this line: 'demonstrate iss' should be 'demonstrates' and the description doesn't match the actual functionality (meeting AI insights, not general OAuth).
| description: This sample app demonstrate iss how to use the Bot Framework support for oauth in your bot. | |
| description: This sample demonstrates how to use the Microsoft Graph API to retrieve AI insights generated by Copilot for recorded meetings in Microsoft Teams. |
Copilot uses AI. Check for mistakes.
| The online meeting ID or None if failed. | ||
| """ | ||
| try: | ||
| url = f"https://graph.microsoft.com/beta/me/onlineMeetings?$filter=JoinWebUrl eq '{meeting_url}'" |
There was a problem hiding this comment.
The meeting_url is directly interpolated into the query string without proper URL encoding, which could lead to injection attacks or malformed requests. Use urllib.parse.quote() to properly encode the URL parameter.
| url = f"https://graph.microsoft.com/beta/me/onlineMeetings?$filter=JoinWebUrl eq '{meeting_url}'" | |
| encoded_meeting_url = quote(meeting_url, safe='') |
Copilot uses AI. Check for mistakes.
| @@ -0,0 +1,3 @@ | |||
| botbuilder-integration-aiohttp>=4.17.0 | |||
| botbuilder-dialogs>=4.16.2 | |||
| aiohttp>=3.8.0 | |||
There was a problem hiding this comment.
aiohttp version 3.8.0 has known security vulnerabilities. Consider upgrading to aiohttp>=3.9.0 which includes security fixes.
| aiohttp>=3.8.0 | |
| aiohttp>=3.9.0 |
Copilot uses AI. Check for mistakes.
| logging.error(f'Error getting user information: {error}') | ||
| raise error | ||
|
|
||
| async def get_photo_async(self, token: str) -> Optional[str]: |
There was a problem hiding this comment.
[nitpick] The method name 'get_photo_async' has redundant '_async' suffix since it's already an async method. Consider renaming to 'get_photo' for better API consistency.
| async def get_photo_async(self, token: str) -> Optional[str]: | |
| async def get_photo(self, token: str) -> Optional[str]: |
Copilot uses AI. Check for mistakes.
| import sys | ||
| import os | ||
| sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | ||
| from simple_graph_client import SimpleGraphClient |
There was a problem hiding this comment.
The sys.path.append() pattern on lines 15-17 is a code smell. This suggests incorrect project structure or import configuration. Consider using proper package imports or relative imports instead.
| import sys | |
| import os | |
| sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) | |
| from simple_graph_client import SimpleGraphClient | |
| from ..simple_graph_client import SimpleGraphClient |
Copilot uses AI. Check for mistakes.
| # Remove <at>...</at> patterns using regex | ||
| at_pattern = r'<at>.*?</at>\s*' | ||
| if re.search(at_pattern, cleaned_text): | ||
| logging.info(f"Found <at> pattern in text: '{cleaned_text}'") | ||
| cleaned_text = re.sub(at_pattern, '', cleaned_text).strip() |
There was a problem hiding this comment.
The regex pattern is compiled on every method call. For better performance, compile the regex pattern once as a module-level constant.
| # Remove <at>...</at> patterns using regex | |
| at_pattern = r'<at>.*?</at>\s*' | |
| if re.search(at_pattern, cleaned_text): | |
| logging.info(f"Found <at> pattern in text: '{cleaned_text}'") | |
| cleaned_text = re.sub(at_pattern, '', cleaned_text).strip() | |
| # Remove <at>...</at> patterns using pre-compiled regex | |
| if AT_PATTERN.search(cleaned_text): | |
| logging.info(f"Found <at> pattern in text: '{cleaned_text}'") | |
| cleaned_text = AT_PATTERN.sub('', cleaned_text).strip() |
Copilot uses AI. Check for mistakes.
No description provided.