dbeaver/pro#9798 adds sorting for columns in user tables#4484
dbeaver/pro#9798 adds sorting for columns in user tables#4484sergeyteleshev wants to merge 5 commits into
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 27 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| return NEW_USER_SYMBOL in user && user[NEW_USER_SYMBOL] === true && 'createdAt' in user && Boolean(user.createdAt); | ||
| } | ||
|
|
||
| export function isUser(user: object): user is AdminUser { |
There was a problem hiding this comment.
Should not be here, please move it to the compareGrantSubjects.ts
There was a problem hiding this comment.
added new file for all of these helpers
| function getTeamRoleRank(user: AdminUser) { | ||
| const granted = tabState.state.grantedUsers.find(grantedUser => grantedUser.userId === user.userId); | ||
|
|
||
| if (!granted) { | ||
| return 0; | ||
| } | ||
|
|
||
| return granted.teamRole === USER_TEAM_ROLE_SUPERVISOR ? 2 : 1; | ||
| } | ||
|
|
||
| const columns = [...COLUMNS]; | ||
|
|
||
| if (teamRolesResource.data.length > 0) { | ||
| columns.push(TEAM_ROLE_COLUMN); | ||
| columns.push({ | ||
| key: TEAM_ROLE_COLUMN_KEY, | ||
| label: 'plugin_authentication_administration_team_user_team_role_supervisor', | ||
| compare: (a, b) => getTeamRoleRank(a) - getTeamRoleRank(b), | ||
| }); |
There was a problem hiding this comment.
so for every single compare we do 2 x O(n) searches among grantedUsers.
It might be a problem on big lists.
Let's optimize it
There was a problem hiding this comment.
in data structure & algorithms world 2 * O(N) = O(N) and its fine. computers and browsers can handle that easy peasy
I think this matters only if you develop search engine or computer chip architecture or something like that
There was a problem hiding this comment.
but how the compare function is used?
it's basically O(n) * compare (O(n))
| export function compareUsers<T extends Pick<AdminUser, 'userId'>>(a: T, b: T): number { | ||
| return a.userId.localeCompare(b.userId); | ||
| } | ||
|
|
||
| export function compareUsersByLastLogin<T extends Pick<AdminUser, 'lastLoginTime'>>(a: T, b: T): number { | ||
| const aTime = a.lastLoginTime ? new Date(a.lastLoginTime).getTime() : 0; | ||
| const bTime = b.lastLoginTime ? new Date(b.lastLoginTime).getTime() : 0; | ||
| return aTime - bTime; | ||
| } | ||
|
|
||
| export function compareNewUsers<T extends AdminUser>(a: T, b: T): number { |
There was a problem hiding this comment.
compareUsers and compareNewUsers are really confusing names, because without looking on implementation you can't understand what it does and how to use it.
so it's better to use compareUserById and compareUsersByNewness how you did with compareUsersByLastLogin
| import classes from './GrantManagementTable.module.css'; | ||
|
|
||
| export interface IGrantManagementTableColumn { | ||
| export interface IGrantManagementTableColumn<T = any> { |
There was a problem hiding this comment.
we loose all the types because of this any, if we want to use types properly, we have to use unknown here or put some constraint how you did it earlier with extends
3 participants
closes https://github.com/dbeaver/pro/issues/9798