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

[feature request] fire `room-join` when someone joins from a QR Code #155

Closed
dcsan opened this Issue Dec 22, 2016 · 12 comments

Comments

Projects
None yet
3 participants
@dcsan

dcsan commented Dec 22, 2016

I was trying to create a welcome message when someone joins from scanning a QR Code but I don't think that room-join event is fired.

Instead all I see is the message (in english mode)

$username joined the group chat via the QR Code shared by $invitee

So I think the current API only works if you use [+] to add someone to a room.

It would be nice to have a common interface for dealing with room-join events but perhaps an extra param for source=invite|qrcode ?

Perhaps the event does get sent the first time someone joins a room. I am quitting and re-joining the group and have run out of devices to test, so not 100% sure...

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Dec 22, 2016

Member

Yes, the join event is fired by matching the join message.

Could you please help me to post both English and Chinese version of that string? So I could write unite test for it. :)

Member

zixia commented Dec 22, 2016

Yes, the join event is fired by matching the join message.

Could you please help me to post both English and Chinese version of that string? So I could write unite test for it. :)

@dcsan

This comment has been minimized.

Show comment
Hide comment
@dcsan

dcsan Dec 22, 2016

haha, I was just adding a TODO that I need the Chinese versions too :)

I was writing my own kind of test harness to add to these messages since I didn't have time to dive into the Wechaty test suite yet. I'll share that repo with you as it has the basic tests and messages.

dcsan commented Dec 22, 2016

haha, I was just adding a TODO that I need the Chinese versions too :)

I was writing my own kind of test harness to add to these messages since I didn't have time to dive into the Wechaty test suite yet. I'll share that repo with you as it has the basic tests and messages.

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Dec 22, 2016

Member

Great! :)

Also have a look at https://github.com/wechaty/wechaty/blob/master/src/puppet-web/firer.spec.ts , I'll add to this file

Member

zixia commented Dec 22, 2016

Great! :)

Also have a look at https://github.com/wechaty/wechaty/blob/master/src/puppet-web/firer.spec.ts , I'll add to this file

@dcsan

This comment has been minimized.

Show comment
Hide comment
@dcsan

dcsan Dec 22, 2016

I made a gist here with some basic regex tests for the english messages
https://gist.github.com/dcsan/6975c84389ceac19d6677389d910506d.js

also in this repo (shared with you)
https://github.com/dcsan/ta-bot/blob/master/kbot/test/JoinRoomRegex-test.ts

I could try and merge this into Wechaty's own test suite next week sometime but I'm not sure where the other regex matches are
https://github.com/wechaty/wechaty/blob/master/test/room.spec.ts

Another thing to note is that WeChat added the "restricted rooms" recently, so they may have other messages. It seems so fragile to build this on top of regex messages rather than some type of data structures... it won't work in other languages for a start.

dcsan commented Dec 22, 2016

I made a gist here with some basic regex tests for the english messages
https://gist.github.com/dcsan/6975c84389ceac19d6677389d910506d.js

also in this repo (shared with you)
https://github.com/dcsan/ta-bot/blob/master/kbot/test/JoinRoomRegex-test.ts

I could try and merge this into Wechaty's own test suite next week sometime but I'm not sure where the other regex matches are
https://github.com/wechaty/wechaty/blob/master/test/room.spec.ts

Another thing to note is that WeChat added the "restricted rooms" recently, so they may have other messages. It seems so fragile to build this on top of regex messages rather than some type of data structures... it won't work in other languages for a start.

@dcsan

This comment has been minimized.

Show comment
Hide comment
@dcsan

dcsan Dec 22, 2016

