-
Notifications
You must be signed in to change notification settings - Fork 72
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
fix recalled message #81
Conversation
@wxul Thanks, you did a great job! The analysis looks good and the code basically should work. This issue is related to wechaty/wechaty#1728, so please go to there and have a look, and also I'd like to invite @windmemory to review this PR for you. @windmemory Would you like to review this PR so that we can make sure it will compatible with your PR at wechaty/wechaty#1735? Thank you very much. |
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 PR makes sense to me.
src/wechaty-bro.js
Outdated
|
||
// add recalled message | ||
m.MsgType = confFactory.MSGTYPE_RECALLED | ||
m.MMActualContent = JSON.stringify(revokemsg) |
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.
From the design in wechaty/wechaty#1735, the content should be the recalled message id, so looks like here should be:
m.MMActualContent = revokemsg.msgid
Then using the toRecalled
method on Message
instance, the recalled message can be retrieved back.
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.
Yes, I think so.
Please go to review interface and add your suggestions as a required change for this PR, thanks!
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.
Please revise the code according to the comments in the code.
最近使用发现撤回消息不会被触发, 在分析源码和进行调试后发现有以下几个问题并提交此PR修复:
MessageType.Recalled
will not be triggered. After analyzing the source code and debugging, I found the following problems and submitted this PR to fix:<br/>
导致xml2json
解析失败 (The<br/>
tag in message text causingxml2json
parsing to fail.)src/wechaty-bro.js
MessageType.Text
类型 (WebMessageType.RECALLED
message will be converted toMessageType.Text
)src/pure-function-helpers/web-message-type.ts
MessageType.Recalled
类型的源消息实例, 无法判断其原有类型 (Whenmessage
triggered, the received message is revoked message and it's type was overridden byMessageType.Recalled
.)根据我的设想,
MessageType.Recalled
消息应该保存被撤销消息的id, 然后由用户去获取被撤销的消息实例According to my assumption, user should get message ID from
MessageType.Recalled
message, then find the revoked message.src/wechaty-bro.js
old
new
usage
END
PS: 我看源码的时间并不多, 对wechaty以及puppet的理解也仅限于业务接触到的地方, 如果有不合理的设计, 还望指正.
PS(from Google translate): I don't have much time to look at the source code. The understanding of wechaty and puppet is limited to the places where the business comes into contact. If there is an unreasonable design, I still want to correct me.