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
Updated pinging logic. #289
Conversation
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.
So is it correct to say that we used to display a contact as online as soon as we received a p2p message from them, while now it can only result from a ping?
const contactDetails = this.contactP2pDetails[pubKey]; | ||
return !!(contactDetails && contactDetails.isOnline); | ||
if (!this.contactP2pDetails[pubKey]) return null; | ||
return { ...this.contactP2pDetails[pubKey] }; |
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.
Was this returning a reference instead of a copy?
If so, good catch.
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.
Ya, don't think it was affecting the actual functionality but it was picked up by the tests
Ye, now we only mark the contact is online if our p2p handshake went through successfully. |
There were a few minor cases that we were missing when checking whether we should ping or just mark the user as online.
This PR simplifies that logic down according to the following diagram:
EDIT: The image is missing a connection between
Was a user marked as online before?
andPing contact
Here we assume:
online