fix(chatwoot): Corrige Reabertura de Conversas e Loop de Mensagem de Conexão #2023
Conversation
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
Este commit aborda duas questões críticas na integração com o Chatwoot para melhorar a estabilidade e a experiência do agente. Primeiro, as conversas que já estavam marcadas como "resolvidas" no Chatwoot não eram reabertas automaticamente quando o cliente enviava uma nova mensagem. Isso foi corrigido para que o sistema verifique o status da conversa e a reabra, garantindo que nenhuma nova interação seja perdida. Segundo, um bug no tratamento do evento de conexão fazia com que a mensagem de status "Conexão estabelecida com sucesso" fosse enviada repetidamente, poluindo o histórico da conversa. A lógica foi ajustada para garantir que esta notificação seja enviada apenas uma vez por evento de conexão.
Reviewer's GuideImplements conditional reopening of resolved Chatwoot conversations only when no open ones exist, and adds rate-limiting to connection status notifications to prevent infinite loops. Sequence diagram for conditional reopening of Chatwoot conversationssequenceDiagram
participant User
participant ChatwootService
participant ChatwootClient
participant Cache
User->>ChatwootService: Send new message
ChatwootService->>ChatwootClient: Query contact conversations
ChatwootClient-->>ChatwootService: Return conversations
alt There is an open conversation
ChatwootService->>Cache: Cache conversation ID
ChatwootService-->>User: Use open conversation
else All conversations are resolved
ChatwootService->>ChatwootClient: Reopen last resolved conversation
ChatwootService->>Cache: Cache conversation ID
ChatwootService-->>User: Use reopened conversation
end
Sequence diagram for rate-limited connection status notificationsequenceDiagram
participant "WhatsApp Instance"
participant ChatwootService
participant ChatwootClient
"WhatsApp Instance"->>ChatwootService: connection.update (status: open)
alt QR code count > 0
ChatwootService->>ChatwootClient: Send 'Connection established' message
ChatwootService->>"WhatsApp Instance": Reset QR code count
ChatwootService->>"WhatsApp Instance": Set lastConnectionNotification timestamp
else QR code count == 0
ChatwootService->>"WhatsApp Instance": Check lastConnectionNotification
alt Notification sent too recently
ChatwootService-->>ChatwootClient: Skip notification
end
end
Class diagram for updated ChatwootService connection handlingclassDiagram
class ChatwootService {
+reopenConversation: boolean
+conversationPending: boolean
+waMonitor: object
+cache: object
+getConversationId()
+createBotMessage()
}
class waInstance {
+qrCode: { count: number }
+lastConnectionNotification: number
}
ChatwootService --> waInstance : uses
ChatwootService --> cache : uses
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Refactor the
reopenConversationblock by extracting the conversation lookup and reopen logic into smaller helper methods to reduce nesting and improve readability. - Extract the hardcoded
minIntervalMsvalue into a named constant or configuration parameter to make the notification throttling interval more transparent and adjustable. - Double-check the connection update logic to ensure that notifications are correctly sent again once the throttle interval has elapsed rather than dropped indefinitely.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Refactor the `reopenConversation` block by extracting the conversation lookup and reopen logic into smaller helper methods to reduce nesting and improve readability.
- Extract the hardcoded `minIntervalMs` value into a named constant or configuration parameter to make the notification throttling interval more transparent and adjustable.
- Double-check the connection update logic to ensure that notifications are correctly sent again once the throttle interval has elapsed rather than dropped indefinitely.
## Individual Comments
### Comment 1
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:753-751` </location>
<code_context>
+ let inboxConversation = null;
+
+ if (this.provider.reopenConversation) {
+ inboxConversation = contactConversations.payload.find(
+ (conversation) =>
+ conversation && conversation.status !== 'resolved' && conversation.inbox_id == filterInbox.id,
+ );
+
+ if (inboxConversation) {
+ this.logger.verbose(
+ `Found open conversation in reopenConversation mode: ${JSON.stringify(inboxConversation)}`,
+ );
} else {
inboxConversation = contactConversations.payload.find(
- (conversation) =>
- conversation && conversation.status !== 'resolved' && conversation.inbox_id == filterInbox.id,
+ (conversation) => conversation.inbox_id == filterInbox.id,
);
- this.logger.verbose(`Found conversation: ${JSON.stringify(inboxConversation)}`);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The fallback search for resolved conversations may return conversations with unexpected statuses.
The fallback logic does not filter for 'resolved' status, so it may select conversations with unintended statuses. Please add a check to ensure only conversations with 'resolved' status are considered for reopening.
Suggested implementation:
```typescript
inboxConversation = contactConversations.payload.find(
(conversation) =>
conversation &&
conversation.status === 'resolved' &&
conversation.inbox_id == filterInbox.id,
);
```
```typescript
if (inboxConversation) {
this.logger.verbose(`Found resolved conversation to reopen: ${JSON.stringify(inboxConversation)}`);
if (this.provider.conversationPending && inboxConversation.status !== 'open') {
await client.conversations.toggleStatus({
```
</issue_to_address>
### Comment 2
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:2425-2427` </location>
<code_context>
if (event === 'connection.update') {
if (body.status === 'open') {
+ const waInstance = this.waMonitor.waInstances[instance.instanceName];
// if we have qrcode count then we understand that a new connection was established
- if (this.waMonitor.waInstances[instance.instanceName].qrCode.count > 0) {
+ if (waInstance && waInstance.qrCode.count > 0) {
const msgConnection = i18next.t('cw.inbox.connected');
await this.createBotMessage(instance, msgConnection, 'incoming');
</code_context>
<issue_to_address>
**issue (bug_risk):** Accessing waInstance.qrCode.count assumes qrCode is always defined.
Add a check to confirm waInstance.qrCode is defined before accessing its count property to prevent runtime errors.
</issue_to_address>
### Comment 3
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:2432` </location>
<code_context>
- this.waMonitor.waInstances[instance.instanceName].qrCode.count = 0;
+ waInstance.qrCode.count = 0;
+
+ waInstance.lastConnectionNotification = Date.now();
+
chatwootImport.clearAll(instance);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** lastConnectionNotification is set only when qrCode.count > 0.
Currently, lastConnectionNotification is only updated when qrCode.count > 0, which may cause repeated notifications if the count stays at zero. To maintain correct throttling, update lastConnectionNotification in both branches.
</issue_to_address>
### Comment 4
<location> `src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts:2423-2446` </location>
<code_context>
if (event === 'connection.update') {
if (body.status === 'open') {
const waInstance = this.waMonitor.waInstances[instance.instanceName];
// if we have qrcode count then we understand that a new connection was established
if (waInstance && waInstance.qrCode.count > 0) {
const msgConnection = i18next.t('cw.inbox.connected');
await this.createBotMessage(instance, msgConnection, 'incoming');
waInstance.qrCode.count = 0;
waInstance.lastConnectionNotification = Date.now();
chatwootImport.clearAll(instance);
} else if (waInstance) {
const timeSinceLastNotification = Date.now() - (waInstance.lastConnectionNotification || 0);
const minIntervalMs = 30000; // 30 seconds
if (timeSinceLastNotification < minIntervalMs) {
this.logger.warn(
`Connection notification skipped for ${instance.instanceName} - too frequent (${timeSinceLastNotification}ms since last)`,
);
}
}
}
}
</code_context>
<issue_to_address>
**suggestion (code-quality):** Merge nested if conditions ([`merge-nested-ifs`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/TypeScript/Default-Rules/merge-nested-ifs))
```suggestion
if (event === 'connection.update' && body.status === 'open') {
const waInstance = this.waMonitor.waInstances[instance.instanceName];
// if we have qrcode count then we understand that a new connection was established
if (waInstance && waInstance.qrCode.count > 0) {
const msgConnection = i18next.t('cw.inbox.connected');
await this.createBotMessage(instance, msgConnection, 'incoming');
waInstance.qrCode.count = 0;
waInstance.lastConnectionNotification = Date.now();
chatwootImport.clearAll(instance);
} else if (waInstance) {
const timeSinceLastNotification = Date.now() - (waInstance.lastConnectionNotification || 0);
const minIntervalMs = 30000; // 30 seconds
if (timeSinceLastNotification < minIntervalMs) {
this.logger.warn(
`Connection notification skipped for ${instance.instanceName} - too frequent (${timeSinceLastNotification}ms since last)`,
);
}
}
}
```
<br/><details><summary>Explanation</summary>Reading deeply nested conditional code is confusing, since you have to keep track of which
conditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two `if` conditions can be combined using
`and` is an easy win.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts
Outdated
Show resolved
Hide resolved
src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts
Outdated
Show resolved
Hide resolved
src/api/integrations/chatbot/chatwoot/services/chatwoot.service.ts
Outdated
Show resolved
Hide resolved
…ão de conexão Este commit introduz melhorias na integração com o Chatwoot, focando na reabertura de conversas e na notificação de conexão. A lógica foi refatorada para centralizar a busca por conversas abertas e a reabertura de conversas resolvidas, garantindo que interações não sejam perdidas. Além disso, foi implementado um intervalo mínimo para notificações de conexão, evitando mensagens excessivas e melhorando a experiência do usuário.
Este commit modifica a lógica de verificação de conversas no serviço Chatwoot, garantindo que a busca por conversas ativas seja priorizada em relação ao uso de cache. A verificação de cache foi removida em pontos críticos para evitar que conversas desatualizadas sejam utilizadas, melhorando a precisão na recuperação de dados. Além disso, a lógica de reabertura de conversas foi refinada para garantir que as interações sejam tratadas corretamente, mantendo a experiência do usuário mais fluida.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This Pull Request resolves two distinct issues in the Chatwoot integration, aiming to improve the agent experience and increase service stability. The fixes ensure that resolved conversations are reopened when new messages are received, if and only if there are no open conversations for that contact, and eliminate a loop of connection status notifications.
Fixes:
Problem: When a customer sent a new message to a conversation that had already been marked as “resolved” in the Chatwoot panel, the conversation was automatically reopened even if there were open conversations for the same contact. As a result, the new message ended up being associated with an old conversation.
Solution: A check has been implemented in the Chatwoot service that, before forwarding a new message, queries the status of the corresponding conversations. If all conversations are marked as resolved, it reopens the last conversation used. If there are any open conversations, it will use the first open conversation it finds.
Elimination of Infinite Loop in Connection Status Message:
Summary by Sourcery
Ensure resolved Chatwoot conversations are only reopened when no open conversation exists, and throttle Chatwoot connection notifications to prevent message loops.
Bug Fixes: