Skip to content

Topcoder Admin App - Roles Management Update #1056

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 29, 2025
Merged

Topcoder Admin App - Roles Management Update #1056

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d3c37e
Topcoder Admin App - Roles Management Update
suppermancool Apr 23, 2025
1c6ab4c
Merge branch 'dev' into diazz-admin-f2f-30376659
suppermancool Apr 28, 2025
26face4
Fix for GitHub bot suggestions
suppermancool Apr 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions src/apps/admin/src/lib/components/FieldSingleSelect/FieldSingleSelect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,22 @@ import {
useMemo,
useRef,
} from 'react'
import ReactSelect, { components, SingleValue } from 'react-select'
import { components, SingleValue } from 'react-select'
Copy link

Choose a reason for hiding this comment

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

The import statement for ReactSelect has been split into two separate imports. Consider consolidating them into a single import statement for better readability and to reduce redundancy.

import CreatableReactSelect from 'react-select/creatable'
import classNames from 'classnames'

import { IconOutline, InputWrapper, LoadingSpinner } from '~/libs/ui'

import { SelectOption } from '../../models'
import ReactSelect from '../common/ReactSelectExport'
Copy link

Choose a reason for hiding this comment

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

The import path for ReactSelect has been changed to a relative path. Ensure that this path is correct and that the ReactSelectExport module is available at the specified location. If this is intentional, verify that it aligns with the project's structure and conventions.


import styles from './FieldSingleSelect.module.scss'

