Conversation
This comment has been minimized.
This comment has been minimized.
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (16)
📝 WalkthroughWalkthroughThe changes add command-app authorization tests, translated accessibility labels, incident-channel loading tracking, frozen-channel edit protection, and committed-state synchronization for bottom sheets and side drawers. ChangesAuthorization and accessibility
Incident-channel loading
Chat editing and overlay state
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CommandScreen
participant ChatStore
participant IncidentChannelAPI
participant Toast
CommandScreen->>ChatStore: loadIncidentChannels(callId)
ChatStore->>IncidentChannelAPI: Request incident channels
ChatStore-->>CommandScreen: Set loading marker
CommandScreen->>ChatStore: openChatChannel(channelId)
ChatStore-->>CommandScreen: Suppress unavailable toast while loading
IncidentChannelAPI-->>ChatStore: Return channels or failure
ChatStore-->>CommandScreen: Clear loading marker
CommandScreen->>Toast: Show unavailable toast after loading
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/app/chat/[channelId].tsx (1)
270-295: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for the frozen-edit state transitions.
Cover these paths:
- Freezing an active edit clears
editMessageandeditTextand showschat.frozen_notice.- Saving while frozen does not call
useChatStore.getState().editMessage.- Saving while active trims the submitted text.
As per coding guidelines, generate tests for new logic. Mock stores and hooks, and call
unmount()during cleanup.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/chat/`[channelId].tsx around lines 270 - 295, Add tests for the frozen-edit logic around the useEffect and handleSaveEdit symbols, mocking the relevant stores and hooks. Verify freezing an active edit clears editMessage and editText and shows chat.frozen_notice, saving while frozen does not invoke useChatStore.getState().editMessage, and active saves submit trimmed text. Ensure each test calls unmount() during cleanup.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/security-integration.test.ts`:
- Around line 147-155: Replace the test-only runCommandAppGate implementation
with coverage of the production authorization gate in initializeApp from the app
layout. Prefer extracting the gate into a shared function used by initializeApp
and importing that function in the tests, or render the layout with its
dependencies mocked; verify denial shows the authorization toast, logs out, and
does not continue initialization.
In `@src/stores/chat/__tests__/incident-channels-loading.test.ts`:
- Around line 47-52: Update the lazy ChatStoreApi import and the beforeAll
require in the incident channel loading test to use the configured
"`@/stores/chat/store`" alias instead of "../store", preserving the existing
lazy-loading behavior.
In `@src/stores/chat/store.ts`:
- Around line 288-296: Update loadIncidentChannels to prevent overlapping
requests for the same callId: return immediately when
incidentChannelsLoadingByCallId already marks that ID as loading, while
preserving the existing loading, success, error, and cleanup updates. Add a test
using two deferred requests for one call ID to verify only one request runs and
the loading marker remains correct.
- Around line 82-87: Add a per-call status to incident channel loading in
src/stores/chat/store.ts#L82-L87, distinguishing loading, loaded, and failed;
update the request catch handling at src/stores/chat/store.ts#L293-L296 to
record failed without marking the result as completed. Update
src/stores/chat/__tests__/incident-channels-loading.test.ts#L91-L98 to assert
failed separately and preserve empty results as the only loaded-unavailable
case. In src/app/(app)/command.tsx#L210-L229, render a translated t()
load-failure or retry message for failed requests and show the unavailable
message only for successfully loaded empty results.
---
Nitpick comments:
In `@src/app/chat/`[channelId].tsx:
- Around line 270-295: Add tests for the frozen-edit logic around the useEffect
and handleSaveEdit symbols, mocking the relevant stores and hooks. Verify
freezing an active edit clears editMessage and editText and shows
chat.frozen_notice, saving while frozen does not invoke
useChatStore.getState().editMessage, and active saves submit trimmed text.
Ensure each test calls unmount() during cleanup.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3ad8f441-5476-4448-ac71-d191015a4c3c
📒 Files selected for processing (8)
src/__tests__/security-integration.test.tssrc/app/(app)/_layout.tsxsrc/app/(app)/command.tsxsrc/app/chat/[channelId].tsxsrc/components/ui/bottom-sheet.tsxsrc/components/ui/side-drawer.tsxsrc/stores/chat/__tests__/incident-channels-loading.test.tssrc/stores/chat/store.ts
| // Loaded lazily so the mock factories above run after their `mock*` consts exist. | ||
| type ChatStoreApi = typeof import('../store').useChatStore; | ||
| let useChatStore: ChatStoreApi; | ||
|
|
||
| beforeAll(() => { | ||
| useChatStore = require('../store').useChatStore as ChatStoreApi; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the configured store alias.
Replace both ../store references with @/stores/chat/store. This keeps test module resolution consistent with the project imports.
As per coding guidelines, use configured path aliases (@/*) instead of relative imports.
Proposed fix
-type ChatStoreApi = typeof import('../store').useChatStore;
+type ChatStoreApi = typeof import('`@/stores/chat/store`').useChatStore;
let useChatStore: ChatStoreApi;
beforeAll(() => {
- useChatStore = require('../store').useChatStore as ChatStoreApi;
+ useChatStore = require('`@/stores/chat/store`').useChatStore as ChatStoreApi;
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Loaded lazily so the mock factories above run after their `mock*` consts exist. | |
| type ChatStoreApi = typeof import('../store').useChatStore; | |
| let useChatStore: ChatStoreApi; | |
| beforeAll(() => { | |
| useChatStore = require('../store').useChatStore as ChatStoreApi; | |
| // Loaded lazily so the mock factories above run after their `mock*` consts exist. | |
| type ChatStoreApi = typeof import('@/stores/chat/store').useChatStore; | |
| let useChatStore: ChatStoreApi; | |
| beforeAll(() => { | |
| useChatStore = require('@/stores/chat/store').useChatStore as ChatStoreApi; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/chat/__tests__/incident-channels-loading.test.ts` around lines 47
- 52, Update the lazy ChatStoreApi import and the beforeAll require in the
incident channel loading test to use the configured "`@/stores/chat/store`" alias
instead of "../store", preserving the existing lazy-loading behavior.
Source: Coding guidelines
|
|
||
| {/* Edit message sheet */} | ||
| <Actionsheet isOpen={editMessage !== null} onClose={() => setEditMessage(null)}> | ||
| <Actionsheet isOpen={editMessage !== null && !isFrozen} onClose={() => setEditMessage(null)}> |
There was a problem hiding this comment.
Inline arrow function in the onClose prop creates a new function on every render, degrading performance. Move the function definition outside the render method.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 451:
Inline arrow function in the `onClose` prop creates a new function on every render, degrading performance. Move the function definition outside the render method.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return; | ||
| } | ||
| if (editMessage && channelId && editText.trim()) { | ||
| void useChatStore.getState().editMessage(editMessage.ChatMessageId, channelId, editText.trim()); |
There was a problem hiding this comment.
The void operator suppresses the promise returned by editMessage, silently swallowing network errors and violating Rule 1. Attach a .catch handler or convert handleSaveEdit to an async function using try/catch.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 291:
The `void` operator suppresses the promise returned by `editMessage`, silently swallowing network errors and violating Rule 1. Attach a `.catch` handler or convert `handleSaveEdit` to an async function using try/catch.
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:
|
| // load failed and let the commander retry instead of declaring the channel missing. | ||
| if (didIncidentChannelsFail) { | ||
| showToast('error', t('command.chat_load_failed')); | ||
| void useChatStore.getState().loadIncidentChannels(boardCallId ?? ''); |
There was a problem hiding this comment.
Unhandled promise rejection occurs when the void operator discards the returned promise from loadIncidentChannels, rendering retry failures invisible. Wrap the call in a try/catch block or chain a .catch handler to catch errors and invoke showToast('error', t('command.chat_load_failed')).
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/command.tsx:
Line 234:
Unhandled promise rejection occurs when the `void` operator discards the returned promise from `loadIncidentChannels`, rendering retry failures invisible. Wrap the call in a `try/catch` block or chain a `.catch` handler to catch errors and invoke `showToast('error', t('command.chat_load_failed'))`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
2 participants
Pull Request Description
This PR delivers a set of bug fixes for the Resgrid IC (Incident Command) app, covering authorization enforcement, chat reliability, accessibility, and component stability.
Key Changes
Command App Authorization Gate
CanLoginToCommandApp === falseare denied access to the IC app — they receive a localized denial toast and are automatically signed out. The check uses a strict false comparison so that missing rights data or older servers that omit the field do not inadvertently lock out authorized users.Incident Channels Loading State
incidentChannelsLoadingByCallId) to the chat store so the command board can distinguish between "channels still loading" and "incident genuinely has no channels." Previously, tapping a chat action mid-load would incorrectly display a "chat unavailable" message. The UI now stays silent while loading and only reports unavailable once the request has completed.Frozen Channel Edit Race Condition
Accessibility
accessibilityRoleand localizedaccessibilityLabelto the drawer menu button and header back button in the app layout.Render Safety for Sheets and Drawers
isOpenRefwrites in both the bottom sheet and side drawer from inline render assignments touseLayoutEffect, preventing abandoned renders from leaving the ref in a state that was never committed — which could cause incorrect close-animation behavior.Test Coverage