-
-
Notifications
You must be signed in to change notification settings - Fork 392
bug Quoted iphone #254
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
bug Quoted iphone #254
Conversation
Summary of ChangesHello @cleitonme, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug related to quoted messages, particularly in scenarios involving iPhones, by enhancing the system's ability to handle full Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces a new optional QuotedMessage field to several message sending endpoints. This allows providing a complete message object for quoting, which fixes an issue with hidden tagging in groups on iPhones, while maintaining backward compatibility.
The implementation is correct and consistent across all affected handlers. My review focuses on improving maintainability by reducing code duplication. I've suggested refactoring the repeated logic for handling quoted messages into helper functions. This will make the codebase cleaner and easier to manage in the future. Overall, this is a good addition to fix the reported bug.
| if t.ContextInfo.StanzaID != nil { | ||
| msg.DocumentMessage.ContextInfo = &waE2E.ContextInfo{ | ||
| StanzaID: proto.String(*t.ContextInfo.StanzaID), | ||
| Participant: proto.String(*t.ContextInfo.Participant), | ||
| QuotedMessage: &waE2E.Message{Conversation: proto.String("")}, | ||
| var qm *waE2E.Message | ||
|
|
||
| // If QuotedMessage was provided, use it. | ||
| if t.QuotedMessage != nil { | ||
| qm = t.QuotedMessage | ||
| } else { | ||
| // Otherwise, it uses the old logic (empty message). | ||
| qm = &waE2E.Message{Conversation: proto.String("")} | ||
| } | ||
|
|
||
| if msg.DocumentMessage.ContextInfo == nil { | ||
| msg.DocumentMessage.ContextInfo = &waE2E.ContextInfo{ | ||
| StanzaID: proto.String(*t.ContextInfo.StanzaID), | ||
| Participant: proto.String(*t.ContextInfo.Participant), | ||
| QuotedMessage: qm, | ||
| } | ||
| } | ||
| } |
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.
There's significant code duplication across several Send... handlers for building the ContextInfo with a quoted message. This logic is repeated in SendAudio, SendImage, SendSticker, SendVideo, SendContact, and SendLocation.
To improve maintainability and reduce redundancy, you could extract this logic into a helper function. For example, you could create a function that takes a pointer to the message's ContextInfo field and sets it accordingly.
Here's an example of what such a helper function could look like:
func (s *server) setQuotedContextInfo(
contextInfo **waE2E.ContextInfo,
reqContextInfo waE2E.ContextInfo,
quotedMessage *waE2E.Message,
) {
if reqContextInfo.StanzaID == nil {
return
}
// Don't overwrite if it's already set (e.g., by another part of the handler)
if *contextInfo != nil {
return
}
qm := quotedMessage
if qm == nil {
qm = &waE2E.Message{Conversation: proto.String("")}
}
*contextInfo = &waE2E.ContextInfo{
StanzaID: proto.String(*reqContextInfo.StanzaID),
Participant: proto.String(*reqContextInfo.Participant),
QuotedMessage: qm,
}
}You could then call it from SendDocument like this:
s.setQuotedContextInfo(&msg.DocumentMessage.ContextInfo, t.ContextInfo, t.QuotedMessage)And similarly for the other 6 handlers, just changing &msg.DocumentMessage.ContextInfo to &msg.AudioMessage.ContextInfo, etc. This would make the code much cleaner and easier to maintain.
| var qm *waE2E.Message | ||
|
|
||
| // If QuotedMessage was provided, use it. | ||
| if t.QuotedMessage != nil { | ||
| qm = t.QuotedMessage | ||
| } else { | ||
| qm.Conversation = proto.String("") | ||
| // Otherwise, use the old logic with QuotedText. | ||
| qm = &waE2E.Message{} | ||
| if t.QuotedText != "" { | ||
| qm.ExtendedTextMessage = &waE2E.ExtendedTextMessage{ | ||
| Text: proto.String(t.QuotedText), | ||
| } | ||
| } else { | ||
| qm.Conversation = proto.String("") | ||
| } | ||
| } |
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.
This logic for determining qm can be simplified by removing the var declaration and the outer if/else structure. This makes the code more concise and easier to read.
qm := t.QuotedMessage
if qm == nil {
// Otherwise, use the old logic with QuotedText.
qm = &waE2E.Message{}
if t.QuotedText != "" {
qm.ExtendedTextMessage = &waE2E.ExtendedTextMessage{
Text: proto.String(t.QuotedText),
}
} else {
qm.Conversation = proto.String("")
}
}…one) - Adicionar campo QuotedMessage opcional em todos os handlers de envio - Usar QuotedMessage se fornecido, senão fallback para mensagem vazia - Baseado no PR asternic#254 do asternic/wuzapi - Corrige problema de citações invisíveis em grupos (especialmente iPhones)
bug Quoted iphone
tulir/whatsmeow#1048 (comment)
Include complete message to avoid hidden tagging errors in groups; I keep it optional for compatibility with the previous version.