Conversation
|
Caution Review failedThe pull request is closed. WalkthroughApp-level location lifecycle moved to _layout (start/stop guarded); per-screen tracking removed. Realtime geolocation support added (storage gating, updater registration, API send gating). Bluetooth audio init now permission-gated with a 500ms delay. Login form adds Server URL flow and analytics; notifications badge capping and accessibility added. Tests and translations updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App (_layout)
participant Offline as offlineQueueService
participant Loc as locationService
participant Auth as AuthState
App->>Offline: initialize()
Offline-->>App: resolved
App->>Loc: startLocationUpdates() (guarded)
alt started
Loc-->>App: started (log)
else error
Loc-->>App: error (logged, init continues)
end
Auth-->>App: status -> signedOut
App->>Loc: stopLocationUpdates() (guarded)
Loc-->>App: stopped or error (logged)
sequenceDiagram
autonumber
participant UI as LoginForm
participant Analytics as useAnalytics
participant Sheet as ServerUrlBottomSheet
UI->>Analytics: trackEvent('login_server_url_pressed')
Analytics-->>UI: ack / error logged
UI->>Sheet: open(isOpen=true)
Sheet-->>UI: onClose -> set isOpen=false
sequenceDiagram
autonumber
participant Hook as useRealtimeGeolocation
participant Store as Storage (MMKV)
participant Updater as LocationServiceUpdater
participant Hub as GeolocationHub
Hook->>Store: saveRealtimeGeolocationState(enabled)
alt updater registered
Hook->>Updater: locationServiceRealtimeUpdater(enabled)
end
alt enabled
Hook->>Hub: connect()
else disabled
Hook->>Hub: disconnect()
end
sequenceDiagram
autonumber
participant BA as bluetoothAudioService.initialize()
participant Delay as 500ms timer
participant Perm as Permissions
participant Ble as BleManager
BA->>Delay: wait 500ms
Delay-->>BA: done
BA->>Perm: request/check perms
alt granted
BA->>Ble: start() & setupEventListeners()
else denied/failure
BA-->>BA: skip init (logged)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/components/notifications/NotificationButton.tsx (1)
22-33: Add accessibilityRole/label and badge testID; localize "99+".
Improve a11y, testability, and i18n without changing visuals.- <Pressable onPress={onPress} className="mr-2 p-2" testID="notification-button"> + <Pressable + onPress={onPress} + accessibilityRole="button" + accessibilityLabel={t('notifications.buttonLabel', { count: unread, defaultValue: unread > 0 ? `Notifications, ${unread} unread` : 'Notifications' })} + className="mr-2 p-2" + hitSlop={8} + testID="notification-button" + > <View className="relative"> <BellIcon size={24} className="text-gray-700 dark:text-gray-300" strokeWidth={2} /> - {counts?.[0]?.count && counts?.[0]?.count > 0 ? ( - <View className="absolute -right-2 -top-2 flex size-5 items-center justify-center rounded-full bg-red-500"> - <Text className="text-xs font-bold text-white">{counts?.[0]?.count > 99 ? '99+' : counts?.[0]?.count}</Text> + {unread > 0 ? ( + <View testID="notification-badge" className="absolute -right-2 -top-2 flex size-5 items-center justify-center rounded-full bg-red-500"> + <Text className="text-xs font-bold text-white"> + {unread > 99 ? t('notifications.ninetyNinePlus', '99+') : unread} + </Text> </View> ) : null}Outside-range additions required:
// imports import { useTranslation } from 'react-i18next'; // inside component, before return const { t } = useTranslation();src/services/bluetooth-audio.service.ts (2)
346-350: checkBluetoothState likely misuses BleManager.checkState (returns void in most versions).Awaiting checkState and mapping its return risks always reading undefined -> State.Unknown. Prefer getState (if available) or listen once for onDidUpdateState after checkState.
Replace with:
async checkBluetoothState(): Promise<State> { try { const maybeGetState = (BleManager as any).getState; if (typeof maybeGetState === 'function') { const s = (await maybeGetState()) as BleState; return this.mapBleStateToState(s); } await BleManager.checkState(); const s: BleState | 'unknown' = await new Promise((resolve) => { const sub = BleManager.onDidUpdateState(({ state }: { state: BleState }) => { sub.remove(); resolve(state); }); setTimeout(() => { sub.remove(); resolve('unknown'); }, 1500); }); return this.mapBleStateToState(s as BleState); } catch (error) { logger.error({ message: 'Failed to check Bluetooth state', context: { error } }); return State.Unknown; } }
317-344: Gate BLE permissions by API level and parameterize startup delay
- Make the startup delay configurable (
startupDelayMs: number = 500) instead of hard-coding 500 ms.- For Android API ≤ 30 (Android 10/11), request
PermissionsAndroid.PERMISSIONS.ACCESS_FINE_LOCATION; for API ≥ 31 (Android 12+), requestBLUETOOTH_SCAN&BLUETOOTH_CONNECT.- (Optional) Pre-check with
PermissionsAndroid.check()and return early if already granted.- async requestPermissions(): Promise<boolean> { - // Add delay to prevent conflicts with other permission requests during app startup - await new Promise((resolve) => setTimeout(resolve, 500)); + async requestPermissions(startupDelayMs: number = 500): Promise<boolean> { + // Small startup delay to avoid contention with other permission prompts + await new Promise((resolve) => setTimeout(resolve, startupDelayMs)); if (Platform.OS === 'android') { try { - const permissions = [PermissionsAndroid.PERMISSIONS.BLUETOOTH_SCAN, PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT]; + const apiLevel = typeof Platform.Version === 'number' ? Platform.Version : parseInt(String(Platform.Version), 10); + const permissions = + apiLevel >= 31 + ? [PermissionsAndroid.PERMISSIONS.BLUETOOTH_SCAN, PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT] + : [PermissionsAndroid.PERMISSIONS.ACCESS_FINE_LOCATION]; const results = await PermissionsAndroid.requestMultiple(permissions);src/app/login/login-form.tsx (4)
101-111: Fix field-level validation: current schema usage will fail username validation due to missing password.You're calling
loginFormSchema.parseAsync({ username: value })which requirespasswordtoo, causing wrong errors. Let zodResolver handle field validation and remove customrules.Apply this diff to remove redundant rules for username:
- <Controller + <Controller defaultValue="" name="username" control={control} - rules={{ - validate: async (value) => { - try { - await loginFormSchema.parseAsync({ username: value }); - return true; - } catch (error: any) { - return error.message; - } - }, - }} render={({ field: { onChange, onBlur, value } }) => (
141-149: Same issue for password Controller. Remove custom rules.Let the resolver surface errors.
- <Controller + <Controller defaultValue="" name="password" control={control} - rules={{ - validate: async (value) => { - try { - await loginFormSchema.parseAsync({ password: value }); - return true; - } catch (error: any) { - return error.message; - } - }, - }} render={({ field: { onChange, onBlur, value } }) => (
53-56: Remove unused local validation state and non-localized fallback text.
validatedis never updated; it gates errors and adds an unlocalized message. Rely on resolver errors only and drop the fallback string.- const [validated] = useState({ - usernameValid: true, - passwordValid: true, - }); + // removed unused local validation flags - <FormControl isInvalid={!!errors?.username || !validated.usernameValid} className="w-full"> + <FormControl isInvalid={!!errors?.username} className="w-full"> ... - <FormControlErrorText className="text-red-500">{errors?.username?.message || (!validated.usernameValid && 'Username not found')}</FormControlErrorText> + <FormControlErrorText className="text-red-500">{errors?.username?.message}</FormControlErrorText> ... - <FormControl isInvalid={!!errors.password || !validated.passwordValid} className="w-full"> + <FormControl isInvalid={!!errors?.password} className="w-full"> ... - <FormControlErrorText className="text-red-500">{errors?.password?.message || (!validated.passwordValid && t('login.password_incorrect'))}</FormControlErrorText> + <FormControlErrorText className="text-red-500">{errors?.password?.message}</FormControlErrorText>Also applies to: 93-97, 126-129, 132-136, 170-172
87-91: Internationalize all user-facing strings and fix typos.Hardcoded "Sign In", the description text (typos: “login in to”, “orginization”), and "Log in" bypass i18n. Use existing keys to avoid new translations.
- <Text className="pb-6 text-center text-4xl font-bold">Sign In</Text> + <Text className="pb-6 text-center text-4xl font-bold">{t('login.title')}</Text> ... - <Text className="mb-6 max-w-xl text-center text-gray-500"> - To login in to the Resgrid Responder app, please enter your username and password. Resgrid Responder is an app designed to allow a person to interact with their orginization in the Resgrid system. - </Text> + <Text className="mb-6 max-w-xl text-center text-gray-500"> + {t('login.login_button_description')} + </Text> ... - <Button className="mt-8 w-full" variant="solid" action="primary" onPress={handleSubmit(onSubmit)}> - <ButtonText>Log in</ButtonText> + <Button className="mt-8 w-full" variant="solid" action="primary" onPress={handleSubmit(onSubmit)}> + <ButtonText>{t('login.login_button')}</ButtonText> </Button>Also applies to: 182-183
src/services/__tests__/location.test.ts (1)
467-469: Same: avoid runtime use of erased TS types in matchers.- expect(mockSetUnitLocation).toHaveBeenCalledWith(expect.any(SaveUnitLocationInput)); + expect(mockSetUnitLocation).toHaveBeenCalled();
🧹 Nitpick comments (22)
src/components/notifications/NotificationButton.tsx (2)
12-21: Compute unread once to avoid repeated optional chaining.
Small readability/perf win.const { counts, isLoading } = useCounts({ filters: [ { read: false, }, ], }); - if (isLoading) return <ActivityIndicator />; + const unread = counts?.[0]?.count ?? 0; + if (isLoading) return <ActivityIndicator testID="notifications-loading" accessibilityLabel={t('notifications.loading', 'Loading notifications')} />;
11-11: Type the component as React.FC per repo guidelines.-export const NotificationButton = ({ onPress }: NotificationButtonProps) => { +export const NotificationButton: React.FC<NotificationButtonProps> = ({ onPress }) => {src/components/notifications/__tests__/NotificationButton.test.tsx (4)
28-39: Verify loading indicator presence (more robust).
If you adopt the proposed testID, assert the spinner shows instead of inferring via absence.- // Since ActivityIndicator doesn't have a testID, we check that no notification button is rendered - expect(screen.queryByTestId('notification-button')).toBeNull(); + expect(screen.getByTestId('notifications-loading')).toBeTruthy(); + expect(screen.queryByTestId('notification-button')).toBeNull();
41-56: Avoid coupling tests to exact className strings.
Asserting the icon’s accessibilityLabel equals a specific class list is brittle. Prefer structural checks (icon rendered) and, if needed, assert presence of a semantic a11y label (e.g., “Notifications”) once you add it on the button.
116-129: Duplicate intent with prior color test.
This “Dark Mode Support” test repeats the class assertion; consider consolidating into a single test.
69-90: Add badge test via testID for clarity.
If you add testID="notification-badge", assert badge presence/absence directly instead of querying text.Example:
render(<NotificationButton onPress={mockOnPress} />); -expect(screen.getByText('5')).toBeTruthy(); +expect(screen.getByTestId('notification-badge')).toBeTruthy();And for zero:
render(<NotificationButton onPress={mockOnPress} />); -expect(screen.queryByText('0')).toBeNull(); +expect(screen.queryByTestId('notification-badge')).toBeNull();Also applies to: 81-90
src/services/bluetooth-audio.service.ts (1)
391-403: Avoid double “scan stopped” handling; clear scan timeout when stop event fires.You both rely on onStopScan and also fire a local timeout; today the timeout remains pending and triggers a second stop/log.
Apply this diff:
private handleScanStopped(): void { + if (this.scanTimeout) { + clearTimeout(this.scanTimeout); + this.scanTimeout = null; + } useBluetoothAudioStore.getState().setIsScanning(false); logger.info({ message: 'Bluetooth scan stopped', }); }Also applies to: 276-281
src/services/__tests__/bluetooth-audio.service.test.ts (3)
34-58: Augment Platform mock with Version and avoid the bare side-effect import.Add Platform.Version to support API-gated permission tests, and drop the top-level
import 'react-native'to prevent loading the real module before mocks.Apply this diff to the mock:
Platform: { OS: 'android', + Version: 33, },Then remove Line 2 (
import 'react-native';) or move all mocks above it.
91-115: Strengthen assertion: verify requested permission set and remove unused fine location in the grant map.Assert the exact array passed to requestMultiple and align the mock results for clarity.
mockRequestMultiple.mockResolvedValue({ - 'android.permission.BLUETOOTH_SCAN': 'granted', - 'android.permission.BLUETOOTH_CONNECT': 'granted', - 'android.permission.ACCESS_FINE_LOCATION': 'granted', + 'android.permission.BLUETOOTH_SCAN': 'granted', + 'android.permission.BLUETOOTH_CONNECT': 'granted', }); ... const result = await permissionPromise; expect(result).toBe(true); - expect(mockRequestMultiple).toHaveBeenCalled(); + expect(mockRequestMultiple).toHaveBeenCalledWith([ + PermissionsAndroid.PERMISSIONS.BLUETOOTH_SCAN, + PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT, + ]);
154-174: Denied permissions: add argument assertion and add a pre-Android 12 case.Also verify the requested array, and (after implementing API gating) cover API 30 with ACCESS_FINE_LOCATION.
Minimal additions:
expect(result).toBe(false); +expect(PermissionsAndroid.requestMultiple).toHaveBeenCalledWith([ + PermissionsAndroid.PERMISSIONS.BLUETOOTH_SCAN, + PermissionsAndroid.PERMISSIONS.BLUETOOTH_CONNECT, +]);I can add a new test:
it('requests ACCESS_FINE_LOCATION on API 30', async () => { (Platform as any).OS = 'android'; (Platform as any).Version = 30; const mockRequestMultiple = PermissionsAndroid.requestMultiple as jest.Mock; mockRequestMultiple.mockResolvedValue({ 'android.permission.ACCESS_FINE_LOCATION': 'granted', }); const service = bluetoothAudioService as any; const p = service.requestPermissions(); jest.advanceTimersByTime(500); await p; expect(mockRequestMultiple).toHaveBeenCalledWith([ PermissionsAndroid.PERMISSIONS.ACCESS_FINE_LOCATION, ]); });Want me to push the test along with the service change?
src/app/login/login-form.tsx (3)
71-81: Make analytics call promise-safe; current try/catch won’t catch async rejections.Wrap with
Promise.resolve(...).catch(...)(orawaitinside an async callback).- const handleServerUrlPress = useCallback(() => { - try { - trackEvent('login_server_url_pressed', { - timestamp: new Date().toISOString(), - }); - } catch (error) { - console.warn('Failed to track login server URL press analytics:', error); - } - setShowServerUrl(true); - }, [trackEvent]); + const handleServerUrlPress = useCallback(() => { + Promise.resolve( + trackEvent('login_server_url_pressed', { timestamp: new Date().toISOString() }) + ).catch((error) => { + console.warn('Failed to track login server URL press analytics:', error); + }); + setShowServerUrl(true); + }, [trackEvent]);
186-189: Add accessibility and testing hooks to the new button.Improves a11y and simplifies test selection.
- <Button className="mt-14 w-full" variant="outline" onPress={handleServerUrlPress}> + <Button + className="mt-14 w-full" + variant="outline" + onPress={handleServerUrlPress} + accessibilityRole="button" + accessibilityLabel={t('login.change_server_url')} + testID="changeServerUrlButton" + > <ButtonText className="text-sm">{t('login.change_server_url')}</ButtonText> </Button>
42-42: Type the component as React.FC for consistency with guidelines.Aligns with repo guidance.
-export const LoginForm = ({ onSubmit = () => { }, isLoading = false, error = undefined }: LoginFormProps) => { +export const LoginForm: React.FC<LoginFormProps> = ({ onSubmit = () => {}, isLoading = false, error }) => {src/app/(app)/map.tsx (1)
107-110: Remove no-op useEffect; centralize documentation elsewhere.
This effect does nothing. Drop it to reduce render overhead and align with “minimize useEffect” guideline. Also confirm that app-level tracking now surfaces permission/error feedback to the user (toast/snackbar) since the per-screen toast was removed.- useEffect(() => { - // Location tracking is now handled at the app level when user is logged in - // No need to start/stop here as it should persist across screen changes - }, []);src/app/(app)/_layout.tsx (1)
111-124: startLocationUpdates invoked only once; add user-facing error notification
- Verified no other call sites in the codebase.
- On failure, show a user-visible notification (e.g. toast/snackbar) alongside the existing log.
src/services/__tests__/location.test.ts (1)
106-106: Remove now-unused import after matcher change.
Prevents linter errors.-import { SaveUnitLocationInput } from '@/models/v4/unitLocation/saveUnitLocationInput';src/lib/hooks/use-realtime-geolocation.ts (2)
8-9: Avoid double writes to MMKV storage.
useMMKVBoolean’s setter already persists; callingsaveRealtimeGeolocationStatewrites again. Consider removing the extra write for clarity and fewer I/Os. If you keep it for consistency, note it’s redundant.-import { getRealtimeGeolocationStorageKey, saveRealtimeGeolocationState } from '../storage/realtime-geolocation'; +import { getRealtimeGeolocationStorageKey } from '../storage/realtime-geolocation';
35-45: Remove redundant save; rely on the hook setter.try { _setRealtimeGeolocationEnabled(enabled); - saveRealtimeGeolocationState(enabled); // Update the location service if the updater is registered if (locationServiceRealtimeUpdater) { await locationServiceRealtimeUpdater(enabled); }src/lib/hooks/__tests__/use-realtime-geolocation.test.ts (1)
45-58: Reset registered updater between tests to avoid cross-test coupling.
Keeps module-level state clean regardless of test order.beforeEach(() => { jest.clearAllMocks(); @@ mockUseSignalRStore.mockReturnValue({ isGeolocationHubConnected: false, connectGeolocationHub: mockConnectGeolocationHub, disconnectGeolocationHub: mockDisconnectGeolocationHub, }); }); + + afterEach(() => { + // Ensure no updater leak across tests + registerLocationServiceRealtimeUpdater(null as any); + });src/services/location.ts (3)
107-113: Background task: avoid storage read on every tick (optional).
loadRealtimeGeolocationState()per update adds I/O overhead. Cache the value with a short TTL or memoize in a module-level var and refresh periodically; background toggles will still propagate because the UI persists to storage.
188-209: DRY up background task options and ensure consistent permission checks.
- You check background permission here (good), but
updateBackgroundGeolocationSetting()registers the task without rechecking permission—mirror this guard there.- Extract the task options into a shared constant to avoid drift across call sites.
Example (outside this file’s changed lines):
// top-level const BACKGROUND_TASK_OPTIONS: Location.LocationTaskOptions = { accuracy: Location.Accuracy.Balanced, timeInterval: 15000, distanceInterval: 10, foregroundService: { notificationTitle: 'Location Tracking', notificationBody: 'Tracking your location in the background', }, };Then use
BACKGROUND_TASK_OPTIONSin both registration sites.If you want, I can generate a small patch updating both sites and adding the missing permission guard in
updateBackgroundGeolocationSetting().
227-227: Consider deduping foreground sends.Add a small “significant movement” or “min interval” gate to avoid spamming the API when jitter causes frequent foreground points with minimal movement.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
src/app/(app)/_layout.tsx(3 hunks)src/app/(app)/map.tsx(1 hunks)src/app/login/__tests__/login-form.test.tsx(1 hunks)src/app/login/login-form.tsx(5 hunks)src/components/notifications/NotificationButton.tsx(1 hunks)src/components/notifications/__tests__/NotificationButton.test.tsx(1 hunks)src/lib/hooks/__tests__/use-realtime-geolocation.test.ts(1 hunks)src/lib/hooks/use-realtime-geolocation.ts(1 hunks)src/services/__tests__/bluetooth-audio.service.test.ts(2 hunks)src/services/__tests__/location.test.ts(19 hunks)src/services/bluetooth-audio.service.ts(2 hunks)src/services/location.ts(11 hunks)src/translations/ar.json(1 hunks)src/translations/en.json(1 hunks)src/translations/es.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/translations/en.jsonsrc/translations/ar.jsonsrc/app/(app)/map.tsxsrc/lib/hooks/__tests__/use-realtime-geolocation.test.tssrc/app/login/__tests__/login-form.test.tsxsrc/components/notifications/__tests__/NotificationButton.test.tsxsrc/services/__tests__/location.test.tssrc/services/bluetooth-audio.service.tssrc/app/login/login-form.tsxsrc/components/notifications/NotificationButton.tsxsrc/translations/es.jsonsrc/services/location.tssrc/services/__tests__/bluetooth-audio.service.test.tssrc/lib/hooks/use-realtime-geolocation.tssrc/app/(app)/_layout.tsx
src/translations/**/*.json
📄 CodeRabbit inference engine (.cursorrules)
Store translation dictionary files under src/translations as JSON resources
Files:
src/translations/en.jsonsrc/translations/ar.jsonsrc/translations/es.json
src/translations/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store translation dictionary files in src/translations
Files:
src/translations/en.jsonsrc/translations/ar.jsonsrc/translations/es.json
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/app/(app)/map.tsxsrc/lib/hooks/__tests__/use-realtime-geolocation.test.tssrc/app/login/__tests__/login-form.test.tsxsrc/components/notifications/__tests__/NotificationButton.test.tsxsrc/services/__tests__/location.test.tssrc/services/bluetooth-audio.service.tssrc/app/login/login-form.tsxsrc/components/notifications/NotificationButton.tsxsrc/services/location.tssrc/services/__tests__/bluetooth-audio.service.test.tssrc/lib/hooks/use-realtime-geolocation.tssrc/app/(app)/_layout.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/app/(app)/map.tsxsrc/app/login/__tests__/login-form.test.tsxsrc/components/notifications/__tests__/NotificationButton.test.tsxsrc/app/login/login-form.tsxsrc/components/notifications/NotificationButton.tsxsrc/app/(app)/_layout.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/lib/hooks/__tests__/use-realtime-geolocation.test.tssrc/app/login/__tests__/login-form.test.tsxsrc/components/notifications/__tests__/NotificationButton.test.tsxsrc/services/__tests__/location.test.tssrc/services/__tests__/bluetooth-audio.service.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/lib/hooks/__tests__/use-realtime-geolocation.test.tssrc/app/login/__tests__/login-form.test.tsxsrc/components/notifications/__tests__/NotificationButton.test.tsxsrc/services/__tests__/location.test.tssrc/services/__tests__/bluetooth-audio.service.test.ts
🧠 Learnings (3)
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps, mapping, or vehicle navigation
Applied to files:
src/app/(app)/map.tsx
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or navigation features
Applied to files:
src/app/(app)/map.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to src/**/*.test.{ts,tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/app/login/__tests__/login-form.test.tsx
🧬 Code graph analysis (8)
src/lib/hooks/__tests__/use-realtime-geolocation.test.ts (2)
src/lib/storage/realtime-geolocation.ts (1)
saveRealtimeGeolocationState(30-32)src/lib/hooks/use-realtime-geolocation.ts (2)
useRealtimeGeolocation(30-75)registerLocationServiceRealtimeUpdater(20-22)
src/app/login/__tests__/login-form.test.tsx (1)
src/app/login/login-form.tsx (1)
LoginForm(42-195)
src/components/notifications/__tests__/NotificationButton.test.tsx (1)
src/components/notifications/NotificationButton.tsx (1)
NotificationButton(11-35)
src/services/__tests__/location.test.ts (4)
src/lib/hooks/use-realtime-geolocation.ts (1)
registerLocationServiceRealtimeUpdater(20-22)src/lib/storage/background-geolocation.ts (1)
loadBackgroundGeolocationState(16-31)src/lib/storage/realtime-geolocation.ts (1)
loadRealtimeGeolocationState(10-25)src/services/location.ts (1)
locationService(366-366)
src/services/location.ts (2)
src/lib/storage/realtime-geolocation.ts (1)
loadRealtimeGeolocationState(10-25)src/lib/hooks/use-realtime-geolocation.ts (1)
registerLocationServiceRealtimeUpdater(20-22)
src/services/__tests__/bluetooth-audio.service.test.ts (1)
src/services/bluetooth-audio.service.ts (1)
bluetoothAudioService(1686-1686)
src/lib/hooks/use-realtime-geolocation.ts (2)
src/lib/storage/realtime-geolocation.ts (2)
getRealtimeGeolocationStorageKey(37-39)saveRealtimeGeolocationState(30-32)src/lib/storage/index.tsx (1)
storage(5-5)
src/app/(app)/_layout.tsx (1)
src/services/location.ts (1)
locationService(366-366)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (15)
src/components/notifications/NotificationButton.tsx (1)
25-25: Icon color swap aligns with dark/light contrast — LGTM.
The new gray palette improves contrast and adheres to dark-mode guidelines.src/services/bluetooth-audio.service.ts (1)
85-90: Good: initialize only after permissions.Deferring BleManager.start and listener setup until permissions are granted is correct and avoids spurious native errors.
src/services/__tests__/bluetooth-audio.service.test.ts (3)
65-70: Timers: OK.Using fake timers to drive the startup delay is correct; restoring after each test prevents leakage.
116-136: iOS path: OK.Bypassing PermissionsAndroid and returning true (after the delay) is fine; you also reset Platform.OS for subsequent tests.
137-153: Failure path: OK.Reject → false is correct and observable with the current logger.
src/translations/en.json (2)
526-526: LGTM: new key added and used by UI.Key
login.change_server_urllooks correct and consistent with settings terminology.
526-526: All locale files includelogin.change_server_url. Verification script exited cleanly with no missing keys—no further action needed.src/translations/es.json (1)
526-526: LGTM: Spanish translation is clear and idiomatic.
Cambiar URL del servidormatches the English intent.src/translations/ar.json (1)
526-526: LGTM: Arabic translation added.Key aligns with English. Consider future RTL layout checks when this label appears in buttons.
src/services/__tests__/location.test.ts (1)
374-377: Keep the existing matcher; SaveUnitLocationInput is a runtime class
TheSaveUnitLocationInputsymbol is exported as a class (not erased), soexpect.any(SaveUnitLocationInput)will work as intended.src/lib/hooks/__tests__/use-realtime-geolocation.test.ts (1)
1-171: Solid coverage and behavior assertions.
Mocks and flows exercise enable/disable, updater registration, error paths, and logger outputs well.src/services/location.ts (4)
9-13: Realtime updater wiring looks good.Registering
updateRealtimeGeolocationSettingvia the hook keeps the service reactive to runtime changes without circular deps.
123-130: Realtime state field and updater registration: LGTM.Keeps foreground watcher aware of runtime setting changes.
181-184: Loading both background and realtime settings on start: LGTM.Keeps runtime flags in sync on boot.
233-236: Startup log context: LGTM.Helpful visibility for toggles after boot.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/app/login/login-form.tsx (1)
21-32: Localize validation: use translation keys and render via t().Avoid hardcoded English in schema; surface keys and translate at render.
const loginFormSchema = z.object({ username: z .string({ - required_error: 'Username is required', + required_error: 'form.required', }) - .min(3, 'Username must be at least 3 characters'), + .min(3, { message: 'login.username_min_length' }), password: z .string({ - required_error: 'Password is required', + required_error: 'form.required', }) - .min(6, 'Password must be at least 6 characters'), + .min(6, { message: 'login.password_min_length' }), });- <FormControlErrorText className="text-red-500">{errors?.username?.message}</FormControlErrorText> + <FormControlErrorText className="text-red-500"> + {errors?.username?.message ? t(errors.username.message as string) : null} + </FormControlErrorText>- <FormControlErrorText className="text-red-500">{errors?.password?.message}</FormControlErrorText> + <FormControlErrorText className="text-red-500"> + {errors?.password?.message ? t(errors.password.message as string) : null} + </FormControlErrorText>Also applies to: 112-113, 145-146
src/translations/es.json (1)
520-539: Add validation strings for new schema keys.Required by the localized zod messages.
"login": { "change_server_url": "Cambiar URL del servidor", @@ "password": "Contraseña", "password_incorrect": "La contraseña era incorrecta", "password_placeholder": "Introduce tu contraseña", + "password_min_length": "La contraseña debe tener al menos 6 caracteres", "title": "Iniciar sesión", "username": "Nombre de usuario", - "username_placeholder": "Introduce tu nombre de usuario" + "username_placeholder": "Introduce tu nombre de usuario", + "username_min_length": "El nombre de usuario debe tener al menos 3 caracteres" },src/services/location.ts (1)
279-306: Reflect background-enabled state in the store when enabling.
You flip isBackgroundGeolocationEnabled and register the task but never set locationStore.setBackgroundEnabled(true). Set it after permission grant so UI/state stay consistent.if (enabled) { // Request background permissions when enabling background geolocation const { status: backgroundStatus } = await Location.requestBackgroundPermissionsAsync(); if (backgroundStatus !== 'granted') { logger.warn({ message: 'Background location permission not granted, cannot enable background geolocation', context: { backgroundStatus }, }); this.isBackgroundGeolocationEnabled = false; return; } + // Reflect background tracking enabled in app state + useLocationStore.getState().setBackgroundEnabled(true);
♻️ Duplicate comments (2)
src/app/login/__tests__/login-form.test.tsx (1)
1-12: Good: Top-level mocks and user interaction assertions address prior feedback.Mocks are now effective; the button press triggers analytics and opens the sheet. This directly verifies the workflow.
Also applies to: 29-45, 61-67, 69-88
src/services/location.ts (1)
45-50: PII-safe logging on realtime-disabled path — good.
You removed coordinates from the disabled branch as previously requested.
🧹 Nitpick comments (12)
src/app/login/__tests__/login-form.test.tsx (3)
55-59: Remove redundant mockClear calls.jest.clearAllMocks() already clears all jest.fn calls. The extra mockClear calls are unnecessary.
beforeEach(() => { jest.clearAllMocks(); - mockTrackEvent.mockClear(); - mockServerUrlBottomSheet.mockClear(); });
69-88: Add a test to assert onClose closes the sheet.This verifies the callback wiring.
it('should track analytics and show bottom sheet when server URL button is pressed', () => { render(<LoginForm onSubmit={mockOnSubmit} />); @@ ); }); + + it('closes the ServerUrlBottomSheet when onClose is invoked', () => { + render(<LoginForm onSubmit={mockOnSubmit} />); + fireEvent.press(screen.getByText('login.change_server_url')); + const openProps = mockServerUrlBottomSheet.mock.calls.at(-1)?.[0]; + expect(openProps?.isOpen).toBe(true); + // trigger close + openProps?.onClose(); + const closedProps = mockServerUrlBottomSheet.mock.calls.at(-1)?.[0]; + expect(closedProps?.isOpen).toBe(false); + });
90-116: Add submit test with valid credentials; prune unused error-prop rerender.Covers the happy path; avoids relying on an unused prop.
it('should render with loading state', () => { render(<LoginForm onSubmit={mockOnSubmit} isLoading={true} />); expect(screen.getByText('login.login_button_loading')).toBeTruthy(); }); - it('should render with different prop combinations', () => { - const { rerender } = render( - <LoginForm - onSubmit={mockOnSubmit} - isLoading={false} - error={undefined} - /> - ); - expect(screen.getByText('login.title')).toBeTruthy(); - - // Test with error - rerender( - <LoginForm - onSubmit={mockOnSubmit} - error="Test error" - /> - ); - expect(screen.getByText('login.title')).toBeTruthy(); - }); + it('submits when fields are valid', () => { + render(<LoginForm onSubmit={mockOnSubmit} />); + fireEvent.changeText(screen.getByPlaceholderText('login.username_placeholder'), 'user123'); + fireEvent.changeText(screen.getByPlaceholderText('login.password_placeholder'), 'secret123'); + fireEvent.press(screen.getByText('login.login_button')); + expect(mockOnSubmit).toHaveBeenCalledWith( + expect.objectContaining({ username: 'user123', password: 'secret123' }), + expect.anything() + ); + });src/app/login/login-form.tsx (4)
36-42: Drop unused error prop; type the component as React.FC.error isn’t rendered anymore; remove it to avoid confusion and align with guidelines (use React.FC).
export type LoginFormProps = { onSubmit?: SubmitHandler<FormType>; isLoading?: boolean; - error?: string; }; -export const LoginForm = ({ onSubmit = () => { }, isLoading = false, error = undefined }: LoginFormProps) => { +export const LoginForm: React.FC<LoginFormProps> = ({ onSubmit = () => {}, isLoading = false }) => {
57-61: Stabilize handlers; avoid creating functions during render.Memoize toggles and submit handler to reduce re-renders and adhere to handler guidelines.
- const handleState = () => { - setShowPassword((showState) => { - return !showState; - }); - }; + const togglePasswordVisibility = useCallback(() => { + setShowPassword((prev) => !prev); + }, []);- <InputSlot onPress={handleState} className="pr-3"> + <InputSlot onPress={togglePasswordVisibility} className="pr-3">+ const onLoginPress = useCallback(() => handleSubmit(onSubmit)(), [handleSubmit, onSubmit]);- <Button className="mt-8 w-full" variant="solid" action="primary" onPress={handleSubmit(onSubmit)}> + <Button className="mt-8 w-full" variant="solid" action="primary" onPress={onLoginPress}> <ButtonText>{t('login.login_button')}</ButtonText> </Button>Also applies to: 137-139, 149-158, 66-66, 77-77
101-106: Use platform autoComplete hints.Improve UX by using specific autocomplete modes.
- autoComplete="off" + autoComplete="username"- autoComplete="off" + autoComplete="password"Also applies to: 133-136
42-42: Fix linter warning about stray whitespace.Line 42: remove the extra space flagged by the test check.
src/services/__tests__/location.test.ts (2)
152-164: Construction-time updater registrations verified.
Lightweight boolean gate works here; consider asserting exact call counts in the dedicated test (see comment below) to avoid reliance on beforeAll state.- expect(registrationCallsVerified).toBe(true); + expect(mockRegisterLocationServiceUpdater).toHaveBeenCalledTimes(1); + expect(mockRegisterLocationServiceRealtimeUpdater).toHaveBeenCalledTimes(1);
258-274: Add regression guard: ensure no background prompt during foreground-only permission flow.
Requesting background permission here is irrelevant; assert it wasn’t called to prevent future regressions.const result = await locationService.requestPermissions(); expect(result).toBe(true); + expect(mockLocation.requestBackgroundPermissionsAsync).not.toHaveBeenCalled();src/services/location.ts (3)
265-274: Avoid awaiting a sync function (saveRealtimeGeolocationState).
saveRealtimeGeolocationState returns void; awaiting it is misleading and may trigger lint warnings. Either remove await or make the saver async.- await saveRealtimeGeolocationState(enabled); + saveRealtimeGeolocationState(enabled);Alternatively, change saveRealtimeGeolocationState to return Promise and keep await.
63-67: Tighten numeric validation to avoid coercion edge cases.
Using global isFinite can coerce strings; prefer typeof + Number.isFinite to keep types strict.// Suggested helper update (outside this hunk): const safeNumericString = (value: number | null | undefined, field: string): string => { if (typeof value !== 'number' || !Number.isFinite(value)) { logger.warn({ message: `Invalid ${field} value: ${value}, defaulting to '0'` }); return '0'; } return String(value); };
112-118: Process all reported locations from TaskManager, not just the first.
Expo can batch multiple points; currently only locations[0] is handled. Iterate to avoid drops.// Suggested change (broader than this hunk): const { locations } = data as { locations: Location.LocationObject[] }; for (const loc of locations) { logger.info({ message: 'Background location update received', context: { latitude: loc.coords.latitude, longitude: loc.coords.longitude, heading: loc.coords.heading }, }); useLocationStore.getState().setLocation(loc); const isRealtimeEnabled = await loadRealtimeGeolocationState(); await sendLocationToAPI(loc, isRealtimeEnabled); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
src/app/(app)/_layout.tsx(3 hunks)src/app/login/__tests__/login-form.test.tsx(1 hunks)src/app/login/login-form.tsx(6 hunks)src/components/notifications/NotificationButton.tsx(3 hunks)src/components/notifications/__tests__/NotificationButton.test.tsx(1 hunks)src/services/__tests__/location.test.ts(20 hunks)src/services/location.ts(8 hunks)src/translations/ar.json(2 hunks)src/translations/en.json(2 hunks)src/translations/es.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/translations/ar.json
- src/components/notifications/tests/NotificationButton.test.tsx
- src/translations/en.json
- src/app/(app)/_layout.tsx
- src/components/notifications/NotificationButton.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/app/login/__tests__/login-form.test.tsxsrc/services/location.tssrc/app/login/login-form.tsxsrc/services/__tests__/location.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/app/login/__tests__/login-form.test.tsxsrc/app/login/login-form.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/app/login/__tests__/login-form.test.tsxsrc/services/location.tssrc/app/login/login-form.tsxsrc/translations/es.jsonsrc/services/__tests__/location.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/app/login/__tests__/login-form.test.tsxsrc/services/__tests__/location.test.ts
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/app/login/__tests__/login-form.test.tsxsrc/services/__tests__/location.test.ts
src/translations/**/*.json
📄 CodeRabbit inference engine (.cursorrules)
Store translation dictionary files under src/translations as JSON resources
Files:
src/translations/es.json
src/translations/**/*.{ts,tsx,json}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Store translation dictionary files in src/translations
Files:
src/translations/es.json
🧬 Code graph analysis (2)
src/services/location.ts (3)
src/lib/storage/realtime-geolocation.ts (2)
loadRealtimeGeolocationState(10-25)saveRealtimeGeolocationState(30-32)src/lib/hooks/use-realtime-geolocation.ts (1)
registerLocationServiceRealtimeUpdater(20-22)src/lib/storage/background-geolocation.ts (1)
loadBackgroundGeolocationState(16-31)
src/services/__tests__/location.test.ts (4)
src/lib/hooks/use-realtime-geolocation.ts (1)
registerLocationServiceRealtimeUpdater(20-22)src/lib/storage/background-geolocation.ts (1)
loadBackgroundGeolocationState(16-31)src/lib/storage/realtime-geolocation.ts (2)
loadRealtimeGeolocationState(10-25)saveRealtimeGeolocationState(30-32)src/services/location.ts (2)
locationService(352-352)sendLocationToAPI(355-355)
🪛 GitHub Check: test
src/app/login/login-form.tsx
[warning] 42-42:
Delete ·
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
src/translations/es.json (1)
521-539: LGTM: Added login.change_server_url and notifications keys.Translations read well and align with usage in the LoginForm and settings.
Also applies to: 724-729
src/services/__tests__/location.test.ts (5)
19-21: Mocks align with new realtime + background flows — good coverage.
Realtime updater/storage and getBackgroundPermissionsAsync are mocked correctly; logger.debug added for new paths.Also applies to: 27-27, 33-36, 64-65, 71-71
347-365: Background task registration gating — LGTM.
Correctly skips registration when background permission is denied and logs a warning.
367-381: Foreground path no longer sends to API — LGTM.
Store update only, with explicit “TaskManager handles API sends” expectation.
441-446: Stop background updates path — LGTM.
State flip only (task handled elsewhere) matches the refactor.
677-699: Realtime setting persistence tests — LGTM.
Verifies in-memory flip and persisted state write.
src/services/location.ts
Outdated
| // Always update local store regardless of realtime setting | ||
| useLocationStore.getState().setLocation(location); | ||
|
|
||
| // Only send to API if realtime geolocation is enabled | ||
| if (!isRealtimeEnabled) { | ||
| logger.debug({ | ||
| message: 'Realtime geolocation disabled, skipping API call', | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Remove duplicate local store updates.
sendLocationToAPI updates the store, and the TaskManager handler also updates it before calling sendLocationToAPI. This double-writes the same point. Make sendLocationToAPI send-only.
- // Always update local store regardless of realtime setting
- useLocationStore.getState().setLocation(location);
+ // Caller is responsible for store updates to avoid duplicatesAlso applies to: 112-118
🤖 Prompt for AI Agents
In src/services/location.ts around lines 41-50 (also apply same change at
112-118): sendLocationToAPI currently writes the location into the local store,
causing a duplicate update because callers (TaskManager handler) already set the
store before calling; remove the local store write from sendLocationToAPI so it
becomes send-only (delete the useLocationStore.getState().setLocation(location)
calls in both places), keep the realtime-enabled check and API request logic
intact, and ensure callers remain responsible for updating the local store
before invoking sendLocationToAPI.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit