Merge use storage feature/capability into main#89
Merged
Conversation
Member
|
🧪 Details on Ubuntu 24.04.2 LTSfrom dotnet-retest v0.7.1 on .NET 8.0.16 with 💜 by @devlooped |
There was a problem hiding this comment.
Pull Request Overview
This PR integrates a storage capability into the WhatsApp handler pipeline, updates the handler builder to include storage and send handlers, and refines response types to persist and expose message IDs.
- Introduce
IStorageServiceandStorageServicefor persisting incoming messages and responses. - Enhance
WhatsAppHandlerBuilderand service collection extensions to wire up storage and send handlers in the pipeline. - Update response records to assign and expose
Idwhen sending messages.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/WhatsApp/WhatsAppServiceCollectionExtensions.cs | Configure storage and send handlers via the builder pipeline. |
| src/WhatsApp/WhatsAppHandlerBuilder.cs | Capture and expose IServiceCollection in the handler builder. |
| src/WhatsApp/WhatsApp.csproj | Reordered package references and added Devlooped.TableStorage. |
| src/WhatsApp/TextResponse.cs | Awaited ReplyAsync, assigned Id, and enriched documentation. |
| src/WhatsApp/TemplateResponse.cs | Expanded XML docs for template-based responses. |
| src/WhatsApp/StorageService.cs | New primary-constructor storage service implementing IStorageService. |
| src/WhatsApp/StorageHandlerExtensions.cs | Added UseStorage extension to register the storage service. |
| src/WhatsApp/SendResponsesHandler.cs | New handler for sending responses through IWhatsAppClient. |
| src/WhatsApp/ResponseStorageHandler.cs | New handler for storing outgoing responses. |
| src/WhatsApp/Response.cs | Made Response implement IMessage, added Id/Number props. |
| src/WhatsApp/ReactionResponse.cs | Updated base constructor call to pass the message upward. |
| src/WhatsApp/MessageStorageHandler.cs | New handler to persist incoming UserMessage instances. |
| src/WhatsApp/Message.cs | Made Message implement IMessage and added Number prop. |
| src/WhatsApp/IStorageService.cs | Introduced the IStorageService interface. |
| src/WhatsApp/IMessage.cs | Introduced the IMessage interface for polymorphic messages/responses. |
| src/WhatsApp/AzureFunctions.cs | Switched to ToArrayAsync() to drive the full handler pipeline. |
| src/SampleApp/Sample/Program.cs | Registered storage in the sample app and updated startup calls. |
Comments suppressed due to low confidence (1)
src/SampleApp/Sample/Program.cs:4
- [nitpick] The
using Devlooped;directive appears unused; consider removing it to clean up imports.
using Devlooped;
|
|
||
| /// <summary> | ||
| /// A simple text response to a user message. | ||
| /// Represents a response containing text and optional interactive buttons, which can be sent as a reply to a message. |
There was a problem hiding this comment.
[nitpick] Remove the extra space before 'which' to maintain consistent spacing in the XML comment.
Suggested change
| /// Represents a response containing text and optional interactive buttons, which can be sent as a reply to a message. | |
| /// Represents a response containing text and optional interactive buttons, which can be sent as a reply to a message. |
| /// <remarks>This abstract record serves as a base type for specific response implementations. It encapsulates the | ||
| /// message being sent and provides functionality for sending the response asynchronously using a WhatsApp | ||
| /// client.</remarks> | ||
| /// <param name="Message"></param> |
There was a problem hiding this comment.
The <param name="Message"> tag is empty; please provide a description such as 'The message being wrapped by this response.'
Comment on lines
+54
to
+79
| builder.Use((inner, services) => | ||
| { | ||
| // Check if the storage capability was enabled by getting the storage service | ||
| if (services.GetService<IStorageService>() is IStorageService storageService) | ||
| { | ||
| return new ResponseStorageHandler(inner, storageService); | ||
| } | ||
|
|
||
| return WhatsAppHandler.Empty; | ||
| }); | ||
|
|
||
| // Add the handler for sending responses | ||
| builder.Use((inner, services) => new SendResponsesHandler(inner, services.GetRequiredService<IWhatsAppClient>())); | ||
|
|
||
| // Add storage handler for incoming messages | ||
| builder.Use((inner, services) => | ||
| { | ||
| // Check if the storage capability was enabled by getting the storage service | ||
| if (services.GetService<IStorageService>() is IStorageService storageService) | ||
| { | ||
| return new MessageStorageHandler(inner, storageService); | ||
| } | ||
|
|
||
| return WhatsAppHandler.Empty; | ||
| }); | ||
|
|
There was a problem hiding this comment.
[nitpick] The storage-check lambda logic is duplicated for both response and message handlers; consider extracting it into a shared helper or extension to reduce duplication.
Suggested change
| builder.Use((inner, services) => | |
| { | |
| // Check if the storage capability was enabled by getting the storage service | |
| if (services.GetService<IStorageService>() is IStorageService storageService) | |
| { | |
| return new ResponseStorageHandler(inner, storageService); | |
| } | |
| return WhatsAppHandler.Empty; | |
| }); | |
| // Add the handler for sending responses | |
| builder.Use((inner, services) => new SendResponsesHandler(inner, services.GetRequiredService<IWhatsAppClient>())); | |
| // Add storage handler for incoming messages | |
| builder.Use((inner, services) => | |
| { | |
| // Check if the storage capability was enabled by getting the storage service | |
| if (services.GetService<IStorageService>() is IStorageService storageService) | |
| { | |
| return new MessageStorageHandler(inner, storageService); | |
| } | |
| return WhatsAppHandler.Empty; | |
| }); | |
| builder.Use((inner, services) => | |
| CreateStorageHandler(inner, services, (i, s) => new ResponseStorageHandler(i, s))); | |
| // Add the handler for sending responses | |
| builder.Use((inner, services) => new SendResponsesHandler(inner, services.GetRequiredService<IWhatsAppClient>())); | |
| // Add storage handler for incoming messages | |
| builder.Use((inner, services) => | |
| CreateStorageHandler(inner, services, (i, s) => new MessageStorageHandler(i, s))); |
kzu
reviewed
May 30, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #87