-
-
Notifications
You must be signed in to change notification settings - Fork 720
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: show users with multiple parallel sessions #8756
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import type { FC } from 'react'; | ||
import { IconCell } from 'component/common/Table/cells/IconCell/IconCell'; | ||
import WarningIcon from '@mui/icons-material/WarningAmber'; | ||
import { Tooltip } from '@mui/material'; | ||
import useUiConfig from 'hooks/api/getters/useUiConfig/useUiConfig'; | ||
import { useVariant } from 'hooks/useVariant'; | ||
|
||
type UserSessionsCellProps = { | ||
count?: number; | ||
}; | ||
|
||
export const UserSessionsCell: FC<UserSessionsCellProps> = ({ count }) => { | ||
const { uiConfig } = useUiConfig(); | ||
const minimumCountToShow = useVariant<number>( | ||
uiConfig.flags.showUserDeviceCount, | ||
); | ||
|
||
if (!count || count < (minimumCountToShow ? minimumCountToShow : 5)) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<IconCell | ||
icon={ | ||
<> | ||
<Tooltip title={`Multiple parallel sessions (${count})`}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe multiple browser sessions? for users it's probably more important that it's many browsers using the same app. It's more important for us that they are parallel. |
||
<WarningIcon | ||
aria-label='Multiple parallel sessions' | ||
color='warning' | ||
Tymek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/> | ||
</Tooltip> | ||
</> | ||
} | ||
/> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,6 +39,7 @@ import { StyledUsersLinkDiv } from '../Users.styles'; | |
import { useUiFlag } from 'hooks/useUiFlag'; | ||
import useUiConfig from '../../../../hooks/api/getters/useUiConfig/useUiConfig'; | ||
import { useScimSettings } from 'hooks/api/getters/useScimSettings/useScimSettings'; | ||
import { UserSessionsCell } from './UserSessionsCell/UserSessionsCell'; | ||
|
||
const UsersList = () => { | ||
const navigate = useNavigate(); | ||
|
@@ -57,6 +58,8 @@ const UsersList = () => { | |
open: false, | ||
}); | ||
const userAccessUIEnabled = useUiFlag('userAccessUIEnabled'); | ||
const showUserDeviceCount = useUiFlag('showUserDeviceCount'); | ||
|
||
const { | ||
settings: { enabled: scimEnabled }, | ||
} = useScimSettings(); | ||
|
@@ -139,7 +142,7 @@ const UsersList = () => { | |
id: 'name', | ||
Header: 'Name', | ||
accessor: (row: any) => row.name || '', | ||
minWidth: 200, | ||
minWidth: 180, | ||
Cell: ({ row: { original: user } }: any) => ( | ||
<HighlightCell | ||
value={user.name} | ||
|
@@ -148,6 +151,21 @@ const UsersList = () => { | |
), | ||
searchable: true, | ||
}, | ||
...(showUserDeviceCount | ||
? [ | ||
{ | ||
id: 'warning', | ||
Tymek marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Header: ' ', | ||
accessor: (row: any) => row.name || '', | ||
maxWidth: 40, | ||
Cell: ({ row: { original: user } }: any) => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I normally specify a proper type I expect instead of any here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's an issue with old react-table. It's the same for other columns in this file. maybe we need to clean this up and migrate fully |
||
<UserSessionsCell count={user.activeSessions} /> | ||
), | ||
searchable: false, | ||
disableSortBy: true, | ||
}, | ||
] | ||
: []), | ||
{ | ||
id: 'role', | ||
Header: 'Role', | ||
|
@@ -283,7 +301,7 @@ const UsersList = () => { | |
}, | ||
{ | ||
condition: isSmallScreen, | ||
columns: ['createdAt', 'last-login'], | ||
columns: ['createdAt', 'last-login', 'warning'], | ||
}, | ||
], | ||
setHiddenColumns, | ||
|
@@ -356,7 +374,10 @@ const UsersList = () => { | |
} | ||
elseShow={ | ||
<TablePlaceholder> | ||
No users available. Get started by adding one. | ||
<span data-loading> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it some extra cleanup you're doing here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes. Quick fix to not show "no users available" when loading. This table needs proper placeholder loading, but that will be done in another PR |
||
No users available. Get started by adding | ||
one. | ||
</span> | ||
</TablePlaceholder> | ||
} | ||
/> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import { PayloadType, type Variant } from 'unleash-client'; | ||
import { parseEnvVarBoolean } from '../util'; | ||
import { parseEnvVarBoolean, parseEnvVarNumber } from '../util'; | ||
import { getDefaultVariant } from 'unleash-client/lib/variant'; | ||
|
||
export type IFlagKey = | ||
|
@@ -289,10 +289,20 @@ const flags: IFlags = { | |
process.env.UNLEASH_EXPERIMENTAL_FLAG_OVERVIEW_REDESIGN, | ||
false, | ||
), | ||
showUserDeviceCount: parseEnvVarBoolean( | ||
process.env.UNLEASH_EXPERIMENTAL_SHOW_USER_DEVICE_COUNT, | ||
false, | ||
), | ||
showUserDeviceCount: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cool, didn't know we could do that :) |
||
name: 'showUserDeviceCount', | ||
enabled: parseEnvVarBoolean( | ||
process.env.UNLEASH_EXPERIMENTAL_SHOW_USER_DEVICE_COUNT, | ||
false, | ||
), | ||
payload: { | ||
type: PayloadType.NUMBER, | ||
value: `${parseEnvVarNumber( | ||
process.env.UNLEASH_EXPERIMENTAL_WARN_ABOVE_SESSION_COUNT, | ||
0, | ||
)}`, | ||
}, | ||
}, | ||
}; | ||
|
||
export const defaultExperimentalOptions: IExperimentalOptions = { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not a string as in other flags?