Skip to content

regression: sorting irregularities in new members list#35004

Merged
kodiakhq[bot] merged 6 commits intorelease-7.3.0from
regression/members-sort-by-statusConnection
Jan 28, 2025
Merged

regression: sorting irregularities in new members list#35004
kodiakhq[bot] merged 6 commits intorelease-7.3.0from
regression/members-sort-by-statusConnection

Conversation

@abhinavkrin
Copy link
Member

@abhinavkrin abhinavkrin commented Jan 22, 2025 •
edited by atlassian bot
Loading

Proposed changes (including videos or screenshots)

  • Fix: When assigning an Owner/Moderator to a channel, the correct user now appears in the "Owners" list immediately (no longer shows the first user in the list by mistake).
  • Fix: Restore sorting by status (online/busy/away/offline), ensuring members are ordered by rolePriority, then status, and finally username/name.

Issue(s)

Steps to test or reproduce

Further comments

  • CORE-937 – Adding a new Owner/Moderator shows incorrect user on the list
  • CORE-951 – Missing sorting by status in rooms.membersOrderedByRole

@abhinavkrin abhinavkrin requested a review from a team as a code owner January 22, 2025 15:36
@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Jan 22, 2025 •
edited
Loading

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Jan 22, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: ca1899b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codecov
Copy link

codecov bot commented Jan 22, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (release-7.3.0@8731b6d). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##             release-7.3.0   #35004   +/-   ##
================================================
  Coverage                 ?   59.17%           
================================================
  Files                    ?     2821           
  Lines                    ?    68137           
  Branches                 ?    15155           
================================================
  Hits                     ?    40322           
  Misses                   ?    24981           
  Partials                 ?     2834
Flag Coverage Δ
unit 74.99% <ø> (?)

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

@github-actions
Copy link
Contributor

github-actions bot commented Jan 22, 2025 •
edited
Loading

PR Preview Action v1.6.0

🚀 View preview at
https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-35004/

Built to branch gh-pages at 2025-01-27 19:17 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@abhinavkrin abhinavkrin added this to the 7.3.0 milestone Jan 22, 2025
@abhinavkrin abhinavkrin force-pushed the regression/members-sort-by-statusConnection branch from 2dde1d1 to e37af2b Compare January 22, 2025 17:03
@abhinavkrin abhinavkrin changed the title regression: no sorting by statusConnection in rooms.membersByOrderedByRole regression: no sorting by status in rooms.membersByOrderedByRole Jan 22, 2025
@abhinavkrin abhinavkrin requested a review from a team as a code owner January 22, 2025 17:47
MarcosSpessatto
MarcosSpessatto suggested changes Jan 23, 2025
View reviewed changes
@abhinavkrin abhinavkrin force-pushed the regression/members-sort-by-statusConnection branch from 803e91c to 135840b Compare January 23, 2025 19:03
KevLehman
KevLehman previously approved these changes Jan 23, 2025
View reviewed changes
dougfabris
dougfabris requested changes Jan 23, 2025
View reviewed changes
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

please use our pkg mock-providers in order to mock the unit test

@abhinavkrin
fix: no sorting by status
725cf16 
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin
minor fix
42030ca 
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin
added unit test for useMembersList
7c77b19 
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin
requested changes and minor fixes
a0b3c5e 
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin abhinavkrin force-pushed the regression/members-sort-by-statusConnection branch from 135840b to a0b3c5e Compare January 24, 2025 09:03
@abhinavkrin
fix: no sorting across pages
a073625 
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin
fix: status sorting logic
ca1899b 
Signed-off-by: Abhinav Kumar <abhinav@avitechlab.com>
@abhinavkrin abhinavkrin changed the title regression: no sorting by status in rooms.membersByOrderedByRole regression: sorting irregularities in new members list Jan 27, 2025
@jessicaschelly jessicaschelly added the stat: QA assured Means it has been tested and approved by a company insider label Jan 28, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Jan 28, 2025
dougfabris
dougfabris approved these changes Jan 28, 2025
View reviewed changes
Copy link
Member

@dougfabris dougfabris left a comment

Choose a reason for hiding this comment

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

LGTM!

@kodiakhq kodiakhq bot merged commit 3c4a9b8 into release-7.3.0 Jan 28, 2025
53 checks passed
@kodiakhq kodiakhq bot deleted the regression/members-sort-by-statusConnection branch January 28, 2025 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dougfabris dougfabris dougfabris approved these changes

@KevLehman KevLehman Awaiting requested review from KevLehman

+1 more reviewer

@MarcosSpessatto MarcosSpessatto MarcosSpessatto approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Milestone

7.3.0

Development

Successfully merging this pull request may close these issues.

5 participants

@abhinavkrin @KevLehman @MarcosSpessatto @dougfabris @jessicaschelly