interface Props {
label?: string
className?: string
placeholder?: string
readonly value?: SelectOption
readonly value?: SelectOption | null
readonly onChange?: (event: SelectOption) => void
readonly disabled?: boolean
readonly dirty?: boolean
Expand All @@ -32,6 +34,10 @@ interface Props {
readonly onBlur?: (event: FocusEvent<HTMLInputElement>) => void
readonly options: SelectOption[]
readonly isLoading?: boolean
readonly classNameWrapper?: string
readonly onSearchChange?: (value: string) => void
readonly creatable?: boolean
readonly createLabel?: (inputValue: string) => string
}

// eslint-disable-next-line react/function-component-definition
Expand Down Expand Up @@ -60,6 +66,10 @@ export const FieldSingleSelect: FC<Props> = (props: Props) => {
[],
)

const Input = useMemo(() => (
props.creatable ? CreatableReactSelect : ReactSelect
Copy link

Choose a reason for hiding this comment

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

Consider renaming the Input variable to something more descriptive, such as SelectComponent, to better convey its purpose and improve code readability.

), [props.creatable])

return (
<InputWrapper
{...props}
Expand All @@ -72,7 +82,7 @@ export const FieldSingleSelect: FC<Props> = (props: Props) => {
hideInlineErrors={props.hideInlineErrors}
ref={wrapRef as MutableRefObject<HTMLDivElement>}
>
<ReactSelect
<Input
Copy link

Choose a reason for hiding this comment

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

The component ReactSelect has been replaced with Input. Ensure that Input supports all the necessary props and functionality that ReactSelect provided, such as custom components, styling, and placeholder handling. Verify that asyncSelectComponents is compatible with Input.

components={asyncSelectComponents}
className={classNames(props.className, styles.select)}
placeholder={props.placeholder ?? 'Enter'}
Expand All @@ -88,9 +98,13 @@ export const FieldSingleSelect: FC<Props> = (props: Props) => {
}
}}
value={props.value}
defaultValue={props.value}
Copy link

Choose a reason for hiding this comment

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

The defaultValue prop is being set to props.value. Ensure that this is the intended behavior, as defaultValue is typically used for uncontrolled components, while value is for controlled components. Using both can lead to unexpected behavior.

isDisabled={props.disabled || props.isLoading}
onBlur={props.onBlur}
options={props.options}
onInputChange={props.onSearchChange}
Copy link

Choose a reason for hiding this comment

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

The onInputChange prop is being set to props.onSearchChange. Verify that props.onSearchChange is correctly handling the input change event and that it is intended to replace any existing input change logic.

createOptionPosition='first'
Copy link

Choose a reason for hiding this comment

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

The createOptionPosition prop is set to 'first'. Ensure this is the desired behavior, as it will affect where new options are added in the list.

formatCreateLabel={props.createLabel}
Copy link

Choose a reason for hiding this comment

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

The formatCreateLabel prop is set to props.createLabel. Ensure that props.createLabel is a function that returns a string or React element, as expected by the formatCreateLabel prop.

/>
{props.isLoading && (
<div className={styles.blockActionLoading}>
Expand Down
11 changes: 11 additions & 0 deletions src/apps/admin/src/lib/components/RoleMembersFilters/RoleMembersFilters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ export const RoleMembersFilters: FC<Props> = props => {
inputControl={register('userHandle')}
disabled={props.isLoading}
/>
<InputText
type='text'
name='email'
label='Email'
placeholder='Enter'
tabIndex={0}
onChange={_.noop}
Copy link

Choose a reason for hiding this comment

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

The onChange handler is set to _.noop, which means it does nothing. If the email input field is intended to be functional, consider implementing an appropriate handler to manage changes to the email input value.

classNameWrapper={styles.field}
inputControl={register('email')}
disabled={props.isLoading}
/>
</div>

<div className={styles.blockBottom}>
Expand Down
71 changes: 28 additions & 43 deletions src/apps/admin/src/lib/components/RoleMembersTable/RoleMembersTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
/**
* Role members table.
*/
import { FC, useContext, useEffect, useMemo } from 'react'
import { FC, useMemo } from 'react'
Copy link

Choose a reason for hiding this comment

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

The useContext and useEffect hooks have been removed from the imports. Ensure that their removal does not affect any functionality that depends on context or side effects.

import _ from 'lodash'
import classNames from 'classnames'

import { useWindowSize, WindowSize } from '~/libs/shared'
import { Button, InputCheckbox, Table, TableColumn } from '~/libs/ui'

import { AdminAppContext } from '../../contexts'
import { useTableFilterLocal, useTableFilterLocalProps } from '../../hooks'
import { AdminAppContextType, RoleMemberInfo } from '../../models'
import { RoleMemberInfo } from '../../models'
import { MobileTableColumn } from '../../models/MobileTableColumn.model'
import { Pagination } from '../common/Pagination'
import { TableMobile } from '../common/TableMobile'
Expand All @@ -28,8 +27,6 @@ interface Props {
}

export const RoleMembersTable: FC<Props> = (props: Props) => {
const { loadUser, usersMapping, cancelLoadUser }: AdminAppContextType
= useContext(AdminAppContext)
const {
page,
setPage,
Expand All @@ -51,27 +48,6 @@ export const RoleMembersTable: FC<Props> = (props: Props) => {
unselectAll,
}: useTableSelectionProps<string> = useTableSelection<string>(datasIds)

useEffect(() => {
// clear queue of currently loading user handles
cancelLoadUser()
// load user handles for members visible on the current page
_.forEach(results, result => {
loadUser(result.id)
})

return () => {
// clear queue of currently loading user handles after exit ui
cancelLoadUser()
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [results])

useEffect(() => {
_.forEach(results, result => {
result.handle = usersMapping[result.id]
})
}, [usersMapping, results])

const columns = useMemo<TableColumn<RoleMemberInfo>[]>(
() => [
{
Expand Down Expand Up @@ -108,20 +84,12 @@ export const RoleMembersTable: FC<Props> = (props: Props) => {
{
label: 'Handle',
Copy link

Choose a reason for hiding this comment

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

The propertyName for the 'Handle' column is set to 'handle', but the renderer logic has been removed. Ensure that usersMapping is correctly handled elsewhere to display the handle, or consider re-implementing the renderer if necessary.

propertyName: 'handle',
renderer: (data: RoleMemberInfo) => {
if (!data.id) {
return <></>
}

return (
<>
{!usersMapping[data.id]
? 'loading...'
: usersMapping[data.id]}
</>
)
},
type: 'element',
type: 'text',
},
{
label: 'Email',
Copy link

Choose a reason for hiding this comment

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

The addition of the 'Email' column with propertyName: 'email' assumes that the data source includes an 'email' field. Verify that the data source provides this field to avoid potential runtime errors.

propertyName: 'email',
type: 'text',
},
{
className: styles.blockColumnAction,
Expand All @@ -145,7 +113,6 @@ export const RoleMembersTable: FC<Props> = (props: Props) => {
[
isSelectAll,
selectedDatas,
usersMapping,
props.isRemoving,
props.isRemovingBool,
props.doRemoveRoleMember,
Expand All @@ -171,12 +138,30 @@ export const RoleMembersTable: FC<Props> = (props: Props) => {
),
type: 'element',
},
], [
{
...columns[3],
className: '',
label: `${columns[3].label as string} label`,
Copy link

Choose a reason for hiding this comment

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

The label concatenation with 'label' seems redundant. Consider revising the label assignment to avoid potential confusion.

mobileType: 'label',
renderer: () => (
<div>
{columns[3].label as string}
:
</div>
),
type: 'element',
},
{
...columns[3],
Copy link

Choose a reason for hiding this comment

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

The use of ...columns[3] is repeated here. Ensure that this is intentional and that the properties from columns[3] are correctly applied to this new column.

mobileType: 'last-value',
},
],
[
{
...columns[3],
...columns[4],
Copy link

Choose a reason for hiding this comment

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

The spread operator is changed from columns[3] to columns[4]. Verify that this change is correct and that the intended column is being referenced.

className: classNames(
columns[3].className,
columns[4].className,
styles.blockRightColumn,
),
colSpan: 2,
Expand Down
Loading