Conversation
This comment has been minimized.
This comment has been minimized.
📝 WalkthroughWalkthroughThe PR adds command-app authorization, incident-scoped chat loading, direct messaging, responsive command-board controls, archived-channel restrictions, shared icon usage, localization, and cleanup for interrupted sheet and drawer animations. ChangesIncident command and chat integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CommandBoard
participant ChatStore
participant ChatApi
participant ChatScreen
CommandBoard->>ChatStore: loadIncidentChannels(callId)
ChatStore->>ChatApi: getChannels(includeArchived=true)
ChatApi-->>ChatStore: return incident channels
CommandBoard->>ChatScreen: open available command, lead, dispatch, or lane channel
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/translations/ar.json (1)
327-375: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new locale values.
The added Arabic and Ukrainian values display English text to localized users. Translate the action labels, unavailable-channel messages, frozen notice, direct-message errors, and authorization error. Preserve interpolation tokens such as
{{name}}.
src/translations/ar.json#L327-L375: translate the new chat labels and messages.src/translations/ar.json#L478-L489: translate command and dispatch channel labels and unavailable messages.src/translations/ar.json#L573-L598: translate lane and leads channel labels and unavailable messages.src/translations/ar.json#L625-L626: translate lead and role-holder message actions.src/translations/ar.json#L1171-L1171: translate the command authorization error.src/translations/uk.json#L327-L375: translate the new chat labels and messages.src/translations/uk.json#L478-L489: translate command and dispatch channel labels and unavailable messages.src/translations/uk.json#L573-L598: translate lane and leads channel labels and unavailable messages.src/translations/uk.json#L625-L626: translate lead and role-holder message actions.src/translations/uk.json#L1171-L1171: translate the command authorization error.🤖 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/translations/ar.json` around lines 327 - 375, Translate every newly added English value in the listed ranges of src/translations/ar.json and src/translations/uk.json, including chat labels, action labels, unavailable-channel messages, frozen notices, direct-message errors, lead/role-holder actions, and the command authorization error. Preserve all interpolation tokens such as {{count}} and {{name}}, and ensure no user-facing English remains in those specified entries.src/translations/sv.json (1)
341-375: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTranslate the new Swedish UI text.
These values remain in English in
sv.json. They produce a mixed-language interface for Swedish users. Translate the direct-message errors, incident chat labels, unavailable-channel messages, lead-message labels, and authorization error.Also applies to: 478-489, 573-598, 625-626, 1171-1171
🤖 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/translations/sv.json` around lines 341 - 375, Translate the remaining English UI values in sv.json into Swedish, including the direct-message error labels near dm_failed and dm_unavailable, incident chat labels such as incident_command_channel and incident_lane_channel, unavailable-channel messages, lead-message labels, and the authorization error in the referenced sections. Preserve all translation keys and placeholders while ensuring no user-facing value in these areas remains in English.
🧹 Nitpick comments (4)
src/api/chat/chat.ts (1)
38-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the approved API endpoint abstraction.
Line 47 calls
api.getdirectly. Move this endpoint tocreateApiEndpointorcreateCachedApiEndpointwhile preserving the typed response and conditional query parameters.🤖 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/api/chat/chat.ts` around lines 38 - 48, Update getChannels to use the approved createApiEndpoint or createCachedApiEndpoint abstraction instead of calling api.get directly. Preserve the ChatV4Response<ChatChannelResultData[]> response type, optional AbortSignal, and conditional activeUnitId/includeArchived query parameters, including omitting params when empty.Source: Coding guidelines
src/app/chat/[channelId].tsx (1)
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRender Lucide icons directly.
Render
ArrowLeftandArchivedirectly. Do not wrap them with the GluestackIconcomponent.As per coding guidelines, use
lucide-react-nativeicons directly, not through the gluestack Icon wrapper.Also applies to: 100-107, 346-350
🤖 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 21 - 23, Remove the Gluestack Icon import and update the chat screen’s ArrowLeft and Archive usages to render the corresponding lucide-react-native components directly. Apply this consistently at the referenced header and archive action locations, preserving their existing size, color, and interaction behavior.Source: Coding guidelines
src/components/command/landscape-structure-board.tsx (2)
89-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDefine
LaneResourceListprops with an interface.Replace the inline props object with a named
LaneResourceListPropsinterface. This keeps component props consistent with the TypeScript requirement.🤖 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/components/command/landscape-structure-board.tsx` at line 89, Define a named LaneResourceListProps interface containing the existing children and optional testID props, then update LaneResourceList to use that interface instead of the inline props object.Source: Coding guidelines
246-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRender Lucide icons directly.
Replace the
Icon as={...}wrappers at the listed sites with the correspondinglucide-react-nativecomponents. Preserve each icon’s size, class, and accessibility props.🤖 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/components/command/landscape-structure-board.tsx` around lines 246 - 264, Replace each Icon as={...} wrapper with the corresponding direct lucide-react-native icon while preserving its existing size, className, and accessibility props. Apply this at src/components/command/landscape-structure-board.tsx lines 246-264 and 424-487, src/components/command/lane-details-sheet.tsx lines 115-121, src/components/command/structure-section.tsx lines 135-152, and src/components/command/resource-cards.tsx lines 59-61.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`:
- Line 29: Add a security integration test alongside the existing
CanLoginToCommandApp fixtures that sets the field to false and verifies the
localized access-denied toast and logout behavior; also add coverage for an
omitted field if the API contract distinguishes missing authorization from
false. Keep the existing authorized cases unchanged and follow the suite’s
established test setup and assertions.
In `@src/app/`(app)/_layout.tsx:
- Around line 648-660: Add accessibilityRole="button" and localized
accessibilityLabel props to both CreateDrawerMenuButton and
CreateHeaderBackButton. Use t('common.back') for the back button and add or
reuse a translated drawer/menu label for the drawer button; keep the existing
testIDs and press behavior unchanged.
- Line 659: Update the header controls in the layout to render Menu and
ArrowLeft directly from lucide-react-native instead of through Icon’s as prop,
supplying theme-resolved color values. Add translated accessibility labels using
t('sidebar.menu') for the menu control and t('common.back') for the back
control.
In `@src/app/`(app)/command.tsx:
- Around line 208-223: Update the incident channel flow around incidentChannels,
commandChatChannelId, laneChatChannelId, and openChatChannel to distinguish an
undefined/loading channels state from a completed request with no matching
channel. Track or reuse the channel-loading state, disable or defer the related
actions while loading, and only show the unavailable toast after loading has
completed.
In `@src/app/chat/`[channelId].tsx:
- Line 401: Update the edit sheet flow alongside MessageActionsSheet’s isFrozen
handling so an edit opened before archival cannot remain active or save
afterward. Guard the edit save handler against isFrozen and close or disable the
edit sheet when the channel becomes archived, while preserving normal editing
behavior for unfrozen channels.
In `@src/components/ui/__tests__/bottom-sheet.test.tsx`:
- Around line 435-453: Update both affected bottom-sheet tests to capture the
render result’s unmount function, then call unmount and restore the
Animated.parallel spy from a finally block so cleanup runs even when assertions
fail. Apply this to the tests around the interrupted close-animation case and
the additional range noted in the comment.
In `@src/components/ui/bottom-sheet.tsx`:
- Around line 50-53: Update isOpenRef in both src/components/ui/bottom-sheet.tsx
(lines 50-53) and src/components/ui/side-drawer.tsx (lines 39-42) using
useLayoutEffect so the ref changes only after a committed render; import
useLayoutEffect from react in both components and preserve the existing
close-animation callback behavior.
In `@src/hooks/__tests__/use-command-board-layout.test.ts`:
- Around line 1-28: Update the test setup around layoutFor to capture the
renderHook result, call unmount() before returning its current value, and
replace the relative useCommandBoardLayout import with
`@/hooks/use-command-board-layout`; keep the react-native mock before imports.
In `@src/hooks/use-direct-message.ts`:
- Around line 20-43: Prevent concurrent opens in openDirectMessage by adding a
synchronous useRef guard checked before createDirectMessage. Set the guard
before starting the request, return immediately when already active, and clear
it in the finally block alongside setIsOpening(false), preserving the existing
request and navigation behavior.
- Around line 16-48: Add focused tests for the new chat and lane behaviors: in
src/hooks/use-direct-message.ts lines 16-48, test missing IDs, API failures,
missing channel IDs, successful routing, and isOpening transitions; in
src/app/(app)/command.tsx lines 202-237, test command, leads, dispatch, lane,
role-holder, and lead direct-message routing; in
src/components/command/landscape-structure-board.tsx lines 83-110, test
scrollEnabled and showsVerticalScrollIndicator at the lane-content viewport
threshold; and in src/components/command/lane-details-sheet.tsx lines 115-122,
test message actions for Resgrid users and their absence for external leads.
In `@src/models/v4/security/departmentRightsResultData.ts`:
- Around line 12-16: The CanLoginToCommandApp field currently fails open for
missing or invalid JSON values. Remove its true initializer and, wherever
SecurityDepartmentRightsResultData is validated or consumed during
initialization, require the value to be a literal boolean before storing or
continuing; reject missing, null, and non-boolean values rather than allowing
them to enable command access.
In `@src/stores/chat/store.ts`:
- Around line 265-267: Add a shared channel-by-ID resolver used by both the
incident-channel cache and ChannelConversationScreen, storing archived channels
separately from the normal channels list so they remain excluded from list UI.
Update the detail-screen fallback refresh to request archived records and
resolve the opened channel by ID, ensuring archived channels set isFrozen and
disable the composer. Add a regression test covering opening an archived command
channel from the board.
In `@src/translations/de.json`:
- Around line 341-342: Translate all newly added chat and authorization
localization keys—including dm_*, frozen_notice, channel labels, unavailable
messages, message labels, and authorization text—into their respective languages
in src/translations/de.json (lines 341-342), src/translations/es.json (lines
341-342), src/translations/fr.json (lines 341-342), src/translations/it.json
(lines 341-342), and src/translations/pl.json (lines 341-342), ensuring no
English values remain in these non-English locale files.
---
Outside diff comments:
In `@src/translations/ar.json`:
- Around line 327-375: Translate every newly added English value in the listed
ranges of src/translations/ar.json and src/translations/uk.json, including chat
labels, action labels, unavailable-channel messages, frozen notices,
direct-message errors, lead/role-holder actions, and the command authorization
error. Preserve all interpolation tokens such as {{count}} and {{name}}, and
ensure no user-facing English remains in those specified entries.
In `@src/translations/sv.json`:
- Around line 341-375: Translate the remaining English UI values in sv.json into
Swedish, including the direct-message error labels near dm_failed and
dm_unavailable, incident chat labels such as incident_command_channel and
incident_lane_channel, unavailable-channel messages, lead-message labels, and
the authorization error in the referenced sections. Preserve all translation
keys and placeholders while ensuring no user-facing value in these areas remains
in English.
---
Nitpick comments:
In `@src/api/chat/chat.ts`:
- Around line 38-48: Update getChannels to use the approved createApiEndpoint or
createCachedApiEndpoint abstraction instead of calling api.get directly.
Preserve the ChatV4Response<ChatChannelResultData[]> response type, optional
AbortSignal, and conditional activeUnitId/includeArchived query parameters,
including omitting params when empty.
In `@src/app/chat/`[channelId].tsx:
- Around line 21-23: Remove the Gluestack Icon import and update the chat
screen’s ArrowLeft and Archive usages to render the corresponding
lucide-react-native components directly. Apply this consistently at the
referenced header and archive action locations, preserving their existing size,
color, and interaction behavior.
In `@src/components/command/landscape-structure-board.tsx`:
- Line 89: Define a named LaneResourceListProps interface containing the
existing children and optional testID props, then update LaneResourceList to use
that interface instead of the inline props object.
- Around line 246-264: Replace each Icon as={...} wrapper with the corresponding
direct lucide-react-native icon while preserving its existing size, className,
and accessibility props. Apply this at
src/components/command/landscape-structure-board.tsx lines 246-264 and 424-487,
src/components/command/lane-details-sheet.tsx lines 115-121,
src/components/command/structure-section.tsx lines 135-152, and
src/components/command/resource-cards.tsx lines 59-61.
🪄 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: ea76658a-595c-4002-8a26-c7300128a1b2
📒 Files selected for processing (32)
src/__tests__/security-integration.test.tssrc/api/chat/chat.tssrc/app/(app)/_layout.tsxsrc/app/(app)/chat.tsxsrc/app/(app)/chatbot.tsxsrc/app/(app)/command.tsxsrc/app/chat/[channelId].tsxsrc/components/chat/message-actions-sheet.tsxsrc/components/command/assistant-sheet.tsxsrc/components/command/landscape-structure-board.tsxsrc/components/command/lane-details-sheet.tsxsrc/components/command/resource-cards.tsxsrc/components/command/structure-section.tsxsrc/components/ui/__tests__/bottom-sheet.test.tsxsrc/components/ui/bottom-sheet.tsxsrc/components/ui/side-drawer.tsxsrc/hooks/__tests__/use-command-board-layout.test.tssrc/hooks/use-command-board-layout.tssrc/hooks/use-direct-message.tssrc/models/v4/chat/chatEnums.tssrc/models/v4/security/departmentRightsResultData.tssrc/stores/chat/store.tssrc/stores/security/__tests__/store.test.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
| CanCreateCalls: true, | ||
| CanAddNote: false, | ||
| CanCreateMessage: false, | ||
| CanLoginToCommandApp: true, |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add a denied-access test case.
Every changed fixture sets CanLoginToCommandApp to true. The suite can therefore pass while the explicit false authorization path remains broken. Add a case that denies command-app access and verifies the localized toast and logout behavior. Also cover an omitted field if the API contract treats missing values specially.
As per coding guidelines: generate tests for new components, services, and logic.
Also applies to: 48-48, 70-70, 91-91, 114-114
🤖 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/__tests__/security-integration.test.ts` at line 29, Add a security
integration test alongside the existing CanLoginToCommandApp fixtures that sets
the field to false and verifies the localized access-denied toast and logout
behavior; also add coverage for an omitted field if the API contract
distinguishes missing authorization from false. Keep the existing authorized
cases unchanged and follow the suite’s established test setup and assertions.
Source: Coding guidelines
| <Pressable | ||
| className="p-2" | ||
| hitSlop={4} | ||
| className="p-3" | ||
| hitSlop={8} | ||
| testID="drawer-menu-button" | ||
| onPress={() => { | ||
| setIsOpen(true); | ||
| }} | ||
| > | ||
| <Menu size={24} color="currentColor" className="text-gray-700 dark:text-gray-300" /> | ||
| {/* Routed through the Icon wrapper, not a bare lucide element: className alone never reaches a | ||
| raw lucide icon (no cssInterop is registered for them), so it falls back to currentColor and | ||
| renders solid black — invisible against a dark header. */} | ||
| <Icon as={Menu} size={24} className="text-gray-700 dark:text-gray-200" /> | ||
| </Pressable> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
file='src/app/(app)/_layout.tsx'
printf '%s\n' '--- target section ---'
sed -n '620,695p' "$file"
printf '%s\n' '--- relevant imports and translation setup ---'
sed -n '1,120p' "$file"
printf '%s\n' '--- Pressable and accessibility usages in this file ---'
rg -n -C 3 'Pressable|accessibility(Label|Role|Hint)' "$file"Repository: Resgrid/IC
Length of output: 9224
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Pressable implementation ---'
pressable=$(fd -i '^pressable\.(tsx|ts)$' src | head -n 1)
printf 'file=%s\n' "$pressable"
sed -n '1,220p' "$pressable"
printf '%s\n' '--- localized icon-button patterns ---'
rg -n -C 4 'accessibilityLabel|accessibilityRole' src --glob '*.{ts,tsx}' | head -n 240
printf '%s\n' '--- translation keys related to navigation and drawer ---'
rg -n -i 'drawer|menu|back|navigation' src --glob '*.{json,ts,tsx}' | head -n 240Repository: Resgrid/IC
Length of output: 231
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate Pressable files ---'
git ls-files | rg '(^|/)pressable\.(tsx|ts)$|(^|/)Pressable\.(tsx|ts)$'
printf '%s\n' '--- UI component directory ---'
git ls-files 'src/components/ui/*' | rg -i 'pressable|button|icon|text'
printf '%s\n' '--- accessibility API usage ---'
rg -n -C 4 'accessibilityLabel|accessibilityRole' src --glob '*.{ts,tsx}' | head -n 240
printf '%s\n' '--- navigation translation references ---'
rg -n -i -C 3 'drawer|menu|back|navigation' src --glob '*.{json,ts,tsx}' | head -n 240Repository: Resgrid/IC
Length of output: 182
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- repository files under the imported UI path ---'
find src/components/ui -maxdepth 3 -type f 2>/dev/null | sort | rg -i 'pressable|button|icon|text' || true
printf '%s\n' '--- Pressable declarations and imports ---'
rg -n -C 5 'export .*Pressable|const Pressable|function Pressable|from .*pressable' src --glob '*.{ts,tsx}' || true
printf '%s\n' '--- accessibility API usage ---'
rg -n -C 4 'accessibilityLabel|accessibilityRole' src --glob '*.{ts,tsx}' | head -n 240 || true
printf '%s\n' '--- navigation translation references ---'
rg -n -i -C 3 'drawer|menu|back|navigation' src --glob '*.{json,ts,tsx}' | head -n 240 || trueRepository: Resgrid/IC
Length of output: 50366
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Pressable wrapper ---'
cat -n src/components/ui/pressable/index.tsx
printf '%s\n' '--- existing localized accessibility labels ---'
rg -n -C 2 'accessibilityLabel=' src --glob '*.{ts,tsx}' | head -n 160
printf '%s\n' '--- translation resource files ---'
find . -type f \( -name '*.json' -o -name '*.ts' -o -name '*.tsx' \) -not -path './node_modules/*' | rg -i 'locale|locales|translation|i18n|lang' | head -n 120
printf '%s\n' '--- common accessibility/navigation keys ---'
rg -n -i -C 2 'dismiss|drawer|menu|back|accessibility' . --glob '*.json' --glob '*.ts' --glob '*.tsx' --glob '!node_modules/**' | head -n 220Repository: Resgrid/IC
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
import json
from pathlib import Path
for name in ('en', 'fr', 'es', 'de', 'it', 'pl', 'sv', 'uk', 'ar'):
path = Path('src/translations') / f'{name}.json'
data = json.loads(path.read_text())
common = data.get('common', {})
print(name, {
'back': common.get('back'),
'menu': common.get('menu'),
'open_menu': common.get('open_menu'),
'open_drawer': common.get('open_drawer'),
})
PY
printf '%s\n' '--- comparable back-button implementations ---'
rg -n -C 5 "testID=.*back|accessibilityLabel=.*common.back|ArrowLeft" src/app src/components --glob '*.{ts,tsx}' | head -n 180Repository: Resgrid/IC
Length of output: 14457
Add accessible names to both navigation buttons.
Set accessibilityRole="button" and localized accessibilityLabel values on CreateDrawerMenuButton and CreateHeaderBackButton. Reuse t('common.back') for the back button and add a translated drawer/menu key. testID does not provide an accessible name for VoiceOver or TalkBack users.
🤖 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/`(app)/_layout.tsx around lines 648 - 660, Add
accessibilityRole="button" and localized accessibilityLabel props to both
CreateDrawerMenuButton and CreateHeaderBackButton. Use t('common.back') for the
back button and add or reuse a translated drawer/menu label for the drawer
button; keep the existing testIDs and press behavior unchanged.
Source: Coding guidelines
| {/* Routed through the Icon wrapper, not a bare lucide element: className alone never reaches a | ||
| raw lucide icon (no cssInterop is registered for them), so it falls back to currentColor and | ||
| renders solid black — invisible against a dark header. */} | ||
| <Icon as={Menu} size={24} className="text-gray-700 dark:text-gray-200" /> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file='src/app/(app)/_layout.tsx'
printf '%s\n' '--- file outline ---'
if command -v ast-grep >/dev/null 2>&1; then
ast-grep outline "$file" || true
fi
printf '%s\n' '--- relevant source ---'
sed -n '1,90p' "$file"
sed -n '610,700p' "$file"
printf '%s\n' '--- icon imports and usages in target file ---'
rg -n -C 3 'Icon|Menu|ArrowLeft|Pressable|Touchable|accessib' "$file"
printf '%s\n' '--- repository guidance and comparable usage ---'
rg -n -g '*.{ts,tsx,md,json}' 'lucide-react-native|Icon as=|accessibilityLabel|aria-label|<Menu|<ArrowLeft' . | head -250Repository: Resgrid/IC
Length of output: 47271
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Icon and Pressable implementations ---'
icon_file=$(fd -i -t f '^icon\.(tsx|ts)$' src/components/ui | head -1)
pressable_file=$(fd -i -t f '^pressable\.(tsx|ts)$' src/components/ui | head -1)
printf 'icon_file=%s\npressable_file=%s\n' "$icon_file" "$pressable_file"
cat -n "$icon_file"
cat -n "$pressable_file"
printf '%s\n' '--- theme and color scheme usage ---'
rg -n -g '*.{ts,tsx}' 'useColorScheme|colorScheme|resolvedColor|theme.*color|ColorValue|--color|dark:' src | head -250
printf '%s\n' '--- accessible header controls and translation keys ---'
rg -n -C 4 -g '*.{ts,tsx,json}' 'common\.back|common\.menu|accessibilityLabel=.*(back|menu)|header-back|drawer-menu' src | head -250
printf '%s\n' '--- project icon rules ---'
sed -n '1,35p' AGENTS.md
sed -n '360,385p' AGENTS.mdRepository: Resgrid/IC
Length of output: 253
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- tracked UI files ---'
git ls-files 'src/components/ui/*' | head -200
printf '%s\n' '--- Icon definitions ---'
rg -n -C 8 'export.*Icon|const Icon|function Icon|createIcon' src/components/ui src | head -250
printf '%s\n' '--- Pressable definitions ---'
rg -n -C 8 'export.*Pressable|const Pressable|function Pressable|createPressable' src/components/ui src | head -250
printf '%s\n' '--- theme and color scheme usage ---'
rg -n -g '*.{ts,tsx}' 'useColorScheme|colorScheme|resolvedColor|theme.*color|ColorValue|--color' src | head -250
printf '%s\n' '--- accessible header controls and translation keys ---'
rg -n -C 4 -g '*.{ts,tsx,json}' 'common\.back|common\.menu|accessibilityLabel=.*(back|menu)|header-back|drawer-menu' src | head -250
printf '%s\n' '--- project icon rules ---'
sed -n '1,35p' AGENTS.md
sed -n '360,385p' AGENTS.mdRepository: Resgrid/IC
Length of output: 50366
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- native Icon wrapper ---'
cat -n src/components/ui/icon/index.tsx | sed -n '1,85p'
printf '%s\n' '--- translation files and relevant keys ---'
git ls-files | rg '(^|/)(locales?|translations?|i18n)(/|\\.)|\\.json$' | head -100
rg -n -C 2 '"(back|menu|drawer|navigation)"|common\.(back|menu)' . \
-g '*.json' -g '*.{ts,tsx}' \
--glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -180
printf '%s\n' '--- direct Lucide color usage ---'
rg -n -C 2 '<(Menu|ArrowLeft|ChevronRight|Search|X)[^>]*(color=|className=)' src -g '*.{ts,tsx}' | head -120Repository: Resgrid/IC
Length of output: 23306
Render the header icons directly and label the controls.
The project rules require direct lucide-react-native icons. Replace Icon as={Menu} and Icon as={ArrowLeft} with direct components and pass theme-resolved color values. Add translated labels using t('sidebar.menu') and t('common.back').
🤖 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/`(app)/_layout.tsx at line 659, Update the header controls in the
layout to render Menu and ArrowLeft directly from lucide-react-native instead of
through Icon’s as prop, supplying theme-resolved color values. Add translated
accessibility labels using t('sidebar.menu') for the menu control and
t('common.back') for the back control.
Source: Coding guidelines
| const incidentChannels = useChatStore((state) => (boardCallId ? state.incidentChannelsByCallId[boardCallId] : undefined)); | ||
|
|
||
| const commandChatChannelId = useMemo(() => incidentChannels?.find((channel) => channel.ChannelType === ChatChannelType.IncidentCommand)?.ChatChannelId ?? null, [incidentChannels]); | ||
|
|
||
| const laneChatChannelId = useCallback((nodeId: string) => incidentChannels?.find((channel) => channel.CommandStructureNodeId === nodeId)?.ChatChannelId ?? null, [incidentChannels]); | ||
|
|
||
| const openChatChannel = useCallback( | ||
| (channelId: string | null, unavailableMessage: string) => { | ||
| if (!channelId) { | ||
| showToast('info', unavailableMessage); | ||
| return; | ||
| } | ||
| router.push(`/chat/${channelId}`); | ||
| }, | ||
| [showToast] | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not report unavailable channels while channels are loading.
incidentChannels is undefined during the initial request. The resolvers convert that state to null, so an immediate tap shows an unavailable-channel toast even when the channel exists. Track loading separately and disable the actions, or wait for loading to complete before showing the unavailable message.
🤖 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/`(app)/command.tsx around lines 208 - 223, Update the incident
channel flow around incidentChannels, commandChatChannelId, laneChatChannelId,
and openChatChannel to distinguish an undefined/loading channels state from a
completed request with no matching channel. Track or reuse the channel-loading
state, disable or defer the related actions while loading, and only show the
unavailable toast after loading has completed.
| onClose={() => setActionsMessage(null)} | ||
| isOwn={!!actionsMessage?.SenderUserId && actionsMessage.SenderUserId === currentUserId} | ||
| isModerator={isModerator} | ||
| frozen={isFrozen} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Close or disable an edit sheet when the channel freezes.
Line 401 freezes MessageActionsSheet, but an edit sheet opened before archival remains active. Its save handler can still call editMessage after isFrozen becomes true. Guard the save handler and close or disable the edit sheet when the channel becomes archived.
🤖 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 at line 401, Update the edit sheet flow
alongside MessageActionsSheet’s isFrozen handling so an edit opened before
archival cannot remain active or save afterward. Guard the edit save handler
against isFrozen and close or disable the edit sheet when the channel becomes
archived, while preserving normal editing behavior for unfrozen channels.
| export const useDirectMessage = () => { | ||
| const { t } = useTranslation(); | ||
| const [isOpening, setIsOpening] = useState(false); | ||
|
|
||
| const openDirectMessage = useCallback( | ||
| async (targetUserId?: string | null) => { | ||
| if (!targetUserId) { | ||
| // External lane leads have a name and phone but no Resgrid account to message. | ||
| useToastStore.getState().showToast('info', t('chat.dm_unavailable')); | ||
| return; | ||
| } | ||
|
|
||
| setIsOpening(true); | ||
| try { | ||
| const channel = await createDirectMessage({ TargetUserId: targetUserId }); | ||
| const channelId = channel?.Data?.ChatChannelId; | ||
| if (!channelId) { | ||
| useToastStore.getState().showToast('error', t('chat.dm_failed')); | ||
| return; | ||
| } | ||
| router.push(`/chat/${channelId}`); | ||
| } catch (error) { | ||
| logger.error({ message: 'chat: failed to open direct message', context: { error, targetUserId } }); | ||
| useToastStore.getState().showToast('error', t('chat.dm_failed')); | ||
| } finally { | ||
| setIsOpening(false); | ||
| } | ||
| }, | ||
| [t] | ||
| ); | ||
|
|
||
| return { openDirectMessage, isOpening }; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --glob '*.{test,spec}.{ts,tsx}' \
'useDirectMessage|command-open-chat|command-open-leads-chat|command-open-dispatch-chat|landscape-lane-resources|lane-lead-(primary|secondary)-message' \
src || trueRepository: Resgrid/IC
Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- changed files ---'
git diff --stat
git status --short
printf '%s\n' '--- relevant source files ---'
for f in \
src/hooks/use-direct-message.ts \
'src/app/(app)/command.tsx' \
src/components/command/landscape-structure-board.tsx \
src/components/command/lane-details-sheet.tsx
do
if [ -f "$f" ]; then
echo "### $f"
wc -l "$f"
sed -n '1,280p' "$f"
fi
done
printf '%s\n' '--- test files and test configuration ---'
git ls-files | rg '(^|/)(__tests__|tests?)/|(\.|/)(test|spec)\.(ts|tsx|js|jsx)$|jest|vitest|testing-library|playwright|detox' || true
rg -n 'jest|vitest|`@testing-library`|renderHook|testID|useDirectMessage|loadIncidentChannels|onContentSizeChange' \
package.json package-lock.json yarn.lock pnpm-lock.yaml src 2>/dev/null | head -300 || trueRepository: Resgrid/IC
Length of output: 50366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- test configuration ---'
sed -n '1,220p' jest.config.js
sed -n '1,180p' package.json
printf '%s\n' '--- focused test file sizes ---'
for f in \
'src/app/(app)/__tests__/command.test.tsx' \
src/components/command/__tests__/command-board.test.tsx \
src/components/command/__tests__/lane-details-sheet.test.tsx \
src/components/command/__tests__/work-time-light.test.tsx
do
echo "### $f"
wc -l "$f"
done
printf '%s\n' '--- focused test references ---'
rg -n -C 3 \
'chat|Chat|channel|message|lead|LaneDetailsSheet|LandscapeStructureBoard|scroll|overflow|testID|router|useDirectMessage|loadIncidentChannels' \
'src/app/(app)/__tests__/command.test.tsx' \
src/components/command/__tests__/command-board.test.tsx \
src/components/command/__tests__/lane-details-sheet.test.tsx \
src/components/command/__tests__/work-time-light.test.tsx || true
printf '%s\n' '--- exports and render paths for LaneResourceList ---'
rg -n -C 4 'LaneResourceList|LandscapeStructureBoard|onOpenLaneChat|onMessageLead|handleOpen(Command|Leads|Dispatch|Lane)Chat|openDirectMessage' \
src/components/command/landscape-structure-board.tsx \
'src/app/(app)/command.tsx' \
src/components/command/lane-details-sheet.tsxRepository: Resgrid/IC
Length of output: 39941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- command test cases and relevant mocks ---'
rg -n '^(describe|it|test)\(|jest\.mock|CommandBoard|command-open|assignment-message|lane-details|LandscapeStructureBoard|LaneDetailsSheet' \
'src/app/(app)/__tests__/command.test.tsx' \
src/components/command/__tests__/command-board.test.tsx \
src/components/command/__tests__/lane-details-sheet.test.tsx
printf '%s\n' '--- exact behavior inventory across tracked test files ---'
python3 - <<'PY'
from pathlib import Path
import re
patterns = [
r'useDirectMessage',
r'openDirectMessage',
r'handleOpenCommandChat',
r'handleOpenLeadsChat',
r'handleOpenDispatchChat',
r'handleOpenLaneChat',
r'loadIncidentChannels',
r'command-open-chat',
r'command-open-leads-chat',
r'command-open-dispatch-chat',
r'landscape-lane-chat-',
r'landscape-lane-resources',
r'lane-lead-(primary|secondary)-message',
r'assignment-message-',
r'onContentSizeChange',
r'scrollEnabled',
r'showsVerticalScrollIndicator',
]
test_files = [
p for p in Path('.').rglob('*')
if p.is_file()
and p.suffix in {'.ts', '.tsx', '.js', '.jsx'}
and ('.test.' in p.name or '.spec.' in p.name or '__tests__' in p.parts)
]
for pattern in patterns:
matches = []
rx = re.compile(pattern)
for path in test_files:
try:
text = path.read_text()
except UnicodeDecodeError:
continue
if rx.search(text):
matches.append(str(path))
print(f'{pattern}: {matches or "NONE"}')
PY
printf '%s\n' '--- source testability points ---'
sed -n '1,130p' src/components/command/__tests__/lane-details-sheet.test.tsx
sed -n '1,100p' src/components/command/__tests__/command-board.test.tsx
sed -n '1,180p' 'src/app/(app)/__tests__/command.test.tsx'Repository: Resgrid/IC
Length of output: 17099
Add focused tests for the new command chat and lane scrolling paths.
src/hooks/use-direct-message.ts: cover missing IDs, API failures, missing channel IDs, successful routing, andisOpeningtransitions.src/app/(app)/command.tsx: cover command, leads, dispatch, lane, role-holder, and lead direct-message routing.src/components/command/landscape-structure-board.tsx: coverscrollEnabledandshowsVerticalScrollIndicatorwhen lane content crosses the viewport threshold.src/components/command/lane-details-sheet.tsx: cover message actions for Resgrid users and their absence for external leads.
📍 Affects 4 files
src/hooks/use-direct-message.ts#L16-L48(this comment)src/app/(app)/command.tsx#L202-L237src/components/command/landscape-structure-board.tsx#L83-L110src/components/command/lane-details-sheet.tsx#L115-L122
🤖 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/hooks/use-direct-message.ts` around lines 16 - 48, Add focused tests for
the new chat and lane behaviors: in src/hooks/use-direct-message.ts lines 16-48,
test missing IDs, API failures, missing channel IDs, successful routing, and
isOpening transitions; in src/app/(app)/command.tsx lines 202-237, test command,
leads, dispatch, lane, role-holder, and lead direct-message routing; in
src/components/command/landscape-structure-board.tsx lines 83-110, test
scrollEnabled and showsVerticalScrollIndicator at the lane-content viewport
threshold; and in src/components/command/lane-details-sheet.tsx lines 115-122,
test message actions for Resgrid users and their absence for external leads.
Source: Coding guidelines
| const openDirectMessage = useCallback( | ||
| async (targetUserId?: string | null) => { | ||
| if (!targetUserId) { | ||
| // External lane leads have a name and phone but no Resgrid account to message. | ||
| useToastStore.getState().showToast('info', t('chat.dm_unavailable')); | ||
| return; | ||
| } | ||
|
|
||
| setIsOpening(true); | ||
| try { | ||
| const channel = await createDirectMessage({ TargetUserId: targetUserId }); | ||
| const channelId = channel?.Data?.ChatChannelId; | ||
| if (!channelId) { | ||
| useToastStore.getState().showToast('error', t('chat.dm_failed')); | ||
| return; | ||
| } | ||
| router.push(`/chat/${channelId}`); | ||
| } catch (error) { | ||
| logger.error({ message: 'chat: failed to open direct message', context: { error, targetUserId } }); | ||
| useToastStore.getState().showToast('error', t('chat.dm_failed')); | ||
| } finally { | ||
| setIsOpening(false); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- hook outline ---'
ast-grep outline src/hooks/use-direct-message.ts --view expanded
printf '%s\n' '--- hook source ---'
cat -n src/hooks/use-direct-message.ts
printf '%s\n' '--- hook usages ---'
rg -n -C 4 'useDirectMessage|openDirectMessage|isOpening' src --glob '*.{ts,tsx}'Repository: Resgrid/IC
Length of output: 7345
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- createDirectMessage definition and callers ---'
rg -n -C 8 'createDirectMessage' src --glob '*.{ts,tsx}'
printf '%s\n' '--- command message controls ---'
sed -n '850,895p' 'src/app/(app)/command.tsx'
sed -n '990,1020p' 'src/app/(app)/command.tsx'
printf '%s\n' '--- related tests and route handling ---'
rg -n -C 5 'dm_unavailable|dm_failed|ChatChannelId|router\.push\(`/chat' . --glob '*.{ts,tsx}' --glob '*.{test,spec}.{ts,tsx}'Repository: Resgrid/IC
Length of output: 48726
🏁 Script executed:
#!/bin/bash
set -eu
node - <<'JS'
const pushes = [];
const requests = [];
let renderedIsOpening = false;
let pendingState = null;
function setIsOpening(value) {
// React batches this update; the current callback still sees the
// previously rendered state until React renders again.
pendingState = value;
}
function flushRender() {
if (pendingState !== null) {
renderedIsOpening = pendingState;
pendingState = null;
}
}
function createDirectMessage(targetUserId) {
let resolve;
const promise = new Promise((r) => { resolve = r; });
requests.push({ targetUserId, resolve });
return promise;
}
async function openDirectMessage(targetUserId) {
if (!targetUserId || renderedIsOpening) return;
setIsOpening(true);
const channel = await createDirectMessage(targetUserId);
pushes.push(`/chat/${channel}`);
setIsOpening(false);
}
// Two synchronous calls occur before a render flush.
const first = openDirectMessage('user-1');
const second = openDirectMessage('user-2');
console.log(JSON.stringify({
requestsStartedBeforeRender: requests.map(({ targetUserId }) => targetUserId),
renderedIsOpening,
secondCallWasBlocked: requests.length < 2,
}));
// Resolve both requests in reverse order.
requests[1].resolve('channel-2');
requests[0].resolve('channel-1');
Promise.all([first, second]).then(() => {
console.log(JSON.stringify({ pushesInResolutionOrder: pushes }));
});
JSRepository: Resgrid/IC
Length of output: 321
Prevent concurrent direct-message opens. Two synchronous presses start both requests before isOpening re-renders, and each completion pushes its own chat route. Add a synchronous useRef guard before createDirectMessage.
🤖 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/hooks/use-direct-message.ts` around lines 20 - 43, Prevent concurrent
opens in openDirectMessage by adding a synchronous useRef guard checked before
createDirectMessage. Set the guard before starting the request, return
immediately when already active, and clear it in the finally block alongside
setIsOpening(false), preserving the existing request and navigation behavior.
| /** | ||
| * Whether this user may act as a commander: use the IC app, establish command, and view command | ||
| * boards. Defaults to true for everyone when the department has never configured the permission. | ||
| */ | ||
| public CanLoginToCommandApp: boolean = true; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'CanLoginToCommandApp|DepartmentRightsResult(Data)?|plainToInstance|new DepartmentRightsResultData|getCurrentUsersRights' srcRepository: Resgrid/IC
Length of output: 17015
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- model and result wrapper ---'
cat -n src/models/v4/security/departmentRightsResultData.ts
cat -n src/models/v4/security/departmentRightsResult.ts
printf '%s\n' '--- rights store ---'
cat -n src/stores/security/store.ts
printf '%s\n' '--- API client and layout flow ---'
cat -n src/api/security/security.ts
sed -n '140,215p' 'src/app/(app)/_layout.tsx'
printf '%s\n' '--- repository-wide references and serializers ---'
rg -n -C 3 'CanLoginToCommandApp|DepartmentRightsResultData|DepartmentRightsResult|JSON\.parse|deserialize|plainToInstance|class-transformer|axios' . \
-g '!node_modules' -g '!dist' -g '!build' -g '!coverage'Repository: Resgrid/IC
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- API endpoint implementation ---'
sed -n '130,175p' src/api/common/client.tsx
printf '%s\n' '--- storage hydration implementation ---'
cat -n src/lib/storage/index.tsx | sed -n '1,80p'
rg -n -C 5 'security-storage|persist\(|hydrate|CanLoginToCommandApp' src --glob '!**/__tests__/**'
printf '%s\n' '--- repository identity ---'
git remote -v
git ls-files | rg -i 'security|rights|department' | head -100Repository: Resgrid/IC
Length of output: 21278
🌐 Web query:
Resgrid server GetCurrentUsersRights CanLoginToCommandApp DepartmentRightsResult
💡 Result:
In the Resgrid platform, GetCurrentUsersRights, CanLoginToCommandApp, and DepartmentRightsResult are internal components related to the system's security, permissions, and API data structures [1][2]. GetCurrentUsersRights is typically a method or endpoint used within the Resgrid backend (often found in controllers or services) to retrieve the permission set for the currently authenticated user [1]. This method evaluates the user's specific access levels within their department to determine what actions they are authorized to perform across the platform [3][2]. CanLoginToCommandApp is a boolean property or flag often included in the data returned by permission-related methods [1]. It explicitly indicates whether the user has the necessary rights to log in to and use the Resgrid Incident Command (IC) application [1][4]. DepartmentRightsResult is a data structure or model object (often a DTO) used to encapsulate these permission settings [1]. It acts as a container that holds various flags and configurations—such as CanLoginToCommandApp—providing the client-side applications (like the IC app, Dispatch, or the Resgrid API client) with a clear view of what the user is permitted to do [1][5][6]. These elements are part of the broader claims and rights system managed by the Resgrid backend, ensuring that access to sensitive tools like the Command application is properly gated based on department-level settings and individual user roles [3][2][6].
Citations:
- 1: Resgrid/Core@1b3d252
- 2: https://resgrid-core.readthedocs.io/_/downloads/en/stable/pdf/
- 3: https://resgrid-core.readthedocs.io/en/latest/setup/
- 4: https://resgrid.com/apps/ic
- 5: https://github.com/Resgrid/ngxresgridlib
- 6: https://resgrid.com/apps
🏁 Script executed:
#!/bin/bash
set -euo pipefail
commit='1b3d2522f173a6f20fbd0999e68a226e3f290cec'
python3 - <<'PY'
import json
import urllib.request
url = "https://api.github.com/repos/Resgrid/Core/commits/1b3d2522f173a6f20fbd0999e68a226e3f290cec"
request = urllib.request.Request(url, headers={"Accept": "application/vnd.github+json", "User-Agent": "review-inspection"})
with urllib.request.urlopen(request) as response:
payload = json.load(response)
print("commit:", payload.get("sha"))
for file in payload.get("files", []):
patch = file.get("patch", "")
if any(term.lower() in (file.get("filename", "") + "\n" + patch).lower()
for term in ("CanLoginToCommandApp", "GetCurrentUsersRights", "DepartmentRightsResult")):
print("\nFILE:", file.get("filename"))
print(patch)
PYRepository: Resgrid/IC
Length of output: 2434
🏁 Script executed:
#!/bin/bash
set -euo pipefail
curl -fsSLk \
-H 'Accept: application/vnd.github+json' \
-H 'User-Agent: review-inspection' \
'https://api.github.com/repos/Resgrid/Core/commits/1b3d2522f173a6f20fbd0999e68a226e3f290cec' |
python3 -c '
import json, sys
payload = json.load(sys.stdin)
print("commit:", payload.get("sha"))
for file in payload.get("files", []):
text = file.get("filename", "") + "\n" + file.get("patch", "")
if any(term.lower() in text.lower() for term in (
"CanLoginToCommandApp", "GetCurrentUsersRights", "DepartmentRightsResult"
)):
print("\nFILE:", file.get("filename"))
print(file.get("patch", ""))
'Repository: Resgrid/IC
Length of output: 854
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
def layout_allows_initialization(rights):
return rights.get("CanLoginToCommandApp") is not False
def class_default_applies_to_missing_field():
# A class initializer applies only when the class is constructed.
constructed = {"CanLoginToCommandApp": True}
plain_api_data = json.loads('{"DepartmentName":"Example"}')
persisted_data = json.loads('{"DepartmentName":"Example"}')
return (
constructed.get("CanLoginToCommandApp") is True,
"CanLoginToCommandApp" not in plain_api_data,
"CanLoginToCommandApp" not in persisted_data,
layout_allows_initialization(plain_api_data),
layout_allows_initialization(persisted_data),
)
cases = {
"explicit true": {"CanLoginToCommandApp": True},
"explicit false": {"CanLoginToCommandApp": False},
"missing": {},
"invalid string": {"CanLoginToCommandApp": "false"},
"invalid null": {"CanLoginToCommandApp": None},
}
print("constructed default, API missing field, persisted missing field, "
"API gate, persisted gate:", class_default_applies_to_missing_field())
for name, rights in cases.items():
print(name, "=> allows initialization:", layout_allows_initialization(rights))
PYRepository: Resgrid/IC
Length of output: 491
Fail closed for missing or invalid CanLoginToCommandApp values.
Axios and Zustand use plain JSON, so the class initializer does not apply when the field is missing. The layout logs out only for literal false; missing, null, and non-boolean values allow initialization. Remove the permissive default and validate the response before storing or continuing initialization.
🤖 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/models/v4/security/departmentRightsResultData.ts` around lines 12 - 16,
The CanLoginToCommandApp field currently fails open for missing or invalid JSON
values. Remove its true initializer and, wherever
SecurityDepartmentRightsResultData is validated or consumed during
initialization, require the value to be a literal boolean before storing or
continuing; reject missing, null, and non-boolean values rather than allowing
them to enable command access.
| const response = await chatApi.getChannels(undefined, true); | ||
| const forCall = (response.Data ?? []).filter((channel) => channel.CallId === numericCallId); | ||
| set((state) => ({ incidentChannelsByCallId: { ...state.incidentChannelsByCallId, [callId]: forCall } })); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make archived incident channels resolvable by the conversation screen.
Line 267 stores archived records only in incidentChannelsByCallId. ChannelConversationScreen reads only channels, and its fallback refresh omits includeArchived. An archived channel opened from the command board is therefore unresolved, so isFrozen is false and the composer remains enabled.
Add a channel-by-ID resolver shared by the incident cache and detail screen. Keep archived records out of the normal list UI. Add a regression test for opening an archived command channel from the board.
🤖 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/store.ts` around lines 265 - 267, Add a shared channel-by-ID
resolver used by both the incident-channel cache and ChannelConversationScreen,
storing archived channels separately from the normal channels list so they
remain excluded from list UI. Update the detail-screen fallback refresh to
request archived records and resolve the opened channel by ID, ensuring archived
channels set isFrozen and disable the composer. Add a regression test covering
opening an archived command channel from the board.
| "dm_failed": "Couldn't open that conversation.", | ||
| "dm_unavailable": "That contact has no Resgrid account to message.", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Translate the new chat and authorization strings in every non-English locale. The same English values appear in all five locale files, so the new flows display English text for non-English users.
- src/translations/de.json#L341-L342: translate the new
dm_*,frozen_notice, channel labels, unavailable messages, message labels, and authorization text. - src/translations/es.json#L341-L342: translate the corresponding new chat and authorization values into Spanish.
- src/translations/fr.json#L341-L342: translate the corresponding new chat and authorization values into French.
- src/translations/it.json#L341-L342: translate the corresponding new chat and authorization values into Italian.
- src/translations/pl.json#L341-L342: translate the corresponding new chat and authorization values into Polish.
📍 Affects 5 files
src/translations/de.json#L341-L342(this comment)src/translations/es.json#L341-L342src/translations/fr.json#L341-L342src/translations/it.json#L341-L342src/translations/pl.json#L341-L342
🤖 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/translations/de.json` around lines 341 - 342, Translate all newly added
chat and authorization localization keys—including dm_*, frozen_notice, channel
labels, unavailable messages, message labels, and authorization text—into their
respective languages in src/translations/de.json (lines 341-342),
src/translations/es.json (lines 341-342), src/translations/fr.json (lines
341-342), src/translations/it.json (lines 341-342), and src/translations/pl.json
(lines 341-342), ensuring no English values remain in these non-English locale
files.
| // initialization — the server refuses them the board endpoints anyway, so signing them straight | ||
| // back out is far clearer than an app that loads and then fails every request. | ||
| if (!isCurrentRun()) return; | ||
| if (securityStore.getState().rights?.CanLoginToCommandApp === false) { |
There was a problem hiding this comment.
Authorization gate fails open. The check for explicit false (=== false) instead of requiring explicit true (!== true) lets null rights or missing CanLoginToCommandApp proceed past the gate, defeating deny-by-default and silently failing the client-side clean sign-out intent. Change to if (securityStore.getState().rights?.CanLoginToCommandApp !== true) so any non-confirmed permission blocks access.
Kody rule violation: Implement RBAC with least privilege and deny-by-default
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 184:
Authorization gate fails open. The check for explicit `false` (`=== false`) instead of requiring explicit `true` (`!== true`) lets `null` `rights` or missing `CanLoginToCommandApp` proceed past the gate, defeating deny-by-default and silently failing the client-side clean sign-out intent. Change to `if (securityStore.getState().rights?.CanLoginToCommandApp !== true)` so any non-confirmed permission blocks access.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // back out is far clearer than an app that loads and then fails every request. | ||
| if (!isCurrentRun()) return; | ||
| if (securityStore.getState().rights?.CanLoginToCommandApp === false) { | ||
| logger.warn({ message: 'User is not authorized to use the IC app; signing out', context: { userId } }); |
There was a problem hiding this comment.
Non-compliant security audit logging. The unauthorized access attempt is logged via logger.warn with only message and context: { userId }, missing actor.role, action (as a structured field), resource.id, result, trace_id, ip, and user_agent. Rule 41 requires security-relevant actions to write to an immutable, append-only audit log with structured fields plus WORM/signed storage and SIEM forwarding. Emit a dedicated audit record via auditLog.write({ action: 'command_app.access_denied', actor: { user_id: userId, role }, resource: { id: 'command_app' }, result: 'denied', trace_id, ip, user_agent, timestamp: new Date().toISOString() }).
Kody rule violation: Emit tamper-evident audit logs with required fields
Prompt for LLM
File src/app/(app)/_layout.tsx:
Line 185:
Non-compliant security audit logging. The unauthorized access attempt is logged via `logger.warn` with only `message` and `context: { userId }`, missing actor.role, action (as a structured field), resource.id, result, trace_id, ip, and user_agent. Rule 41 requires security-relevant actions to write to an immutable, append-only audit log with structured fields plus WORM/signed storage and SIEM forwarding. Emit a dedicated audit record via `auditLog.write({ action: 'command_app.access_denied', actor: { user_id: userId, role }, resource: { id: 'command_app' }, result: 'denied', trace_id, ip, user_agent, timestamp: new Date().toISOString() })`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| {/* Shortcut across to the assistant. The app header above carries the title and the way back, | ||
| so this row is actions only. */} | ||
| <HStack className="items-center justify-end border-b border-outline-100 px-4 py-2"> | ||
| <Pressable className="flex-row items-center rounded-full bg-purple-600 px-3 py-2" onPress={() => router.push('/chatbot' as Href)} accessibilityLabel={t('chat.assistant')} testID="chat-open-assistant"> |
There was a problem hiding this comment.
Performance regression in src/app/(app)/chat.tsx (and 17 additional locations including chatbot.tsx, command.tsx, landscape-structure-board.tsx, lane-details-sheet.tsx, and structure-section.tsx). Inline arrow functions inside JSX props create new function instances on every render, violating the team rule against .bind() or arrow functions in JSX props. 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/app/(app)/chat.tsx:
Line 140:
Performance regression in `src/app/(app)/chat.tsx` (and 17 additional locations including `chatbot.tsx`, `command.tsx`, `landscape-structure-board.tsx`, `lane-details-sheet.tsx`, and `structure-section.tsx`). Inline arrow functions inside JSX props create new function instances on every render, violating the team rule against `.bind()` or arrow functions in JSX props. 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.
| {/* 1:1 with whoever holds this ICS position. */} | ||
| <Pressable | ||
| accessibilityLabel={t('command.message_role_holder')} | ||
| onPress={() => void openDirectMessage(assignment.UserId)} |
There was a problem hiding this comment.
Unhandled promise rejection in src/app/(app)/command.tsx:204 and src/app/(app)/command.tsx:1012. openDirectMessage returns a promise discarded via void with no rejection handler, violating Rule [1] for async error handling and risking a crash or silent failure. Wrap the call in an async handler with try/catch, or ensure openDirectMessage internally catches and surfaces errors to the UI.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/app/(app)/command.tsx:
Line 877:
Unhandled promise rejection in `src/app/(app)/command.tsx:204` and `src/app/(app)/command.tsx:1012`. `openDirectMessage` returns a promise discarded via `void` with no rejection handler, violating Rule [1] for async error handling and risking a crash or silent failure. Wrap the call in an async handler with try/catch, or ensure `openDirectMessage` internally catches and surfaces errors to the UI.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| const incidentChannels = useChatStore((state) => (boardCallId ? state.incidentChannelsByCallId[boardCallId] : undefined)); | ||
|
|
||
| const commandChatChannelId = useMemo(() => incidentChannels?.find((channel) => channel.ChannelType === ChatChannelType.IncidentCommand)?.ChatChannelId ?? null, [incidentChannels]); |
There was a problem hiding this comment.
Duplicated logic in src/app/(app)/command.tsx at lines 210, 227, and 231. The pattern incidentChannels?.find((channel) => channel.ChannelType === X)?.ChatChannelId ?? null is repeated three times with only the channel type differing. Rule [14] requires extracting duplicated logic into a named function or utility; create a helper such as const findChannelId = useCallback((type: ChatChannelType) => incidentChannels?.find((c) => c.ChannelType === type)?.ChatChannelId ?? null, [incidentChannels]) and reuse it for all three lookups.
Kody rule violation: Extract duplicated logic into functions
Prompt for LLM
File src/app/(app)/command.tsx:
Line 210:
Duplicated logic in `src/app/(app)/command.tsx` at lines 210, 227, and 231. The pattern `incidentChannels?.find((channel) => channel.ChannelType === X)?.ChatChannelId ?? null` is repeated three times with only the channel type differing. Rule [14] requires extracting duplicated logic into a named function or utility; create a helper such as `const findChannelId = useCallback((type: ChatChannelType) => incidentChannels?.find((c) => c.ChannelType === type)?.ChatChannelId ?? null, [incidentChannels])` and reuse it for all three lookups.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // The server validates the user actually holds command (CanSendAsIcAsync) and rejects otherwise. | ||
| const isCommandChannel = isCommandChannelType(channel?.ChannelType); | ||
|
|
||
| /** | ||
| * Archived channel = point-in-time record. A closed incident freezes its command and lane chat, and | ||
| * a closed call freezes its incident chat: no posting, no editing, no reactions. The server enforces | ||
| * all of it; this just stops the UI offering actions that would bounce. Flagging stays available. | ||
| */ | ||
| const isFrozen = !!channel?.IsArchived; |
There was a problem hiding this comment.
isFrozen is always false for archived channels because channel is resolved from s.channels, which never includes archived channels — fetchChannels calls getChannels() without includeArchived, and loadIncidentChannels stores results only in incidentChannelsByCallId, never merging into s.channels. The entire frozen-channel feature is non-functional for closed incidents navigated from the command board, and message sends fail server-side silently. Resolve the channel from incidentChannelsByCallId as a fallback, or call getChannel(channelId) to fetch the specific archived channel when not found in s.channels, then set isFrozen from that.
// Fall back to a direct fetch for channels not in the main list (archived incident channels).
const channel = useChatStore((s) => s.channels.find((c) => c.ChatChannelId === channelId));
const [fallbackChannel, setFallbackChannel] = useState<ChatChannelResultData | null>(null);
useEffect(() => {
if (channel || !channelId || !isChatEnabled || fallbackChannel) return;
let cancelled = false;
chatApi.getChannel(channelId).then((res) => { if (!cancelled) setFallbackChannel(res.Data ?? null); }).catch(() => { if (!cancelled) setFallbackChannel(null); });
return () => { cancelled = true; };
}, [channel, channelId, isChatEnabled, fallbackChannel]);
const resolvedChannel = channel ?? fallbackChannel;
...
const isFrozen = !!resolvedChannel?.IsArchived;Prompt for LLM
File src/app/chat/[channelId].tsx:
Line 77 to 85:
`isFrozen` is always false for archived channels because `channel` is resolved from `s.channels`, which never includes archived channels — `fetchChannels` calls `getChannels()` without `includeArchived`, and `loadIncidentChannels` stores results only in `incidentChannelsByCallId`, never merging into `s.channels`. The entire frozen-channel feature is non-functional for closed incidents navigated from the command board, and message sends fail server-side silently. Resolve the channel from `incidentChannelsByCallId` as a fallback, or call `getChannel(channelId)` to fetch the specific archived channel when not found in `s.channels`, then set `isFrozen` from that.
Suggested Code:
// Fall back to a direct fetch for channels not in the main list (archived incident channels).
const channel = useChatStore((s) => s.channels.find((c) => c.ChatChannelId === channelId));
const [fallbackChannel, setFallbackChannel] = useState<ChatChannelResultData | null>(null);
useEffect(() => {
if (channel || !channelId || !isChatEnabled || fallbackChannel) return;
let cancelled = false;
chatApi.getChannel(channelId).then((res) => { if (!cancelled) setFallbackChannel(res.Data ?? null); }).catch(() => { if (!cancelled) setFallbackChannel(null); });
return () => { cancelled = true; };
}, [channel, channelId, isChatEnabled, fallbackChannel]);
const resolvedChannel = channel ?? fallbackChannel;
...
const isFrozen = !!resolvedChannel?.IsArchived;
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| // A FIXED height, clamped so the board never eats more than a screenful. Previously this was a | ||
| // minHeight, so a lane with a lot of crews grew without bound and the page scrolled forever to get | ||
| // past the structure section. Each lane now scrolls its own resources instead. | ||
| const laneHeight = Math.max(280, Math.min(560, viewportHeight - 360)); |
There was a problem hiding this comment.
Layout-critical magic numbers in src/components/command/landscape-structure-board.tsx. The literals 280 (min lane height), 560 (max lane height), and 360 (vertical offset for headers/controls above the lanes) appear inline, making design intent opaque and easy to misuse or inconsistently update elsewhere. Define named constants such as MIN_LANE_HEIGHT, MAX_LANE_HEIGHT, and LANE_VERTICAL_OFFSET and reference them in the calculation.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/components/command/landscape-structure-board.tsx:
Line 312:
Layout-critical magic numbers in `src/components/command/landscape-structure-board.tsx`. The literals 280 (min lane height), 560 (max lane height), and 360 (vertical offset for headers/controls above the lanes) appear inline, making design intent opaque and easy to misuse or inconsistently update elsewhere. Define named constants such as `MIN_LANE_HEIGHT`, `MAX_LANE_HEIGHT`, and `LANE_VERTICAL_OFFSET` and reference them in the calculation.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| <HStack className="items-center" space="sm"> | ||
| <Box className="size-8 items-center justify-center rounded-lg bg-primary-50 dark:bg-primary-950"> | ||
| <Truck className="text-primary-600 dark:text-primary-400" size={16} /> | ||
| <Icon as={Truck} className="text-blue-600 dark:text-blue-400" size={16} /> |
There was a problem hiding this comment.
Design-system violation in src/components/command/resource-cards.tsx and src/components/command/assistant-sheet.tsx (lines 122, 159). The Truck icon's color was changed from the semantic token text-primary-600 dark:text-primary-400 to a raw Tailwind color text-blue-600 dark:text-blue-400, bypassing the centrally managed, themeable source of truth and risking visual inconsistency if the primary palette is updated. Revert to text-primary-600 dark:text-primary-400 or the appropriate official design-system token.
Kody rule violation: Prefer the company design system over hand-rolled UI styles
Prompt for LLM
File src/components/command/resource-cards.tsx:
Line 93:
Design-system violation in `src/components/command/resource-cards.tsx` and `src/components/command/assistant-sheet.tsx` (lines 122, 159). The Truck icon's color was changed from the semantic token `text-primary-600 dark:text-primary-400` to a raw Tailwind color `text-blue-600 dark:text-blue-400`, bypassing the centrally managed, themeable source of truth and risking visual inconsistency if the primary palette is updated. Revert to `text-primary-600 dark:text-primary-400` or the appropriate official design-system token.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const response = await chatApi.getChannels(undefined, true); | ||
| const forCall = (response.Data ?? []).filter((channel) => channel.CallId === numericCallId); |
There was a problem hiding this comment.
loadIncidentChannels calls getChannels(undefined, true) on every board open (command.tsx:204), fetching the user's entire channel history and filtering client-side by callId, producing O(total channels) payload that grows unbounded as incidents accumulate. Pass callId as a query parameter to getChannels so the server filters before responding, or add a dedicated getIncidentChannels(callId) endpoint.
// Pass callId to the API so the server filters before sending.
const response = await chatApi.getChannels(undefined, true, undefined, numericCallId);
const forCall = response.Data ?? [];Prompt for LLM
File src/stores/chat/store.ts:
Line 265 to 266:
`loadIncidentChannels` calls `getChannels(undefined, true)` on every board open (command.tsx:204), fetching the user's entire channel history and filtering client-side by `callId`, producing O(total channels) payload that grows unbounded as incidents accumulate. Pass `callId` as a query parameter to `getChannels` so the server filters before responding, or add a dedicated `getIncidentChannels(callId)` endpoint.
Suggested Code:
// Pass callId to the API so the server filters before sending.
const response = await chatApi.getChannels(undefined, true, undefined, numericCallId);
const forCall = response.Data ?? [];
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| * channels: once a command closes its chat stays readable as a point-in-time record. | ||
| */ | ||
| incidentChannelsByCallId: Record<string, ChatChannelResultData[]>; | ||
| loadIncidentChannels: (callId: string) => Promise<void>; |
There was a problem hiding this comment.
Missing async function documentation in src/stores/chat/store.ts and src/api/chat/chat.ts. loadIncidentChannels returns Promise<void> but lacks @returns or @throws JSDoc; the JSDoc block above documents the incidentChannelsByCallId field, not this function. Rule 22 requires async/Promise functions to document resolve value, rejection conditions, and await usage — the current implementation swallows errors internally, which callers cannot infer from the signature. Add a JSDoc comment with @returns {Promise<void>} and a note that failures are caught and logged.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/stores/chat/store.ts:
Line 82:
Missing async function documentation in `src/stores/chat/store.ts` and `src/api/chat/chat.ts`. `loadIncidentChannels` returns `Promise<void>` but lacks `@returns` or `@throws` JSDoc; the JSDoc block above documents the `incidentChannelsByCallId` field, not this function. Rule 22 requires async/Promise functions to document resolve value, rejection conditions, and await usage — the current implementation swallows errors internally, which callers cannot infer from the signature. Add a JSDoc comment with `@returns {Promise<void>}` and a note that failures are caught and logged.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
jest-setup.ts (1)
6-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
anyprops with typed component props.Lines 8 and 12 disable strict type checking for both mock components. Type
childrenwithPropsWithChildrenand use typed functional components.As per coding guidelines, “Use TypeScript strict mode; never use
any, and prefer precise types and interfaces.”🤖 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 `@jest-setup.ts` around lines 6 - 12, Replace the any-typed children props in the SafeAreaView mock and SafeAreaProvider mock with React PropsWithChildren types, and define both as typed functional components while preserving their existing rendering behavior.Source: Coding guidelines
src/components/chat/new-conversation-sheet.tsx (1)
107-108: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd recipient-type payload regression tests.
Add tests that verify a person recipient sends only
TargetUserIdand a unit recipient sends onlyTargetUnitId. This branch controls the direct-message API payload identity.As per coding guidelines, “generate tests for new components, services, and logic.”
🤖 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/components/chat/new-conversation-sheet.tsx` around lines 107 - 108, Add regression tests for the direct-message payload branch around createDirectMessage: verify person recipients send only TargetUserId, while unit recipients send only TargetUnitId with the parsed numeric identifier. Use the existing recipientUserId and isUnitRecipient behavior and assert the complete payload passed to createDirectMessage.Source: Coding guidelines
src/hooks/__tests__/use-signalr-lifecycle.test.tsx (1)
53-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the untyped SignalR store response.
Line 58 casts the selector response to
any. Use a typed SignalR store fixture so this test fails when the hook requires a missing store field.As per coding guidelines, “Use TypeScript strict mode; never use
any, and prefer precise types and interfaces.”🤖 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/hooks/__tests__/use-signalr-lifecycle.test.tsx` around lines 53 - 58, Replace the `as any` cast in the SignalR store fixture used by the selector mock with the concrete SignalR store type or a precisely typed fixture. Update the fixture to satisfy all required fields so `useSignalR` tests detect missing store properties while preserving the existing mock connection behavior.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/components/chat/new-conversation-sheet.tsx`:
- Around line 187-203: Replace the hardcoded `#6b7280` color on the Truck
component in the isUnit branch with the appropriate semantic theme color token,
preserving sufficient contrast in both light and dark system themes.
- Around line 66-72: Update the labels in the new-conversation sheet currently
using chat.search_people and chat.no_people so DM mode uses recipient-specific
localized text reflecting both people and units, while preserving the existing
people-only labels for group mode. Add the required translation keys to every
catalog and resolve them through the existing react-i18next t() helper.
---
Nitpick comments:
In `@jest-setup.ts`:
- Around line 6-12: Replace the any-typed children props in the SafeAreaView
mock and SafeAreaProvider mock with React PropsWithChildren types, and define
both as typed functional components while preserving their existing rendering
behavior.
In `@src/components/chat/new-conversation-sheet.tsx`:
- Around line 107-108: Add regression tests for the direct-message payload
branch around createDirectMessage: verify person recipients send only
TargetUserId, while unit recipients send only TargetUnitId with the parsed
numeric identifier. Use the existing recipientUserId and isUnitRecipient
behavior and assert the complete payload passed to createDirectMessage.
In `@src/hooks/__tests__/use-signalr-lifecycle.test.tsx`:
- Around line 53-58: Replace the `as any` cast in the SignalR store fixture used
by the selector mock with the concrete SignalR store type or a precisely typed
fixture. Update the fixture to satisfy all required fields so `useSignalR` tests
detect missing store properties while preserving the existing mock connection
behavior.
🪄 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: 69f36e69-c42b-42b1-ba91-d4db4ea3c53a
📒 Files selected for processing (18)
jest-setup.tsplugins/__tests__/with-app-icon-badge.test.tsscripts/__tests__/extract-release-notes.test.tssrc/components/chat/new-conversation-sheet.tsxsrc/hooks/__tests__/use-signalr-lifecycle.test.tsxsrc/lib/__tests__/navigation.test.tssrc/services/__tests__/push-notification.test.tssrc/stores/chat/__tests__/hub-invoke-args.test.tssrc/stores/chat/store.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
🚧 Files skipped from review as they are similar to previous changes (9)
- src/translations/uk.json
- src/translations/sv.json
- src/translations/pl.json
- src/translations/fr.json
- src/translations/en.json
- src/translations/ar.json
- src/translations/es.json
- src/translations/it.json
- src/translations/de.json
| // DM mode also offers units (IC can open a 1:1 with a unit); | ||
| // group membership only supports users, so group mode stays people-only. | ||
| const includeUnits = mode === 'dm'; | ||
| getRecipients(true, includeUnits) | ||
| .then((result) => { | ||
| if (cancelled) return; | ||
| setRecipients((result.Data ?? []).filter(isPersonRecipient)); | ||
| setRecipients((result.Data ?? []).filter((r) => isPersonRecipient(r) || (includeUnits && isUnitRecipient(r)))); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use recipient labels in direct-message mode.
Direct-message mode now includes units. The labels at Lines 162 and 175 still use chat.search_people and chat.no_people. Use localized recipient-specific labels for this mode and add the keys to each translation catalog.
As per coding guidelines, “Wrap all user-visible text in t() from react-i18next” and use localized UI text.
🤖 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/components/chat/new-conversation-sheet.tsx` around lines 66 - 72, Update
the labels in the new-conversation sheet currently using chat.search_people and
chat.no_people so DM mode uses recipient-specific localized text reflecting both
people and units, while preserving the existing people-only labels for group
mode. Add the required translation keys to every catalog and resolve them
through the existing react-i18next t() helper.
Source: Coding guidelines
| {isUnit ? ( | ||
| <Center className="size-8 rounded-full bg-secondary-200"> | ||
| <Truck size={16} color="#6b7280" /> | ||
| </Center> | ||
| ) : ( | ||
| <Avatar size="sm"> | ||
| <AvatarImage source={{ uri: getAvatarUrl(userId) }} /> | ||
| </Avatar> | ||
| )} | ||
| <Text className="flex-1 text-typography-900" numberOfLines={1}> | ||
| {recipient.Name} | ||
| </Text> | ||
| {isUnit ? ( | ||
| <Box className="rounded-full bg-secondary-200 px-2 py-0.5"> | ||
| <Text className="text-xs text-typography-600">{t('chat.unit')}</Text> | ||
| </Box> | ||
| ) : null} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the fixed truck icon color.
Line 189 uses #6b7280. This color does not adapt to the system color scheme. Use a semantic theme token that preserves contrast in light and dark modes.
As per coding guidelines, “Support light and dark modes through the system color scheme and use semantic Tailwind color tokens instead of hardcoded hex colors.”
🤖 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/components/chat/new-conversation-sheet.tsx` around lines 187 - 203,
Replace the hardcoded `#6b7280` color on the Truck component in the isUnit branch
with the appropriate semantic theme color token, preserving sufficient contrast
in both light and dark system themes.
Source: Coding guidelines
| const targetId = recipientUserId(recipient); | ||
| const response = await createDirectMessage(isUnitRecipient(recipient) ? { TargetUnitId: parseInt(targetId, 10) } : { TargetUserId: targetId }); |
There was a problem hiding this comment.
Unguarded parseInt(targetId, 10) passes silently truncated or NaN values to createDirectMessage, causing invalid requests or incorrect unit targeting. Validate the parsed payload with Number.isFinite before invoking createDirectMessage.
const targetId = recipientUserId(recipient);
const payload = isUnitRecipient(recipient)
? { TargetUnitId: parseInt(targetId, 10) }
: { TargetUserId: targetId };
if ('TargetUnitId' in payload && !Number.isFinite(payload.TargetUnitId)) {
useToastStore.getState().showToast('error', t('chat.create_conversation_failed'));
return;
}
const response = await createDirectMessage(payload);Prompt for LLM
File src/components/chat/new-conversation-sheet.tsx:
Line 107 to 108:
Unguarded parseInt(targetId, 10) passes silently truncated or NaN values to createDirectMessage, causing invalid requests or incorrect unit targeting. Validate the parsed payload with Number.isFinite before invoking createDirectMessage.
Suggested Code:
const targetId = recipientUserId(recipient);
const payload = isUnitRecipient(recipient)
? { TargetUnitId: parseInt(targetId, 10) }
: { TargetUserId: targetId };
if ('TargetUnitId' in payload && !Number.isFinite(payload.TargetUnitId)) {
useToastStore.getState().showToast('error', t('chat.create_conversation_failed'));
return;
}
const response = await createDirectMessage(payload);
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:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/stores/chat/__tests__/hub-invoke-args.test.ts`:
- Line 167: Update the no-resend assertion in the hub-invocation test to
explicitly match null arguments for the SetActiveChannel call, replacing the
expect.anything() placeholders while preserving the expectation that this
null-clear invocation is not made.
🪄 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: 8934344e-1200-4f5c-99a1-1bfd0d22e642
📒 Files selected for processing (2)
src/stores/chat/__tests__/hub-invoke-args.test.tssrc/stores/chat/store.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/stores/chat/store.ts
| useChatStore.getState().handleChatConnected(); | ||
| await flush(); | ||
|
|
||
| expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', expect.anything(), expect.anything()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
In Jest 29.7.0, does expect.anything()matchnullorundefined?
💡 Result:
In Jest 29.7.0, expect.anything does not match null or undefined [1][2]. According to official documentation, expect.anything is explicitly designed to match anything except for null or undefined [1][2].
Citations:
- 1: https://jestjs.io/docs/29.7/expect
- 2: https://github.com/jestjs/jest/blob/v29.7.0/docs/ExpectAPI.md
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- focused test ---'
sed -n '130,180p' src/stores/chat/__tests__/hub-invoke-args.test.ts
printf '%s\n' '--- relevant symbols ---'
rg -n -C 4 'handleChatConnected|SetActiveChannel|mockInvoke' src/stores/chatRepository: Resgrid/IC
Length of output: 13423
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- sync and reconnect implementation ---'
sed -n '230,260p' src/stores/chat/store.ts
sed -n '850,885p' src/stores/chat/store.ts
printf '%s\n' '--- test setup and imports ---'
sed -n '1,140p' src/stores/chat/__tests__/hub-invoke-args.test.tsRepository: Resgrid/IC
Length of output: 7954
🏁 Script executed:
#!/bin/bash
set -eu
node - <<'JS'
const fs = require('fs');
const testSource = fs.readFileSync('src/stores/chat/__tests__/hub-invoke-args.test.ts', 'utf8');
const storeSource = fs.readFileSync('src/stores/chat/store.ts', 'utf8');
// Jest's `expect.anything()` predicate: every value except null and undefined.
const anything = (value) => value !== null && value !== undefined;
for (const value of [null, undefined, 'channel-1', 0, false, {}]) {
console.log(`${String(value)} => ${anything(value)}`);
}
if (anything(null) || anything(undefined)) {
throw new Error('expect.anything() must not match null or undefined');
}
if (!testSource.includes(
"expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', expect.anything(), expect.anything());"
)) {
throw new Error('target negative assertion not found');
}
if (!storeSource.includes(
"await signalRService.invoke(Env.CHAT_HUB_NAME, 'SetActiveChannel', channelId, null);"
)) {
throw new Error('SetActiveChannel null-argument call shape not found');
}
console.log('The current negative matcher cannot detect the reachable SetActiveChannel(null, null) call.');
JSRepository: Resgrid/IC
Length of output: 339
🏁 Script executed:
#!/bin/bash
set -eu
node - <<'JS'
const fs = require('fs');
const testSource = fs.readFileSync('src/stores/chat/__tests__/hub-invoke-args.test.ts', 'utf8');
const storeSource = fs.readFileSync('src/stores/chat/store.ts', 'utf8');
const anything = (value) => value !== null && value !== undefined;
for (const value of [null, undefined, 'channel-1', 0, false, {}]) {
console.log(`${String(value)} => ${anything(value)}`);
}
if (anything(null) || anything(undefined)) {
throw new Error('expect.anything() must not match null or undefined');
}
if (!testSource.includes(
"expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', expect.anything(), expect.anything());"
)) {
throw new Error('target negative assertion not found');
}
if (!storeSource.includes(
"await signalRService.invoke(Env.CHAT_HUB_NAME, 'SetActiveChannel', channelId, null);"
)) {
throw new Error('SetActiveChannel null-argument call shape not found');
}
console.log('The current negative matcher cannot detect the reachable SetActiveChannel(null, null) call.');
JSRepository: Resgrid/IC
Length of output: 339
Match the null arguments in the no-resend assertion.
expect.anything() does not match null or undefined. The assertion therefore passes when handleChatConnected() invokes SetActiveChannel(null, null). Match the null-clear call explicitly.
Proposed fix
- expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', expect.anything(), expect.anything());
+ expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', null, null);📝 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.
| expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', expect.anything(), expect.anything()); | |
| expect(mockInvoke).not.toHaveBeenCalledWith('chatHub', 'SetActiveChannel', null, null); |
🤖 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__/hub-invoke-args.test.ts` at line 167, Update the
no-resend assertion in the hub-invocation test to explicitly match null
arguments for the SetActiveChannel call, replacing the expect.anything()
placeholders while preserving the expectation that this null-clear invocation is
not made.
| // Only clear if no newer marker superseded this one while in flight. | ||
| if (pendingActiveChannelSync === marker) pendingActiveChannelSync = null; | ||
| } catch (error) { | ||
| logger.debug({ message: 'chat: invoke SetActiveChannel skipped', context: { error } }); |
There was a problem hiding this comment.
Missing structured log fields violate Rule [3], as the operation name and channelId are embedded only in the message string, preventing queryable logs. Emit them as structured fields, e.g., logger.debug({ message: 'chat: invoke skipped', context: { op: 'SetActiveChannel', channelId, error } });.
Kody rule violation: Include error context in structured logs
Prompt for LLM
File src/stores/chat/store.ts:
Line 249:
Missing structured log fields violate Rule [3], as the operation name and `channelId` are embedded only in the message string, preventing queryable logs. Emit them as structured fields, e.g., `logger.debug({ message: 'chat: invoke skipped', context: { op: 'SetActiveChannel', channelId, error } });`.
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 introduces several major enhancements to the IC (Incident Commander) app:
Command App Authorization
Adds a
CanLoginToCommandApppermission check during app initialization. Users not authorized by their department to act as commanders are now signed out immediately with a clear error message, rather than loading an app where every request would fail.Chat Integration on the Command Board
Brings chat directly into the command board workflow:
useDirectMessagehook)IncidentLeadsandIncidentDispatchchannel typesResponsive Command Board Layout
Introduces a
useCommandBoardLayouthook that adapts the board's controls based on screen size — compact icon-only buttons on phones in portrait, full-size labeled buttons on tablets, landscape phones, and desktop windows. The landscape structure board lanes now have fixed heights with self-contained scrolling, preventing busy lanes from stretching the page indefinitely.Critical Bug Fix: Invisible Modal Overlay
Fixes a bug where an interrupted close animation on bottom sheets and the side drawer left a transparent, full-screen Modal mounted over the app, swallowing all touch input and making screens appear frozen until a route change. The modal now unmounts regardless of animation completion status, guarded against stale state.
Icon Rendering Fix
Routes all lucide icons through the
Iconwrapper component instead of using bare elements. Bare lucide icons don't receive className styling (no cssInterop registered), causing them to render solid black and disappear against dark headers.Chat Screen Header Consolidation
Chat and chatbot screens now use the standard app header (with back button and notifications) instead of custom in-screen toolbars. The chat detail screen also handles back navigation for deep-link and notification entry points where there's no navigation history to pop.
Summary by CodeRabbit
New Features
Bug Fixes
Localization