Repository navigation
Conversation
This comment has been minimized.
This comment has been minimized.
|
Warning Review paused — included plan limit reachedKeep your review moving with free on-demand reviews.
On-demand reviews are free for the next 3 days.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Promotion and pricing detailsOn-demand reviews are free for the next 3 days. After that, they cost $0.25 per reviewed file. Review limit detailsOr wait 4 minutes for your next included review. Limit details: You’ve used all 3 included reviews currently available. Your 42 included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (33)
Included review availability: This review used your included allowance. 1 included review remains after this review. Your included PR review attempts over the past 7 days set your current allowance at 4 reviews per hour. Your free on-demand review promotion remains active until October 9, 2026 at 6:00 PM UTC. 📝 WalkthroughWalkthroughThe pull request adds call-close notification controls and server error-message handling. It adds configurable status filtering and hold-to-confirm interactions for personnel status changes. It also expands push-notification event-code parsing. ChangesCall closure
Personnel status flow
Push notification parsing
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant User
participant HoldToConfirmButton
participant StatusButtons
participant PersonnelStatusStore
User->>HoldToConfirmButton: Hold a status button
HoldToConfirmButton->>StatusButtons: Confirm held status
StatusButtons->>PersonnelStatusStore: Call confirmHeldStatus with selected status
Merge Risk: ⚪ Minimal · up to The identified timer and held-status concerns do not block the intended interactions. The PR is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
| <HoldToConfirmButton | ||
| key={status.Id} | ||
| testID={`status-hold-button-${status.Id}`} | ||
| onConfirm={() => handleStatusHold(status)} |
There was a problem hiding this comment.
Per-render function allocation occurs when .bind() or inline arrow functions are used in JSX props, impacting performance; this affects src/components/home/status-buttons.tsx:94-94, 115-115, 125-125, and 129-129, plus src/components/status/personnel-status-bottom-sheet.tsx:582-583, 607-607, 655-656, 742-742, and 746-746. Move these function definitions outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/components/home/status-buttons.tsx:
Line 93:
Per-render function allocation occurs when `.bind()` or inline arrow functions are used in JSX props, impacting performance; this affects `src/components/home/status-buttons.tsx:94-94`, `115-115`, `125-125`, and `129-129`, plus `src/components/status/personnel-status-bottom-sheet.tsx:582-583`, `607-607`, `655-656`, `742-742`, and `746-746`. Move these function definitions outside the render method.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const handleStatusHold = (statusData: StatusesResultData) => { | ||
| // The sheet opens on the status and saves it at once, or stays on the step that still needs the member. | ||
| setIsOpen(true, statusData); | ||
| void confirmHeldStatus?.(statusData); |
There was a problem hiding this comment.
Unhandled promise rejections can occur when confirmHeldStatus rejects without a catch handler, including at src/components/home/status-buttons.tsx, src/components/status/personnel-status-bottom-sheet.tsx:308-308, 655-655, src/stores/status/personnel-status-store.ts:573-573, src/stores/status/__tests__/personnel-status-store.test.ts:1826-1827, 1846-1846, 1854-1854, 1860-1860, 1868-1868, and 1877-1878, src/api/calls/__tests__/closeCall.test.ts:20-20, 23-23, and 28-28, and src/components/calls/__tests__/close-call-bottom-sheet.test.tsx:380-380 and 403-403. Attach a catch handler to the promise returned by confirmHeldStatus and pass the error and statusData.Id to handleConfirmError.
Kody rule violation: Handle async operations with proper error handling
void confirmHeldStatus?.(statusData).catch((error) => handleConfirmError(error, statusData.Id));Prompt for LLM
File src/components/home/status-buttons.tsx:
Line 57:
Unhandled promise rejections can occur when `confirmHeldStatus` rejects without a catch handler, including at `src/components/home/status-buttons.tsx`, `src/components/status/personnel-status-bottom-sheet.tsx:308-308`, `655-655`, `src/stores/status/personnel-status-store.ts:573-573`, `src/stores/status/__tests__/personnel-status-store.test.ts:1826-1827`, `1846-1846`, `1854-1854`, `1860-1860`, `1868-1868`, and `1877-1878`, `src/api/calls/__tests__/closeCall.test.ts:20-20`, `23-23`, and `28-28`, and `src/components/calls/__tests__/close-call-bottom-sheet.test.tsx:380-380` and `403-403`. Attach a catch handler to the promise returned by `confirmHeldStatus` and pass the error and `statusData.Id` to `handleConfirmError`.
Suggested Code:
void confirmHeldStatus?.(statusData).catch((error) => handleConfirmError(error, statusData.Id));
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| statusDetail: status.Detail, | ||
| }); | ||
| } catch (error) { | ||
| console.warn('Failed to track status hold analytics:', error); |
There was a problem hiding this comment.
Message-only console warnings from status hold analytics tracking omit the operation name, status identifier, and error object. Emit a structured warning through logger.warn with op: 'trackStatusHold', statusId: status.Id, and err: error.
Kody rule violation: Include error context in structured logs
logger.warn('status hold analytics tracking failed', { op: 'trackStatusHold', statusId: status.Id, err: error });Prompt for LLM
File src/components/status/personnel-status-bottom-sheet.tsx:
Line 304:
Message-only console warnings from status hold analytics tracking omit the operation name, status identifier, and error object. Emit a structured warning through `logger.warn` with `op: 'trackStatusHold'`, `statusId: status.Id`, and `err: error`.
Suggested Code:
logger.warn('status hold analytics tracking failed', { op: 'trackStatusHold', statusId: status.Id, err: error });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| const current = all.find((status) => toId(status.Id) === currentStatusId); | ||
| const nextIds = new Set((current?.NextIds ?? []).map((id) => toId(id)).filter((id) => id !== '' && id !== '0')); |
There was a problem hiding this comment.
Invalid status filtering causes getOfferedStatuses to discard next-status ID "0", so a transition such as NextIds: [0] produces an empty offered set and incorrectly falls back to showing every status instead of only status 0. Remove the id !== '0' filter, retain the empty-ID guard, and handle the separate unknown-status/"0 means Unknown" ambiguity in current-status resolution.
const nextIds = new Set((current?.NextIds ?? []).map((id) => toId(id)).filter((id) => id !== ''));Prompt for LLM
File src/lib/status-flow.ts:
Line 88:
Invalid status filtering causes getOfferedStatuses to discard next-status ID "0", so a transition such as NextIds: [0] produces an empty offered set and incorrectly falls back to showing every status instead of only status 0. Remove the `id !== '0'` filter, retain the empty-ID guard, and handle the separate unknown-status/"0 means Unknown" ambiguity in current-status resolution.
Suggested Code:
const nextIds = new Set((current?.NextIds ?? []).map((id) => toId(id)).filter((id) => id !== ''));
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| (statusData: StatusesResultData) => { | ||
| // The sheet opens on the status and saves it at once, or stays on the step that still needs the member. | ||
| setIsOpen(true, statusData); | ||
| void confirmHeldStatus?.(statusData); |
There was a problem hiding this comment.
Unhandled promise rejections occur when confirmHeldStatus returns a rejected promise that void does not handle, including at src/components/status/personnel-status-bottom-sheet.tsx:401 and :540. Attach .catch() to capture or log each status confirmation failure with operation context.
Kody rule violation: Handle async operations with proper error handling
confirmHeldStatus?.(statusData).catch((error) => {
// handle or log the failure with operation context
});Prompt for LLM
File src/components/home/status-buttons.tsx:
Line 124:
Unhandled promise rejections occur when `confirmHeldStatus` returns a rejected promise that `void` does not handle, including at `src/components/status/personnel-status-bottom-sheet.tsx:401` and `:540`. Attach `.catch()` to capture or log each status confirmation failure with operation context.
Suggested Code:
confirmHeldStatus?.(statusData).catch((error) => {
// handle or log the failure with operation context
});
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| statusDetail: status.Detail, | ||
| }); | ||
| } catch (error) { | ||
| console.warn('Failed to track status option analytics:', error); |
There was a problem hiding this comment.
Insufficient error context occurs when src/components/status/personnel-status-bottom-sheet.tsx:397 logs only a message and raw error, making failures difficult to correlate with the selected status. Include operation: 'trackStatusOptionSelected' and statusId: status.Id as structured fields in the error log.
Kody rule violation: Include error context in structured logs
console.warn('Status option analytics failed', { operation: 'trackStatusOptionSelected', statusId: status.Id, error });Prompt for LLM
File src/components/status/personnel-status-bottom-sheet.tsx:
Line 368:
Insufficient error context occurs when `src/components/status/personnel-status-bottom-sheet.tsx:397` logs only a message and raw `error`, making failures difficult to correlate with the selected status. Include `operation: 'trackStatusOptionSelected'` and `statusId: status.Id` as structured fields in the error log.
Suggested Code:
console.warn('Status option analytics failed', { operation: 'trackStatusOptionSelected', statusId: status.Id, error });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
2 participants
Pull Request Summary
This PR improves call closure, personnel status progression, and push notification handling.
Call closure updates
sendNotificationflag to the close-call API request.Personnel status flow improvements
NextIdsconfiguration to show only valid next statuses by default.NextIdsstatus model field and shared status-flow helpers.Hold-to-confirm status setting
StatusHoldToConfirmconfiguration.Push notification parsing
NC:{callId}notifications indicating that a call has been closed.C{callId}call codes andM{messageId}message codes.CT123from being incorrectly interpreted as call notifications.Validation