-
-
Notifications
You must be signed in to change notification settings - Fork 2
Minor CallToAction cleanup #230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
kzu
commented
Jul 9, 2025
- SendXXX is used when there isn't a clear verb we can use. In this case, CallToAction is already a verb/action so we don't need the Send prefix (just like Reply/React).
- Calls to action can (for now?) never be a reply to anything, they are standalone messages. Remove the Context everywhere therefore, since we don't use it at all anyway in the actual sending implementation.
- Unify naming on Action for the button text.
🧪 Details on Ubuntu 24.04.2 LTSfrom dotnet-retest v0.7.1 on .NET 8.0.18 with 💜 by @devlooped |
- SendXXX is used when there isn't a clear verb we can use. In this case, CallToAction is already a verb/action so we don't need the Send prefix (just like Reply/React). - Calls to action can (for now?) never be a reply to anything, they are standalone messages. Remove the Context everywhere therefore, since we don't use it at all anyway in the actual sending implementation. - Unify naming on Action for the button text.
There was a problem hiding this 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 refactors the CallToAction API to improve naming consistency and remove unused context functionality. The changes align the CallToAction naming pattern with other message types like Reply/React that don't use a "Send" prefix, and simplify the API by removing context parameters that aren't utilized.
- Renamed
SendCallToActionAsynctoCallToActionAsyncto follow consistent naming patterns - Unified parameter naming from
buttonTexttoactionacross all CallToAction methods - Removed unused
Contextparameter from CallToActionResponse constructors and method calls
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| WhatsAppClientExtensions.cs | Updates method name and parameter naming for CallToAction extension method |
| MessageExtensions.cs | Removes context parameter from CallToActionResponse constructor calls and adds documentation |
| CallToActionResponse.cs | Refactors record definition to remove context parameter and unify action parameter naming |
| /// <param name="Text">The content of the message calling to action.</param> | ||
| /// <param name="Action">The action button text.</param> | ||
| /// <param name="Url">The URL to navigate to when the action button is clicked.</param> | ||
| /// /// <param name="ConversationId">The conversation id where this response was generated</param> |
Copilot
AI
Jul 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are extra forward slashes in the XML documentation comment. It should be '/// ...' instead of '/// /// ...'
| /// /// <param name="ConversationId">The conversation id where this response was generated</param> | |
| /// <param name="ConversationId">The conversation id where this response was generated</param> |