Skip to content

Topcoder Admin App - Misc Update 0601 #1093

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

Merged
merged 2 commits into from
Jun 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { useManageAddBillingAccount, useManageAddBillingAccountProps } from '../
import { BILLING_ACCOUNT_STATUS_EDIT_OPTIONS } from '../../config/index.config'
import { PageContent, PageHeader } from '../../lib'
import { FormEditBillingAccount, SelectOption } from '../../lib/models'
import { formEditBillingAccountSchema } from '../../lib/utils'
import { formAddBillingAccountSchema, formEditBillingAccountSchema } from '../../lib/utils'

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

Expand All @@ -46,6 +46,7 @@ export const BillingAccountNewPage: FC<Props> = (props: Props) => {
const { accountId = '' }: { accountId?: string } = useParams<{
accountId: string
}>()
const isEdit = useMemo(() => !!accountId, [accountId])
const {
isLoading,
isAdding,
Expand All @@ -55,8 +56,8 @@ export const BillingAccountNewPage: FC<Props> = (props: Props) => {
billingAccount,
}: useManageAddBillingAccountProps = useManageAddBillingAccount(accountId)
const pageTitle = useMemo(
() => (accountId ? 'Edit Billing Account' : 'New Billing Account'),
[accountId],
() => (isEdit ? 'Edit Billing Account' : 'New Billing Account'),
[isEdit],
)
const {
register,
Expand All @@ -81,7 +82,9 @@ export const BillingAccountNewPage: FC<Props> = (props: Props) => {
subscriptionNumber: '' as any,
},
mode: 'all',
resolver: yupResolver(formEditBillingAccountSchema),
resolver: yupResolver(
isEdit ? formEditBillingAccountSchema : formAddBillingAccountSchema,
),
})

const endDate = watch('endDate')
Expand Down Expand Up @@ -354,7 +357,9 @@ export const BillingAccountNewPage: FC<Props> = (props: Props) => {
onChange={_.noop}
classNameWrapper={styles.field}
inputControl={register('paymentTerms')}
disabled={isAdding || isUpdating}
disabled={
isEdit ? true : isAdding || isUpdating
}
error={_.get(errors, 'paymentTerms.message')}
dirty
/>
Expand Down
27 changes: 2 additions & 25 deletions src/apps/admin/src/lib/components/ReviewerList/ReviewerList.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FC, useMemo } from 'react'
import { format } from 'date-fns'

import { CheckIcon, XIcon } from '@heroicons/react/solid'
import { CheckIcon } from '@heroicons/react/solid'
Copy link

Choose a reason for hiding this comment

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

The XIcon import has been removed. Ensure that this icon is not used elsewhere in the component. If it is needed, it should be re-imported.

import { useWindowSize, WindowSize } from '~/libs/shared'
import {
Button,
Expand All @@ -27,7 +27,6 @@ export interface ReviewerListProps {
approvingReviewerId: number
onPageChange: (page: number) => void
onApproveApplication: (reviewer: Reviewer) => void
onUnapproveApplication: (reviewer: Reviewer) => void
onToggleSort: (sort: Sort) => void
}

Expand All @@ -36,22 +35,15 @@ const ApproveButton: FC<{
openReviews: number
approvingReviewerId: number
onApproveApplication: ReviewerListProps['onApproveApplication']
onUnapproveApplication: ReviewerListProps['onUnapproveApplication']
}> = props => {
const handleApprove = useEventCallback((): void => {
Copy link

Choose a reason for hiding this comment

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

The onUnapproveApplication prop has been removed from the component's props. Ensure that this is intentional and that the functionality for unapproving an application is no longer required.

props.onApproveApplication(props.reviewer)
})

const handleRemove = useEventCallback((): void => {
props.onUnapproveApplication(props.reviewer)
})

const isApproving = props.approvingReviewerId === props.reviewer.userId
const isOtherApproving = props.approvingReviewerId > 0
const hideApproveButton
Copy link

Choose a reason for hiding this comment

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

The handleRemove function has been removed. Verify that the removal of this function aligns with the intended functionality changes, as it was previously used to handle unapproving applications.

= props.openReviews < 1 || props.reviewer.applicationStatus !== 'Pending'
const showRemoveButton
= props.reviewer.applicationStatus === 'Approved'

return (
<>
Expand All @@ -61,19 +53,7 @@ const ApproveButton: FC<{
className={styles.approvingLoadingSpinner}
Copy link

Choose a reason for hiding this comment

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

The showRemoveButton logic has been removed. Confirm that the removal of this logic is intended and that the UI no longer requires a remove button for approved applications.

/>
) : (
hideApproveButton ? (
showRemoveButton && (
<Button
primary
variant='danger'
onClick={handleRemove}
>
<XIcon className='icon icon-fill' />
{' '}
Remove Reviewer
</Button>
)
) : (
!hideApproveButton && (
<Button
Copy link

Choose a reason for hiding this comment

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

The variant='danger' prop was removed from the Button component. If this was intentional, ensure that the styling and behavior of the button still meet the requirements. If it was not intentional, consider adding it back to maintain the previous styling.

primary
onClick={handleApprove}
Expand Down Expand Up @@ -125,15 +105,13 @@ const Actions: FC<{
openReviews: number
approvingReviewerId: number
onApproveApplication: ReviewerListProps['onApproveApplication']
onUnapproveApplication: ReviewerListProps['onUnapproveApplication']
}> = props => (
<div className={styles.rowActions}>
<ApproveButton
reviewer={props.reviewer}
openReviews={props.openReviews}
approvingReviewerId={props.approvingReviewerId}
onApproveApplication={props.onApproveApplication}
onUnapproveApplication={props.onUnapproveApplication}
/>
</div>
)
Expand Down Expand Up @@ -202,7 +180,6 @@ const ReviewerList: FC<ReviewerListProps> = props => {
openReviews={props.openReviews}
approvingReviewerId={props.approvingReviewerId}
onApproveApplication={props.onApproveApplication}
onUnapproveApplication={props.onUnapproveApplication}
/>
),
type: 'action',
Expand Down
5 changes: 0 additions & 5 deletions src/apps/admin/src/lib/components/RolesFilter/RolesFilter.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,6 @@ export const RolesFilter: FC<Props> = props => {

useEffect(() => {
props.setFilters({
createdAtString: roleName,
createdByHandle: roleName,
id: roleName,
modifiedAtString: roleName,
modifiedByHandle: roleName,
roleName,
})
}, [roleName]) // eslint-disable-line react-hooks/exhaustive-deps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export interface FormEditBillingAccount {
poNumber: string
subscriptionNumber?: number
description: string
paymentTerms: number
paymentTerms?: number
salesTax?: number
client: {
id: number
Expand Down
10 changes: 5 additions & 5 deletions src/apps/admin/src/lib/models/TableRolesFilter.type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
* Model for table roles filter
*/
export type TableRolesFilter = {
id: string
id?: string
Copy link

Choose a reason for hiding this comment

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

The id field has been changed to optional. Ensure that this change aligns with the intended functionality and that all parts of the application that rely on this field being present are updated accordingly.

roleName: string
createdAtString: string
modifiedAtString: string
createdByHandle: string
modifiedByHandle: string
createdAtString?: string
Copy link

Choose a reason for hiding this comment

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

The createdAtString field has been made optional. Verify that this change does not affect any logic that assumes this field is always present.

modifiedAtString?: string
Copy link

Choose a reason for hiding this comment

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

The modifiedAtString field is now optional. Check if there are any dependencies or logic that require this field to be non-optional.

createdByHandle?: string
Copy link

Choose a reason for hiding this comment

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

The createdByHandle field has been changed to optional. Ensure that this change does not break any existing functionality that expects this field to be mandatory.

modifiedByHandle?: string
Copy link

Choose a reason for hiding this comment

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

The modifiedByHandle field is now optional. Confirm that this change is consistent with the application's requirements and does not introduce any issues.


}
55 changes: 54 additions & 1 deletion src/apps/admin/src/lib/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function isValidNumber(value: number | undefined): boolean {
/**
* validation schema for form new billing account
*/
export const formEditBillingAccountSchema: Yup.ObjectSchema<FormEditBillingAccount>
export const formAddBillingAccountSchema: Yup.ObjectSchema<FormEditBillingAccount>
Copy link

Choose a reason for hiding this comment

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

The variable name formAddBillingAccountSchema suggests that this schema is for adding a new billing account, but the type FormEditBillingAccount implies it is for editing. Ensure the naming is consistent with the intended use of the schema.

= Yup.object({
budgetAmount: Yup.number()
.transform(value => (Number.isNaN(value) ? undefined : value))
Expand Down Expand Up @@ -183,6 +183,59 @@ export const formEditBillingAccountSchema: Yup.ObjectSchema<FormEditBillingAccou
.min(1, 'Subscription number must be greater than or equal 1.'),
})

/**
* validation schema for form new billing account
*/
export const formEditBillingAccountSchema: Yup.ObjectSchema<FormEditBillingAccount>
Copy link

Choose a reason for hiding this comment

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

The variable formEditBillingAccountSchema is declared twice in the file. Consider renaming the newly added schema to avoid conflicts and ensure clarity.

= Yup.object({
budgetAmount: Yup.number()
.transform(value => (Number.isNaN(value) ? undefined : value))
.optional()
.typeError('Invalid number.')
.min(1, 'Budget amount must be greater than or equal 1.'),
client: Yup.object()
.shape({
id: Yup.number()
.typeError('Invalid number.')
.required('Id is required.'),
name: Yup.string()
.required('Name is required.'),
})
.default(undefined)
.required('Client is required.'),
companyId: Yup.number()
.typeError('Invalid number.')
.required('Customer number is required.')
.min(1, 'Customer number must be greater than or equal 1.'),
description: Yup.string()
.trim()
.required('Description is required.'),
endDate: Yup.date()
.required('End date is required.'),
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 .nullable() method to the endDate field to handle cases where the date might be null.

name: Yup.string()
.trim()
.required('Name is required.'),
paymentTerms: Yup.number()
.transform(value => (Number.isNaN(value) ? undefined : value))
.optional()
.typeError('Invalid number.')
.min(1, 'Payment terms must be greater than or equal 1.'),
poNumber: Yup.string()
.trim()
.required('PO Number is required.'),
salesTax: Yup.number()
.required('Sales tax is required.'),
Copy link

Choose a reason for hiding this comment

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

The salesTax field should have a .typeError('Invalid number.') similar to other number fields for consistency in error messaging.

startDate: Yup.date()
.required('Start date is required.'),
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 .nullable() method to the startDate field to handle cases where the date might be null.

status: Yup.string()
.required('Status is required.'),
subscriptionNumber: Yup.number()
.transform(value => (Number.isNaN(value) ? undefined : value))
.optional()
.typeError('Invalid number.')
.min(1, 'Subscription number must be greater than or equal 1.'),
})

/**
* validation schema for form new billing account
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
gap: 30px;
margin-left: auto;
margin-top: 16px;
flex-wrap: wrap;
}

.loadingSpinnerContainer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
useRef,
useState,
} from 'react'
import { NavigateFunction, useNavigate, useParams } from 'react-router-dom'
import { useParams } from 'react-router-dom'
Copy link

Choose a reason for hiding this comment

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

The NavigateFunction and useNavigate imports have been removed. Ensure that these are not used elsewhere in the file, as their removal could lead to runtime errors if they are still needed.

import { sortBy } from 'lodash'

import { XIcon } from '@heroicons/react/solid'
Expand Down Expand Up @@ -63,7 +63,6 @@ export const ManageReviewerPage: FC = () => {
challengeId: string
}>()
const [challengeUuid, setChallengeUuid] = useState('')
const navigate: NavigateFunction = useNavigate()
const [filterCriteria, setFilterCriteria]: [
Copy link

Choose a reason for hiding this comment

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

The navigate variable declaration has been removed. Ensure that this removal does not affect any navigation functionality within the component. If navigate was used elsewhere in the code, consider refactoring those parts to avoid errors.

ReviewFilterCriteria,
Dispatch<SetStateAction<ReviewFilterCriteria>>
Expand Down Expand Up @@ -137,12 +136,6 @@ export const ManageReviewerPage: FC = () => {
})
})

const unapprove = useEventCallback((): void => {
if (challengeUuid) {
navigate(`${rootRoute}/challenge-management/${challengeUuid}/manage-user`)
}
})

// Init
useEffect(() => {
search()
Expand Down Expand Up @@ -204,6 +197,14 @@ export const ManageReviewerPage: FC = () => {
<PageHeader>
<h2>{`${pageTitle} ${challengeId}`}</h2>
<div className={styles.headerActions}>
<LinkButton
primary
onClick={handleRejectPendingConfirmDialog}
size='lg'
to={`${rootRoute}/challenge-management/${challengeUuid}/manage-user`}
Copy link

Choose a reason for hiding this comment

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

The to prop in the LinkButton component should be reviewed to ensure it correctly constructs the URL. Verify that rootRoute, challengeUuid, and the rest of the path are correctly defined and concatenated to form a valid URL.

>
User Management
</LinkButton>
<Button
primary
variant='danger'
Expand Down Expand Up @@ -232,7 +233,6 @@ export const ManageReviewerPage: FC = () => {
reviewers={reviewers}
openReviews={openReviews}
onApproveApplication={approve}
Copy link

Choose a reason for hiding this comment

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

The onUnapproveApplication prop has been removed. Ensure that this is intentional and that the functionality for unapproving applications is no longer needed. If it is still required, consider re-adding it or implementing the functionality elsewhere.

onUnapproveApplication={unapprove}
approvingReviewerId={userId}
paging={{
page: filterCriteria.page,
Expand Down