fix(puppet-web): send any type file. #714

binsee · 2017-08-08T12:32:30Z

Fix can't send pdf and more type file.
Now, you can use m.say(new MediaMessage(file)) send any type file.

Fix can't send pdf and more type file. Now, you can use `m.say(new MediaMessage(file))` send any type file. Fix wechaty#710

huan

Hi @binsee, Thanks for fixing this issue!

Your code looks great, there's only one problem with the typing of any. Please follow my reviews, I believe the one more job we need to do is to add an Interface for the new MediaData

Please let me know if you have any problem, I will be able to merge this PR after you replace any with the Interface.

Thanks!

huan · 2017-08-08T12:50:59Z

src/puppet-web/bridge.ts

@@ -397,15 +397,14 @@ export class Bridge {
    }
  }

-  public sendMedia(toUserName: string, mediaId: string, type: number): Promise<boolean> {
-    if (!toUserName) {
+  public sendMedia(mediaData: any): Promise<boolean> {


Using a type like any can make you feel very convenience at start, But it will cause problems in the future because it disables all the benefits from the static type checking.

So please do not use an any type here. I have two suggestions:

Keep the original style.

Then we will have a function like:

public sendMedia( toUserName: string, mediaId: string, msgType, fileName, fileMd5, fileSize, fileType, mmFileExt, mmFileId, )

It's very verbose, but it's more clear and prevent us to make mistakes in the future.

Or do it the better way: create an interface for this data type.

interface MediaData { toUserName: string, mediaId: string, msgType, fileName, fileMd5, fileSize, fileType, mmFileExt, mmFileId, } public sendMedia(mediaData: MediaData): Promise<boolean> {

I'll prefer the method 2, but you can use either of them, but not the any one.

huan · 2017-08-08T12:53:02Z

src/puppet-web/puppet-web.ts

@@ -336,7 +338,7 @@ export class PuppetWeb extends Puppet {
    }
  }

-  private async uploadMedia(mediaMessage: MediaMessage, toUserName: string): Promise<string> {
+  private async uploadMedia(mediaMessage: MediaMessage, toUserName: string): Promise<any> {


The same suggestion as the above review: do not use any.

I think here need interface too.

huan · 2017-08-08T12:54:56Z

src/puppet-web/puppet-web.ts

@@ -437,7 +450,7 @@ export class PuppetWeb extends Puppet {
    })
    if (!mediaId)
      throw new Error('upload fail')
-    return mediaId as string
+    return { mediaId, mediaData }


Could you add mediaId into mediaData?

like:

Object.assign(mediaData, { mediaId. })

If we could do this, then here will be more clear, because we can return the interface of MediaData here.

huan · 2017-08-08T12:55:50Z

src/puppet-web/puppet-web.ts

@@ -455,17 +468,22 @@ export class PuppetWeb extends Puppet {
      destinationId = to.id
    }

-    const mediaId = await this.uploadMedia(message, destinationId)
+    const { mediaId, mediaData } = await this.uploadMedia(message, destinationId)


Use one return value with an interface here instead of using {}.

OK, I'll make changes as you suggest.

Code refactoring has been done

lijiarui

Thanks for your pr! It helps a lot.
But I fill a little bit confused as I commented.

lijiarui · 2017-08-08T13:33:20Z

src/puppet-web/puppet-web.ts

    const msgType = UtilLib.msgType(message.ext())

+    mediaData.FileType = 7


what does 7 means here? Can it use enum?

Reference source: https://res.wx.qq.com/a/wx_fed/webwx/res/static/js/index_4a48aef.js

But the use is unknown, and I'll test it again

var d, u = { FromUserName: c.getUserName(), ToUserName: e.ToUserName, FileSize: e.size, FileMd5: t, FileName: e.name, FileType: 7 }; if (i) { var m = angular.extend(u, c.getBaseRequest()); d = angular.extend({}, u), n({ method: "POST", url: r.API_checkupload, data: m }).success(function(t) { 0 == t.BaseResponse.Ret ? (d = angular.extend(d, { AESKey: t.AESKey, Signature: t.Signature }), e.Signature = t.Signature, a(e, t.MediaId, t)) : (e.MMSendMsg && (e.MMSendMsg.MMFileStatus = r.MM_SEND_FILE_STATUS_FAIL, e.MMSendMsg.MMStatus = r.MSG_SEND_STATUS_FAIL), alert(t.BaseResponse.ErrMsg)) }).error(function(t) { e.MMSendMsg && (e.MMSendMsg.MMFileStatus = r.MM_SEND_FILE_STATUS_FAIL, e.MMSendMsg.MMStatus = r.MSG_SEND_STATUS_FAIL), alert("上传失败") })

After testing, no valid use of the file type parameter has been found and has been removed.

binsee · 2017-08-08T17:57:46Z

Known issues:
After the file is sent, the file status on the web page is not updated, but the file has actually been sent successfully.

Sender (bot):

Receiver (wechat on pc)

binsee · 2017-08-08T19:11:34Z

use: await m.say(new MediaMessage(file))

demo code:

  .on('message', async m => {
    try {
      const room = m.room()
      console.log((room ? '[' + room.topic() + ']' : '')
        + '<' + m.from().name() + '>'
        + ':' + m.toStringDigest(),
      )

      if (/^([a-zA-z0-9]{1,5})$/i.test(m.content()) && !m.self()) {

        const file = __dirname + '/test.' + m.content()
        const pathInfo = path.parse(file)
        const fileStat = fs.statSync(file)

        if (fs.existsSync(file) && fileStat.isFile()) {
          await m.say(`请求 ${m.content()} 类型文件`)
          log.info('Bot', `REPLY: File[${pathInfo.base}] size: ${fileStat.size ? fileStat.size : 0}`)
          await m.say(new MediaMessage(file))
        } else {
          await m.say(`请求 ${m.content()} 类型文件失败！没有此类型的文件。`)
        }
      }
    } catch (e) {
      log.error('Bot', 'on(message) exception: %s', e)
    }
  })

huan

Thank you very much for the code reflecting, with the new interface MediaData, it looks fantastic!

I have a tiny question for you, which had posted as reviews, but I think that's not a problem.

In order to get this PR merged, please get Approved from at least 3 reviewers.

I'll approve this PR now for my vote.

Thanks again for your great work, Cheers!

huan · 2017-08-09T01:47:30Z

src/puppet-web/puppet-web.ts

@@ -384,6 +389,8 @@ export class PuppetWeb extends Puppet {
    const first = cookie.find(c => c.name === 'webwx_data_ticket')
    const webwxDataTicket = first && first.value
    const size = buffer.length
+    const id = 'WU_FILE_' + this.fileId


Here you save WU_FILE_fileId to id, but in the following code of formData you use this.fileId instead.

Is this a mistake, or by design?

huan · 2017-08-09T01:48:13Z

src/puppet-web/puppet-web.ts

    const formData = {
-      id: 'WU_FILE_1',
+      id: this.fileId,


Here. fileId or just id? (const id = 'WU_FILE_' + this.fileId)?

This is a mistake and I will fix it

hailiang-wang

LGTM - thanks!

binsee added 2 commits August 8, 2017 20:30

fix(puppet-web): send any type file.

c490162

Fix can't send pdf and more type file. Now, you can use `m.say(new MediaMessage(file))` send any type file. Fix wechaty#710

fix(puppet-web): Delete unused comments

62f9234

huan requested changes Aug 8, 2017

View reviewed changes

huan requested a review from lijiarui August 8, 2017 13:01

lijiarui requested changes Aug 8, 2017

View reviewed changes

fix(puppet-web): add interface MediaData

72bf68f

binsee mentioned this pull request Aug 8, 2017

Support Message Type of Image/Video #4

Closed

huan requested review from a team and mukaiu and removed request for a team August 9, 2017 01:26

huan approved these changes Aug 9, 2017

View reviewed changes

huan requested a review from a team August 9, 2017 01:56

fix(puppet-web): formData.id Allocation error

750a42a

lijiarui approved these changes Aug 9, 2017

View reviewed changes

fix(wechaty-bro): remove trailing comma

a8636f2

huan requested review from dcsan, xinbenlv, a user, Gcaufy and imWildCat August 9, 2017 08:30

hailiang-wang approved these changes Aug 9, 2017

View reviewed changes

huan merged commit 6c576c5 into wechaty:master Aug 10, 2017

binsee deleted the fix-send-pdf-file branch August 10, 2017 07:48

This was referenced Aug 10, 2017

Add support for send url rich media message #718

Closed

Sending Images #690

Closed

lijiarui mentioned this pull request Aug 11, 2017

the latest docker version 139 cannot send file #720

Closed

huan mentioned this pull request Sep 6, 2017

Invite @binsee to write a blog about what he like. wechaty/docusaurus#42

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(puppet-web): send any type file. #714

fix(puppet-web): send any type file. #714

binsee commented Aug 8, 2017

huan left a comment

huan Aug 8, 2017

huan Aug 8, 2017

huan Aug 8, 2017

huan Aug 8, 2017

binsee Aug 8, 2017

binsee Aug 8, 2017

lijiarui left a comment

lijiarui Aug 8, 2017

binsee Aug 8, 2017 •

edited

binsee Aug 8, 2017

lijiarui Aug 9, 2017

binsee commented Aug 8, 2017

binsee commented Aug 8, 2017 •

edited

huan left a comment

huan Aug 9, 2017

huan Aug 9, 2017

binsee Aug 9, 2017

hailiang-wang left a comment

		const msgType = UtilLib.msgType(message.ext())

		mediaData.FileType = 7

fix(puppet-web): send any type file. #714

fix(puppet-web): send any type file. #714

Conversation

binsee commented Aug 8, 2017

huan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lijiarui left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binsee Aug 8, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

binsee commented Aug 8, 2017

binsee commented Aug 8, 2017 • edited

huan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hailiang-wang left a comment

Choose a reason for hiding this comment

binsee Aug 8, 2017 •

edited

binsee commented Aug 8, 2017 •

edited