Skip to content

Topcoder Admin App - Misc Update 0505 #1069

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

Closed
wants to merge 1 commit into from
Closed

Topcoder Admin App - Misc Update 0505 #1069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
50 changes: 23 additions & 27 deletions src/apps/admin/src/challenge-management/ChallengeManagementPage/ChallengeManagementPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
useState,
} from 'react'
import { useSearchParams } from 'react-router-dom'
import _ from 'lodash'
Copy link

Choose a reason for hiding this comment

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

The lodash library is imported but not used in the current code changes. Consider removing the import if it is not needed to avoid unnecessary dependencies.


import { LoadingSpinner, PageDivider, PageTitle } from '~/libs/ui'
import { PaginatedResponse } from '~/libs/core'
Expand All @@ -36,6 +37,17 @@ import { useEventCallback } from '../../lib/hooks'

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

const defaultFilter: ChallengeFilterCriteria = {
challengeId: '',
legacyId: 0,
name: '',
page: 1,
perPage: 25,
status: ChallengeStatus.Active,
track: null!, // eslint-disable-line @typescript-eslint/no-non-null-assertion, unicorn/no-null
Copy link

Choose a reason for hiding this comment

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

Using null! with non-null assertion can lead to runtime errors if track is accessed before being properly initialized. Consider using a more type-safe approach, such as optional chaining or default values.

type: null!, // eslint-disable-line @typescript-eslint/no-non-null-assertion, unicorn/no-null
Copy link

Choose a reason for hiding this comment

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

Using null! with non-null assertion can lead to runtime errors if type is accessed before being properly initialized. Consider using a more type-safe approach, such as optional chaining or default values.

}

/**
* Challenge Management page.
*/
Expand All @@ -45,15 +57,9 @@ export const ChallengeManagementPage: FC = () => {
ChallengeFilterCriteria,
Dispatch<SetStateAction<ChallengeFilterCriteria>>,
] = useState<ChallengeFilterCriteria>({
challengeId: '',
legacyId: 0,
name: '',
page: 1,
perPage: 25,
status: ChallengeStatus.Active,
track: null!, // eslint-disable-line @typescript-eslint/no-non-null-assertion, unicorn/no-null
type: null!, // eslint-disable-line @typescript-eslint/no-non-null-assertion, unicorn/no-null
...defaultFilter,
})
const disableReset = useMemo(() => _.isEqual(filterCriteria, defaultFilter), [filterCriteria])
Copy link

Choose a reason for hiding this comment

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

Consider importing only the isEqual function from lodash instead of the entire lodash library to reduce bundle size: import isEqual from 'lodash/isEqual';

const [challenges, setChallenges]: [
Array<Challenge>,
Dispatch<SetStateAction<Array<Challenge>>>,
Expand Down Expand Up @@ -119,19 +125,6 @@ export const ChallengeManagementPage: FC = () => {
}
}, [pageChangeEvent]) // eslint-disable-line react-hooks/exhaustive-deps -- missing dependency: search

// Reset
const [resetEvent, setResetEvent] = useState(false)
useEffect(() => {
if (resetEvent) {
search()
setResetEvent(false)
}
}, [resetEvent]) // eslint-disable-line react-hooks/exhaustive-deps -- missing dependency: search

