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(puppet-web): send any type file. #714

Merged
merged 5 commits into from Aug 10, 2017

Conversation

Projects
None yet
4 participants
@binsee
Member

binsee commented Aug 8, 2017

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

Fix #710

binsee added some commits Aug 8, 2017

fix(puppet-web): 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 #710
@zixia

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!

Show outdated Hide outdated src/puppet-web/bridge.ts
Show outdated Hide outdated src/puppet-web/puppet-web.ts
Show outdated Hide outdated src/puppet-web/puppet-web.ts
Show outdated Hide outdated src/puppet-web/puppet-web.ts

@zixia zixia requested a review from lijiarui Aug 8, 2017

@lijiarui

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

Show outdated Hide outdated src/puppet-web/puppet-web.ts
@binsee

This comment has been minimized.

Show comment
Hide comment
@binsee

binsee Aug 8, 2017

Member

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):
image

Receiver (wechat on pc)
image

Member

binsee commented Aug 8, 2017

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):
image

Receiver (wechat on pc)
image

@binsee

This comment has been minimized.

Show comment
Hide comment
@binsee

binsee Aug 8, 2017

Member

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)
    }
  })
Member

binsee commented Aug 8, 2017

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)
    }
  })

@zixia zixia requested review from Chatie/wechaty and mukaiu and removed request for Chatie/wechaty Aug 9, 2017

@zixia

zixia approved these changes Aug 9, 2017

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!

@@ -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

This comment has been minimized.

@zixia

zixia Aug 9, 2017

Member

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?

@zixia

zixia Aug 9, 2017

Member

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?

Show outdated Hide outdated src/puppet-web/puppet-web.ts

@zixia zixia requested a review from Chatie/contributor Aug 9, 2017

@zixia zixia requested review from dcsan, xinbenlv, kungfu-software, Gcaufy, imWildCat and Chatie/contributor Aug 9, 2017

@Samurais

LGTM - thanks!

@zixia zixia merged commit 6c576c5 into Chatie:master Aug 10, 2017

4 of 6 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
ci/circleci Your tests passed on CircleCI!
Details
codacy/pr Good work! A positive pull request.
Details
codeclimate All good!
Details
security/snyk No new vulnerabilities
Details

@binsee binsee deleted the binsee:fix-send-pdf-file branch Aug 10, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment