Hotfixes for minor issues #1299
Conversation
…sion phases open - prioritise submission.
…challenges and better group searching in system admin app
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| usersMapping, | ||
| ) | ||
|
|
||
| const filteredGroups = useMemo(() => { |
There was a problem hiding this comment.
[performance]
Using useMemo for filteredGroups is appropriate here to avoid unnecessary computations when groups or searchTerm change. However, ensure that groups is a stable reference (e.g., not recreated on every render) to fully benefit from memoization.
| return id.includes(normalized) || name.includes(normalized) | ||
| }) | ||
| }, [groups, searchTerm]) | ||
| const hasSearchTerm = useMemo( |
There was a problem hiding this comment.
[💡 performance]
The useMemo for hasSearchTerm is not necessary since the computation is trivial (just a string length check). Removing it could simplify the code without a performance hit.
| timeLeftColor: displayTimeLeftColor, | ||
| timeLeftStatus: displayTimeLeftStatus, | ||
| }: PhaseDisplayInfo = useMemo( | ||
| () => computePhaseDisplayInfo(props.challengeInfo), |
There was a problem hiding this comment.
[❗❗ correctness]
The computePhaseDisplayInfo function relies on props.challengeInfo but does not handle the case where props.challengeInfo might be undefined or null. Consider adding a check to ensure props.challengeInfo is defined before using it, to prevent potential runtime errors.
| function selectPhaseForDisplay(data?: ChallengeInfo): BackendPhase | undefined { | ||
| if (!data) return undefined | ||
|
|
||
| const phases = Array.isArray(data.phases) ? data.phases : [] |
There was a problem hiding this comment.
[❗❗ correctness]
In selectPhaseForDisplay, the function assumes data.phases is an array but does not handle the case where data.phases might be undefined. Consider adding a check to ensure data.phases is defined before attempting to use it as an array.
| return {} | ||
| } | ||
|
|
||
| const endDate = new Date(rawEndDate) |
There was a problem hiding this comment.
[correctness]
The computePhaseTiming function uses new Date(rawEndDate) without checking if rawEndDate is a valid date string. Consider validating rawEndDate before using it to create a Date object to avoid potential invalid date issues.
| const firstPage = await xhrGetPaginatedAsync<BackendResource[]>( | ||
| `${resourceBaseUrl}/resources?${qs.stringify(baseQuery)}`, | ||
| ) | ||
| const totalPages = firstPage.totalPages ?? 0 |
There was a problem hiding this comment.
[correctness]
Consider handling the case where firstPage.totalPages is undefined more explicitly. Currently, it defaults to 0, which might not be the intended behavior if the API response is malformed or missing this field.
| perPage: DEFAULT_PER_PAGE, | ||
| } | ||
| const firstPage = await xhrGetPaginatedAsync<BackendResource[]>( | ||
| `${resourceBaseUrl}/resources?${qs.stringify(baseQuery)}`, |
There was a problem hiding this comment.
[💡 maintainability]
The xhrGetPaginatedAsync call is repeated with similar query parameters. Consider refactoring this into a helper function to improve maintainability and reduce duplication.
| `${resourceBaseUrl}/resources?${qs.stringify({ | ||
| ...query, | ||
| page, | ||
| perPage: DEFAULT_PER_PAGE, |
There was a problem hiding this comment.
[performance]
Using a high DEFAULT_PER_PAGE value like 1000 might lead to performance issues if the API or client cannot handle large payloads efficiently. Consider verifying the API's capability to handle such requests or making this configurable.
Fix handles not showing for submissions for large challenges, and add searching to groups in system-admin app
Fix for ordering when there are multiple phases open, to ensure we show submission at the priority, not registration.