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

[FIX] Empty DMs go to sidebar's top after updating user preferences #27912

Merged
merged 4 commits into from
Feb 15, 2023

Conversation

matheusbsilva137
Copy link
Member

@matheusbsilva137 matheusbsilva137 commented Jan 31, 2023

Proposed changes (including videos or screenshots)

  • Do not use _updatedAt as lm field when merging info from subscriptions and rooms (since this field is updated along with the user preferences).

Issue(s)

Fix #23617

Steps to test or reproduce

  1. Create a new direct message and send a message in another room (so that the DM is not shown on top of the list);
  2. Go to My Account > Preferences > Notifications and update Desktop/Push/Email notification setting (any of them);
  3. Return to the main screen and check the sidebar.
    Current behavior: empty DMs go to the top of the list.
    Expected behavior: empty DMs should now stay at the bottom of the list.

Further comments

The lastRoomUpdate variable is used as a backup field to the lm field when it is not available in the room (that is, when no message has been sent in the room). Changing user notification preferences also updates the _updatedAt field in every subscription, which implies that empty DMs will be sorted to top when the sidebar is sorted. Instead, we could keep this field as undefined so that they're sorted to the bottom of the list.

TC-79

@codecov
Copy link

codecov bot commented Jan 31, 2023

Codecov Report

Merging #27912 (2c2fcdc) into develop (88f7024) will increase coverage by 1.48%.
The diff coverage is 54.36%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #27912      +/-   ##
===========================================
+ Coverage    42.24%   43.73%   +1.48%     
===========================================
  Files          812      790      -22     
  Lines        16052    15261     -791     
  Branches      2072     2085      +13     
===========================================
- Hits          6781     6674     -107     
+ Misses        8991     8308     -683     
+ Partials       280      279       -1     
Flag Coverage Δ
e2e 43.73% <54.36%> (+1.48%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@tassoevan tassoevan modified the milestones: 6.0.0, 6.0.0-prep Feb 14, 2023
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Feb 14, 2023
@kodiakhq kodiakhq bot merged commit f85d57a into develop Feb 15, 2023
@kodiakhq kodiakhq bot deleted the fix/dms-go-to-top branch February 15, 2023 03:59
@sampaiodiego sampaiodiego mentioned this pull request Feb 17, 2023
@sampaiodiego sampaiodiego mentioned this pull request Mar 9, 2023
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stat: QA skipped stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Chat order changes when user change notification preferences
3 participants