actually I made a much simpler check now to just get the person who Joined the room
(this doesn't get the invitee though...)

  regexCheckJoinMessage: (input) => {
    let checks = [
      { regex: /.* invited "(.*)" to .*/i },  // You've | $inviter
      { regex: /.* invited (.*) to .*/i },  // without quotes
      { regex: /"(.*)" joined the group chat .*/i },
      //// dont need these
      // { regex: /"(.*)" joined the group chat via the QR Code shared by "(.*)"/i },
      // { regex: /"(.*)" joined the group chat via your .*/i }
    ]
    let match
    for (let elem of checks) {
      match = input.match(elem.regex)
      if (match) {
        // debug('found join match', match)
        break
      } else {
        // debug("NO match\n", input, "\n", elem.regex)
      }
    }
    return match
  },

checking against these messages

let msgs = [
  '"$invitee😀" joined the group chat via the QR Code shared by "$invitee😀"',
  '"😀inviter" invited "$invitee😀" to the group chat',
  '"😀inviter" invited "$invitee😀, $invitee2, $invitee3" to the group chat',
  'You\'ve invited "$invitee😀" to the group chat  Revoke',
  'You\'ve invited "Quoted name, Person name two" to join the group chat',
  'You\'ve invited Person Name one, Person name two to join the group chat',
]

dcsan commented Dec 22, 2016

actually I made a much simpler check now to just get the person who Joined the room
(this doesn't get the invitee though...)

  regexCheckJoinMessage: (input) => {
    let checks = [
      { regex: /.* invited "(.*)" to .*/i },  // You've | $inviter
      { regex: /.* invited (.*) to .*/i },  // without quotes
      { regex: /"(.*)" joined the group chat .*/i },
      //// dont need these
      // { regex: /"(.*)" joined the group chat via the QR Code shared by "(.*)"/i },
      // { regex: /"(.*)" joined the group chat via your .*/i }
    ]
    let match
    for (let elem of checks) {
      match = input.match(elem.regex)
      if (match) {
        // debug('found join match', match)
        break
      } else {
        // debug("NO match\n", input, "\n", elem.regex)
      }
    }
    return match
  },

checking against these messages

let msgs = [
  '"$invitee😀" joined the group chat via the QR Code shared by "$invitee😀"',
  '"😀inviter" invited "$invitee😀" to the group chat',
  '"😀inviter" invited "$invitee😀, $invitee2, $invitee3" to the group chat',
  'You\'ve invited "$invitee😀" to the group chat  Revoke',
  'You\'ve invited "Quoted name, Person name two" to join the group chat',
  'You\'ve invited Person Name one, Person name two to join the group chat',
]
@lijiarui

This comment has been minimized.

Show comment
Hide comment
@lijiarui

lijiarui Dec 23, 2016

Member

Chinese version of QR info String:

"$invitee"通过扫描"$inviter"分享的二维码加入群聊

Member

lijiarui commented Dec 23, 2016

Chinese version of QR info String:

"$invitee"通过扫描"$inviter"分享的二维码加入群聊

@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Dec 23, 2016

Member

Support to fire room-join when someone joins from a QR Code is not hard to do, so I'd like to leave this issue for Welcome Pull Request for a week. :)

Member

zixia commented Dec 23, 2016

Support to fire room-join when someone joins from a QR Code is not hard to do, so I'd like to leave this issue for Welcome Pull Request for a week. :)

@zixia zixia added the enhancement label Dec 23, 2016

lijiarui added a commit to lijiarui/wechaty that referenced this issue Dec 24, 2016

@dcsan

This comment has been minimized.

Show comment
Hide comment
@dcsan

dcsan Dec 31, 2016

with version 7.5 which includes @lijiarui pull request I don't get a room join event fired...

Wechaty Doctor

  1. Wechaty version: #git[df619ab Merge pull request #177 from wechaty/greenkeeper/@types/sinon-1.16.34]
  2. Darwin x64 version 16.3.0 memory 160/8192 MB
  3. Docker: false
  4. Node version: v6.9.1
  bot:Kbot RECV: "DC-helper" joined the group chat via your shared QR Code. +21s
[TestBotGroup]<TestBotGroup>:{SYS}"DC-helper" joined the group chat via your shared QR Code.
  bot:Kbot roomName TestBotGroup +2ms
  bot:KBrain getItemData - no item for "dc-helper" joined the group chat via your shared qr code. +1ms
  bot:KBrain lookup TestBotGroup "DC-helper" joined the group chat via your shared QR Code. +0ms
  bot:KBrain examples TestBotGroup "DC-helper" joined the group chat via your shared QR Code. +0ms
check.res= undefined

dcsan commented Dec 31, 2016

with version 7.5 which includes @lijiarui pull request I don't get a room join event fired...

Wechaty Doctor

  1. Wechaty version: #git[df619ab Merge pull request #177 from wechaty/greenkeeper/@types/sinon-1.16.34]
  2. Darwin x64 version 16.3.0 memory 160/8192 MB
  3. Docker: false
  4. Node version: v6.9.1
  bot:Kbot RECV: "DC-helper" joined the group chat via your shared QR Code. +21s
[TestBotGroup]<TestBotGroup>:{SYS}"DC-helper" joined the group chat via your shared QR Code.
  bot:Kbot roomName TestBotGroup +2ms
  bot:KBrain getItemData - no item for "dc-helper" joined the group chat via your shared qr code. +1ms
  bot:KBrain lookup TestBotGroup "DC-helper" joined the group chat via your shared QR Code. +0ms
  bot:KBrain examples TestBotGroup "DC-helper" joined the group chat via your shared QR Code. +0ms
check.res= undefined

@dcsan

This comment has been minimized.

Show comment
Hide comment
@dcsan

dcsan Dec 31, 2016

also noting the messages are all a bit different

from the person who shared the QR Code

"DC-helper" joined the group chat via your shared QR Code.

the joining person themselves sees:

You've joined the group chat via QR Code. Group chat members: (list of names)

others who did NOT send the QR code would see another message

"$invitee😀" joined the group chat via the QR Code shared by "$invitee😀"

dcsan commented Dec 31, 2016

also noting the messages are all a bit different

from the person who shared the QR Code

"DC-helper" joined the group chat via your shared QR Code.

the joining person themselves sees:

You've joined the group chat via QR Code. Group chat members: (list of names)

others who did NOT send the QR code would see another message

"$invitee😀" joined the group chat via the QR Code shared by "$invitee😀"

@lijiarui

This comment has been minimized.

Show comment
Hide comment
@lijiarui

lijiarui Dec 31, 2016

Member

@dcsan it seems you are not running the correct version...
maybe you should clone the repo I forked to check whether it works... because @zixia didn't merge my pr.

Thanks

Member

lijiarui commented Dec 31, 2016

@dcsan it seems you are not running the correct version...
maybe you should clone the repo I forked to check whether it works... because @zixia didn't merge my pr.

Thanks

@dcsan

This comment has been minimized.

Show comment
Hide comment
@dcsan

dcsan Jan 1, 2017

(oops I thought I saw some release notes for a 7.5 version that had already had this merged in)

OK great I confirmed this works using your fork @lijiarui
tested with users from a QR code and from a direct [+] add

Thanks!

dcsan commented Jan 1, 2017

(oops I thought I saw some release notes for a 7.5 version that had already had this merged in)

OK great I confirmed this works using your fork @lijiarui
tested with users from a QR code and from a direct [+] add

Thanks!

zixia added a commit that referenced this issue Jan 2, 2017

Merge pull request #162 from lijiarui/master
enhance #155 fire `room-join` when someone joins from a QR Code
@zixia

This comment has been minimized.

Show comment
Hide comment
@zixia

zixia Jan 2, 2017

Member

PR merged. Please let me know if there's any other problem with this issue.

Thank @dcsan @lijiarui for the contribution!

Member

zixia commented Jan 2, 2017

PR merged. Please let me know if there's any other problem with this issue.

Thank @dcsan @lijiarui for the contribution!

@zixia zixia closed this Jan 2, 2017

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