Skip to content
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

feat: fetch the users presence asynchronously (isOnline) #1101

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

domw30
Copy link
Collaborator

@domw30 domw30 commented Oct 16, 2023

What does this do?

  • replaces how we're currently setting the users isOnline status by fetching the correct data using matrixs getPresence method.

Why are we making this change?

  • currently using getUser, to set the users online status, this accidentally works as the user will be defaulted to offline. Using getPresence asynchronously will return the users correct online status. Similar to how we are getting the last seen at data.

How do I test this?

  • open messenger and check users online status in the conversation list.

Key decisions and Risk Assessment:

Things to consider:

  1. How will this affect security?
  2. How will this affect performance?
  3. Does this change any APIs?

@domw30 domw30 requested a review from a team October 16, 2023 08:02
src/lib/chat/matrix-client.ts Show resolved Hide resolved

if (conversation.isOneOnOne && matrixId) {
if (isOneOnOne && matrixId) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we still show the green dot on group conversations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah actually I don't think this will won't work for group conversations. I'll need to take another look.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, Updated this!

Screenshot 2023-10-17 at 13 12 44

Comment on lines 87 to 88
if (isOnline) {
member.isOnline = isOnline;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if isOnline is false? don't we need to set it to false for the user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah if the user is offline (i.e., if the presence value is anything other than 'online'), the method will return isOnline: false handled in getUserPresence

const isOnline = presence === 'online';

@domw30 domw30 merged commit 0924ea3 into main Oct 17, 2023
4 checks passed
@domw30 domw30 deleted the feat/replace-user-presence-data branch October 17, 2023 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants