Find group chats by chat id

[cvincent]

Fixes #31.

[nr23730]

@michaelnew Can you also have a look at this?

[michaelnew]

I think #31 happens because matrix-appservice-bridge escapes characters like @ and + when setting user IDs in Matrix, but it doesn't un-escape them when we read the user ID back out and try to match it with an iMessage user. In the case of 1-to-1 messages we use the room name in Matrix (which is just a not escaped version of the user ID) to find the iMessage conversation, but for group messages we use the list of user IDs. So we end up looking for a group chat with usernames like mikenew=40gmail.com, which don't exist.

Anyway, it looks like this avoids the issue by not relying on user IDs to match up group chats at all and just using the chat ID from the room name. Which is a way more sane way of doing it anyway. We should probably still try and get a fix in with matrix-appservice-bridge in case we want to match up user IDs in the future, but that's really beside the point.

I'm a big fan of this change. I've had issues (even with a hacky fix for the character escaping) of group messages forking into different matrix rooms because the list of users was out of order or who knows what. So I'm hoping this will end up fixing that too. I'm testing it right now, so far so good. 👍 from me.

[cvincent]

I believe the previous method was only creating text chats, which are distinct from chats. My first attempt to fix it was by massaging the handles it used to create the chat, and it "worked" as in it created a duplicate text chat with the same members, rather than using the already existing chat.

There are probably additional cases to handle. I didn't test if this works with chats that have at least one non-iMessage user (green text bubbles). One of the advantages to being my friend is occasionally receiving a random message reading "testing" followed by "wow it worked!!" but I didn't want to push it too far all at once, haha.

[michaelnew]

Ha, yeah my friends get those messages too :)

I didn't realize text chat was a thing, so maybe that's what I was seeing with the duplicate rooms. BTW if you poke around the iOS headers there's some funny stuff in there, like isAppleChat, isBusinessChat, and isMakoChat, and I have no idea what those are supposed to mean.

I had the same thought about SMS. I'm going to see if I can set up a group chat with an SMS user and test that out.

[michaelnew]

Actually scratch that. I don't think I can create a group chat with an SMS user since my iMessage account no longer has a phone number associated with it. So I won't be able to test that.

Regardless, I still think we should merge this change since group chats are currently broken. Even if there are edge cases that aren't covered here, it's better than what we have.

[cvincent]

I'd be willing to carve out some time to circle back and test out SMS stuff since I do have a working account with a phone number. Can't hard-commit on when, but possibly this week?

[ericmigi]

seeing this error when using the branch. the message goes through but this is reported in the puppet control room

Error in handleMatrixEvent Error: exited nonzero
at ChildProcess. (/Users/eric/dev/imessagefix/matrix-puppet-imessage/src/imessage-send-group.js:48:18)
at ChildProcess.emit (events.js:210:5)
at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12) {
[stack]: 'Error: exited nonzero\n' +
' at ChildProcess. (/Users/eric/dev/imessagefix/matrix-puppet-imessage/src/imessage-send-group.js:48:18)\n' +
' at ChildProcess.emit (events.js:210:5)\n' +
' at Process.ChildProcess._handle.onexit (internal/child_process.js:272:12)',
[message]: 'exited nonzero'
} {
age: 82,
content: { body: 'ask me a question only Liz would know', msgtype: 'm.text' },
event_id: '$Hk9-4MJXmHsddi-mAT0U5hmHelCWKkrfklpht8vQiBE',
origin_server_ts: 1581299445632,
room_id: '!VsStpkBKNMJcVagdiA:eric.nova.chat',
sender: '@liz:eric.nova.chat',
type: 'm.room.message',
unsigned: { age: 82 },
user_id: '@liz:eric.nova.chat',
getRoomId: [Function] { [length]: 0, [name]: '' },
getId: [Function] { [length]: 0, [name]: '' }
} 

[nr23730]

I think this requires also a change in https://github.com/matrix-hacks/matrix-puppet-imessage/blob/master/index.js#L196

This probably resolved the issue of @ericmigi

[nr23730]

I think #31 happens because matrix-appservice-bridge escapes characters like @ and + when setting user IDs in Matrix, but it doesn't un-escape them when we read the user ID back out and try to match it with an iMessage user. In the case of 1-to-1 messages we use the room name in Matrix (which is just a not escaped version of the user ID) to find the iMessage conversation, but for group messages we use the list of user IDs. So we end up looking for a group chat with usernames like mikenew=40gmail.com, which don't exist.

Anyway, it looks like this avoids the issue by not relying on user IDs to match up group chats at all and just using the chat ID from the room name. Which is a way more sane way of doing it anyway. We should probably still try and get a fix in with matrix-appservice-bridge in case we want to match up user IDs in the future, but that's really beside the point.

I'm a big fan of this change. I've had issues (even with a hacky fix for the character escaping) of group messages forking into different matrix rooms because the list of users was out of order or who knows what. So I'm hoping this will end up fixing that too. I'm testing it right now, so far so good. 👍 from me.

Thank you for your reply! :) I'm going to merge if the issue of @ericmigi is resolved.

[rootkit007]

I have been running this branch for about a week with no issues. Everything works, including group messages