-
Notifications
You must be signed in to change notification settings - Fork 95
Log webhook #7
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
base: main
Are you sure you want to change the base?
Log webhook #7
Conversation
…ename no envio de mídia document em WABA
fix sending media in groups where the group number is greater than 23…
Reviewer's GuideThis PR enhances the WhatsApp Baileys integration by logging and overriding the WebVersion dynamically, tightening cache settings, enriching and streamlining webhook payloads (including initial history and group metadata), standardizing media handling, and exposing new instance- and media-download endpoints, alongside cleanup and configuration updates. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @jhonsw1 - I've reviewed your changes - here's some feedback:
- Remove large blocks of commented-out logic in the Baileys and Business services or extract them behind feature flags to keep the codebase clean and maintainable.
- Replace raw console.log/console.error calls with the configured this.logger at the appropriate levels for consistent logging.
- Ensure the new historySetData parameter added to sendDataWebhook is propagated through all event emitters (webhook, websocket, RabbitMQ) and update their interfaces accordingly.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
}); | ||
const webVersion = integrationData.webVersion | ||
? integrationData.webVersion.split('.').map(Number) | ||
: version |
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.
issue (bug_risk): Fallback for webVersion
uses version
string directly
Always convert the fallback version
to a number[]
(e.g., version.split('.').map(Number)
) to avoid passing a string where an array is expected.
received.message?.pollUpdateMessage || | ||
!received?.message | ||
) { | ||
continue; | ||
return; |
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.
issue: Using return
inside the loop exits the entire handler
Use continue
here so you skip this message but still process the rest of the batch.
@@ -257,7 +257,7 @@ export class ChannelStartupService { | |||
return data; | |||
} | |||
|
|||
public async sendDataWebhook<T = any>(event: Events, data: T, local = true, integration?: string[]) { | |||
public async sendDataWebhook<T = any>(event: Events, data: T, local = true, integration?: string[], historySetData?: wa.HistorySetData) { |
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.
issue (bug_risk): New historySetData
parameter may not be handled by all event channels
Update WebsocketController and other event emitters to forward historySetData as well; otherwise non-webhook integrations will drop it.
@@ -131,7 +131,8 @@ export class InstanceController { | |||
throw new BadRequestException('number is required'); | |||
} | |||
const urlServer = this.configService.get<HttpServer>('SERVER').URL; | |||
webhookWaBusiness = `${urlServer}/webhook/meta`; | |||
const webHookTest = this.configService.get<WaBusiness>('WA_BUSINESS').WEBHOOK_TEST; | |||
webhookWaBusiness = `${webHookTest ? webHookTest : urlServer}/webhook/meta`; |
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.
suggestion (code-quality): Avoid unneeded ternary statements (simplify-ternary
)
webhookWaBusiness = `${webHookTest ? webHookTest : urlServer}/webhook/meta`; | |
webhookWaBusiness = `${webHookTest || urlServer}/webhook/meta`; |
Explanation
It is possible to simplify certain ternary statements into either use of an||
or !
.This makes the code easier to read, since there is no conditional logic.
const response = { | ||
id: instanceByName.id, | ||
name: instanceByName.name, | ||
ownerJid: instanceByName.ownerJid, | ||
connectionStatus: instanceByName.connectionStatus, | ||
profileName: instanceByName.profileName, | ||
} | ||
return response; |
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.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable
)
const response = { | |
id: instanceByName.id, | |
name: instanceByName.name, | |
ownerJid: instanceByName.ownerJid, | |
connectionStatus: instanceByName.connectionStatus, | |
profileName: instanceByName.profileName, | |
} | |
return response; | |
return { | |
id: instanceByName.id, | |
name: instanceByName.name, | |
ownerJid: instanceByName.ownerJid, | |
connectionStatus: instanceByName.connectionStatus, | |
profileName: instanceByName.profileName, | |
}; | |
Explanation
Something that we often see in people's code is assigning to a result variableand then immediately returning it.
Returning the result directly shortens the code and removes an unnecessary
variable, reducing the mental load of reading the function.
Where intermediate variables can be useful is if they then get used as a
parameter or a condition, and the name can act like a comment on what the
variable represents. In the case where you're returning it from a function, the
function name is there to tell you what the result is, so the variable name
is unnecessary.
if (!findMessage) { | ||
return; | ||
} | ||
if (item.status === 'read' && key.fromMe) return; |
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.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces
)
if (item.status === 'read' && key.fromMe) return; | |
if (item.status === 'read' && key.fromMe) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
@@ -954,7 +940,7 @@ export class BusinessStartupService extends ChannelStartupService { | |||
public async mediaMessage(data: SendMediaDto, file?: any) { | |||
const mediaData: SendMediaDto = { ...data }; | |||
|
|||
if (file) mediaData.media = file.buffer.toString('base64'); | |||
if (file) mediaData.media = file.buffer; |
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.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces
)
if (file) mediaData.media = file.buffer; | |
if (file) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
} else if(received.message?.protocolMessage){ | ||
if(!received.message?.protocolMessage?.editedMessage){ | ||
this.logger.verbose('message rejected'); | ||
return; | ||
} | ||
} |
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.
suggestion (code-quality): Merge nested if conditions (merge-nested-ifs
)
} else if(received.message?.protocolMessage){ | |
if(!received.message?.protocolMessage?.editedMessage){ | |
this.logger.verbose('message rejected'); | |
return; | |
} | |
} | |
} else if (received.message?.protocolMessage && !received.message?.protocolMessage?.editedMessage) { | |
this.logger.verbose('message rejected'); | |
return; | |
} | |
Explanation
Reading deeply nested conditional code is confusing, since you have to keep track of whichconditions relate to which levels. We therefore strive to reduce nesting where
possible, and the situation where two
if
conditions can be combined usingand
is an easy win.
|
||
if (file) mediaData.media = file.buffer.toString('base64'); | ||
if (file) mediaData.media = file.buffer; |
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.
suggestion (code-quality): Use block braces for ifs, whiles, etc. (use-braces
)
if (file) mediaData.media = file.buffer; | |
if (file) { |
Explanation
It is recommended to always use braces and create explicit statement blocks.Using the allowed syntax to just write a single statement can lead to very confusing
situations, especially where subsequently a developer might add another statement
while forgetting to add the braces (meaning that this wouldn't be included in the condition).
}, | ||
); | ||
|
||
return buffer.toString('base64');; |
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.
suggestion (code-quality): Remove unreachable code. (remove-unreachable-code
)
return buffer.toString('base64');; | |
return buffer.toString('base64'); |
Explanation
Statements after areturn
, break
, continue
or throw
will never be executed.Leaving them in the code confuses the reader, who may believe that these
statements have some effect. They should therefore be removed.
Summary by Sourcery
Introduce webhook enhancements and history sync to WhatsApp Baileys integration, add new API endpoints for instance management and media download, and remove legacy database writes.
New Features:
Bug Fixes:
Enhancements:
Build:
Chores: