saros-project / saros

Open Source IDE plugin for distributed collaborative software development
https://www.saros-project.org
GNU General Public License v2.0
160 stars 52 forks source link

[FIX] #1121 [S] New contact will be added through invite command if necessary #1140

Open FelixSelter opened 3 years ago

FelixSelter commented 3 years ago

Your checklist for this pull request

Please, check that:

Description

In this Pull request issue, #1121 is fixed. If someone is invited by the invite command of the Server it will check if this person is an actual contact and if not add him as one. Note the invited person still has to accept the contactInvite. After he accepted the server admin has to reinvite him so he will get the session invite request

Also its my first time contributing so I hope everything is fine. Else tell me. I'm a human and got the ability learn

Thank you for the awesome work and to the reviewers!

srossbach commented 3 years ago

I still have issues figuring out what is going wrong ? Neither the commit description or the the description of the issue mention what is going wrong. And before try to respond I do not accept the answer: You have to add the contact. I want to know what part of Saros is not working as expected and WHY.

srossbach commented 3 years ago

Ok after digging through the code, Stefan changed how Saros support is determined. This now requires presence information which you will not obtain as long as there is no subscription status. So the minimum requirement is subscription='to': .

This does not fix anything unless the user somehow (depending on Saros/E or Saros/I) accepts the invitation. To be more specific, the user has to accept the invitation ASAP which it simply cannot.

FelixSelter commented 3 years ago

As far as I understood the issue is that, users are confused if they try to invite someone but it's not working because they are not on the contact list of the server. My approach to fixing this was to send a contact request in this case. It would make more sense if I would print the warning and then return it so they won't be covered by the error traces. The user then has time to accept and the admin will reinvite him. I also suggested adding this contact invitation as an external command but this won't fix the issue.

To my understanding, the issue is fixed because the admin gets notified and knows that the user has to accept the contact invitation. May @Drakulix can take a look because he knows the most about the issue due to the title change etc

I will try to take a look into this subscription system but as I started I'm not really familiar with saros. As far as I understood there's only a subscription if he's already a contact.

Any other suggestions? What would fix the issue in your opinion? Is it possible to override the rights of the other user so that there's no need to accept the contact invite (I don't think that should be done but seems to be the only other solution)?

FelixSelter commented 3 years ago

Example session till I fix the issue with the subscription:

# Welcome to Saros Server (type 'help' for available commands)
> invite name@saros-con.imp.fu-berlin.de
Adding name@saros-con.imp.fu-berlin.de as new contact. You will have to reinvite him after he accepted being added to the contacts
10:47:17.751 [main] ERROR saros.server.console.InviteCommand - Unknown ErrorAn unknown error has occured:

subscription-required(407) Not subscribed

It will be continued anyway
The contact was added successfully. Please invite him again after he accepted
> invite name@saros-con.imp.fu-berlin.de
> [-] Starting session negotiation...
[\] Checking Saros support...
[|] Checking version compatibility...
[/] Sending negotiation request...
[-] Waiting for negotiation acknowledgement...
[\] Waiting for name@saros-con.imp.fu-berlin.de to accept invitation...
[|] Waiting for remote session configuration data...
[/] Sending final session configuration data...
[-] Waiting for name@saros-con.imp.fu-berlin.de to connect...
[\] Establishing connection and performing final initialization...
[|] Synchronizing user list...