feat: Add Metadata Update Flow Control and New State for Connection Monitoring #73
Conversation
lotharking
changed the title
| @@ -454,6 +457,14 @@ export const messageEvents = async (agent: ServiceAgent, config: ServerConfig) = | |||
| }) | |||
|
|
|||
| await sendMessageReceivedEvent(agent, msg, msg.timestamp, config) | |||
|
|
|||
| const body = new ConnectionStateUpdated({ | |||
There was a problem hiding this comment.
With the code base like it is right now, I don't see why are we generating such a new webhook event, since we can catch the data from the ProfileMessage message-received-event. Also, it would not be completely correct to generate a connection-state-update, since we are not actually updating the connection itself within Service Agent's main module (no actual ConnectionRecord metadata is being updated).
Is there any reason why you cannot hook into message received event to process this user profile? If so, would it make sense to create a new webhook event specific for user profile? I propose this thinking on the future, since we want to get rid of current Service Agent main API and if we want to use Credo core directly, we'll get different events for each situation (one for connection metadata updated, another for user profile received).
There was a problem hiding this comment.
Each event is handled independently, and the start state is used. A connection is not considered completed until the capabilities and profile have been validated
| @@ -14,7 +14,7 @@ export class ConnectionEntity { | |||
| status?: ExtendedDidExchangeState | |||
|
|
|||
| @Column({ type: 'varchar', nullable: true }) | |||
| language?: string | |||
| lang?: string | |||
There was a problem hiding this comment.
I think it would be a lot better if, instead of having a plain field with this single element, you could store the whole User Profile here. This way you can store user's display picture, display name and other stuff that could be useful for client apps.
There was a problem hiding this comment.
added new userProfile
| @@ -14,7 +14,7 @@ export class ConnectionEntity { | |||
| status?: ExtendedDidExchangeState | |||
|
|
|||
| @Column({ type: 'varchar', nullable: true }) | |||
| language?: string | |||
| lang?: string | |||
|
|
|||
There was a problem hiding this comment.
More on this: what about creating a specific field for features, which will contain an object with all capabilities, protocols, etc. discovered? This could make nestjs-client a bit more friendly to manage.
There was a problem hiding this comment.
By making good use of the metadata field, I would consider it unnecessary to create a further breakdown, as it currently allows the user to use all the values of the capabilities regardless of their structure.
This proposal would be good to implement, but perhaps standardizing the capabilities in the models would be better. However, since the capabilities are being added to Credo, it might be best to wait until they are standardized.
user profile created for handling params
| if (!conn?.createdTs) { | ||
| throw new Error(`No connection found with id: ${id}. The connection may not have been created properly`) | ||
| } | ||
| const { lang, metadata, createdTs } = conn |
There was a problem hiding this comment.
I find this logic far more complicated (and maybe not as stable) as it could if we'd manage user profile and features in separate events and database fields.
Summary
A concurrency issue was detected when starting the chatbot flow, beginning with the profile submission as specified in the specifications. This issue occurred because the sending of the capabilities was delayed compared to the sending of the profiles with the language, which led to a lack of a clear starting point for the chatbot flow controlled by the NestJS library.
The need to add a new state called "Updated" to the
ExtendedDidExchangeStatewas identified to address this. This state monitors updates to the metadata. This state aims to send the necessary updates to inform the chatbots that the connection contains relevant and useful information.With this new state in place, we can now monitor when the connection is updated, and use this update as an indication of a new connection. This allows the user to map the connection according to their specific interests.
Changes
ExtendedDidExchangeState: Introduced a new state to track connection updates.updateMetadatamethod: Developed a method to update metadata as neededisCompletedmethod for complete validation: Added a method to validate if the connection has been completed after save capabilities and profile.ExtendedDidExchangeState.Updated: Implemented flow control based on the new "Updated" state inExtendedDidExchangeState.Related Issues
Testing
Checklist