const handleReset = useEventCallback(() => {
previousPageChangeEvent.current = false
setResetEvent(true)
})
const handlePageChange = useEventCallback((page: number) => {
setFilterCriteria({ ...filterCriteria, page })
setPageChangeEvent(true)
Expand All @@ -149,12 +142,15 @@ export const ChallengeManagementPage: FC = () => {
onFilterCriteriaChange={setFilterCriteria}
onSearch={search}
disabled={searching || !filtersInited}
showResetButton={
previousPageChangeEvent.current
&& searched
&& challenges.length === 0
}
onReset={handleReset}
onReset={function onReset() {
Copy link

Choose a reason for hiding this comment

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

Consider using an arrow function for onReset to maintain consistency with other function definitions in the component.

setFilterCriteria({
...defaultFilter,
})
setTimeout(() => {
search()
Copy link

Choose a reason for hiding this comment

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

The setTimeout function is used here without a delay parameter. If the intention is to execute search() immediately, consider removing setTimeout for clarity.

})
}}
disableReset={disableReset}
/>
<PageDivider />
{searching && (
Expand Down
18 changes: 0 additions & 18 deletions ...ps/admin/src/lib/components/BillingAccountResourcesTable/BillingAccountResourcesTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,6 @@ export const BillingAccountResourcesTable: FC<Props> = (props: Props) => {
propertyName: 'name',
type: 'text',
},
{
label: 'Status',
propertyName: 'status',
type: 'text',
},
{
className: styles.blockColumnAction,
label: '',
Expand Down Expand Up @@ -81,21 +76,8 @@ export const BillingAccountResourcesTable: FC<Props> = (props: Props) => {
},
],
[
{
label: 'Status label',
mobileType: 'label',
propertyName: 'status',
renderer: () => <div>Status:</div>,
type: 'element',
},
{
...columns[1],
mobileType: 'last-value',
},
],
[
{
...columns[2],
colSpan: 2,
mobileType: 'last-value',
},
Expand Down
2 changes: 1 addition & 1 deletion src/apps/admin/src/lib/components/BillingAccountsFilter/BillingAccountsFilter.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
display: flex;
justify-content: flex-end;
align-items: flex-start;
gap: 30px;
gap: 15px;
flex-wrap: wrap;

@include ltemd {
Expand Down
29 changes: 25 additions & 4 deletions src/apps/admin/src/lib/components/BillingAccountsFilter/BillingAccountsFilter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,22 +27,30 @@ interface Props {
onSubmitForm?: (data: FormBillingAccountsFilter) => void
}

const defaultValues: FormBillingAccountsFilter = {
endDate: undefined,
name: '',
startDate: undefined,
status: '1',
user: '',
}

export const BillingAccountsFilter: FC<Props> = (props: Props) => {
const maxDate = useMemo(() => moment()
.add(20, 'y')
.toDate(), [])
const {
register,
reset,
handleSubmit,
control,
formState: { isValid },
formState: { isValid, isDirty },
Copy link

Choose a reason for hiding this comment

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

The isDirty property is being destructured from formState but is not used anywhere in the component. Consider removing it if it's not needed.

}: UseFormReturn<FormBillingAccountsFilter> = useForm({
defaultValues: {
status: '1',
},
defaultValues,
mode: 'all',
resolver: yupResolver(formBillingAccountsFilterSchema),
})

const onSubmit = useCallback(
(data: FormBillingAccountsFilter) => {
props.onSubmitForm?.(data)
Expand Down Expand Up @@ -162,6 +170,19 @@ export const BillingAccountsFilter: FC<Props> = (props: Props) => {
>
Filter
</Button>
<Button
secondary
onClick={function onClick() {
reset(defaultValues)
setTimeout(() => {
onSubmit(defaultValues)
Copy link

Choose a reason for hiding this comment

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

Consider providing a delay duration for setTimeout to ensure consistent behavior across different environments.

})
}}
size='lg'
disabled={!isDirty}
>
Reset
</Button>
</div>
</form>
)
Expand Down
11 changes: 7 additions & 4 deletions src/apps/admin/src/lib/components/ChallengeFilters/ChallengeFilters.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@
gap: 15px 30px;
}

.searchButton {
margin-left: auto;
}

@media (max-width: #{$lg-max}) {
.filters {
grid-template-columns: 1fr 1fr;
Expand All @@ -32,3 +28,10 @@
}
}
}

.blockBtns {
margin-left: auto;
display: flex;
gap: 15px;
justify-content: flex-end;
}
12 changes: 4 additions & 8 deletions src/apps/admin/src/lib/components/ChallengeFilters/ChallengeFilters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ import styles from './ChallengeFilters.module.scss'
interface ChallengeFiltersProps {
filterCriteria: ChallengeFilterCriteria
disabled: boolean
showResetButton: boolean
onFilterCriteriaChange: (newFilterCriteria: ChallengeFilterCriteria) => void
onSearch: () => void
onReset: () => void
disableReset: boolean
}

const ChallengeFilters: FC<ChallengeFiltersProps> = props => {
Expand Down Expand Up @@ -188,28 +188,24 @@ const ChallengeFilters: FC<ChallengeFiltersProps> = props => {
disabled={props.disabled}
/>
</div>
{!props.showResetButton && (
<div className={styles.blockBtns}>
<Button
primary
className={styles.searchButton}
onClick={props.onSearch}
disabled={props.disabled}
size='lg'
>
Search
</Button>
)}
{props.showResetButton && (
<Button
secondary
className={styles.searchButton}
onClick={handleReset}
disabled={props.disabled}
disabled={props.disabled || props.disableReset}
Copy link

Choose a reason for hiding this comment

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

Consider renaming props.disableReset to props.isResetDisabled for consistency with boolean naming conventions.

size='lg'
>
Reset
</Button>
)}
</div>
</div>
)
}
Expand Down
8 changes: 7 additions & 1 deletion src/apps/admin/src/lib/components/ChallengeList/ChallengeList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,13 @@ const ChallengeList: FC<ChallengeListProps> = props => {
propertyName: 'name',
renderer: (challenge: Challenge) => (
// eslint-disable-next-line jsx-a11y/anchor-is-valid
<a href='#' className={styles.challengeTitle}>
<a
href={`${EnvironmentConfig.ADMIN.CHALLENGE_URL}/${challenge.id}`}
className={styles.challengeTitle}
onClick={function onClick() {
Copy link

Choose a reason for hiding this comment

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

The onClick handler is setting window.location.href to the same URL as the href attribute. This is redundant and can be removed unless there is a specific reason for having both.

window.location.href = `${EnvironmentConfig.ADMIN.CHALLENGE_URL}/${challenge.id}`
}}
>
{challenge.name}
</a>
),
Expand Down
2 changes: 1 addition & 1 deletion src/apps/admin/src/lib/components/ClientsFilter/ClientsFilter.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
display: flex;
justify-content: flex-end;
align-items: flex-start;
gap: 30px;
gap: 15px;
flex-wrap: wrap;

@include ltemd {
Expand Down
27 changes: 23 additions & 4 deletions src/apps/admin/src/lib/components/ClientsFilter/ClientsFilter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,25 @@ interface Props {
onSubmitForm?: (data: FormClientsFilter) => void
}

const defaultValues: FormClientsFilter = {
endDate: undefined,
name: '',
startDate: undefined,
status: '1',
}

export const ClientsFilter: FC<Props> = (props: Props) => {
const maxDate = useMemo(() => moment()
.add(20, 'y')
.toDate(), [])
const {
register,
reset,
handleSubmit,
control,
formState: { isValid },
formState: { isValid, isDirty },
Copy link

Choose a reason for hiding this comment

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

The isDirty property is being destructured from formState but is not used anywhere in the component. If it's not needed, consider removing it to keep the code clean.

}: UseFormReturn<FormClientsFilter> = useForm({
defaultValues: {
status: '1',
},
defaultValues,
mode: 'all',
resolver: yupResolver(formClientsFilterSchema),
})
Expand Down Expand Up @@ -149,6 +155,19 @@ export const ClientsFilter: FC<Props> = (props: Props) => {
>
Filter
</Button>
<Button
secondary
onClick={function onClick() {
reset(defaultValues)
setTimeout(() => {
onSubmit(defaultValues)
Copy link

Choose a reason for hiding this comment

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

Consider adding a delay duration to the setTimeout function to ensure consistent behavior. Currently, it defaults to 0 milliseconds, which might not be necessary.

})
}}
size='lg'
disabled={!isDirty}
>
Reset
</Button>
</div>
</form>
)
Expand Down
5 changes: 4 additions & 1 deletion src/apps/admin/src/lib/components/ReviewSummaryList/ReviewSummaryList.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@
}
}

.challengeTitle {
.challengeTitleText,
.challengeTitleLink {
min-width: 200px;
padding: 0;
justify-content: flex-start;
border-radius: 0;
color: $body-color;
line-height: 16px;
white-space: break-spaces;
}

.challengeTitleLink {
Copy link

Choose a reason for hiding this comment

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

The class .challengeTitleLink is defined but not used in this diff. Ensure that it is applied to the appropriate elements in the HTML/JSX to have the intended effect.

&:hover {
color: $blue-110;
}
Expand Down
11 changes: 8 additions & 3 deletions src/apps/admin/src/lib/components/ReviewSummaryList/ReviewSummaryList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { FC, useMemo } from 'react'
import { useNavigate } from 'react-router-dom'

import { EnvironmentConfig } from '~/config'
import { useWindowSize, WindowSize } from '~/libs/shared'
import { Button, LinkButton, Table, type TableColumn } from '~/libs/ui'
import { Sort } from '~/apps/gamification-admin/src/game-lib/pagination'
Expand Down Expand Up @@ -45,13 +46,17 @@ const ChallengeTitle: FC<{
review: ReviewSummary
}> = props => {
const goToChallenge = useEventCallback(() => {
window.location.href = `https://www.topcoder.com/challenges/${props.review.legacyChallengeId}`
window.location.href = `${EnvironmentConfig.ADMIN.CHALLENGE_URL}/${props.review.legacyChallengeId}`
Copy link

Choose a reason for hiding this comment

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

Consider verifying that EnvironmentConfig.ADMIN.CHALLENGE_URL is correctly defined and accessible to avoid potential runtime errors.

})

return (
<LinkButton onClick={goToChallenge} className={styles.challengeTitle}>
return props.review.legacyChallengeId ? (
<LinkButton onClick={goToChallenge} className={styles.challengeTitleLink}>
Copy link

Choose a reason for hiding this comment

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

Ensure that props.review.legacyChallengeId is always defined when expected. If it can be undefined, consider handling this case appropriately to avoid unexpected behavior.

{props.review.challengeName}
Copy link

Choose a reason for hiding this comment

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

The class name styles.challengeTitleLink should be verified to ensure it matches the intended styling. If this is a new class name, ensure it is defined in the styles.

</LinkButton>
) : (
<span className={styles.challengeTitleText}>
Copy link

Choose a reason for hiding this comment

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

The class name styles.challengeTitleText should be verified to ensure it matches the intended styling. If this is a new class name, ensure it is defined in the styles.

{props.review.challengeName}
</span>
)
}

Expand Down
6 changes: 6 additions & 0 deletions src/apps/admin/src/lib/components/UsersFilters/UsersFilters.module.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,9 @@
font-size: 14px;
color: $black-60;
}

.blockBtns {
Copy link

Choose a reason for hiding this comment

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

Consider using a more descriptive class name than .blockBtns to improve readability and maintainability of the code. A name that reflects the purpose or context of the buttons would be beneficial.

display: flex;
gap: 15px;
justify-content: flex-end;
}
Loading