-
Notifications
You must be signed in to change notification settings - Fork 53
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
[FIX] #1121 [S] New contact will be added through invite command if necessary #1140
base: master
Are you sure you want to change the base?
Conversation
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. |
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 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. |
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)? |
Example session till I fix the issue with the subscription:
|
for (JID jid : jids.get(true)) { | ||
sessionManager.startSharingReferencePoints(jid); | ||
|
||
// --------------add contact if not already-------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comments are pretty pointless unless you are not able to read code at all.
out.println( | ||
"Adding " | ||
+ jid | ||
+ " as new contact. You will have to reinvite him after he accepted being added to the contacts"); // the previous warning wont be displayed in the console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you really want to show these messages in the log file ? It is an interactive command at all.
+ jid | ||
+ " as new contact. You will have to reinvite him after he accepted being added to the contacts"); // the previous warning wont be displayed in the console | ||
|
||
final BiPredicate<String, String> questionDialogHandler = // error handler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an automated process, either you are query the STDIN or simply return true, As a side node, this will executed mostly all the time because of privacy reasons, i.e the server will not tell you if such a JID exists.
"The contact was added successfully. Please invite him again after he accepted"); | ||
return; // exit because the user cant accept the invite in time | ||
} catch (OperationCanceledException e) { | ||
log.error(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this your intention to continue with the code on line 102 ? I do not think so.
} | ||
// --------------end add contact if not already-------------- | ||
|
||
sessionManager.invite( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do not see the contact online (I do not know if XMPP supports invisibility, but that is another issue) then it makes no sense to continue. The invitation will simply fail.
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 requestAlso 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!