Skip to content

[CB] put new user to top after creation #3903

Merged
Wroud merged 9 commits intodevelfrom
7506-cb-put-new-user-to-top-after-creation
Nov 19, 2025
Merged

[CB] put new user to top after creation #3903
Wroud merged 9 commits intodevelfrom
7506-cb-put-new-user-to-top-after-creation

Conversation

@sergeyteleshev
Copy link
Copy Markdown
Contributor

@sergeyteleshev sergeyteleshev marked this pull request as ready for review November 17, 2025 09:44
@sergeyteleshev sergeyteleshev self-assigned this Nov 17, 2025
@sergeyteleshev sergeyteleshev requested a review from Wroud November 17, 2025 10:44
Comment thread webapp/packages/core-authentication/src/UsersResource.ts Outdated
@sergeyteleshev sergeyteleshev requested a review from Wroud November 17, 2025 12:28
Wroud
Wroud previously approved these changes Nov 17, 2025
Comment thread webapp/packages/core-authentication/src/UsersResource.ts Outdated
Comment thread webapp/packages/core-authentication/src/UsersResource.ts
return b.createdAt - a.createdAt;
}

return 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

return compareUsers(a,b)? And then you dont need to sort twice in users table

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The sort function helper is called sortByNewUsers. Therefore, it's supposed to adhere to the principle of "single responsibility". If neither user is new, we should do nothing in the context of sorting in order to keep predictable behavior. If we want to, we can always combine these two sorts: sortByNewUsers and sortUsersById, which we actually do

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe responsibility here is to sort users. How we do that does not really matter. And we use both functions anyway only for sorting users, so why not to combine them in a single compareUsers function?

devnaumov
devnaumov previously approved these changes Nov 17, 2025
Copy link
Copy Markdown
Contributor

@SychevAndrey SychevAndrey left a comment

Choose a reason for hiding this comment

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

compareUsers

Wroud
Wroud previously approved these changes Nov 18, 2025
devnaumov
devnaumov previously approved these changes Nov 18, 2025
@Wroud Wroud merged commit 354f13d into devel Nov 19, 2025
10 checks passed
@Wroud Wroud deleted the 7506-cb-put-new-user-to-top-after-creation branch November 19, 2025 08:51
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.

5 participants