Adaptive card sample updates #121
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the adaptive card implementation by introducing a strongly-typed AdaptiveCardPayload model and updating the ActionButtonType enum to use semantic button names. The changes improve type safety and align the codebase with adaptive card schema conventions.
- Introduced
AdaptiveCardPayloadclass to replace the genericobjecttype for better type safety - Refactored
ActionButtonTypeenum from generic styles (Primary/Secondary/Tertiary) to semantic action names (Accept/Copy/Reject/UpdateNote) - Updated sample extension to use lowercase property values for adaptive card elements (e.g., "bolder", "small", "medium") per the adaptive card schema specification
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/Dragon.Copilot.Models/VisualizationResource.cs | Changed AdaptiveCardPayload property from object to strongly-typed AdaptiveCardPayload class |
| src/Dragon.Copilot.Models/AdaptiveCardPayload.cs | New model class representing adaptive card structure with schema, type, version, and body properties |
| src/Dragon.Copilot.Models/ActionButtonType.cs | Refactored enum from generic button styles to semantic action types that better describe button purpose |
| samples/DragonCopilot/Workflow/SampleExtension.Web/Services/ProcessingService.cs | Updated sample to use new typed model, corrected adaptive card property casing, added missing properties (PartnerLogo, Code, References), and updated to use semantic ActionButtonType values |
| body = bodyElements.ToArray() | ||
| Type = "AdaptiveCard", | ||
| Version = "1.3", | ||
| Body = bodyElements.ToArray(), |
There was a problem hiding this comment.
[nitpick] The trailing comma after Body = bodyElements.ToArray() is unnecessary since it's the last property in the object initializer. While not incorrect, removing it would be more consistent with typical C# style.
| Body = bodyElements.ToArray(), | |
| Body = bodyElements.ToArray() |
| /// Adaptive card schema URL | ||
| /// </summary> | ||
| [JsonPropertyName("$schema")] | ||
| public string Schema {get; } = "http://adaptivecards.io/schemas/adaptive-card.json"; |
There was a problem hiding this comment.
Missing space between get and } in the property accessor. Should be { get; } for consistency with C# formatting conventions.
| public string Schema {get; } = "http://adaptivecards.io/schemas/adaptive-card.json"; | |
| public string Schema { get; } = "http://adaptivecards.io/schemas/adaptive-card.json"; |
| /// Type of the card (always "AdaptiveCard") | ||
| /// </summary> | ||
| [JsonPropertyName("type")] | ||
| public string Type { get; set; } = "AdaptiveCard"; |
There was a problem hiding this comment.
The Type property has inconsistent accessor pattern. It uses { get; set; } while the similar Schema property (line 18) uses { get; } (getter-only). Since the Type should always be "AdaptiveCard" according to the comment, consider making this getter-only like Schema: public string Type { get; } = "AdaptiveCard";
| public string Type { get; set; } = "AdaptiveCard"; | |
| public string Type { get; } = "AdaptiveCard"; |
| type = "AdaptiveCard", | ||
| version = "1.3", | ||
| body = bodyElements.ToArray() | ||
| Type = "AdaptiveCard", |
There was a problem hiding this comment.
The Type property assignment is redundant since AdaptiveCardPayload.Type already has a default value of "AdaptiveCard". This line can be removed.
| Type = "AdaptiveCard", |
| version = "1.3", | ||
| body = bodyElements.ToArray() | ||
| Type = "AdaptiveCard", | ||
| Version = "1.3", |
There was a problem hiding this comment.
The Version property assignment is redundant since AdaptiveCardPayload.Version already has a default value of "1.3". This line can be removed.
| Version = "1.3", |
brandonpollett
deleted the
personal/brpoll/adaptive-card-sample-updates
branch
No description provided.