Skip to content
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

add room-leave event #370

Merged
merged 6 commits into from
Apr 16, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions src/puppet-web/firer.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ test('parseRoomJoin()', t => {
})

test('parseRoomLeave()', t => {
const contentList = [
const contentLeaverList = [
[
`You removed "Bruce LEE" from the group chat`,
`Bruce LEE`,
Expand All @@ -122,11 +122,27 @@ test('parseRoomLeave()', t => {
],
]

let result
contentList.forEach(([content, leaver]) => {
result = Firer.parseRoomLeave(content)
t.truthy(result, 'should get leaver for leave message: ' + content)
t.is(result, leaver, 'should get leaver name right')
const contentRemoverList = [
[
`You were removed from the group chat by "桔小秘"`,
`桔小秘`,
],
[
'你被"李佳芮"移出群聊',
'李佳芮',
],
]

contentLeaverList.forEach(([content, leaver]) => {
const resultLeaver = Firer.parseRoomLeave(content)[0]
t.truthy(resultLeaver, 'should get leaver for leave message: ' + content)
t.is(resultLeaver, leaver, 'should get leaver name right')
})

contentRemoverList.forEach(([content, remover]) => {
const resultRemover = Firer.parseRoomLeave(content)[1]
t.truthy(resultRemover, 'should get remover for leave message: ' + content)
t.is(resultRemover, remover, 'should get leaver name right')
})

t.throws(() => {
Expand Down
56 changes: 39 additions & 17 deletions src/puppet-web/firer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,16 @@ const regexConfig = {
/^"(.+)"通过扫描"?(.+?)"?分享的二维码加入群聊$/,
],

roomLeave: [
// no list
roomLeaveByBot: [
/^You removed "(.+)" from the group chat$/,
/^你将"(.+)"移出了群聊$/,
],
roomLeaveByOther: [
/^You were removed from the group chat by "(.+)"$/,
/^你被"(.+)"移出群聊$/,
],

roomTopic: [
/^"?(.+?)"? changed the group name to "(.+)"$/,
/^"?(.+?)"?修改群名为“(.+)”$/,
Expand Down Expand Up @@ -283,15 +289,18 @@ async function checkRoomJoin(m: Message): Promise<void> {
return
}

function parseRoomLeave(content: string): string {
const reList = regexConfig.roomLeave

let found: string[]|null = []
reList.some(re => !!(found = content.match(re)))
if (!found || !found.length) {
function parseRoomLeave(content: string): [string, string] {
const reListByBot = regexConfig.roomLeaveByBot
const reListByOther = regexConfig.roomLeaveByOther
let foundByBot: string[]|null = []
reListByBot.some(re => !!(foundByBot = content.match(re)))
let foundByOther: string[]|null = []
reListByOther.some(re => !!(foundByOther = content.match(re)))
if ((!foundByBot || !foundByBot.length) && (!foundByOther || !foundByOther.length)) {
throw new Error('checkRoomLeave() no matched re for ' + content)
}
return found[1] // leaver
const [leaver, remover] = foundByBot ? [ foundByBot[1], this.userId ] : [ this.userId, foundByOther[1] ]
return [leaver, remover]
}

/**
Expand All @@ -300,9 +309,9 @@ function parseRoomLeave(content: string): string {
async function checkRoomLeave(m: Message): Promise<void> {
log.verbose('PuppetWebFirer', 'fireRoomLeave(%s)', m.content())

let leaver
let leaver: string, remover: string
try {
leaver = parseRoomLeave(m.content())
[leaver, remover] = parseRoomLeave(m.content())
} catch (e) {
return
}
Expand All @@ -315,22 +324,35 @@ async function checkRoomLeave(m: Message): Promise<void> {
}
/**
* FIXME: leaver maybe is a list
* @lijiarui: I have checked, leaver will never be a list. If the bot remove 2 leavers at the same time, it will be 2 sys message, instead of 1 sys message contains 2 leavers.
*/
let leaverContact = room.member(leaver)

if (!leaverContact) {
log.error('PuppetWebFirer', 'fireRoomLeave() leaver %s not found, event `room-leave` & `leave` will not be fired', leaver)
return
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})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (!removerContact) {
log.error('PuppetWebFirer', 'fireRoomLeave() bot is removed from the room, but remover %s not found, event `room-leave` & `leave` will not be fired', remover)
return
}
} else {
removerContact = Contact.load(this.userId)
leaverContact = room.member({alias: remover}) || room.member({name: leaver})
if (!leaverContact) {
log.error('PuppetWebFirer', 'fireRoomLeave() bot removed someone from the room, but leaver %s not found, event `room-leave` & `leave` will not be fired', leaver)
return
}
}

await removerContact.ready()
await leaverContact.ready()
await room.ready()

/**
* FIXME: leaver maybe is a list
* @lijiarui: I have checked, leaver will never be a list. If the bot remove 2 leavers at the same time, it will be 2 sys message, instead of 1 sys message contains 2 leavers.
*/
this.emit('room-leave', room, [leaverContact])
room.emit('leave' , [leaverContact])
this.emit('room-leave', room, leaverContact, removerContact)
room.emit('leave' , leaverContact, removerContact)

setTimeout(_ => { room.refresh() }, 10000) // reload the room data, especially for memberList
}
Expand Down