Fixed sample - graph change notification #1645
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the Graph Change Notification sample to improve developer experience, bump toolkit schema versions, and migrate the Azure AD app manifest to the new schema.
- Added “Try it yourself” sections with demo manifest links in both NodeJS and C# READMEs
- Bumped Teams Toolkit YAML schema to v1.8 and adjusted local provisioning steps
- Refactored AAD manifest to the v2 schema, introduced a static
TokenStore, and enhanced the notification controller to fetch presence via Graph
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/graph-change-notification/nodejs/README.md | Added manifest link under “Try it yourself” |
| samples/graph-change-notification/csharp/README.md | Added manifest link under “Try it yourself” |
| samples/graph-change-notification/csharp/M365Agent/m365agents.yml | Bumped YAML schema and version to v1.8 |
| samples/graph-change-notification/csharp/M365Agent/m365agents.local.yml | Bumped YAML schema to v1.8; refactored writeToEnvironmentFile and provisioning echo steps |
| samples/graph-change-notification/csharp/M365Agent/aad.manifest.json | Migrated AAD app manifest to new v2 schema |
| samples/graph-change-notification/csharp/ChangeNotification/Helper/ChangeNotification.cs | Added TokenStore; unused import introduced |
| samples/graph-change-notification/csharp/ChangeNotification/Dialogs/MainDialog.cs | Storing token in static TokenStore |
| samples/graph-change-notification/csharp/ChangeNotification/Controllers/NotificationController.cs | Enhanced notification handler to log and fetch presence via GraphClient |
| README.md | Added demo-manifest link for Change Notification sample |
Comments suppressed due to low confidence (2)
samples/graph-change-notification/csharp/M365Agent/m365agents.local.yml:16
- The comment is indented under
writeToEnvironmentFile, but ensure the mapping structure aligns with the schema. Verify thatwriteToEnvironmentFileis correctly treated as a mapping rather than a scalar.
# Write the information of created resources into environment file for the specified environment variable(s).
samples/graph-change-notification/csharp/M365Agent/m365agents.local.yml:38
- Splitting the echo across two lines will break the shell command (the second line is just a string literal). Combine both commands onto one line or prefix the second line with
echo.
echo "::set-teamsfx-env MICROSOFT_APP_TYPE=MultiTenant"; echo
| @@ -1,4 +1,5 @@ | |||
| using AdaptiveCards; | |||
| using System.Collections.Concurrent; | |||
There was a problem hiding this comment.
This using directive is not used anywhere in this file. Consider removing it to keep imports clean.
| using System.Collections.Concurrent; |
Copilot uses AI. Check for mistakes.
| @@ -27,4 +28,8 @@ public Microsoft.Bot.Schema.Attachment GetAvailabilityChangeCard(string Header, | |||
| }; | |||
| } | |||
| } | |||
| public static class TokenStore | |||
There was a problem hiding this comment.
Storing and sharing the token in a static field can lead to race conditions and token leakage across conversations. Consider using BotState or dependency injection to store per-user tokens securely.
Copilot uses AI. Check for mistakes.
No description provided.