Merge repo oficial #1922
Closed
Merge repo oficial #1922
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
Contributor
Reviewer's GuideThis PR merges changes from the official repo, introducing a normalizeJid utility, robust session clearing, enhanced logging, automated PENDING status reconciliation, refactored read/ status handling with caching, improved on-WhatsApp number verification, default speech-to-text toggles and credential management in the OpenAI controller, and adds database migrations to align defaults. Sequence diagram for automatic PENDING message status reconciliationsequenceDiagram
participant BaileysStartupService
participant PrismaRepository
participant Logger
participant Webhook
BaileysStartupService->>Logger: Log PENDING message sent
BaileysStartupService->>BaileysStartupService: setTimeout(30s)
BaileysStartupService->>PrismaRepository: findFirst(message with status PENDING)
alt still pending
BaileysStartupService->>Logger: Warn forcing status update
BaileysStartupService->>PrismaRepository: update(message.status = SERVER_ACK)
BaileysStartupService->>Webhook: sendDataWebhook(MESSAGES_UPDATE)
else not pending
BaileysStartupService->>Logger: No action
end
Sequence diagram for OpenaiController credential creation and default settingsequenceDiagram
participant User as actor
participant OpenaiController
participant CredsRepository
participant SettingsRepository
participant Logger
User->>OpenaiController: createCreds(data)
OpenaiController->>CredsRepository: findFirst(name exists?)
alt name exists
OpenaiController->>Logger: error
OpenaiController-->>User: throw error
else name does not exist
OpenaiController->>CredsRepository: create(data)
OpenaiController->>SettingsRepository: findFirst(instanceId)
alt settings exist
OpenaiController->>SettingsRepository: update(OpenaiCreds connect)
else settings do not exist
OpenaiController->>SettingsRepository: create(OpenaiCreds connect)
end
OpenaiController-->>User: return creds
end
Flow diagram for onWhatsappCache number normalization and prioritizationflowchart TD
A["Input remoteJid"] --> B["Check if Brazilian number"]
B -- Yes --> C["Prioritize format with 9"]
C --> D["Add with 9 @domain"]
C --> E["Add without 9 @domain"]
B -- No --> F["Check if Mexican/Argentinian number"]
F -- Yes --> G["Handle prefix and digit"]
G --> H["Add with digit @domain"]
G --> I["Add without digit @domain"]
F -- No --> J["Other countries"]
J --> K["Add remoteJid as is"]
D & E & H & I & K --> L["Return numbersAvailable"]
File-Level Changes
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}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Instead of hardcoding the 30-second timeout for forcing PENDING messages to SERVER_ACK, extract it into a configurable constant or environment variable so it can be tuned without code changes.
- clearCorruptedSessionData currently deletes all cache entries with broad patterns on every startup—consider narrowing its scope to only truly corrupted keys to avoid unintended data loss and heavy cache operations.
- The normalizeJid implementation relies on simple string splits and includes checks—consider centralizing JID parsing into a shared utility or library and handling edge cases like whitespace or unexpected suffixes for greater robustness.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of hardcoding the 30-second timeout for forcing PENDING messages to SERVER_ACK, extract it into a configurable constant or environment variable so it can be tuned without code changes.
- clearCorruptedSessionData currently deletes all cache entries with broad patterns on every startup—consider narrowing its scope to only truly corrupted keys to avoid unintended data loss and heavy cache operations.
- The normalizeJid implementation relies on simple string splits and includes checks—consider centralizing JID parsing into a shared utility or library and handling edge cases like whitespace or unexpected suffixes for greater robustness.
## Individual Comments
### Comment 1
<location> `src/utils/onWhatsappCache.ts:20` </location>
<code_context>
number.slice(4, 5) === '9' && number.length === 13 ? number : `${number.slice(0, 4)}9${number.slice(4)}`;
const numberWithoutDigit = number.length === 12 ? number : number.slice(0, 4) + number.slice(5);
- numbersAvailable.push(numberWithDigit);
- numbersAvailable.push(numberWithoutDigit);
+ // Add the format WITH 9 first (prioritized)
+ numbersAvailable.push(`${numberWithDigit}@${domain || 's.whatsapp.net'}`);
+ // Add the format WITHOUT 9 second (fallback)
+ numbersAvailable.push(`${numberWithoutDigit}@${domain || 's.whatsapp.net'}`);
</code_context>
<issue_to_address>
Avoid duplicate domain appending in getAvailableNumbers.
Check that numbers passed to this function do not already include a domain to prevent appending it twice.
Suggested implementation:
```typescript
// Add the format WITH 9 first (prioritized)
if (!numberWithDigit.includes('@')) {
numbersAvailable.push(`${numberWithDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithDigit);
}
// Add the format WITHOUT 9 second (fallback)
if (!numberWithoutDigit.includes('@')) {
numbersAvailable.push(`${numberWithoutDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithoutDigit);
}
```
```typescript
if (!numberWithDigit.includes('@')) {
numbersAvailable.push(`${numberWithDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithDigit);
}
if (!numberWithoutDigit.includes('@')) {
numbersAvailable.push(`${numberWithoutDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithoutDigit);
}
```
</issue_to_address>
### Comment 2
<location> `src/api/integrations/chatbot/openai/controllers/openai.controller.ts:239` </location>
<code_context>
if (!data.name) throw new BadRequestException('Name is required');
// Check if API key already exists
- const existingApiKey = await this.credsRepository.findFirst({
- where: {
- apiKey: data.apiKey,
- },
- });
+ // const existingApiKey = await this.credsRepository.findFirst({
+ // where: {
+ // apiKey: data.apiKey,
+ // },
+ // });
- if (existingApiKey) {
- throw new BadRequestException('This API key is already registered. Please use a different API key.');
- }
</code_context>
<issue_to_address>
Reconsider removal of duplicate API key check.
If allowing duplicate API keys, confirm that downstream processes are robust against this and that no security or operational issues will arise.
</issue_to_address>
### Comment 3
<location> `prisma/mysql-migrations/20250709000000_change_speech_to_text_default_to_true/migration.sql:5` </location>
<code_context>
+ALTER TABLE `OpenaiSetting` MODIFY COLUMN `speechToText` BOOLEAN NULL DEFAULT true;
+
+-- Update existing records to use the new default
+UPDATE `OpenaiSetting` SET `speechToText` = true WHERE `speechToText` IS NULL OR `speechToText` = false;
</code_context>
<issue_to_address>
Migration may overwrite intentional false values.
Currently, the migration updates both NULL and false values to true. If false values are intentional, you may want to update only NULL values instead.
</issue_to_address>
### Comment 4
<location> `prisma/postgresql-migrations/20250709000000_change_speech_to_text_default_to_true/migration.sql:5` </location>
<code_context>
+ALTER TABLE "OpenaiSetting" ALTER COLUMN "speechToText" SET DEFAULT true;
+
+-- Update existing records to use the new default
+UPDATE "OpenaiSetting" SET "speechToText" = true WHERE "speechToText" IS NULL OR "speechToText" = false;
</code_context>
<issue_to_address>
Migration may overwrite intentional false values.
This update will change all false or NULL values to true, potentially overriding intentional false settings. Please confirm if this aligns with the desired migration behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
-20
to
-21
| numbersAvailable.push(numberWithDigit); | ||
| numbersAvailable.push(numberWithoutDigit); |
Contributor
There was a problem hiding this comment.
suggestion (bug_risk): Avoid duplicate domain appending in getAvailableNumbers.
Check that numbers passed to this function do not already include a domain to prevent appending it twice.
Suggested implementation:
// Add the format WITH 9 first (prioritized)
if (!numberWithDigit.includes('@')) {
numbersAvailable.push(`${numberWithDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithDigit);
}
// Add the format WITHOUT 9 second (fallback)
if (!numberWithoutDigit.includes('@')) {
numbersAvailable.push(`${numberWithoutDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithoutDigit);
} if (!numberWithDigit.includes('@')) {
numbersAvailable.push(`${numberWithDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithDigit);
}
if (!numberWithoutDigit.includes('@')) {
numbersAvailable.push(`${numberWithoutDigit}@${domain || 's.whatsapp.net'}`);
} else {
numbersAvailable.push(numberWithoutDigit);
}
Comment on lines
-239
to
-245
| const existingApiKey = await this.credsRepository.findFirst({ | ||
| where: { | ||
| apiKey: data.apiKey, | ||
| }, | ||
| }); | ||
| // const existingApiKey = await this.credsRepository.findFirst({ | ||
| // where: { | ||
| // apiKey: data.apiKey, | ||
| // }, | ||
| // }); | ||
|
|
||
| if (existingApiKey) { |
Contributor
There was a problem hiding this comment.
🚨 question (security): Reconsider removal of duplicate API key check.
If allowing duplicate API keys, confirm that downstream processes are robust against this and that no security or operational issues will arise.
| ALTER TABLE `OpenaiSetting` MODIFY COLUMN `speechToText` BOOLEAN NULL DEFAULT true; | ||
|
|
||
| -- Update existing records to use the new default | ||
| UPDATE `OpenaiSetting` SET `speechToText` = true WHERE `speechToText` IS NULL OR `speechToText` = false; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Migration may overwrite intentional false values.
Currently, the migration updates both NULL and false values to true. If false values are intentional, you may want to update only NULL values instead.
| ALTER TABLE "OpenaiSetting" ALTER COLUMN "speechToText" SET DEFAULT true; | ||
|
|
||
| -- Update existing records to use the new default | ||
| UPDATE "OpenaiSetting" SET "speechToText" = true WHERE "speechToText" IS NULL OR "speechToText" = false; |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Migration may overwrite intentional false values.
This update will change all false or NULL values to true, potentially overriding intentional false settings. Please confirm if this aligns with the desired migration behavior.
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.
Summary by Sourcery
Improve WhatsApp Baileys integration with JID normalization, corrupted session clearing, enriched logging, automatic PENDING-to-SERVER_ACK transitions, and robust message status handling. Enhance OpenAI controller by defaulting speech-to-text, auto-assigning new credentials, and updating Prisma schema for the new default.
New Features:
Bug Fixes:
Enhancements:
Build: