Conversation
CU-868ex18rd Fix for splash screen
CU-868ex18rd Splash screen changes.
CU-868ex18rd Working on geolocation issue minor fixes.
CU-868ex18rd And16k SignalR update.
CU-868ex18rd Expo 53 update.
CU-868ex18rd Fixing internal test builds
CU-868ex18rd Fix for ad hoc build
CU-868ex18rd Fixing the call images view.
CU-868ex18rd Fixing missing analytic key env.
CU-868ex18rd Fixing map issue.
CU-868ex18rd Migrating to countly
CU-868frkzum Fixed issue with broken onboarding screens.
RR-T39 Added skip button to onboarding page top and scroll.
RR-T39 Fixing the ci build
RR-T40 Logout fix, close call fix, units state fix, contacts fix, pro…
📝 WalkthroughWalkthroughMultiple enhancements across image handling, shift day filtering, and status logic. Cross-platform image picker/upload refactoring with permission handling and base64 encoding. Shift components now filter upcoming days, apply layout constraints, and use translations. Personnel status store filters groups by type. Minor UI and selector improvements. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Image Selection UI
participant Permissions as Permissions Handler
participant Picker as ImagePicker
participant Manipulator as ImageManipulator
participant FileSystem as FileSystem
participant API as Upload API
User->>UI: Select/Capture Image
UI->>Permissions: Check platform & request
alt Web
Permissions-->>UI: Skip permissions
else Native
Permissions->>Permissions: Request camera/library access
alt Permission Granted
Permissions-->>UI: Proceed
else Permission Denied
Permissions-->>UI: Show settings/error toast
end
end
UI->>Picker: Launch image picker/camera
Picker-->>UI: Return image asset with URI
UI->>UI: Validate asset.uri
alt URI Valid
UI->>Manipulator: Manipulate & encode to base64
Manipulator-->>UI: Return base64
alt No base64 returned
UI->>FileSystem: Read from manipulated URI
FileSystem-->>UI: Return base64 via FileSystem
end
UI->>API: Upload base64 + metadata
API-->>User: Success confirmation
else URI Invalid
UI-->>User: Show error toast & validation msg
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 review |
✅ Actions performedReview triggered.
|
![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/shifts/shift-details-sheet.tsx`:
- Around line 260-280: Precompute the upcomingDays array by filtering
selectedShift.Days (using the existing parseISO/startOfDay logic and same
try/catch) into a const upcomingDays and then use a ternary to render the whole
"Upcoming Days" VStack only when upcomingDays.length > 0; replace the inline
filter+slice+map with upcomingDays.slice(0,7).map(d => <ShiftDayCard
key={d.ShiftDayId} shiftDay={d} onPress={() => selectShiftDay(d)} />) so the
header and list are omitted when the filtered list is empty.
In `@src/components/units/unit-card.tsx`:
- Around line 92-97: statusText currently only uses customStatusData?.Text or
unit.CurrentStatus and misses falling back to defaultStatus.text (and uses ||
which lets empty strings through); update the statusText calculation to:
prioritize customStatusData?.Text, then unit.CurrentStatus, then
defaultStatus?.text, and use the nullish coalescing operator (??) instead of ||
so empty strings from the API don't bypass the default. Keep the related logic
for buttonColor unchanged except ensure consistent use of ?? when reading
customStatusData?.BColor and unit.CurrentStatusColor to mirror the defensive
behavior used for statusText; reference variables/functions: statusText,
buttonColor, defaultStatus, getDefaultStatus, statusId, customStatusData, unit.
🧹 Nitpick comments (5)
src/stores/status/personnel-status-store.ts (1)
113-115: LGTM! The filtering logic is correct.The change properly filters groups to only include station groups (TypeId === 1), with appropriate null handling via the
|| []fallback.Consider extracting the magic number
1into a named constant (e.g.,GROUP_TYPE_STATION = 1) for improved readability and maintainability, especially if this type ID is used elsewhere in the codebase.src/components/audio-stream/audio-stream-bottom-sheet.tsx (1)
141-145: Placeholder prop is now redundant.Since
valueis always provided (eithercurrentStream.Nameor the translated "none" string), theplaceholderprop will never be displayed. Consider removing it to avoid confusion.Suggested change
<Select selectedValue={getCurrentStreamValue()} onValueChange={handleStreamSelection} isDisabled={isLoading || isBuffering}> <SelectTrigger> - <SelectInput placeholder={t('audio_streams.select_placeholder')} value={currentStream ? currentStream.Name : t('audio_streams.none')} /> + <SelectInput value={currentStream ? currentStream.Name : t('audio_streams.none')} /> <SelectIcon /> </SelectTrigger>src/components/shifts/__tests__/shift-details-sheet.test.tsx (2)
51-78: Mockedformatdate output is inconsistent with test data.The
formatmock returns'Jan 15, 2024'and'2024-01-15'while the test data uses 2027 dates. Although this works because mocks are static, it reduces test readability and could cause confusion when debugging.Consider updating the mock to return dates consistent with the test data:
🔧 Suggested improvement
jest.mock('date-fns', () => ({ format: jest.fn((date, formatStr) => { if (formatStr === 'MMM dd, yyyy') { - return 'Jan 15, 2024'; + return 'Jan 15, 2027'; } if (formatStr === 'yyyy-MM-dd') { - return '2024-01-15'; + return '2027-01-15'; } return date; }),
916-917: Remove empty duplicatedescribeblock.This empty
describe('Interaction', ...)block is redundant since there's already a populateddescribe('Interaction', ...)block at lines 659-711. Remove this dead code.🧹 Proposed fix
- describe('Interaction', () => { - }); - describe('Calendar Tab', () => {src/components/calls/call-images-modal.tsx (1)
280-293: Handleblob:URIs in the web fallback for future compatibility.The project currently uses expo-image-picker ~16.1.4, which returns
data:URIs on web. However, expo-image-picker 17.0.0 and later (released Aug 13, 2025) returnsblob:URLs instead as a breaking change. When upgrading, the fallback catch block (lines 284–293) will fail to extract base64 from blob URLs, causing uploads to fail. Even though the code comment acknowledges blob URL possibility (line 282), the implementation doesn't handle it. Add blob→base64 conversion to future-proof the error handler:Suggested blob URL fallback
} catch (webError) { console.error('Web image manipulation error:', webError); // On web, the URI might already be a data URI or blob URL // Try to extract base64 from data URI if (selectedImageInfo.uri.startsWith('data:')) { const base64Match = selectedImageInfo.uri.match(/base64,(.*)$/); if (base64Match && base64Match[1]) { base64Image = base64Match[1]; } else { throw new Error('Could not extract base64 from data URI'); } + } else if (selectedImageInfo.uri.startsWith('blob:')) { + const response = await fetch(selectedImageInfo.uri); + const blob = await response.blob(); + const dataUrl = await new Promise<string>((resolve, reject) => { + const reader = new FileReader(); + reader.onloadend = () => { + if (typeof reader.result === 'string') { + resolve(reader.result); + } else { + reject(new Error('Unexpected FileReader result')); + } + }; + reader.onerror = () => reject(reader.error ?? new Error('FileReader error')); + reader.readAsDataURL(blob); + }); + const base64Match = dataUrl.match(/base64,(.*)$/); + if (base64Match && base64Match[1]) { + base64Image = base64Match[1]; + } else { + throw new Error('Could not extract base64 from blob data URL'); + } } else { throw webError; } }
| {/* Upcoming Days */} | ||
| {selectedShift?.Days && selectedShift.Days.length > 0 && ( | ||
| <VStack space="md"> | ||
| <Text size="lg" className="font-semibold text-gray-900 dark:text-white"> | ||
| Recent Shift Days | ||
| {t('shifts.upcoming_shift_days')} | ||
| </Text> | ||
| <VStack space="sm"> | ||
| {selectedShift.Days.slice(0, 7).map((day) => ( | ||
| <ShiftDayCard key={day.ShiftDayId} shiftDay={day} onPress={() => selectShiftDay(day)} /> | ||
| ))} | ||
| {selectedShift.Days.filter((day) => { | ||
| if (!day.ShiftDay) return false; | ||
| try { | ||
| const shiftDate = startOfDay(parseISO(day.ShiftDay)); | ||
| const today = startOfDay(new Date()); | ||
| return shiftDate >= today; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }) | ||
| .slice(0, 7) | ||
| .map((day) => ( | ||
| <ShiftDayCard key={day.ShiftDayId} shiftDay={day} onPress={() => selectShiftDay(day)} /> | ||
| ))} |
There was a problem hiding this comment.
Avoid rendering the “Upcoming Shift Days” section when the filtered list is empty.
If all days are filtered out (e.g., only past dates), the header still shows with no items. Precompute the filtered list once and guard the section. As per coding guidelines, prefer ternary rendering.
🔧 Suggested fix
- {/* Upcoming Days */}
- {selectedShift?.Days && selectedShift.Days.length > 0 && (
- <VStack space="md">
- <Text size="lg" className="font-semibold text-gray-900 dark:text-white">
- {t('shifts.upcoming_shift_days')}
- </Text>
- <VStack space="sm">
- {selectedShift.Days.filter((day) => {
- if (!day.ShiftDay) return false;
- try {
- const shiftDate = startOfDay(parseISO(day.ShiftDay));
- const today = startOfDay(new Date());
- return shiftDate >= today;
- } catch {
- return false;
- }
- })
- .slice(0, 7)
- .map((day) => (
- <ShiftDayCard key={day.ShiftDayId} shiftDay={day} onPress={() => selectShiftDay(day)} />
- ))}
- </VStack>
- </VStack>
- )}
+ const today = startOfDay(new Date());
+ const upcomingDays =
+ selectedShift?.Days?.filter((day) => {
+ if (!day.ShiftDay) return false;
+ try {
+ const shiftDate = startOfDay(parseISO(day.ShiftDay));
+ return shiftDate >= today;
+ } catch {
+ return false;
+ }
+ }).slice(0, 7) ?? [];
+
+ {/* Upcoming Days */}
+ {upcomingDays.length > 0 ? (
+ <VStack space="md">
+ <Text size="lg" className="font-semibold text-gray-900 dark:text-white">
+ {t('shifts.upcoming_shift_days')}
+ </Text>
+ <VStack space="sm">
+ {upcomingDays.map((day) => (
+ <ShiftDayCard key={day.ShiftDayId} shiftDay={day} onPress={() => selectShiftDay(day)} />
+ ))}
+ </VStack>
+ </VStack>
+ ) : null}🤖 Prompt for AI Agents
In `@src/components/shifts/shift-details-sheet.tsx` around lines 260 - 280,
Precompute the upcomingDays array by filtering selectedShift.Days (using the
existing parseISO/startOfDay logic and same try/catch) into a const upcomingDays
and then use a ternary to render the whole "Upcoming Days" VStack only when
upcomingDays.length > 0; replace the inline filter+slice+map with
upcomingDays.slice(0,7).map(d => <ShiftDayCard key={d.ShiftDayId} shiftDay={d}
onPress={() => selectShiftDay(d)} />) so the header and list are omitted when
the filtered list is empty.
| // Fall back to default status if custom status not found (used for colors only) | ||
| const defaultStatus = statusId && !customStatusData ? getDefaultStatus(statusId) : null; | ||
|
|
||
| // Get status text and colors - prioritize custom, then default, then API response | ||
| const statusText = customStatusData?.Text || (defaultStatus?.text ? t(defaultStatus.text) : null) || ('CurrentStatus' in unit ? unit.CurrentStatus : null); | ||
| const buttonColor = customStatusData?.BColor || defaultStatus?.color || ('CurrentStatusColor' in unit ? unit.CurrentStatusColor : null); | ||
| // Get status text - prioritize custom status, then API response (CurrentStatus from unit) | ||
| const statusText = customStatusData?.Text || ('CurrentStatus' in unit ? unit.CurrentStatus : null); | ||
| const buttonColor = customStatusData?.BColor || ('CurrentStatusColor' in unit ? unit.CurrentStatusColor : null) || defaultStatus?.color; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/units/unit-card.tsxRepository: Resgrid/Responder
Length of output: 8166
🏁 Script executed:
find src -type f -name "*.ts" | xargs grep -l "StatusesResultData\|UnitResultData\|UnitInfoResultData" | head -5Repository: Resgrid/Responder
Length of output: 276
🏁 Script executed:
cat -n src/models/v4/statuses/statusesResultData.tsRepository: Resgrid/Responder
Length of output: 565
🏁 Script executed:
cat -n src/models/v4/units/unitResultData.tsRepository: Resgrid/Responder
Length of output: 855
🏁 Script executed:
cat -n src/models/v4/units/unitInfoResultData.tsRepository: Resgrid/Responder
Length of output: 1119
Guard against missing status text when CurrentStatus is unavailable.
The status badge will not render if the API omits CurrentStatus and no custom status is set for the unit type, even though a valid default status exists for the statusId. Additionally, statusText lacks the fallback to defaultStatus.text that buttonColor has, creating an asymmetry. Using ?? instead of || is also safer to avoid empty-string pass-through from the API.
Suggested fix
- const statusText = customStatusData?.Text || ('CurrentStatus' in unit ? unit.CurrentStatus : null);
+ const statusText =
+ customStatusData?.Text ??
+ ('CurrentStatus' in unit ? unit.CurrentStatus : null) ??
+ (defaultStatus ? t(defaultStatus.text) : null);🤖 Prompt for AI Agents
In `@src/components/units/unit-card.tsx` around lines 92 - 97, statusText
currently only uses customStatusData?.Text or unit.CurrentStatus and misses
falling back to defaultStatus.text (and uses || which lets empty strings
through); update the statusText calculation to: prioritize
customStatusData?.Text, then unit.CurrentStatus, then defaultStatus?.text, and
use the nullish coalescing operator (??) instead of || so empty strings from the
API don't bypass the default. Keep the related logic for buttonColor unchanged
except ensure consistent use of ?? when reading customStatusData?.BColor and
unit.CurrentStatusColor to mirror the defensive behavior used for statusText;
reference variables/functions: statusText, buttonColor, defaultStatus,
getDefaultStatus, statusId, customStatusData, unit.
Merge pull request #94 from Resgrid/master
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.