Dev#96
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
feat: Webhook Management System — Complete Frontend Dashboard for CommDesk
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a new Webhook management system with delivery logs and testing capabilities, integrates Vitest for testing, and enhances the ConfirmModal component. Key feedback highlights a broken sort implementation in the reminders utility and an unreliable selection state check in the Webhook table that relies solely on array lengths.
|
|
||
| //sorting acc to priority | ||
| return reminders.sort((a, b) => (a.type === "urgent" ? -1 : 1)).slice(0, 5); | ||
| return reminders.sort((a) => (a.type === "urgent" ? -1 : 1)).slice(0, 5); |
There was a problem hiding this comment.
The sort function is missing the second argument in the comparator, which is required for a valid sort. The current implementation will not sort the array correctly and may cause runtime errors.
| return reminders.sort((a) => (a.type === "urgent" ? -1 : 1)).slice(0, 5); | |
| return reminders.sort((a, b) => Number(b.type === "urgent") - Number(a.type === "urgent")).slice(0, 5); |
| }: Props) { | ||
| const navigate = useNavigate(); | ||
|
|
||
| const allSelected = webhooks.length > 0 && selectedIds.length === webhooks.length; |
There was a problem hiding this comment.
The allSelected check is incorrect because it only compares the length of selectedIds and webhooks. This will lead to incorrect selection states if selectedIds contains IDs not present in the current webhooks list.
| const allSelected = webhooks.length > 0 && selectedIds.length === webhooks.length; | |
| const allSelected = webhooks.length > 0 && webhooks.every(w => selectedIds.includes(w.id)); |
2 participants
No description provided.