Skip to content

Conversation

@adalon
Copy link
Collaborator

@adalon adalon commented May 30, 2025

Fixes #87

@adalon adalon requested review from Copilot and kzu May 30, 2025 20:12
@kzu
Copy link
Member

kzu commented May 30, 2025

32 passed 32 passed 7 skipped

🧪 Details on Ubuntu 24.04.2 LTS

from dotnet-retest v0.7.1 on .NET 8.0.16 with 💜 by @devlooped

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 IStorageService and StorageService for persisting incoming messages and responses.
  • Enhance WhatsAppHandlerBuilder and service collection extensions to wire up storage and send handlers in the pipeline.
  • Update response records to assign and expose Id when 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.
Copy link

Copilot AI May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copilot uses AI. Check for mistakes.
/// <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>
Copy link

Copilot AI May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The <param name="Message"> tag is empty; please provide a description such as 'The message being wrapped by this response.'

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

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;
});

Copy link

Copilot AI May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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)));

Copilot uses AI. Check for mistakes.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not now

@kzu kzu enabled auto-merge (rebase) May 30, 2025 20:27
@kzu kzu merged commit 7f97eee into main May 30, 2025
4 checks passed
@kzu kzu deleted the dev/adalonso/UseStorage branch May 30, 2025 20:27
@kzu kzu restored the dev/adalonso/UseStorage branch May 30, 2025 20:31
@kzu kzu deleted the dev/adalonso/UseStorage branch May 30, 2025 20:36
@devlooped devlooped locked and limited conversation to collaborators Jun 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose IServiceCollection in the WhatsAppHandlerBuilder

3 participants