Conversation
📝 WalkthroughWalkthroughThis PR adds avatar rendering utilities and avatar-based UI (with initials fallback), introduces safe timestamp formatting, configures expo-image-picker permissions, adds Android microphone permission checks before LiveKit connection/foreground service start, fixes a shift translation key, and expands utils tests/mocks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI Component
participant Permissions as Android Permissions
participant LiveKit as LiveKit Client
participant FG as Foreground Service
rect rgba(220,240,255,0.5)
UI->>Permissions: Check microphone permission (android only)
alt permission not granted
Permissions->>Permissions: requestRecordingPermissionsAsync()
end
Permissions-->>UI: permission result (granted/denied)
end
alt granted
UI->>LiveKit: disconnect existing room (if any)
UI->>LiveKit: connectToRoom()
UI->>FG: start foreground service
else denied
UI-->>UI: throw / log error (permission denied)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
🤖 Fix all issues with AI agents
In `@src/components/personnel/personnel-card.tsx`:
- Around line 45-52: The PersonnelCard component retains imageError across prop
changes causing stale fallbacks; add a useEffect in PersonnelCard that watches
the personnel identity (e.g., personnel.UserId or the personnel object) and
calls setImageError(false) whenever it changes so a new avatar load is attempted
for each new person; update the effect to run on mount and on personnel changes
to reset the state used by avatarUrl/initials/fallbackColor.
- Line 148: This component directly calls parseDateISOString and
formatDateForDisplay on personnel.StatusTimestamp which can throw on malformed
input; replace that inline parsing with the existing safeFormatTimestamp
behavior (or extract/reuse the shared safeFormatTimestamp utility used in
PersonnelDetailsSheet) so the formatting is done inside a try/catch and returns
a safe string/null; update the render to use
safeFormatTimestamp(personnel.StatusTimestamp) (or the newly extracted helper)
instead of parseDateISOString/formatDateForDisplay to prevent unhandled
exceptions.
🧹 Nitpick comments (3)
src/components/personnel/personnel-details-sheet.tsx (1)
22-43: Extract shared helper functions to a utility module.
getColorFromStringandgetInitialsare duplicated in bothpersonnel-details-sheet.tsxandpersonnel-card.tsx. Consider extracting these to@/lib/utils.tsalongsidegetAvatarUrlto maintain DRY principles and ensure consistent behavior across components.♻️ Suggested refactor
Add to
src/lib/utils.ts:/** * Generates a deterministic color from a string (user ID or name) * Returns an HSL color string */ export function getColorFromString(str: string): string { let hash = 0; for (let i = 0; i < str.length; i++) { hash = str.charCodeAt(i) + ((hash << 5) - hash); } const hue = Math.abs(hash % 360); return `hsl(${hue}, 65%, 45%)`; } /** * Gets initials from first and last name */ export function getInitials(firstName?: string, lastName?: string): string { const first = firstName?.trim()?.[0]?.toUpperCase() || ''; const last = lastName?.trim()?.[0]?.toUpperCase() || ''; return first + last || '?'; }Then import from both components:
import { getAvatarUrl, getColorFromString, getInitials } from '@/lib/utils';src/components/personnel/personnel-card.tsx (2)
16-38: Duplicate helper functions - see personnel-details-sheet.tsx comment.These helper functions should be extracted to
@/lib/utils.tsas noted in the other file's review.
103-114: Redundant GroupName check.Line 103 already verifies
personnel.GroupNameis truthy, making the nested check on line 105 redundant.♻️ Proposed simplification
- ) : personnel.GroupName ? ( - <VStack space="xs"> - {personnel.GroupName ? ( - <HStack space="xs" className="items-center"> - <Users size={16} className="text-gray-600 dark:text-gray-400" /> - <Text className="text-sm text-gray-600 dark:text-gray-300" numberOfLines={1}> - {personnel.GroupName} - </Text> - </HStack> - ) : null} - </VStack> + ) : personnel.GroupName ? ( + <VStack space="xs"> + <HStack space="xs" className="items-center"> + <Users size={16} className="text-gray-600 dark:text-gray-400" /> + <Text className="text-sm text-gray-600 dark:text-gray-300" numberOfLines={1}> + {personnel.GroupName} + </Text> + </HStack> + </VStack> ) : null}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lib/__tests__/utils.test.ts`:
- Line 1: Add unit tests covering safeFormatTimestamp in
src/lib/__tests__/utils.test.ts: create cases for (1) valid timestamp inputs
with different format strings to assert expected outputs, (2) null and undefined
inputs to assert they return empty string or expected fallback, and (3)
invalid/non-parsable timestamp inputs to assert the function handles errors
gracefully (e.g., returns empty string or a specific error value). Use the
existing test harness and imports (safeFormatTimestamp) and mirror patterns used
for other helpers (formatLocalDateString, getTodayLocalString) to keep tests
consistent and isolated.
🧹 Nitpick comments (5)
src/components/personnel/personnel-details-sheet.tsx (2)
22-24: Remove extra blank lines.Static analysis indicates unnecessary blank lines here. As per the pipeline failure hints.
♻️ Proposed fix
import { VStack } from '../ui/vstack'; - - - export const PersonnelDetailsSheet: React.FC = () => {
100-110: Simplify JSX formatting for avatar components.Static analysis suggests condensing the multi-line JSX to single lines for cleaner formatting.
♻️ Proposed fix
<Avatar size="lg" style={imageError ? { backgroundColor: fallbackColor } : undefined}> - {!imageError && ( - <AvatarImage - source={{ uri: avatarUrl }} - onError={() => setImageError(true)} - /> - )} - {imageError && ( - <AvatarFallbackText className="text-white">{initials}</AvatarFallbackText> - )} + {!imageError ? ( + <AvatarImage source={{ uri: avatarUrl }} onError={() => setImageError(true)} /> + ) : ( + <AvatarFallbackText className="text-white">{initials}</AvatarFallbackText> + )} </Avatar>This also aligns with the coding guideline to use ternary operator
? :for conditional rendering instead of&&.src/components/personnel/personnel-card.tsx (3)
16-17: Remove extra blank lines.Static analysis indicates unnecessary blank lines here.
♻️ Proposed fix
import { VStack } from '../ui/vstack'; - - interface PersonnelCardProps {
41-51: Simplify JSX formatting and use ternary for conditional rendering.Static analysis suggests condensing multi-line JSX. Additionally, per coding guidelines, use ternary operator
? :instead of&&for conditional rendering.♻️ Proposed fix
<Avatar size="md" style={imageError ? { backgroundColor: fallbackColor } : undefined}> - {!imageError && ( - <AvatarImage - source={{ uri: avatarUrl }} - onError={() => setImageError(true)} - /> - )} - {imageError && ( - <AvatarFallbackText className="text-white">{initials}</AvatarFallbackText> - )} + {!imageError ? ( + <AvatarImage source={{ uri: avatarUrl }} onError={() => setImageError(true)} /> + ) : ( + <AvatarFallbackText className="text-white">{initials}</AvatarFallbackText> + )} </Avatar>
5-5: Remove unused importsformatDateForDisplayandparseDateISOStringfrom the import statement on line 5.These functions are imported but not used anywhere in the component. The component uses
safeFormatTimestampfor timestamp formatting instead.♻️ Proposed fix
-import { formatDateForDisplay, getAvatarUrl, getColorFromString, getInitials, parseDateISOString, safeFormatTimestamp } from '@/lib/utils'; +import { getAvatarUrl, getColorFromString, getInitials, safeFormatTimestamp } from '@/lib/utils';
| @@ -1,4 +1,4 @@ | |||
| import { formatLocalDateString, isSameDate, isToday, getTodayLocalString } from '../utils'; | |||
| import { formatLocalDateString, isSameDate, isToday, getTodayLocalString, safeFormatTimestamp, getColorFromString, getInitials } from '../utils'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for safeFormatTimestamp tests in the test file
rg -n "safeFormatTimestamp" src/lib/__tests__/utils.test.ts -C 5Repository: Resgrid/Responder
Length of output: 440
🏁 Script executed:
#!/bin/bash
# Also check the entire test file to see all test cases
wc -l src/lib/__tests__/utils.test.ts && echo "---" && cat -n src/lib/__tests__/utils.test.ts | tail -100Repository: Resgrid/Responder
Length of output: 4075
🏁 Script executed:
#!/bin/bash
# Search for the utils implementation to understand what safeFormatTimestamp does
cat -n src/lib/utils.ts | grep -A 20 "safeFormatTimestamp"Repository: Resgrid/Responder
Length of output: 877
Add test cases for safeFormatTimestamp.
The function is imported but has no test coverage. Add tests for various scenarios: valid timestamps with different formats, null/undefined inputs, and error handling for invalid timestamps.
🤖 Prompt for AI Agents
In `@src/lib/__tests__/utils.test.ts` at line 1, Add unit tests covering
safeFormatTimestamp in src/lib/__tests__/utils.test.ts: create cases for (1)
valid timestamp inputs with different format strings to assert expected outputs,
(2) null and undefined inputs to assert they return empty string or expected
fallback, and (3) invalid/non-parsable timestamp inputs to assert the function
handles errors gracefully (e.g., returns empty string or a specific error
value). Use the existing test harness and imports (safeFormatTimestamp) and
mirror patterns used for other helpers (formatLocalDateString,
getTodayLocalString) to keep tests consistent and isolated.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.