add room-leave event #370
add room-leave event #370
Conversation
|
Sorry, i cannot merge this because your PR didn't pass any of CI test. |
|
Yes, it cannot be merge now, because I should fix #364 first. And I have fix room.member(), please help to merge it first, thanks |
|
Sorry, I have no time to help you about this recently. You need to do it by yourself. |
huan
changed the title
|
Changes Unknown when pulling f0c8176 on lijiarui:room_leave into ** on Chatie:master**. |
|
@zixia maybe it has done... |
src/puppet-web/firer.ts
Outdated
| let leaverContact: Contact | null, removerContact: Contact | null | ||
| if (leaver === this.userId) { | ||
| leaverContact = Contact.load(this.userId) | ||
| removerContact = room.member({alias: remover}) || room.member({name: remover}) |
There was a problem hiding this comment.
Please use the formal query filter name, instead of using an alias.
removerContact = room.member({roomAlias: remover}) || room.member({name: remover})I will more prefer to write this piece of code in this way:
removerContact = room.member(remover)Because Room.member() should do the job for translate the string name to a Contact inside a room.
There was a problem hiding this comment.
I think using room.member() is not strict here， because roomAlias(alias) here is useless, bot just recognize contactAlias and name, if we use roomAlias, then when someone's roomAlias is the same as other's name in the room. it will get the contact we don't mean go get.
So, maybe the new change I commit is the better way. we should may the code easier for developer, but more strict here.
|
Changes Unknown when pulling 77076c7 on lijiarui:room_leave into ** on Chatie:master**. |
src/puppet-web/firer.ts
Outdated
| } | ||
| } else { | ||
| removerContact = Contact.load(this.userId) | ||
| leaverContact = room.member({contactAlias: remover}) || room.member({name: leaver}) |
There was a problem hiding this comment.
I think it is better to use member(name) here for better encapsulation.
huan
requested review from
JasLin,
dcsan,
xinbenlv,
Gcaufy,
imWildCat,
hailiang-wang and
mukaiu
|
Please get 2 more approving from other contributors as well, then I could be able to merge this. Thanks! |
|
@lijiarui This is the way of my favorite for using https://github.com/Chatie/wechaty/blob/master/src/puppet-web/firer.ts#L319 |
|
Changes Unknown when pulling 3e29a82 on lijiarui:room_leave into ** on Chatie:master**. |
1 similar comment
|
Changes Unknown when pulling 3e29a82 on lijiarui:room_leave into ** on Chatie:master**. |
|
The shorter, the better. |
lijiarui
changed the title
|
Thanks all! I can merge this PR now, Cheers! |
fix #250