67P / hyperchannel

Kosmos Chat for the Web
Mozilla Public License 2.0
20 stars 3 forks source link

Add function which processes channel joins, including failures. #112

Closed silverbucket closed 7 years ago

silverbucket commented 7 years ago

Connected to #41

silverbucket commented 7 years ago

This successfully handles JOIN failures, however since the space is looked up by hostname, and the hostname for the XMPP JID is mixed up (in our test case, the user is user@host.com while the server is xmpp.host.com, therefore it's unclear what is actually the JID and/or what is the official way of referencing an account.

If, as we discussed today, the JID is user@host.com/resource then it does not give enough information about where this user exists, it would seem like this means it can't be the read JID.

So, is the real JID user@host.com@xmpp.host.com/resource ? There are so many pieces involved right now its a bit hard to work through this alone. At any rate, this pull request is functional except for one piece @galfert said he'd look into which is that the personId lookup is failing due to an incorrect server name (for xmpp)

gregkare commented 7 years ago

As discussed on Skype the JID is user@host.com/resource even if the server has DNS entries set up to announce to clients that the server is running on another host (xmpp.host.com for example)

galfert commented 7 years ago

I pushed some changes so at least IRC works again in this PR. Haven't gotten to trying XMPP yet. Will do that later.

galfert commented 7 years ago

Update: I fixed some problems and bugs related to XMPP, and also refactored some of the code along the way. But right now I'm at a point where I just don't get any feedback from Sockethub when trying to join a channel, except for the completed events. But no message or failure events.

And I also can't see anything of relevance in the Sockethub logs itself.

I am connected to the server though, as I can see the presence updates of the other users on the server.

@silverbucket: do you maybe have some time on friday to further pair on this a little bit?

silverbucket commented 7 years ago

@galfert sorry I didn't notice you ping me to pair. I'm free this weekend if you are.

galfert commented 7 years ago

I finally managed to successfully join XMPP MUC rooms. The problem was that that you have to add your nickname as a resource to the room id, e.g. room@muc.host.com/nickname.

Messaging in rooms work now. But it's still missing the list of users in the room.

As for direct messages, we can receive them but the outgoing messages don't seem to be sent to the recipient yet. I couldn't see any obvious error messages about that in the Sockethub logs, but I also didn't look into that very thoroughly yet.

raucao commented 7 years ago

The problem was that that you have to add your nickname as a resource to the room

We shouldn't have to do that in Hyperchannel! The whole point of Sockethub platforms is to abstract that away from apps.

raucao commented 7 years ago

I can see tons of changes, and one of them is speeding up the test suite. However, there's not a single new or changed test in here?

galfert commented 7 years ago

The recent changes are just me trying to get everything to work at first. It's not refactored, cleaned up or tested yet.

raucao commented 7 years ago

Got it. Only saw the WIP after my last comment earlier. ;)

galfert commented 7 years ago

I'm still writing some tests for the new methods. But other than that this should be ready for you to have a look and give it a try.

raucao commented 7 years ago

@galfert It's all red. Should I wait maybe?

galfert commented 7 years ago

Not sure yet what the test failure is about. But looks like it might be an issue with the recent remotestorage updates (undefined is not a function (evaluating 'o.defineModule')).

I think I already have an idea how to tackle the Codeclimate issues, but that requires a slightly bigger refactoring I would rather do in a separate PR.

But both should not prevent you from giving it a try already.

galfert commented 7 years ago

Added a bunch of tests as well now. This should be all ready to go now.

raucao commented 7 years ago

There I fixed it. Pushing from the TXL runway, so didn't test in browser anymore, but trusting the specs. :)

raucao commented 7 years ago

@galfert Ok, so is there anything missing now?

To confirm: it's correct that the XMPP join, which under the hood is actually sending presence to a channel, is not resulting in my user being shown in the channel list currently, right?

Also, sending messages is not included here on purpose, correct?

galfert commented 7 years ago

[...] is not resulting in my user being shown in the channel list currently, right?

Why would you show up in the channel list? When joining a channel, you should be seen in the user list of that channel. In Hyperchannel, as well as in other clients. When you receive a direct message, the other user's JID should show up as a channel in the channel list.

Also, sending messages is not included here on purpose, correct?

Not sure I understand. You can send and receive messages in channels. What doesn't work yet is initiating direct message chats with a user by clicking the user in the user list of a channel. But when the other user sends you a direct message first, you can also send direct messages to that user.

raucao commented 7 years ago

Why would you show up in the channel list? When joining a channel, you should be seen in the user list of that channel. In Hyperchannel, as well as in other clients.

That's what I meant. But it didn't work. I got the successful join, but the username wasn't in the list in my other client.

Not sure I understand. You can send and receive messages in channels.

Messages from Hyperchannel didn't show up in my normal client, so I assumed it's not implemented yet.

raucao commented 7 years ago

For posterity, I wrote this on IRC after we seemed to have found the bug, but it was actually a different one:

<raucao> galfert: i'm back to it not working, but without the replace-connection error
<raucao> but also with the same procedure (wrong jid first, then correct one)
<raucao> all the sockethub messages get completed
<raucao> that's what i had before (and i was wondering why there's an error now, which i would have reported then)