RU-T46 Fixing issue with call creation map, perf fix for action. Live… #198
Conversation
📝 WalkthroughWalkthroughAdds Mapbox-backed location pickers with timeouted geolocation and reverse-geocoding, platform CallKeep wiring (Android no-op + iOS usage), cached GetActiveCalls endpoint with TTL, Bluetooth audio adjustments, expanded Jest mocks for LiveKit/CallKeep, multiple test refactors, store timestamping, and new translation keys. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Picker as LocationPicker
participant Geo as GeolocationService
participant Mapbox as Mapbox
participant Geocode as ReverseGeocode
User->>Picker: Open picker
Picker->>Geo: request current position (with timeout)
alt position returned
Geo-->>Picker: coordinates
Picker->>Mapbox: center map (zoom = 15)
else timeout/error
Geo-->>Picker: timeout/error
Picker->>Picker: use DEFAULT_LOCATION
Picker->>Mapbox: center map (zoom = 4)
end
User->>Mapbox: tap map / press My Location
Mapbox-->>Picker: new coordinates
Picker->>Geocode: reverse geocode(coordinates)
Geocode-->>Picker: address
Picker-->>User: onLocationSelected(currentLocation, address)
sequenceDiagram
participant App
participant LiveKit as LiveKitRoom
participant Platform as OS
participant CallKeep as CallKeepService
App->>LiveKit: connectToRoom()
LiveKit-->>App: connected
alt iOS
App->>CallKeep: startCall()
CallKeep-->>App: success (logged)
else Android
App->>App: try register foreground service (non-fatal)
end
User->>App: disconnect
App->>LiveKit: disconnect()
alt iOS
App->>CallKeep: endCall()
end
App-->>App: set isTalking = false
sequenceDiagram
participant StatusStore
participant CallsStore
participant API as BackendAPI
StatusStore->>CallsStore: read calls + lastFetchedAt
alt cache empty or stale
StatusStore->>API: fetch calls
API-->>StatusStore: calls
StatusStore->>CallsStore: set calls + lastFetchedAt = now
else cache fresh
StatusStore->>CallsStore: reuse cached calls
end
StatusStore->>API: fetch groups (always)
API-->>StatusStore: groups
StatusStore-->>Client: availableCalls + availableStations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/stores/app/livekit-store.ts (1)
275-310: Interface signature mismatch fordisconnectFromRoom.The
disconnectFromRoommethod is nowasync(line 275) with multipleawaitcalls, but the interface at line 90 declares it asdisconnectFromRoom: () => void. This mismatch means:
- Callers won't know the function returns a Promise
- Errors from the async operations won't propagate properly if callers don't await
🔧 Suggested fix
Update the interface signature at line 90:
- disconnectFromRoom: () => void; + disconnectFromRoom: () => Promise<void>;src/components/maps/full-screen-location-picker.tsx (1)
97-133: Clear the location timeout to prevent unhandled rejection.When
getCurrentPositionAsyncresolves before the timeout expires, thePromise.racesettles immediately but the timeout'ssetTimeoutcontinues running. After the timeout fires, it rejects the unused promise with no handler, causing an unhandled rejection that can crash the app. Store the timeout ID and clear it in thefinallyblock.Suggested fix
- const timeoutPromise = new Promise<never>((_, reject) => { - setTimeout(() => reject(new Error('Location timeout')), LOCATION_TIMEOUT); - }); + let timeoutId: ReturnType<typeof setTimeout> | undefined; + const timeoutPromise = new Promise<never>((_, reject) => { + timeoutId = setTimeout(() => reject(new Error('Location timeout')), LOCATION_TIMEOUT); + }); ... - } finally { - if (isMountedRef.current) setIsLocating(false); - } + } finally { + if (timeoutId) clearTimeout(timeoutId); + if (isMountedRef.current) setIsLocating(false); + }
🤖 Fix all issues with AI agents
In `@src/api/calls/calls.ts`:
- Around line 9-12: The calls API caches GetActiveCalls for 30s but mutations
don't clear it; after successful mutations in the functions createCall,
updateCall, and closeCall call cacheManager.remove('/Calls/GetActiveCalls')
(same pattern used in src/api/contacts/contacts.ts) to invalidate the cached
callsApi response so new/updated/closed calls reflect immediately; ensure the
cache removal runs only after the mutation completes successfully and handle any
errors from CacheManager.remove gracefully.
In `@src/components/calls/__tests__/call-files-modal.test.tsx`:
- Around line 40-62: The mock store's callFiles currently uses an unsafe cast
("as any"); change its type to CallFileResultData[] for precision by declaring
callFiles: defaultMockFiles as CallFileResultData[] (or initialize with a
shallow clone like [...defaultMockFiles] to avoid shared mutation), and update
the resetMockStoreState assignment to reset mockStoreState.callFiles =
defaultMockFiles as CallFileResultData[] (or a clone) so the types align; keep
the helpers setMockStoreState, mockFetchCallFiles, and mockClearFiles unchanged.
In `@src/components/maps/full-screen-location-picker.tsx`:
- Around line 177-178: The Mapbox.PointAnnotation component is using hard-coded
user-facing text in the title prop ("Selected Location"); replace that with a
localized string by wrapping it in the translation function (t) and using an
appropriate key (e.g., t('maps.selectedLocation')) so the title prop becomes
localized; update the PointAnnotation usage (Mapbox.PointAnnotation
id="selectedLocation" title=...) accordingly and ensure the t function is
imported/available in the component.
- Around line 12-24: The Mapbox access token is being set unconditionally via
Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); add a guard that checks
Env.UNIT_MAPBOX_PUBKEY for falsy/empty values before calling
Mapbox.setAccessToken, and if missing log a clear error (e.g., using
console.error or an existing logger) indicating the token is not configured and
avoid calling Mapbox.setAccessToken to prevent silent failures; include the
symbol Env.UNIT_MAPBOX_PUBKEY and Mapbox.setAccessToken in the check and
message, and in development consider throwing or failing fast so the missing
token is noticed.
In `@src/services/__tests__/app-reset.service.test.ts`:
- Around line 466-468: The test incorrectly assumes clearAllAppData swallows
errors; update the test to either (A) mock/stub the cleanup helpers called by
resetAllStores so they do not throw (e.g., stub the LiveKit disconnect and audio
stream cleanup helpers) and then assert await
expect(clearAllAppData()).resolves.toBeUndefined(), or (B) if you want to verify
propagation, change the assertion to await
expect(clearAllAppData()).rejects.toThrow(...) and assert the specific error;
locate usages of clearAllAppData and resetAllStores in the test and update the
mocks/assertion accordingly.
In `@src/stores/app/livekit-store.ts`:
- Line 11: The import in livekit-store.ts uses a platform-specific suffix
("callkeep.service.ios") which breaks Android bundling; either change the import
to import { callKeepService } from '../../services/callkeep.service' so Metro
can resolve platform-specific files automatically, or add a
callkeep.service.android.ts that exports the same named symbol (callKeepService)
as a no-op shim; update the import in livekit-store.ts to the base module name
and keep existing Platform.OS checks at the call sites (callKeepService usage
around lines referenced 244 and 282) unchanged.
In `@src/stores/status/store.ts`:
- Around line 80-109: The current double-caching causes calls to never refresh
if useCallsStore.calls is non-empty; add a store-level timestamp and staleness
check: extend useCallsStore data to include lastFetchedAt, update lastFetchedAt
when you set calls (useCallsStore.setState and where availableCalls is set), and
change the fetch condition in this function to treat calls as stale when
Date.now() - lastFetchedAt > STORE_TTL_MS (or when existingCalls.length===0) so
you call getCalls() again; ensure getCalls() is awaited and
useCallsStore.setState({ calls: callsResponse.Data || [], lastFetchedAt:
Date.now() }) and then set availableCalls from the fresh response.
🧹 Nitpick comments (7)
src/components/calls/__tests__/close-call-bottom-sheet.test.tsx (1)
124-178: Avoid the unused select callback registry.
selectCallbacksis written but never read or exported here, so it adds global mutable state without a consumer. If there’s no external use, consider keeping this mock stateless by removing the registry/effect.♻️ Suggested simplification
jest.mock('@/components/ui/select', () => { - // Store the callback for each select - const selectCallbacks: Record<string, (value: string) => void> = {}; - return { Select: ({ children, testID, selectedValue, onValueChange, ...props }: any) => { - const React = require('react'); - const { View, TouchableOpacity, Text } = require('react-native'); - - // Store the callback for external access - React.useEffect(() => { - if (testID && onValueChange) { - selectCallbacks[testID] = onValueChange; - } - return () => { - if (testID) { - delete selectCallbacks[testID]; - } - }; - }, [testID, onValueChange]); + const { View } = require('react-native'); return ( <View testID={testID} onValueChange={onValueChange} {...props} > {children} </View> ); },src/components/livekit/__tests__/livekit-bottom-sheet.test.tsx (1)
440-453: Test doesn't verify component behavior.This test directly calls the mocked audio service methods rather than testing that the component correctly invokes them during user interactions. It only proves the mock works, not that the component integrates correctly.
💡 Consider improving test to verify component behavior
- it('should call audio service methods', async () => { - const { audioService } = require('@/services/audio.service'); - - // Clear any previous calls - audioService.playConnectionSound.mockClear(); - audioService.playDisconnectionSound.mockClear(); - - // Test that the audio service methods are called - this confirms the implementation - await audioService.playConnectionSound(); - expect(audioService.playConnectionSound).toHaveBeenCalledTimes(1); - - await audioService.playDisconnectionSound(); - expect(audioService.playDisconnectionSound).toHaveBeenCalledTimes(1); - }); + it('should call audio service when connecting to room', async () => { + const connectToRoom = jest.fn(); + mockUseLiveKitStore.mockReturnValue({ + ...defaultLiveKitState, + isBottomSheetVisible: true, + availableRooms: mockAvailableRooms, + connectToRoom, + }); + + const { getByText } = render(<LiveKitBottomSheet />); + // Simulate user action that triggers connection + // Then verify audio service was called as a result + });src/components/maps/location-picker.tsx (1)
58-69: Minor: Timeout is not cleared if location resolves first.The
setTimeoutin the timeout promise is never cleared, meaning it will still fire (and reject) even after the location successfully resolves. While theisMountedRefcheck prevents state updates, this is a minor resource leak.💡 Optional: Clean up timeout on resolution
+ let timeoutId: NodeJS.Timeout; const timeoutPromise = new Promise<never>((_, reject) => { - setTimeout(() => reject(new Error('Location timeout')), LOCATION_TIMEOUT); + timeoutId = setTimeout(() => reject(new Error('Location timeout')), LOCATION_TIMEOUT); }); - const location = await Promise.race([ - Location.getCurrentPositionAsync({ - accuracy: Location.Accuracy.Balanced, - }), - timeoutPromise, - ]); + try { + const location = await Promise.race([ + Location.getCurrentPositionAsync({ + accuracy: Location.Accuracy.Balanced, + }), + timeoutPromise, + ]); + clearTimeout(timeoutId); + // ... rest of success handling + } catch (error) { + clearTimeout(timeoutId); + throw error; + }src/components/maps/full-screen-location-picker.tsx (4)
63-72: TypeaddressPartsto avoid implicitany[].The empty array infers
any[]. Explicitly type it asstring[]to keep type safety. As per coding guidelines, avoidany.♻️ Suggested fix
- let addressParts = []; + const addressParts: string[] = [];
153-162: Replaceanyin the map-press handler with Mapbox’s event type.Use the exported onPress event type from
@rnmapbox/maps(e.g.,OnPressEventor the appropriate type for v10.1.42-rc.0) to keep handler typing accurate. As per coding guidelines, avoidany.♻️ Example (confirm the exact type in this Mapbox version)
-import Mapbox from '@rnmapbox/maps'; +import Mapbox, { type OnPressEvent } from '@rnmapbox/maps'; ... - const handleMapPress = (event: any) => { + const handleMapPress = (event: OnPressEvent) => {
196-196: Use the ternary operator for conditional rendering.Guidelines prefer
?:over&&for conditional UI. As per coding guidelines, update to a ternary.♻️ Suggested change
- {!hasUserLocation && <Text className="mb-2 text-center text-amber-600">{t('common.tap_map_to_select')}</Text>} + {!hasUserLocation ? ( + <Text className="mb-2 text-center text-amber-600">{t('common.tap_map_to_select')}</Text> + ) : null}
243-257: Make the new “My Location” button theme-aware.The hard-coded white background and blue icon color can clash in dark mode. Consider using themed colors (gluestack tokens or color-scheme-aware styles) for consistent light/dark support. As per coding guidelines, ensure dark-mode compatibility.
| // Create a single object that will be mutated - never reassign! | ||
| const mockStoreState = { | ||
| callFiles: defaultMockFiles as any, | ||
| isLoadingFiles: false, | ||
| errorFiles: null, | ||
| errorFiles: null as string | null, | ||
| fetchCallFiles: mockFetchCallFiles, | ||
| clearFiles: mockClearFiles, | ||
| }; | ||
|
|
||
| // Helper function to update mock state without replacing the object | ||
| const setMockStoreState = (updates: Partial<typeof mockStoreState>) => { | ||
| Object.assign(mockStoreState, updates); | ||
| }; | ||
|
|
||
| // Reset mock state to defaults | ||
| const resetMockStoreState = () => { | ||
| mockStoreState.callFiles = defaultMockFiles; | ||
| mockStoreState.isLoadingFiles = false; | ||
| mockStoreState.errorFiles = null; | ||
| mockStoreState.fetchCallFiles = mockFetchCallFiles; | ||
| mockStoreState.clearFiles = mockClearFiles; | ||
| }; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and read the relevant lines
head -80 src/components/calls/__tests__/call-files-modal.test.tsx | tail -50Repository: Resgrid/Unit
Length of output: 1409
🏁 Script executed:
# Search for existing CallFile or related types in non-test files
rg -n "type CallFile|interface CallFile|type.*CallFile|export.*CallFile" -g '!**/__tests__/**'Repository: Resgrid/Unit
Length of output: 1866
🏁 Script executed:
# Also check for any models or types related to call files
rg -n "class CallFile|type Call.*File|interface Call.*File" --type ts --type tsx -g '!**/__tests__/**' | head -20Repository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
# Let's check the CallFileResultData structure to confirm it matches the mock
head -50 src/models/v4/callFiles/callFileResultData.tsRepository: Resgrid/Unit
Length of output: 407
🏁 Script executed:
# Check if callFiles array is mutated anywhere in the component
rg -A 5 -B 5 "callFiles\." src/components/calls/call-files-modal.tsx | head -40Repository: Resgrid/Unit
Length of output: 1031
Replace as any with CallFileResultData[] type.
Line 42 uses as any, which conflicts with the TS guideline to avoid any and strive for precise types. Since CallFileResultData is already imported in the component and matches the mock object structure exactly, use it directly instead of the type cast.
Cloning defaultMockFiles in the initial state and reset function is optional but recommended for test isolation if mock state handling evolves.
🔧 Suggested fix
const mockStoreState = {
- callFiles: defaultMockFiles as any,
+ callFiles: defaultMockFiles as CallFileResultData[],
isLoadingFiles: false,
errorFiles: null as string | null,
fetchCallFiles: mockFetchCallFiles,
clearFiles: mockClearFiles,
};🤖 Prompt for AI Agents
In `@src/components/calls/__tests__/call-files-modal.test.tsx` around lines 40 -
62, The mock store's callFiles currently uses an unsafe cast ("as any"); change
its type to CallFileResultData[] for precision by declaring callFiles:
defaultMockFiles as CallFileResultData[] (or initialize with a shallow clone
like [...defaultMockFiles] to avoid shared mutation), and update the
resetMockStoreState assignment to reset mockStoreState.callFiles =
defaultMockFiles as CallFileResultData[] (or a clone) so the types align; keep
the helpers setMockStoreState, mockFetchCallFiles, and mockClearFiles unchanged.
| import { Env } from '@/lib/env'; | ||
|
|
||
| // Ensure Mapbox access token is set before using any Mapbox components | ||
| Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); | ||
|
|
||
| // Default location (center of USA) used when user location is unavailable | ||
| const DEFAULT_LOCATION = { | ||
| latitude: 39.8283, | ||
| longitude: -98.5795, | ||
| }; | ||
|
|
||
| // Timeout for location fetching (in milliseconds) | ||
| const LOCATION_TIMEOUT = 10000; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/maps/full-screen-location-picker.tsx | head -100Repository: Resgrid/Unit
Length of output: 4398
🏁 Script executed:
# Check if there are other Mapbox implementations in the codebase
rg "setAccessToken|Mapbox\.setAccessToken" --type ts --type tsx -B 2 -A 2Repository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
# Check the Env module to understand how UNIT_MAPBOX_PUBKEY is defined
rg "UNIT_MAPBOX_PUBKEY" --type ts --type tsx -B 2 -A 2Repository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
# Check for error boundaries or error handling in components
rg "ErrorBoundary|try.*catch.*Mapbox|useEffect.*Mapbox" --type tsx -B 2 -A 5 | head -50Repository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
# Find and read the Env module
fd -t f "env" --extension ts --extension tsx | grep -E "(lib|config)" | head -5Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Search for setAccessToken without type filter
rg "setAccessToken" -B 2 -A 2Repository: Resgrid/Unit
Length of output: 1805
🏁 Script executed:
# Check the full context of Env import and module structure
find . -path "*/lib/env*" -type fRepository: Resgrid/Unit
Length of output: 181
🏁 Script executed:
cat -n src/lib/env.jsRepository: Resgrid/Unit
Length of output: 754
🏁 Script executed:
# Also check if there's validation or error handling in the Map component itself
cat -n src/app/\(app\)/index.tsx | head -80Repository: Resgrid/Unit
Length of output: 3917
Add validation for Mapbox token to provide clearer error messages.
Mapbox.setAccessToken is called unconditionally with Env.UNIT_MAPBOX_PUBKEY; if the token is missing or undefined, the map may fail silently without clear error messaging. Add a guard to log an error and prevent silent failures during development.
Suggested guard
-Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY);
+if (!Env.UNIT_MAPBOX_PUBKEY) {
+ console.error('Missing UNIT_MAPBOX_PUBKEY for Mapbox initialization');
+} else {
+ Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY);
+}📝 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.
| import { Env } from '@/lib/env'; | |
| // Ensure Mapbox access token is set before using any Mapbox components | |
| Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); | |
| // Default location (center of USA) used when user location is unavailable | |
| const DEFAULT_LOCATION = { | |
| latitude: 39.8283, | |
| longitude: -98.5795, | |
| }; | |
| // Timeout for location fetching (in milliseconds) | |
| const LOCATION_TIMEOUT = 10000; | |
| import { Env } from '@/lib/env'; | |
| // Ensure Mapbox access token is set before using any Mapbox components | |
| if (!Env.UNIT_MAPBOX_PUBKEY) { | |
| console.error('Missing UNIT_MAPBOX_PUBKEY for Mapbox initialization'); | |
| } else { | |
| Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); | |
| } | |
| // Default location (center of USA) used when user location is unavailable | |
| const DEFAULT_LOCATION = { | |
| latitude: 39.8283, | |
| longitude: -98.5795, | |
| }; | |
| // Timeout for location fetching (in milliseconds) | |
| const LOCATION_TIMEOUT = 10000; |
🤖 Prompt for AI Agents
In `@src/components/maps/full-screen-location-picker.tsx` around lines 12 - 24,
The Mapbox access token is being set unconditionally via
Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); add a guard that checks
Env.UNIT_MAPBOX_PUBKEY for falsy/empty values before calling
Mapbox.setAccessToken, and if missing log a clear error (e.g., using
console.error or an existing logger) indicating the token is not configured and
avoid calling Mapbox.setAccessToken to prevent silent failures; include the
symbol Env.UNIT_MAPBOX_PUBKEY and Mapbox.setAccessToken in the check and
message, and in development consider throwing or failing fast so the missing
token is noticed.
| <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location"> | ||
| <Box className="items-center justify-center"> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if file exists and get its size
wc -l src/components/maps/full-screen-location-picker.tsxRepository: Resgrid/Unit
Length of output: 111
🏁 Script executed:
# Read the specific lines mentioned in the review
sed -n '170,185p' src/components/maps/full-screen-location-picker.tsxRepository: Resgrid/Unit
Length of output: 1001
🏁 Script executed:
# Check imports at the top of the file to see if react-i18next is already in use
head -40 src/components/maps/full-screen-location-picker.tsxRepository: Resgrid/Unit
Length of output: 1612
🏁 Script executed:
# Search for other instances of "Selected Location" or similar string patterns in the file
rg "Selected Location|PointAnnotation" src/components/maps/full-screen-location-picker.tsx -A 2 -B 2Repository: Resgrid/Unit
Length of output: 636
🏁 Script executed:
# Search for other potential user-facing text in the file that should be localized
rg "title=|placeholder=|label=|aria-label=" src/components/maps/full-screen-location-picker.tsx -A 1Repository: Resgrid/Unit
Length of output: 259
🏁 Script executed:
# Check for other hardcoded strings that aren't wrapped in t()
grep -n "=\"[A-Z][^\"]*\"" src/components/maps/full-screen-location-picker.tsx | grep -v "id=" | grep -v "style=" | grep -v "ref=" | head -20Repository: Resgrid/Unit
Length of output: 38
Wrap the annotation title in the translation function.
The title="Selected Location" prop is user-facing text used for accessibility. Wrap it in t() with an appropriate translation key as per coding guidelines requiring all user-facing text in .tsx files to be localized.
Suggested change
- <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location">
+ <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title={t('common.selected_location')}>📝 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.
| <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location"> | |
| <Box className="items-center justify-center"> | |
| <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title={t('common.selected_location')}> | |
| <Box className="items-center justify-center"> |
🤖 Prompt for AI Agents
In `@src/components/maps/full-screen-location-picker.tsx` around lines 177 - 178,
The Mapbox.PointAnnotation component is using hard-coded user-facing text in the
title prop ("Selected Location"); replace that with a localized string by
wrapping it in the translation function (t) and using an appropriate key (e.g.,
t('maps.selectedLocation')) so the title prop becomes localized; update the
PointAnnotation usage (Mapbox.PointAnnotation id="selectedLocation" title=...)
accordingly and ensure the t function is imported/available in the component.
| // The cleanup error is caught and logged within resetAllStores, | ||
| // so clearAllAppData should complete without throwing | ||
| await expect(clearAllAppData()).resolves.toBeUndefined(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd 'app-reset\.service\.ts$' -x rg -n -C3 'resetAllStores|clearAllAppData' {}Repository: Resgrid/Unit
Length of output: 1099
🏁 Script executed:
fd 'app-reset\.service\.ts$' -x cat -n {}Repository: Resgrid/Unit
Length of output: 10584
The test assumption about error handling is incorrect. clearAllAppData does not swallow errors—it explicitly re-throws them (line 281 in the implementation). Only specific cleanup operations within resetAllStores (LiveKit disconnect and audio stream cleanup) catch and log errors; other errors will propagate up and cause clearAllAppData to throw. The test expectation resolves.toBeUndefined() is only valid if no errors occur during cleanup operations or app storage clearing.
🤖 Prompt for AI Agents
In `@src/services/__tests__/app-reset.service.test.ts` around lines 466 - 468, The
test incorrectly assumes clearAllAppData swallows errors; update the test to
either (A) mock/stub the cleanup helpers called by resetAllStores so they do not
throw (e.g., stub the LiveKit disconnect and audio stream cleanup helpers) and
then assert await expect(clearAllAppData()).resolves.toBeUndefined(), or (B) if
you want to verify propagation, change the assertion to await
expect(clearAllAppData()).rejects.toThrow(...) and assert the specific error;
locate usages of clearAllAppData and resetAllStores in the test and update the
mocks/assertion accordingly.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@src/api/calls/calls.ts`:
- Around line 130-138: There are trailing whitespace characters on several blank
lines causing Prettier/CI failures; remove the stray spaces in the cache
invalidation block (around the cacheManager.remove('/Calls/GetActiveCalls')
try/catch) and the same stray spaces in the updateCall and closeCall functions
so blank lines contain no trailing spaces; search for
cacheManager.remove('/Calls/GetActiveCalls'), updateCall, and closeCall to
locate the blocks and delete trailing spaces on the flagged lines (130, 138,
187, 195, 207, 215) to satisfy formatting checks.
In `@src/components/maps/full-screen-location-picker.tsx`:
- Around line 153-162: The handler handleMapPress currently assumes
event.geometry has coordinates but GeoJSON.Geometry is a union (e.g.,
GeometryCollection lacks coordinates); add a type guard before accessing
coordinates—either narrow the Feature type to a Point (e.g., accept
GeoJSON.Feature<GeoJSON.Point>) or check at runtime (e.g., if
(event.geometry.type !== 'GeometryCollection' && 'coordinates' in
event.geometry) { const coords = event.geometry.coordinates as number[]; ... }).
Then use coords[1]/coords[0] to build newLocation and call setCurrentLocation,
setHasUserLocation and reverseGeocode; this ensures TypeScript understands
coordinates exists and prevents the error.
In `@src/components/maps/location-picker.tsx`:
- Around line 58-73: The variable timeoutId is typed/used incorrectly and may be
considered unassigned; change its declaration to let timeoutId:
ReturnType<typeof setTimeout> | undefined = undefined, assign timeoutId =
setTimeout(...) inside the timeoutPromise executor, and guard the clearTimeout
call with if (timeoutId !== undefined) clearTimeout(timeoutId); keep the
timeoutPromise and Promise.race usage with LOCATION_TIMEOUT and
Location.getCurrentPositionAsync as-is so the timeout semantics are preserved.
In `@src/services/callkeep.service.android.ts`:
- Around line 53-57: The parameter type for setMuteStateCallback is ambiguous
because "(muted: boolean) => void | null" is parsed as a function returning
void|null; change the parameter signature to accept a nullable callback by using
"((muted: boolean) => void) | null" (i.e., setMuteStateCallback(callback:
((muted: boolean) => void) | null): void) and update any references or usage in
setMuteStateCallback to handle callback being null accordingly; ensure the
logger.debug message remains but the TypeScript type reflects a nullable
function.
- Around line 3-10: The Android CallKeepService implementation has
method/signature mismatches with the iOS API; update the Android service so its
public API matches iOS exactly: change startCall to startCall(roomName: string,
handle?: string): Promise<string>, rename isCallActive to isCallActiveNow():
boolean, add getCurrentCallUUID(): string | null, remove Android-only methods
setMuted and updateCallDisplay (or move them into iOS if you deliberately choose
that direction), and fix setMuteStateCallback's type to ((muted: boolean) =>
void) | null; ensure these changes are applied on the CallKeepService class and
any exported types so Metro resolution sees identical method signatures across
platforms.
In `@src/stores/calls/store.ts`:
- Around line 43-48: The fetchCalls implementation should guard against
response.Data being undefined so calls stays typed as an array: in fetchCalls
(which calls getCalls) check the response and normalize the value passed to
set({ calls: ... }) to an array (e.g., use a fallback like [] or Array.isArray
check) before updating state; ensure set is called with calls always being an
array and still update isLoading and lastFetchedAt as before.
♻️ Duplicate comments (3)
src/components/maps/full-screen-location-picker.tsx (2)
14-15: Add validation for Mapbox token to prevent silent failures.Same as the other location picker component—setting the access token unconditionally may cause silent map failures if
Env.UNIT_MAPBOX_PUBKEYis undefined or empty.Suggested guard
-// Ensure Mapbox access token is set before using any Mapbox components -Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +// Ensure Mapbox access token is set before using any Mapbox components +if (!Env.UNIT_MAPBOX_PUBKEY) { + console.error('Missing UNIT_MAPBOX_PUBKEY for Mapbox initialization'); +} else { + Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +}
177-177: Wrap annotation title in translation function.The
title="Selected Location"is user-facing accessibility text and should be localized per coding guidelines.Proposed fix
- <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location"> + <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title={t('common.selected_location')}>src/stores/app/livekit-store.ts (1)
11-11: Module resolution fails during tests — add a basecallkeep.service.tsfile.The static analysis shows the test suite cannot find
'../../services/callkeep.service'. While Metro bundler resolves platform-specific files (.ios.ts,.android.ts) at runtime, Jest requires either:
- A base
callkeep.service.tsthat re-exports the platform service, or- Explicit
moduleNameMapperconfiguration in JestThe simplest fix is to add a base file that re-exports one of the platform implementations (typically the more complete one):
Proposed fix: Create `src/services/callkeep.service.ts`
// src/services/callkeep.service.ts // Re-export for platforms - Metro will resolve to .ios.ts or .android.ts at runtime // This file serves as fallback for test environments and type checking export * from './callkeep.service.ios';Alternatively, if you want the Android no-op as the default:
export * from './callkeep.service.android';
🧹 Nitpick comments (7)
src/api/calls/calls.ts (2)
94-109: Remove commented-out dead code.Multiple lines contain commented-out code (e.g.,
//dispatchEntries.push(...callData.dispatchUsers.map((user) => \U:${user}`))`). This appears to be legacy code from a previous implementation. Per the PR's stated "code cleanup and modernization" objective, consider removing these comments to improve readability.♻️ Proposed cleanup for createCall
if (callData.dispatchUsers) { - //dispatchEntries.push(...callData.dispatchUsers.map((user) => `U:${user}`)); dispatchEntries.push(...callData.dispatchUsers); } if (callData.dispatchGroups) { - //dispatchEntries.push(...callData.dispatchGroups.map((group) => `G:${group}`)); dispatchEntries.push(...callData.dispatchGroups); } if (callData.dispatchRoles) { - //dispatchEntries.push(...callData.dispatchRoles.map((role) => `R:${role}`)); dispatchEntries.push(...callData.dispatchRoles); } if (callData.dispatchUnits) { - //dispatchEntries.push(...callData.dispatchUnits.map((unit) => `U:${unit}`)); dispatchEntries.push(...callData.dispatchUnits); }Apply the same cleanup to
updateCall.Also applies to: 150-165
86-127: Consider extracting duplicated dispatch list logic.The dispatch list building logic (lines 87-112 and 143-168) is nearly identical between
createCallandupdateCall. Extracting this into a helper function would reduce duplication and ease future maintenance.♻️ Suggested helper extraction
interface DispatchOptions { dispatchUsers?: string[]; dispatchGroups?: string[]; dispatchRoles?: string[]; dispatchUnits?: string[]; dispatchEveryone?: boolean; } const buildDispatchList = (options: DispatchOptions): string => { if (options.dispatchEveryone) { return '0'; } const dispatchEntries: string[] = []; if (options.dispatchUsers) { dispatchEntries.push(...options.dispatchUsers); } if (options.dispatchGroups) { dispatchEntries.push(...options.dispatchGroups); } if (options.dispatchRoles) { dispatchEntries.push(...options.dispatchRoles); } if (options.dispatchUnits) { dispatchEntries.push(...options.dispatchUnits); } return dispatchEntries.join('|'); };Then both
createCallandupdateCallcan callconst dispatchList = buildDispatchList(callData);.Also applies to: 142-184
src/components/maps/location-picker.tsx (4)
13-14: Add validation for Mapbox token to prevent silent failures.Setting the access token unconditionally may cause silent map failures if
Env.UNIT_MAPBOX_PUBKEYis undefined or empty. Add a guard for clearer error messaging during development.Suggested guard
-// Ensure Mapbox access token is set before using any Mapbox components -Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +// Ensure Mapbox access token is set before using any Mapbox components +if (!Env.UNIT_MAPBOX_PUBKEY) { + console.error('Missing UNIT_MAPBOX_PUBKEY for Mapbox initialization'); +} else { + Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +}
143-147: Wrap user-facing annotation title in translation function.The
title="Selected Location"is user-facing accessibility text and should be localized per coding guidelines.Proposed fix
- <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location"> + <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title={t('common.selected_location')}>
151-153: Ensure the "My Location" button is accessible.The button lacks an accessibility label, which is important for screen reader users. Add
accessibilityLabelandaccessibilityRoleprops.Proposed fix
- <TouchableOpacity style={styles.myLocationButton} onPress={getUserLocation} disabled={isLocating}> + <TouchableOpacity + style={styles.myLocationButton} + onPress={getUserLocation} + disabled={isLocating} + accessibilityLabel={t('common.my_location')} + accessibilityRole="button" + > {isLocating ? <ActivityIndicator size="small" color="#007AFF" /> : <LocateIcon size={20} color="#007AFF" />} </TouchableOpacity>
124-132: Consider typing the map press event parameter.The
eventparameter is typed asany, which violates the coding guideline to avoidanyand strive for precise types. Use the appropriate Mapbox type.Proposed fix
- const handleMapPress = (event: any) => { + const handleMapPress = (event: GeoJSON.Feature) => { const { coordinates } = event.geometry;Note: You may need to narrow the geometry type or add a type guard since
GeoJSON.Geometryis a union type. See the fix applied infull-screen-location-picker.tsxfor reference.src/components/maps/full-screen-location-picker.tsx (1)
190-192: Ensure the "My Location" button is accessible.The button lacks accessibility attributes for screen reader users. Add
accessibilityLabelandaccessibilityRoleprops.Proposed fix
- <TouchableOpacity style={[styles.myLocationButton, { top: insets.top + 10 }]} onPress={getUserLocation} disabled={isLocating}> + <TouchableOpacity + style={[styles.myLocationButton, { top: insets.top + 10 }]} + onPress={getUserLocation} + disabled={isLocating} + accessibilityLabel={t('common.my_location')} + accessibilityRole="button" + > {isLocating ? <ActivityIndicator size="small" color="#007AFF" /> : <LocateIcon size={24} color="#007AFF" />} </TouchableOpacity>
src/api/calls/calls.ts
Outdated
|
|
||
| // Invalidate cache after successful mutation | ||
| try { | ||
| cacheManager.remove('/Calls/GetActiveCalls'); | ||
| } catch (error) { | ||
| // Silently handle cache removal errors | ||
| console.warn('Failed to invalidate calls cache:', error); | ||
| } | ||
|
|
There was a problem hiding this comment.
Fix trailing space formatting issues flagged by Prettier.
Pipeline failures indicate stray spaces on lines 130, 138, 187, 195, 207, and 215. Remove the trailing whitespace to pass CI checks.
🧹 Proposed fix
const response = await createCallApi.post<SaveCallResult>(data);
-
+
// Invalidate cache after successful mutation
try {
cacheManager.remove('/Calls/GetActiveCalls');
} catch (error) {
// Silently handle cache removal errors
console.warn('Failed to invalidate calls cache:', error);
}
-
+
return response.data;Apply the same pattern (remove trailing spaces on blank lines) to the updateCall and closeCall functions.
Also applies to: 187-195, 207-215
🧰 Tools
🪛 GitHub Actions: React Native CI/CD
[warning] 130-130: Prettier formatting issue. Delete stray spaces.
[warning] 138-138: Prettier formatting issue. Delete stray spaces.
🪛 GitHub Check: test
[warning] 138-138:
Delete ··
[warning] 130-130:
Delete ··
🤖 Prompt for AI Agents
In `@src/api/calls/calls.ts` around lines 130 - 138, There are trailing whitespace
characters on several blank lines causing Prettier/CI failures; remove the stray
spaces in the cache invalidation block (around the
cacheManager.remove('/Calls/GetActiveCalls') try/catch) and the same stray
spaces in the updateCall and closeCall functions so blank lines contain no
trailing spaces; search for cacheManager.remove('/Calls/GetActiveCalls'),
updateCall, and closeCall to locate the blocks and delete trailing spaces on the
flagged lines (130, 138, 187, 195, 207, 215) to satisfy formatting checks.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/maps/full-screen-location-picker.tsx`:
- Around line 192-195: The My Location TouchableOpacity lacks accessibility
metadata; update the TouchableOpacity (styles.myLocationButton) that calls
getUserLocation and uses isLocating/LocateIcon/ActivityIndicator to include
accessibility props: set accessible={true}, accessibilityRole="button",
accessibilityLabel (e.g., "My location"), accessibilityHint (e.g., "Centers map
on your current location"), and accessibilityState={{ busy: isLocating }} so
screen readers properly announce state and purpose.
- Around line 97-109: The timeout promise created for LOCATION_TIMEOUT uses
setTimeout which can fire after Promise.race resolves and cause an unhandled
rejection; modify the race so you capture the timeout ID (e.g., let timeoutId =
setTimeout(...)) when constructing timeoutPromise and after awaiting
Promise.race([...]) clearTimeout(timeoutId) if it exists; update the
timeoutPromise and the surrounding code around Location.getCurrentPositionAsync
and timeoutPromise to share that timeoutId and ensure clearTimeout is called
when the location resolves first (mirror the pattern used in non-full-screen
location-picker.tsx).
♻️ Duplicate comments (3)
src/api/calls/calls.ts (1)
131-139: Verify trailing whitespace has been removed.A previous review flagged trailing spaces in the cache invalidation blocks. Ensure these blank lines contain no trailing whitespace to pass Prettier/CI checks. If the linter still reports issues, remove any stray spaces on the blank lines within these blocks.
Also applies to: 163-171, 183-191
src/components/maps/full-screen-location-picker.tsx (2)
14-16: Guard Mapbox token initialization to avoid silent failures.🛡️ Suggested guard
-Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +if (!Env.UNIT_MAPBOX_PUBKEY) { + console.error('Mapbox access token is not configured. Please set UNIT_MAPBOX_PUBKEY in your environment.'); +} else { + Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +}
179-181: Localize the annotation title.
User‑facing text should be translated. As per coding guidelines, wrap UI strings int().🌐 Suggested change
- <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location"> + <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title={t('common.selected_location')}>
🧹 Nitpick comments (2)
src/services/callkeep.service.ts (1)
9-11: Consider verifying the type equivalence assumption.The comment states types are "the same in both," but only iOS types are exported. This is fine if Android is a no-op that mirrors the iOS interface (which the PR summary suggests), but if the implementations diverge in the future, consumers using Android might encounter type mismatches.
src/components/maps/location-picker.tsx (1)
6-7: Use shared UI primitives for TouchableOpacity/ActivityIndicator.
Keeps styling/theming consistent with the rest of the UI layer. As per coding guidelines, use gluestack‑ui consistently.♻️ Suggested refactor
-import { ActivityIndicator, StyleSheet, TouchableOpacity } from 'react-native'; +import { ActivityIndicator, TouchableOpacity } from '@/components/ui'; +import { StyleSheet } from 'react-native';Also applies to: 155-158
| {/* My Location button */} | ||
| <TouchableOpacity style={[styles.myLocationButton, { top: insets.top + 10 }]} onPress={getUserLocation} disabled={isLocating}> | ||
| {isLocating ? <ActivityIndicator size="small" color="#007AFF" /> : <LocateIcon size={24} color="#007AFF" />} | ||
| </TouchableOpacity> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/components/maps/full-screen-location-picker.tsx | sed -n '185,200p'Repository: Resgrid/Unit
Length of output: 1017
🏁 Script executed:
rg "common.my_location" --type ts --type tsxRepository: Resgrid/Unit
Length of output: 83
🏁 Script executed:
fd -e json -e ts -e tsx | xargs rg "my_location" -lRepository: Resgrid/Unit
Length of output: 170
🏁 Script executed:
cat src/translations/en.json | jq . | grep -A 2 -B 2 "my_location"Repository: Resgrid/Unit
Length of output: 357
🏁 Script executed:
cat src/translations/en.json | jq . | grep -A 2 -B 2 "location"Repository: Resgrid/Unit
Length of output: 4635
🏁 Script executed:
rg "accessibilityRole|accessibilityLabel" src/components/maps/full-screen-location-picker.tsxRepository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
cat -n src/components/maps/full-screen-location-picker.tsx | sed -n '1,50p'Repository: Resgrid/Unit
Length of output: 2423
🏁 Script executed:
rg "accessibility" src/components/maps/full-screen-location-picker.tsxRepository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
rg "accessibilityRole|accessibilityLabel" src/components/maps/ -A 2 -B 2Repository: Resgrid/Unit
Length of output: 609
🏁 Script executed:
cat -n src/components/maps/full-screen-location-picker.tsx | sed -n '180,210p'Repository: Resgrid/Unit
Length of output: 1888
🏁 Script executed:
rg "accessibilityRole|accessibilityLabel|accessibilityHint" src/components/maps/full-screen-location-picker.tsx -B 2 -A 2Repository: Resgrid/Unit
Length of output: 38
Add accessibility metadata to the My Location button.
Improves screen-reader discoverability for this interactive control. A similar component in the codebase (location-picker.tsx) already implements the same accessibility pattern.
♿ Suggested change
- <TouchableOpacity style={[styles.myLocationButton, { top: insets.top + 10 }]} onPress={getUserLocation} disabled={isLocating}>
+ <TouchableOpacity
+ style={[styles.myLocationButton, { top: insets.top + 10 }]}
+ onPress={getUserLocation}
+ disabled={isLocating}
+ accessibilityRole="button"
+ accessibilityLabel={t('common.my_location')}
+ >🤖 Prompt for AI Agents
In `@src/components/maps/full-screen-location-picker.tsx` around lines 192 - 195,
The My Location TouchableOpacity lacks accessibility metadata; update the
TouchableOpacity (styles.myLocationButton) that calls getUserLocation and uses
isLocating/LocateIcon/ActivityIndicator to include accessibility props: set
accessible={true}, accessibilityRole="button", accessibilityLabel (e.g., "My
location"), accessibilityHint (e.g., "Centers map on your current location"),
and accessibilityState={{ busy: isLocating }} so screen readers properly
announce state and purpose.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/components/maps/full-screen-location-picker.tsx`:
- Around line 157-169: The handler handleMapPress currently only excludes
GeometryCollection but may receive non-Point geometries with nested coordinates;
change the guard to explicitly check that event.geometry.type === 'Point' before
extracting coordinates so you only treat coords as number[]; then extract
[longitude, latitude] and pass the newLocation to setCurrentLocation,
setHasUserLocation(true) and reverseGeocode(latitude, longitude) as before
(refer to handleMapPress, setCurrentLocation, setHasUserLocation,
reverseGeocode).
♻️ Duplicate comments (3)
src/components/maps/full-screen-location-picker.tsx (3)
14-15: Add validation for Mapbox token to prevent silent failures.The token is set unconditionally; if
Env.UNIT_MAPBOX_PUBKEYis undefined or empty, the map may fail without clear error messaging.Suggested guard
-// Ensure Mapbox access token is set before using any Mapbox components -Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +// Ensure Mapbox access token is set before using any Mapbox components +if (!Env.UNIT_MAPBOX_PUBKEY) { + console.error('Missing UNIT_MAPBOX_PUBKEY for Mapbox initialization'); +} else { + Mapbox.setAccessToken(Env.UNIT_MAPBOX_PUBKEY); +}
184-185: Wrap the annotation title in the translation function.The
title="Selected Location"prop is user-facing text used for accessibility. Per coding guidelines, all user-facing text in.tsxfiles must be localized witht().Suggested change
- <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title="Selected Location"> + <Mapbox.PointAnnotation id="selectedLocation" coordinate={[currentLocation.longitude, currentLocation.latitude]} title={t('common.selected_location')}>
196-199: Add accessibility metadata to the My Location button.The button lacks
accessibilityRoleandaccessibilityLabel, which are needed for screen reader support per WCAG mobile guidelines. The similar close button pattern and the siblinglocation-picker.tsxcomponent already implement accessibility props.Suggested fix
- <TouchableOpacity style={[styles.myLocationButton, { top: insets.top + 10 }]} onPress={getUserLocation} disabled={isLocating}> + <TouchableOpacity + style={[styles.myLocationButton, { top: insets.top + 10 }]} + onPress={getUserLocation} + disabled={isLocating} + accessibilityRole="button" + accessibilityLabel={t('common.my_location')} + accessibilityState={{ disabled: isLocating }} + >
🧹 Nitpick comments (1)
src/components/maps/full-screen-location-picker.tsx (1)
192-194: Add accessibility metadata to the Close button.For consistency and WCAG compliance, the close button should also include accessibility props.
Suggested fix
- <TouchableOpacity style={[styles.closeButton, { top: insets.top + 10 }]} onPress={onClose}> + <TouchableOpacity + style={[styles.closeButton, { top: insets.top + 10 }]} + onPress={onClose} + accessibilityRole="button" + accessibilityLabel={t('common.close')} + >
| const handleMapPress = (event: GeoJSON.Feature) => { | ||
| if (event.geometry.type !== 'GeometryCollection' && 'coordinates' in event.geometry) { | ||
| const coords = event.geometry.coordinates as number[]; | ||
| const [longitude, latitude] = coords; | ||
| const newLocation = { | ||
| latitude, | ||
| longitude, | ||
| }; | ||
| setCurrentLocation(newLocation); | ||
| setHasUserLocation(true); | ||
| reverseGeocode(newLocation.latitude, newLocation.longitude); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Type guard should explicitly check for Point geometry.
The current guard excludes GeometryCollection but still allows LineString, Polygon, etc., where coordinates is a nested array (e.g., number[][]), causing incorrect latitude/longitude extraction. A map press event should yield a Point geometry.
Suggested fix
const handleMapPress = (event: GeoJSON.Feature) => {
- if (event.geometry.type !== 'GeometryCollection' && 'coordinates' in event.geometry) {
- const coords = event.geometry.coordinates as number[];
+ if (event.geometry.type === 'Point') {
+ const coords = event.geometry.coordinates;
const [longitude, latitude] = coords;🤖 Prompt for AI Agents
In `@src/components/maps/full-screen-location-picker.tsx` around lines 157 - 169,
The handler handleMapPress currently only excludes GeometryCollection but may
receive non-Point geometries with nested coordinates; change the guard to
explicitly check that event.geometry.type === 'Point' before extracting
coordinates so you only treat coords as number[]; then extract [longitude,
latitude] and pass the newLocation to setCurrentLocation,
setHasUserLocation(true) and reverseGeocode(latitude, longitude) as before
(refer to handleMapPress, setCurrentLocation, setHasUserLocation,
reverseGeocode).
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Release Notes
New Features
Bug Fixes
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Localization
✏️ Tip: You can customize this high-level summary in your review settings.