feat: exposes method for updating connection messages and get contact information#76
feat: exposes method for updating connection messages and get contact information#76vieiraricardo wants to merge 1 commit intoreleases/v1.1.0from
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request enhances the SuperDapp Agents SDK by adding message update capabilities and contact retrieval functionality. The changes enable agents to update previously sent messages and fetch contact information by Cognito ID, while also improving the internal routing logic for channel messages.
Changes:
- Added PUT HTTP method support to the client's fetch implementation
- Implemented
getContactByCognitoId()method for retrieving contact information via internal API route - Updated
updateConnectionMessage()to useSendMessageOptionsparameter type and added fetch API support (previously only supported axios) - Added
updateConnectionMessage()wrapper method toSuperDappAgentfor easier message updates - Enhanced
sendReplyMarkupMessage()to properly route channel messages through the channel endpoint
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/core/client.ts | Added PUT method support, implemented getContactByCognitoId method, updated updateConnectionMessage to use SendMessageOptions with dual fetch/axios support |
| src/core/agent.ts | Added updateConnectionMessage wrapper method and improved sendReplyMarkupMessage to route channel messages correctly |
| async updateConnectionMessage( | ||
| connectionId: string, | ||
| messageId: string, | ||
| message: string | { body: string } | ||
| options: SendMessageOptions | ||
| ): Promise<ApiResponse> { |
There was a problem hiding this comment.
The JSDoc comment on line 342 states "Accepts a string or an object with { body }" but the parameter type has been changed to SendMessageOptions, which is more restrictive. The comment should be updated to accurately reflect that this method now expects a SendMessageOptions object containing a message property with a MessageBody structure and an optional isSilent boolean.
| async updateConnectionMessage( | ||
| connectionId: string, | ||
| messageId: string, | ||
| message: string | { body: string } | ||
| options: SendMessageOptions | ||
| ): Promise<ApiResponse> { | ||
| const payload = { message }; | ||
| if (this.useFetch) { | ||
| return this.fetchJson( | ||
| 'PUT', | ||
| `${AGENT_BOTS_CONNECTIONS_ENDPOINT}/${encodeURIComponent(connectionId)}/messages/${encodeURIComponent(messageId)}`, | ||
| options | ||
| ); | ||
| } | ||
| const response = await this.axios.put( | ||
| `${AGENT_BOTS_CONNECTIONS_ENDPOINT}/${encodeURIComponent( | ||
| connectionId | ||
| )}/messages/${encodeURIComponent(messageId)}`, | ||
| payload | ||
| options | ||
| ); | ||
| return response.data; | ||
| } |
There was a problem hiding this comment.
This creates an API design inconsistency: updateConnectionMessage now uses SendMessageOptions with a structured format (message body + isSilent), while updateChannelMessage (line 382-395) still uses the simpler string | { body: string } signature. For consistency and maintainability, these two methods should follow the same pattern. Consider updating updateChannelMessage to also use SendMessageOptions and add fetch support similar to this implementation.
| } | ||
|
|
||
| /** | ||
| * update connection message (DM) |
There was a problem hiding this comment.
The comment should start with a capital letter and end with a period for consistency with other JSDoc comments in this file. It should be: "Update connection message (DM)"
| * update connection message (DM) | |
| * Update connection message (DM). |
| /** | ||
| * update connection message (DM) | ||
| */ | ||
|
|
There was a problem hiding this comment.
There's an extra blank line (line 109) between the JSDoc comment and the method declaration. This inconsistency should be removed to match the pattern used elsewhere in the file where JSDoc comments are immediately followed by the method declaration without blank lines.
| /** | ||
| * Get contact by cognito id (agent-bots internal route) | ||
| */ | ||
| async getContactByCognitoId(cognitoId: string): Promise<ApiResponse> { | ||
| const path = `${AGENT_BOTS_ENDPOINT}contacts/by-cognito/${encodeURIComponent(cognitoId)}`; | ||
| if (this.useFetch) { | ||
| return this.fetchJson('GET', path); | ||
| } | ||
| const response = await this.axios.get(path); | ||
| return response.data; | ||
| } |
There was a problem hiding this comment.
The new getContactByCognitoId method lacks test coverage. While other client methods in the test suite (like getMe, sendChannelMessage) have existence checks, this new method should have a test case verifying its existence and behavior, consistent with the testing pattern used for other API methods in src/tests/client.test.ts.
1 participant
No description provided.