-
Notifications
You must be signed in to change notification settings - Fork 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
enable sending voice message through messageSendFile() #20
Conversation
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.
Thank you very much for adding audio support to our Puppt Official Account!
Please follow my reviews and make the CI green, then it will be ready to be merged.
And please link to the related issue in the PR so we can group all related information together.
Link to: #19
}): Promise<string> { | ||
log.verbose('OfficialAccount', 'sendFile(%s)', JSON.stringify(args)) | ||
// log.verbose('OfficialAccount', 'sendFile(%s)', JSON.stringify(args)) |
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 keep this log verbose output, because we always log a function when it's being called.
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.
ok, i uncommented it. just to note that this line does not work for audio file such as .mp3, and will throw an error.
src/puppet-oa.ts
Outdated
@@ -535,7 +532,8 @@ class PuppetOA extends Puppet { | |||
conversationId: string, | |||
file : FileBox, | |||
): Promise<string> { | |||
return this.messageSend(conversationId, file) | |||
const msgtype: OAMediaType = (file.mimeType === 'image/jpeg') ? 'image' : 'voice' |
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.
Can we make this logic more robust?
I'd like to suggest that:
- we do a
switch
for checkingfile.mimeType
and set theMsgType
correctly - we throw an exception for the unknown
mimeType
instead of failing silently
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.
thanks for the suggestions, have updated
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.
LGTM
Thank you very much for your contribution! You are welcome to join Wechaty Contributor Program1. Join Wechaty Organization
I have invited you to join our Wechaty GitHub Organization, please accept it by following the above message. 2. Update Your Wechaty Contributor ProfilePlease open Contributor Hall of Fame and add yourself to the end of the list, so that other contributors will know you better! 3. Join The Contributor Only WeChat RoomWe also have a WeChat room for contributors only which can discuss Wechaty at a deeper level, you are welcome to join and if you are interested. Please add @lijiarui wechat: ruirui_0914 and send her this pr link. She will invite you into Wechaty Contributor Room Cheers! |
This PR has been published on wechaty-puppet-office-account@0.5.42 |
Support sending amr/mp3 files through messageSendFile(), will check the mimetype of the Filebox file, and correspondingly adjust the request header sent to wechat OA server.
Test plan:
Anticipated result: user should receive a voice message (recorded in 'xxx.mp3') from the wechat OA.
Link to #19