Skip to content
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
5 changes: 4 additions & 1 deletion src/apps/onboarding/src/redux/actions/member.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ const createWorksPayloadData: any = (works: WorkInfo[]) => {
const data: any = works.map(work => {
const {
companyName,
company,
position,
industry,
otherIndustry,
Expand All @@ -222,10 +223,12 @@ const createWorksPayloadData: any = (works: WorkInfo[]) => {
city,
associatedSkills,
}: any = work
const normalizedCompanyName: string = _.trim(companyName || company || '')

Choose a reason for hiding this comment

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

[⚠️ correctness]
Using _.trim on companyName || company || '' is safe, but consider handling cases where both companyName and company are non-empty but different. This could lead to unexpected results if the data is inconsistent.

return {
associatedSkills: Array.isArray(associatedSkills) ? associatedSkills : undefined,
cityName: city,
companyName: companyName || '',
company: normalizedCompanyName || undefined,

Choose a reason for hiding this comment

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

[⚠️ correctness]
The property company is being set to normalizedCompanyName || undefined. If normalizedCompanyName is an empty string, this will result in company being undefined. Ensure this behavior is intentional and aligns with the expected data structure.

companyName: normalizedCompanyName,
description: description || undefined,
// eslint-disable-next-line unicorn/no-null
endDate: endDate ? endDate.toISOString() : null,
Expand Down
146 changes: 118 additions & 28 deletions src/apps/wallet-admin/src/home/tabs/payments/PaymentsTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { toast } from 'react-toastify'
import { AxiosError } from 'axios'
import React, { FC, useCallback, useEffect } from 'react'

import { Collapsible, ConfirmModal, LoadingCircles } from '~/libs/ui'
import { Collapsible, ConfirmModal, InputText, LoadingCircles } from '~/libs/ui'
import { UserProfile } from '~/libs/core'
import { downloadBlob } from '~/libs/shared'

Expand All @@ -13,6 +13,7 @@ import { Winning, WinningDetail } from '../../../lib/models/WinningDetail'
import { FilterBar, formatIOSDateString, PaymentView } from '../../../lib'
import { ConfirmFlowData } from '../../../lib/models/ConfirmFlowData'
import { PaginationInfo } from '../../../lib/models/PaginationInfo'
import { Filter } from '../../../lib/components/filter-bar/FilterBar'
import PaymentEditForm from '../../../lib/components/payment-edit/PaymentEdit'
import PaymentsTable from '../../../lib/components/payments-table/PaymentTable'

Expand Down Expand Up @@ -69,6 +70,7 @@ const ListView: FC<ListViewProps> = (props: ListViewProps) => {
const [isConfirmFormValid, setIsConfirmFormValid] = React.useState<boolean>(false)
const [winnings, setWinnings] = React.useState<ReadonlyArray<Winning>>([])
const [selectedPayments, setSelectedPayments] = React.useState<{ [paymentId: string]: Winning }>({})
const selectedPaymentsCount = Object.keys(selectedPayments).length
const [isLoading, setIsLoading] = React.useState<boolean>(false)
const [filters, setFilters] = React.useState<Record<string, string[]>>({})
const [pagination, setPagination] = React.useState<PaginationInfo>({
Expand Down Expand Up @@ -290,6 +292,50 @@ const ListView: FC<ListViewProps> = (props: ListViewProps) => {
|| props.profile.roles.includes('Payment Editor')
)

const isEngagementPaymentApprover = props.profile.roles.includes('Engagement Payment Approver')
const [bulkOpen, setBulkOpen] = React.useState(false)
const [bulkAuditNote, setBulkAuditNote] = React.useState('')

const onBulkApprove = useCallback(async (auditNote: string) => {
const ids = Object.keys(selectedPayments)
if (ids.length === 0) return

toast.success('Starting bulk approve', { position: toast.POSITION.BOTTOM_RIGHT })

let successfullyUpdated = 0
for (const id of ids) {

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Using any for updates can lead to runtime errors and makes the code harder to maintain. Consider defining a specific type for updates to ensure type safety.

const updates: any = {
auditNote,
paymentStatus: 'OWED',
winningsId: id,
}

try {
// awaiting sequentially to preserve order and server load control
// errors for individual items are caught and reported
// eslint-disable-next-line no-await-in-loop
await editPayment(updates)

Choose a reason for hiding this comment

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

[⚠️ performance]
The editPayment function is awaited inside a loop. This can lead to performance issues if the number of payments is large, as each request waits for the previous one to complete. Consider using Promise.all to execute these requests concurrently, if the order of execution is not critical.

successfullyUpdated += 1
} catch (err:any) {
const paymentName = selectedPayments[id]?.handle || id
if (err?.message) {
toast.error(`Failed to update payment ${paymentName} (${id}): ${err.message}`, { position: toast.POSITION.BOTTOM_RIGHT })
} else {
toast.error(`Failed to update payment ${paymentName} (${id})`, { position: toast.POSITION.BOTTOM_RIGHT })
}
}
}

if (successfullyUpdated === ids.length) {

Choose a reason for hiding this comment

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

[💡 usability]
The success message is only shown if all payments are successfully updated. Consider providing feedback for partial success as well, to inform the user how many payments were successfully updated even if some failed.

toast.success(`Successfully updated ${successfullyUpdated} winnings`, { position: toast.POSITION.BOTTOM_RIGHT })
}

setBulkAuditNote('')
setBulkOpen(false)
setSelectedPayments({})
await fetchWinnings()
}, [selectedPayments, fetchWinnings])

return (
<>
<div className={styles.container}>
Expand All @@ -300,6 +346,8 @@ const ListView: FC<ListViewProps> = (props: ListViewProps) => {
<Collapsible header={<h3>Payment Listing</h3>}>
<FilterBar
showExportButton
selectedCount={selectedPaymentsCount}
onBulkClick={() => setBulkOpen(true)}
onExport={async () => {
toast.success('Downloading payments report. This may take a few moments.', { position: toast.POSITION.BOTTOM_RIGHT })
downloadBlob(
Expand Down Expand Up @@ -354,33 +402,35 @@ const ListView: FC<ListViewProps> = (props: ListViewProps) => {
],
type: 'dropdown',
},
{
key: 'type',
label: 'Type',
options: [
{
label: 'Task Payment',
value: 'TASK_PAYMENT',
},
{
label: 'Contest Payment',
value: 'CONTEST_PAYMENT',
},
{
label: 'Copilot Payment',
value: 'COPILOT_PAYMENT',
},
{
label: 'Review Board Payment',
value: 'REVIEW_BOARD_PAYMENT',
},
{
label: 'Engagement Payment',
value: 'ENGAGEMENT_PAYMENT',
},
],
type: 'dropdown',
},
...(isEngagementPaymentApprover ? [] : [
{
key: 'category',
label: 'Type',
options: [
{
label: 'Task Payment',
value: 'TASK_PAYMENT',
},
{
label: 'Contest Payment',
value: 'CONTEST_PAYMENT',
},
{
label: 'Copilot Payment',
value: 'COPILOT_PAYMENT',
},
{
label: 'Review Board Payment',
value: 'REVIEW_BOARD_PAYMENT',
},
{
label: 'Engagement Payment',
value: 'ENGAGEMENT_PAYMENT',
},
],
type: 'dropdown',
},
] as Filter[]),
{
key: 'date',
label: 'Date',
Expand Down Expand Up @@ -449,11 +499,13 @@ const ListView: FC<ListViewProps> = (props: ListViewProps) => {
{isLoading && <LoadingCircles className={styles.centered} />}
{!isLoading && winnings.length > 0 && (
<PaymentsTable
enableBulkEdit={isEngagementPaymentApprover}
canEdit={isEditingAllowed()}
currentPage={pagination.currentPage}
numPages={pagination.totalPages}
payments={winnings}
selectedPayments={selectedPayments}
onSelectionChange={selected => setSelectedPayments(selected)}
onNextPageClick={async function onNextPageClicked() {
if (pagination.currentPage === pagination.totalPages) {
return
Expand Down Expand Up @@ -513,6 +565,44 @@ const ListView: FC<ListViewProps> = (props: ListViewProps) => {
</Collapsible>
</div>
</div>
{bulkOpen && (
<ConfirmModal
maxWidth='800px'
size='lg'
showButtons
title={`${selectedPaymentsCount > 1 ? 'Bulk ' : ''}Approve Payment${selectedPaymentsCount > 1 ? 's' : ''}`}
action='Approve'
onClose={function onClose() {
setBulkAuditNote('')
setBulkOpen(false)
}}
onConfirm={function onConfirm() {
onBulkApprove(bulkAuditNote)
}}
canSave={bulkAuditNote.trim().length > 0}

Choose a reason for hiding this comment

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

[💡 correctness]
The canSave condition relies on bulkAuditNote.trim().length > 0, which could lead to issues if the user enters only whitespace. Consider trimming the input before validation.

open={bulkOpen}
>
<div>
<p>
You are about to approve
{' '}
{selectedPaymentsCount}
{' '}
payment
{selectedPaymentsCount > 1 ? 's' : ''}
.
</p>
<br />
<InputText
type='text'
label='Audit Note'
name='bulkAuditNote'
value={bulkAuditNote}
onChange={(e: React.ChangeEvent<HTMLInputElement>) => setBulkAuditNote(e.target.value)}
/>
</div>
</ConfirmModal>
)}
{confirmFlow && (
<ConfirmModal
maxWidth='800px'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type FilterOptions = {
value: string;
};

type Filter = {
export type Filter = {
key: string;
label: string;
type: 'input' | 'dropdown' | 'member_autocomplete';
Expand All @@ -27,6 +27,8 @@ interface FilterBarProps {
onFilterChange: (key: string, value: string[]) => void;
onResetFilters?: () => void;
onExport?: () => void;
selectedCount?: number;
onBulkClick?: () => void;
}

const FilterBar: React.FC<FilterBarProps> = (props: FilterBarProps) => {
Expand Down Expand Up @@ -120,6 +122,17 @@ const FilterBar: React.FC<FilterBarProps> = (props: FilterBarProps) => {
size='lg'
/>
)}
{!!props.selectedCount && props.selectedCount > 0 && (

Choose a reason for hiding this comment

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

[💡 readability]
The use of double negation !!props.selectedCount is redundant here since props.selectedCount > 0 already ensures that props.selectedCount is truthy. Consider removing !!props.selectedCount for clarity.

<>
<Button
primary
className={styles.bulkButton}
label={`${props.selectedCount > 1 ? 'Bulk ' : ''}Approve (${props.selectedCount})`}
size='lg'
onClick={() => props.onBulkClick?.()}

Choose a reason for hiding this comment

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

[⚠️ correctness]
Consider providing a default value or handling the case where onBulkClick is not defined, to avoid potential runtime errors if the function is expected to be called.

/>
</>
)}
<Button
primary
className={styles.resetButton}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import { Winning } from '../../models/WinningDetail'
import styles from './PaymentTable.module.scss'

interface PaymentTableProps {
enableBulkEdit?: boolean
payments: ReadonlyArray<Winning>;
selectedPayments?: { [paymentId: string]: Winning };
onSelectionChange?: (selected: { [paymentId: string]: Winning }) => void;
currentPage: number;
numPages: number;
onPaymentEditClick: (payment: Winning) => void;
Expand All @@ -31,12 +33,61 @@ const PaymentsTable: React.FC<PaymentTableProps> = (props: PaymentTableProps) =>
}
}, [props.selectedPayments])

// Only rows with this status are selectable for bulk actions

Choose a reason for hiding this comment

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

[💡 maintainability]
Consider extracting the selectableStatus string into a constant or configuration file to avoid magic strings and improve maintainability.

const selectableStatus = 'On Hold (Admin)'

const onToggleRow = (payment: Winning, checked: boolean) => {
setSelectedPayments(prev => {
const next = { ...prev }
if (checked) {
next[payment.id] = payment
} else {
delete next[payment.id]
}

props.onSelectionChange?.(next)
return next
})
}

const visibleSelectablePayments = props.payments.filter(p => p.status === selectableStatus)
const allVisibleSelected = visibleSelectablePayments.length > 0 && visibleSelectablePayments.every(p => selectedPayments[p.id])
const someVisibleSelected = visibleSelectablePayments.some(p => selectedPayments[p.id]) && !allVisibleSelected

const onToggleSelectAll = (checked: boolean) => {
if (checked) {

Choose a reason for hiding this comment

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

[⚠️ correctness]
The onToggleSelectAll function directly mutates the selectedPayments state, which can lead to bugs if the state is updated elsewhere asynchronously. Consider using a functional state update to ensure consistency.

const next: { [paymentId: string]: Winning } = {}
visibleSelectablePayments.forEach(p => { next[p.id] = p })
setSelectedPayments(next)
props.onSelectionChange?.(next)
} else {
// deselect all visible selectable rows
setSelectedPayments(prev => {
const next = { ...prev }
visibleSelectablePayments.forEach(p => { delete next[p.id] })
props.onSelectionChange?.(next)
return next
})
}
}

return (
<>
<div className={styles.tableContainer}>
<table>
<thead>
<tr>
{props.enableBulkEdit && (
<th>
<input
type='checkbox'
aria-label='Select All'
checked={allVisibleSelected}
ref={el => { if (el) el.indeterminate = someVisibleSelected }}

Choose a reason for hiding this comment

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

[⚠️ correctness]
Using ref to set indeterminate state directly on the DOM element can lead to unexpected behavior if the component re-renders. Consider using a useEffect hook to manage the indeterminate state based on someVisibleSelected.

onChange={e => onToggleSelectAll(e.target.checked)}
/>
</th>
)}
<th className='body-ultra-small-bold'>HANDLE</th>
<th className={`body-ultra-small-bold ${styles.description}`}>DESCRIPTION</th>
<th className='body-ultra-small-bold'>CREATE DATE</th>
Expand All @@ -53,6 +104,17 @@ const PaymentsTable: React.FC<PaymentTableProps> = (props: PaymentTableProps) =>
key={`${payment.id}`}
className={selectedPayments[payment.id] ? 'selected' : ''}
>
{props.enableBulkEdit && (
<td>
<input
type='checkbox'
aria-label={`Select ${payment.handle}`}
checked={!!selectedPayments[payment.id]}
disabled={payment.status !== selectableStatus}
onChange={e => onToggleRow(payment, e.target.checked)}
/>
</td>
)}
<td className='body-small-bold'>{payment.handle}</td>
<td className='body-small'>{payment.description}</td>
<td className='body-small-bold'>{payment.createDate}</td>
Expand Down
1 change: 1 addition & 0 deletions src/apps/wallet-admin/src/lib/services/wallet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ export async function exportSearchResults(filters: Record<string, string[]>): Pr
winnerIds?: string[], [key: string]: string | number | string[] | undefined
} = {
...filteredFilters,
type: 'PAYMENT',
}

if (filters.winnerIds && filters.winnerIds.length > 0) {
Expand Down
4 changes: 3 additions & 1 deletion src/libs/core/lib/profile/user-skill.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ export type UserSkillWithActivity = {
activity: {
certification?: UserSkillActivity
course?: UserSkillActivity
challenge?: UserSkillActivity
challenge?: {

Choose a reason for hiding this comment

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

[⚠️ maintainability]
Changing challenge from UserSkillActivity to an object with dynamic keys of type UserSkillActivity increases flexibility but may introduce issues if not handled properly. Ensure that all parts of the codebase that interact with challenge are updated to handle this new structure. Consider if this change impacts serialization/deserialization or any external API contracts.

[key: string]: UserSkillActivity,
}
engagement?: UserSkillActivity
}
} & UserSkill
Expand Down
Loading
Loading