-
Notifications
You must be signed in to change notification settings - Fork 4
CU-868ffcgaz Fixing unit tests broken by Expo 53 update #169
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Task linked: CU-868ffcgaz Update to Expo 53 |
WalkthroughIntroduces extensive Jest mocks and test refactors across multiple suites, updates CI workflow gating and jq installation, adjusts TypeScript types (timers, refs), adds minimal UI code tweaks (no-op prop spreads, cssInterop), updates i18n locale sourcing, modifies tsconfig include/module resolution, and adds Gluestack UI ambient type declarations. Changes
Sequence Diagram(s)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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ 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)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
__mocks__/expo-task-manager.ts (1)
1-22: Remove location-tracking mocks from expo-task-manager and import Jest
In__mocks__/expo-task-manager.ts, delete thestartLocationTrackingAsync,stopLocationTrackingAsync, andhasStartedLocationTrackingAsyncexports, addimport { jest } from '@jest/globals';at the top, and update the default
TaskManagerto only includedefineTask,getRegisteredTasksAsync,isTaskRegisteredAsync,unregisterTaskAsync, andunregisterAllTasksAsync. Then verify that__mocks__/expo-location.tsexportsstartLocationUpdatesAsync,stopLocationUpdatesAsync, andhasStartedLocationUpdatesAsync..github/workflows/react-native-cicd.yml (2)
257-268: Artifact upload may fail when files don’t exist for the matrix platform.Either split per-platform or ignore missing files.
Option A (ignore missing):
- name: 📦 Upload build artifacts to GitHub uses: actions/upload-artifact@v4 with: name: app-builds-${{ matrix.platform }} path: | ./ResgridUnit-dev.apk ./ResgridUnit-prod.apk ./ResgridUnit-prod.aab ./ResgridUnit-ios-dev.ipa ./ResgridUnit-ios-adhoc.ipa ./ResgridUnit-ios-prod.ipa + if-no-files-found: ignore retention-days: 7Option B: two conditional upload steps (one for android, one for ios).
154-181: CastversionCodeto a number in the CI script
Replace the secondjqinvocation with atonumberconversion soversionCoderemains an integer, for example:jq '.versionCode = ( "7${{ github.run_number }}" | tonumber )' package.json > package.json.tmp && mv package.json.tmp package.jsonsrc/components/notifications/__tests__/NotificationInbox.test.tsx (1)
442-460: Tests call deleteMessage directly; this doesn’t validate component behavior.Drive deletion through the UI: select items, press bulk delete, confirm, and assert API call + toast.
Sketch:
render(<NotificationInbox isOpen onClose={mockOnClose} />); // enter selection, select items, press bulk delete, confirm await waitFor(() => expect(mockDeleteMessage).toHaveBeenCalledWith('1')); expect(mockShowToast).toHaveBeenCalled();src/stores/status/__tests__/store.test.ts (1)
349-355: Assert the thrown error explicitly to prevent false positives.Currently the try/catch swallows failures if nothing throws.
-await act(async () => { - try { - await result.current.saveUnitStatus(input); - } catch (error) { - // Expected to throw now since we re-throw critical errors - } -}); +await act(async () => { + await expect(result.current.saveUnitStatus(input)).rejects.toThrow('Critical error'); +});
🧹 Nitpick comments (42)
src/stores/calls/__tests__/detail-store.test.ts (2)
2-8: Avoid duplicating Platform mocks; prefer the global setup.Given the repo already has jest-platform-setup.ts, in-file Platform mocks can drift and cause order/hoisting surprises. Rely on the global setup or consolidate to a single place.
-// Mock Platform first before any imports -jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn((specifics: any) => specifics.ios || specifics.default), - Version: 17, - }, -})); +// Prefer global Platform mock in jest-platform-setup.ts
11-18: Centralize MMKV mock to avoid divergence across suites.This local MMKV mock is fine, but it’s better as a manual mock in mocks/react-native-mmkv.ts so all tests share the same behavior.
-// Mock MMKV storage -jest.mock('react-native-mmkv', () => ({ - MMKV: jest.fn().mockImplementation(() => ({ - set: jest.fn(), - getString: jest.fn(), - delete: jest.fn(), - })), - useMMKVBoolean: jest.fn(() => [false, jest.fn()]), -})); +// Use a shared manual mock in __mocks__/react-native-mmkv.ts to keep behavior consistent__mocks__/react-native-edge-to-edge.ts (1)
1-4: Import jest globals to satisfy TS and avoid “jest is not defined”.TypeScript may flag jest.fn without types configured. Importing from @jest/globals avoids build time errors.
+import { jest } from '@jest/globals'; export const SystemBars = ({ style, hidden }: any) => null; export const setStatusBarStyle = jest.fn(); export const setNavigationBarStyle = jest.fn();If the real library exports additional members (e.g., providers/hooks), mirror them here to prevent partial-mock surprises.
src/components/sidebar/__tests__/unit-sidebar-minimal.test.tsx (1)
4-15: Avoid per-test Platform mocks; rely on global jest-platform-setup or move mock truly first.This duplicates the global Platform mock and can drift. If you must keep a local override, ensure it executes before any imports (including RTL), otherwise hoisting nuances may still load Platform early.
Suggested: delete this local mock and rely on the global setup.
-// Mock Platform first before any other imports -const mockPlatform = { - OS: 'ios' as const, - select: jest.fn().mockImplementation((obj: any) => obj.ios || obj.default), - Version: 17, - constants: {}, - isTesting: true, -}; - -// Mock react-native Platform -jest.mock('react-native/Libraries/Utilities/Platform', () => mockPlatform); +// Platform mocking handled in jest-platform-setup.tsRun the test suite locally to confirm no regressions after removing this duplicate mock.
.github/workflows/react-native-cicd.yml (2)
156-166: jq install block: refine OS handling and make script fail-fast.Minor robustness tweaks and clearer OS branching.
-# Ensure jq exists on both Linux and macOS -if ! command -v jq >/dev/null 2>&1; then - echo "Installing jq..." - if [[ "$RUNNER_OS" == "Linux" ]]; then - sudo apt-get update && sudo apt-get install -y jq - elif [[ "$RUNNER_OS" == "macOS" ]]; then - brew update && brew install jq - else - echo "Unsupported OS for auto-install of jq" >&2; exit 1 - fi -fi +set -euo pipefail +# Ensure jq exists on both Linux and macOS +if ! command -v jq >/dev/null 2>&1; then + echo "Installing jq..." + case "$RUNNER_OS" in + Linux) sudo apt-get update -y && sudo apt-get install -y jq ;; + macOS) brew update && brew install jq ;; + *) echo "Unsupported OS for auto-install of jq" >&2; exit 1 ;; + esac +fiConfirm the actual values of RUNNER_OS on macos-15 and ubuntu-latest runners (expect “macOS” and “Linux”).
196-199: Tie iOS key filename to EXPO_ASC_KEY_ID and gate to iOS.Avoid hardcoding and unnecessary secret writes on Android jobs.
Example:
- name: 📋 Create iOS Cert if: ${{ matrix.platform == 'ios' }} run: | : "${EXPO_ASC_KEY_ID:?missing}" echo "$UNIT_IOS_CERT" | base64 -d > "AuthKey_${EXPO_ASC_KEY_ID}.p8"__mocks__/expo-device.ts (1)
1-31: Add ES module interop and minimal enums for safer imports.Some codepaths import default or refer to DeviceType. Improve compatibility.
export const isDevice = true; @@ export const manufacturer = 'Apple'; +export enum DeviceType { + UNKNOWN = 0, + PHONE = 1, + TABLET = 2, + DESKTOP = 3, + TV = 4, +} + export default { + __esModule: true as const, isDevice, deviceName, @@ deviceType, manufacturer, + DeviceType, };If your code imports expo-device as a namespace (import * as Device), this change maintains parity; validate against usage sites.
src/components/notifications/__tests__/NotificationInbox.test.tsx (2)
345-363: “Loading state” test doesn’t assert loading UI.It only checks container presence. Assert a loading indicator/text that the real component renders during isLoading.
Example follow-up:
const { getByTestId } = render(<NotificationInbox isOpen onClose={mockOnClose} />); expect(getByTestId('notification-loading')).toBeTruthy();
253-261: General: solid coverage of list/selection interactions — once run against the real component.After removing the SUT mock, keep these scenarios but align selectors to the production UI.
Also applies to: 264-277, 280-299, 301-320, 322-343, 402-416, 418-440
src/services/__tests__/push-notification.test.ts (2)
115-129: Type the notification factory and use nullish coalescing for clarity.Avoid any and make intent explicit for undefined/null normalization.
- const createMockNotification = (data: any): Notifications.Notification => + type MockNotificationData = { + title?: string | null; + body?: string | null; + data?: Record<string, unknown> | null; + }; + const createMockNotification = (data: MockNotificationData): Notifications.Notification => ({ date: Date.now(), request: { identifier: 'test-id', content: { - title: data.title || null, + title: data.title ?? null, subtitle: null, - body: data.body || null, - data: data.data || {}, + body: data.body ?? null, + data: data.data ?? {}, sound: null, }, trigger: null, }, } as Notifications.Notification);
12-18: Replace inline expo-device mock with centralized manual mock
Insrc/services/__tests__/push-notification.test.ts, remove the inlinejest.mock('expo-device', …)block (lines 12–18) so that Jest uses your existing__mocks__/expo-device.tsmanual mock:- jest.mock('expo-device', () => ({ - isDevice: true, - deviceName: 'Test Device', - osName: 'iOS', - osVersion: '15.0', - }));This ensures all suites share the same mock and prevents drift.
__mocks__/expo-av.ts (1)
4-28: Add a few commonly-used Audio.Sound methods to future-proof the mock.Many codepaths call getStatusAsync, setOnPlaybackStatusUpdate, pauseAsync, or replayAsync. Stubbing them prevents surprising test failures later.
Sound: class MockSound { static createAsync = jest.fn().mockResolvedValue({ sound: new this(), status: { isLoaded: true }, }); playAsync = jest.fn().mockResolvedValue({ status: { isPlaying: true } }); + pauseAsync = jest.fn().mockResolvedValue({ status: { isPlaying: false } }); + replayAsync = jest.fn().mockResolvedValue({ status: { isPlaying: true } }); stopAsync = jest.fn().mockResolvedValue({ status: { isPlaying: false } }); unloadAsync = jest.fn().mockResolvedValue(undefined); setVolumeAsync = jest.fn().mockResolvedValue(undefined); + getStatusAsync = jest.fn().mockResolvedValue({ isLoaded: true, isPlaying: false }); + setOnPlaybackStatusUpdate = jest.fn(); },__mocks__/expo-navigation-bar.ts (1)
1-10: Optional: expose a default aggregate for wildcard imports.Some codebases use import * as NavigationBar from 'expo-navigation-bar'. Providing a default export avoids ESM interop edge cases in tests.
export const setVisibilityAsync = jest.fn().mockResolvedValue(undefined); export const getVisibilityAsync = jest.fn().mockResolvedValue('visible'); export const setBackgroundColorAsync = jest.fn().mockResolvedValue(undefined); export const getBackgroundColorAsync = jest.fn().mockResolvedValue('#000000'); export const setBehaviorAsync = jest.fn().mockResolvedValue(undefined); export const getBehaviorAsync = jest.fn().mockResolvedValue('overlay-swipe'); export const setButtonStyleAsync = jest.fn().mockResolvedValue(undefined); export const getButtonStyleAsync = jest.fn().mockResolvedValue('light'); export const setPositionAsync = jest.fn().mockResolvedValue(undefined); export const getPositionAsync = jest.fn().mockResolvedValue('bottom'); + +export default { + setVisibilityAsync, + getVisibilityAsync, + setBackgroundColorAsync, + getBackgroundColorAsync, + setBehaviorAsync, + getBehaviorAsync, + setButtonStyleAsync, + getButtonStyleAsync, + setPositionAsync, + getPositionAsync, +};src/components/status/__tests__/status-bottom-sheet.test.tsx (4)
7-24: Use a single actionsheet mock (prefer the alias path used by the component).Mocking both '../../ui/actionsheet' and '@/components/ui/actionsheet' increases maintenance and risks divergence. Keep only the alias-based mock that the component imports.
-// Mock all UI components based on actual imports -jest.mock('../../ui/actionsheet', () => { - const mockReact = require('react'); - const { View } = require('react-native'); - - return { - Actionsheet: ({ children, isOpen }: any) => - isOpen ? mockReact.createElement(View, { testID: 'actionsheet' }, children) : null, - ActionsheetBackdrop: ({ children }: any) => - mockReact.createElement(View, { testID: 'actionsheet-backdrop' }, children), - ActionsheetContent: ({ children }: any) => - mockReact.createElement(View, { testID: 'actionsheet-content' }, children), - ActionsheetDragIndicator: () => - mockReact.createElement(View, { testID: 'actionsheet-drag-indicator' }), - ActionsheetDragIndicatorWrapper: ({ children }: any) => - mockReact.createElement(View, { testID: 'actionsheet-drag-indicator-wrapper' }, children), - }; -});Also applies to: 253-274
809-819: Make submit button selection less brittle.Scanning all buttons and traversing Text children is fragile. Prefer an explicit testID (e.g., submit-button) or accessibilityLabel for direct lookup and assertion.
-const buttons = screen.getAllByTestId('button'); -const submitButton = buttons.find(button => { - try { - const textElements = button.findAllByType('Text'); - return textElements.some(text => text.props.children === 'Submit'); - } catch (e) { - return false; - } -}); -expect(submitButton?.props.accessibilityState?.disabled).toBe(true); +const submit = screen.getByA11yLabel?.('Submit') ?? screen.getByTestId('submit-button'); +expect(submit.props.accessibilityState?.disabled).toBe(true);If helpful, adjust the Button mock to set accessibilityLabel from its children:
-Button: ({ children, onPress, isDisabled, className, ...props }: any) => +Button: ({ children, onPress, isDisabled, className, ...props }: any) => mockReact.createElement(TouchableOpacity, { onPress: isDisabled ? undefined : onPress, testID: 'button', + accessibilityLabel: typeof children === 'string' ? children : undefined, accessibilityState: { disabled: isDisabled }, ...props }, children),
2704-2711: waitFor usage doesn’t assert completion.await waitFor(() => slowSaveUnitStatus); will pass immediately because the function reference is truthy. Assert the call or resolution.
-// Wait for the operation to complete -await waitFor(() => slowSaveUnitStatus); +// Wait for the operation to complete +await waitFor(() => expect(slowSaveUnitStatus).toHaveBeenCalled());
1-177: Centralize repeated mocks in your global setup
Extract the duplicated UI (Actionsheet, Button, Text, Spinner, Textarea, etc.), storage (MMKV), i18n (react-i18next), and styling (nativewind) mocks from individual tests into yourjest-setup.ts(or a shared helper) to reduce per-test noise and prevent drift.__mocks__/expo-location.ts (1)
32-43: Make timestamp dynamic per call to avoid stale datamockResolvedValue captures Date.now() once at module load. Switch to mockImplementation so each call returns a fresh timestamp.
-export const getCurrentPositionAsync = jest.fn().mockResolvedValue({ - coords: { - latitude: 40.7128, - longitude: -74.006, - altitude: null, - accuracy: 5, - altitudeAccuracy: null, - heading: null, - speed: null, - }, - timestamp: Date.now(), -}); +export const getCurrentPositionAsync = jest.fn().mockImplementation(async () => ({ + coords: { + latitude: 40.7128, + longitude: -74.006, + altitude: null, + accuracy: 5, + altitudeAccuracy: null, + heading: null, + speed: null, + }, + timestamp: Date.now(), +}));src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx (4)
165-173: Brittle displayName assertionReact.memo components often have displayName undefined or “Memo(...)”. Either set displayName in the component or relax the test.
Option A (set in component file):
// in src/components/settings/unit-selection-bottom-sheet.tsx export const UnitSelectionBottomSheet = React.memo<UnitSelectionBottomSheetProps>(/* ... */); +UnitSelectionBottomSheet.displayName = 'UnitSelectionBottomSheet';Option B (relax test):
- expect(UnitSelectionBottomSheet.displayName).toBe('UnitSelectionBottomSheet'); + expect(UnitSelectionBottomSheet.displayName ?? '').toContain('UnitSelectionBottomSheet');
498-517: Use fake timers to avoid real setTimeout-based flakinessThis test depends on wall-clock timing. Switch to fake timers for determinism.
it('does not close when loading', async () => { - // Make setActiveUnit slow to simulate loading state + // Make setActiveUnit slow to simulate loading state mockSetActiveUnit.mockImplementation(() => new Promise((resolve) => setTimeout(resolve, 100))); + jest.useFakeTimers(); render(<UnitSelectionBottomSheet {...mockProps} />); const unitToSelect = screen.getByTestId('unit-item-2'); await act(async () => { fireEvent.press(unitToSelect); }); - // During loading state, pressing cancel should not close + // During loading state, pressing cancel should not close const cancelButton = screen.getByText('Cancel'); fireEvent.press(cancelButton); - // onClose should not be called while loading - await new Promise((resolve) => setTimeout(resolve, 50)); // Wait a bit but not long enough for the async operation + // onClose should not be called while loading + act(() => { + jest.advanceTimersByTime(50); + }); expect(mockProps.onClose).not.toHaveBeenCalled(); + jest.useRealTimers(); });
533-545: Tighten assertions for list/group renderinggetAllByText returns an array; assert lengths for stronger guarantees.
- expect(screen.getAllByText('Engine')).toBeTruthy(); - expect(screen.getAllByText('Ladder')).toBeTruthy(); - expect(screen.getAllByText('Rescue')).toBeTruthy(); + expect(screen.getAllByText('Engine').length).toBeGreaterThan(0); + expect(screen.getAllByText('Ladder').length).toBeGreaterThan(0); + expect(screen.getAllByText('Rescue').length).toBeGreaterThan(0);
18-32: Centralize store and logger mocks with explicit shapesDirectly mutating auto-mocked modules (e.g., useRolesStore.getState) is fragile. Provide factory mocks with the needed API to reduce surprises.
Example:
-jest.mock('@/stores/roles/store'); +jest.mock('@/stores/roles/store', () => ({ + useRolesStore: { + getState: jest.fn(() => ({ fetchRolesForUnit: jest.fn() })), + }, +}));src/stores/contacts/__tests__/store.test.ts (2)
5-6: Minor typing nit: avoid any in Platform.selectUse a generic to keep types while staying lightweight.
- select: jest.fn((specifics: any) => specifics.ios || specifics.default), + select: jest.fn(<T extends Record<string, unknown>>(specifics: T) => (specifics as any).ios ?? (specifics as any).default),
179-193: Optional: wrap async fetch in act and await to silence act warningsThis keeps React state updates contained and deterministic.
- act(() => { - result.current.fetchContacts(); - }); + await act(async () => { + const p = result.current.fetchContacts(); + // Loading should flip immediately + expect(result.current.isLoading).toBe(true); + await p; + });src/components/ui/__tests__/focus-aware-status-bar.test.tsx (2)
5-21: Smoke-only tests: add at least one render-based behavior test (focus + platform + web null).These checks don’t execute hooks or side-effects. Add one iOS/Android render test and a web null-case to cover the real behavior.
Example additions:
+import { render } from '@testing-library/react-native'; +import * as nav from '@react-navigation/native'; +jest.spyOn(nav, 'useIsFocused').mockReturnValue(true); +jest.mock('react-native-edge-to-edge', () => ({ + SystemBars: (props: any) => React.createElement('SystemBars', props), +})); + it('should have correct prop types', () => { @@ }); it('should handle optional props', () => { @@ }); + +it('renders SystemBars when focused on mobile', () => { + const { UNSAFE_getByType } = render(React.createElement(FocusAwareStatusBar, { hidden: true })); + const el = UNSAFE_getByType('SystemBars'); + expect(el.props.hidden).toEqual({ statusBar: true, navigationBar: true }); +}); + +it('returns null on web', async () => { + jest.isolateModules(() => { + jest.doMock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { ...RN, Platform: { ...RN.Platform, OS: 'web' } }; + }); + const { FocusAwareStatusBar: WebStatusBar } = require('../focus-aware-status-bar'); + const { render } = require('@testing-library/react-native'); + const tree = render(React.createElement(WebStatusBar)).toJSON(); + expect(tree).toBeNull(); + }); +});
10-15: Test name nit: it doesn’t validate “prop types.”Rename to reflect intent, e.g., “accepts props.” No functional change.
src/stores/calls/__tests__/integration.test.ts (2)
2-8: Prefer partial mock over replacing the whole react-native module.Overriding only Platform avoids breaking consumers that import other RN exports.
-jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn((specifics: any) => specifics.ios || specifics.default), - Version: 17, - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { + ...RN.Platform, + OS: 'ios', + select: jest.fn((specifics: any) => specifics.ios || specifics.default), + Version: 17, + }, + }; +});
11-18: Keep actual MMKV exports; stub behaviors you need.This reduces drift with library updates and preserves types.
-jest.mock('react-native-mmkv', () => ({ - MMKV: jest.fn().mockImplementation(() => ({ - set: jest.fn(), - getString: jest.fn(), - delete: jest.fn(), - })), - useMMKVBoolean: jest.fn(() => [false, jest.fn()]), -})); +jest.mock('react-native-mmkv', () => { + const actual = jest.requireActual('react-native-mmkv'); + return { + ...actual, + MMKV: jest.fn().mockImplementation(() => ({ + set: jest.fn(), + getString: jest.fn(), + delete: jest.fn(), + })), + useMMKVBoolean: jest.fn(() => [false, jest.fn()]), + }; +});src/stores/dispatch/__tests__/store.test.ts (2)
2-8: Partial mock RN Platform instead of replacing the entire module.Prevents accidental breakage of other RN exports during imports.
-jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn((specifics: any) => specifics.ios || specifics.default), - Version: 17, - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { + ...RN.Platform, + OS: 'ios', + select: jest.fn((specifics: any) => specifics.ios || specifics.default), + Version: 17, + }, + }; +});
11-18: Preserve actual MMKV exports; stub only what’s needed.Aligns with the rest of the suite and avoids type mismatches.
-jest.mock('react-native-mmkv', () => ({ - MMKV: jest.fn().mockImplementation(() => ({ - set: jest.fn(), - getString: jest.fn(), - delete: jest.fn(), - })), - useMMKVBoolean: jest.fn(() => [false, jest.fn()]), -})); +jest.mock('react-native-mmkv', () => { + const actual = jest.requireActual('react-native-mmkv'); + return { + ...actual, + MMKV: jest.fn().mockImplementation(() => ({ + set: jest.fn(), + getString: jest.fn(), + delete: jest.fn(), + })), + useMMKVBoolean: jest.fn(() => [false, jest.fn()]), + }; +});src/stores/status/__tests__/store.test.ts (4)
2-8: Use a partial RN mock to avoid clobbering the whole module.Safer for downstream imports that expect other RN symbols.
-jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn((specifics) => specifics.ios || specifics.default), - Version: 17, - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { + ...RN.Platform, + OS: 'ios', + select: jest.fn((specifics) => specifics.ios || specifics.default), + Version: 17, + }, + }; +});
11-18: Preserve actual MMKV exports; mock behaviors.Reduces breakage risk and keeps types consistent.
-jest.mock('react-native-mmkv', () => ({ - MMKV: jest.fn().mockImplementation(() => ({ - set: jest.fn(), - getString: jest.fn(), - delete: jest.fn(), - })), - useMMKVBoolean: jest.fn(() => [false, jest.fn()]), -})); +jest.mock('react-native-mmkv', () => { + const actual = jest.requireActual('react-native-mmkv'); + return { + ...actual, + MMKV: jest.fn().mockImplementation(() => ({ + set: jest.fn(), + getString: jest.fn(), + delete: jest.fn(), + })), + useMMKVBoolean: jest.fn(() => [false, jest.fn()]), + }; +});
145-147: Avoid asserting exact empty-args calls; this is brittle.APIs may accept optional params. Prefer call count or loose match.
-expect(mockGetCalls).toHaveBeenCalledWith(); -expect(mockGetAllGroups).toHaveBeenCalledWith(); +expect(mockGetCalls).toHaveBeenCalledTimes(1); +expect(mockGetAllGroups).toHaveBeenCalledTimes(1);
79-81: Reset zustand store state between tests to avoid leakage.Clear store to initial defaults in beforeEach for deterministic runs.
src/stores/calls/__tests__/store.test.ts (2)
2-8: Partial RN mock recommended.Same rationale: override Platform only.
-jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn((specifics: any) => specifics.ios || specifics.default), - Version: 17, - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { + ...RN.Platform, + OS: 'ios', + select: jest.fn((specifics: any) => specifics.ios || specifics.default), + Version: 17, + }, + }; +});
11-18: MMKV mock: keep actual exports, stub internals.Consistent and safer.
-jest.mock('react-native-mmkv', () => ({ - MMKV: jest.fn().mockImplementation(() => ({ - set: jest.fn(), - getString: jest.fn(), - delete: jest.fn(), - })), - useMMKVBoolean: jest.fn(() => [false, jest.fn()]), -})); +jest.mock('react-native-mmkv', () => { + const actual = jest.requireActual('react-native-mmkv'); + return { + ...actual, + MMKV: jest.fn().mockImplementation(() => ({ + set: jest.fn(), + getString: jest.fn(), + delete: jest.fn(), + })), + useMMKVBoolean: jest.fn(() => [false, jest.fn()]), + }; +});src/stores/app/__tests__/location-store.test.ts (2)
2-8: Don’t mock the entire react-native module; target Platform or use the central setup.Overriding 'react-native' with only Platform risks breaking consumers that expect other RN exports (Testing Library, jest-host components). Prefer the shared jest-platform-setup or mock only the Platform submodule.
Apply this diff to remove the broad mock and rely on jest-platform-setup:
-// Mock Platform first before any imports -jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn((specifics: any) => specifics.ios || specifics.default), - Version: 17, - }, -}));
11-18: Make the MMKV mock stateful so “persist” scenarios are meaningful.Current fns are no-ops; add an in-memory store so set/get/delete reflect state, and wire useMMKVBoolean to it. This keeps tests deterministic and aligns with our use of react-native-mmkv.
-jest.mock('react-native-mmkv', () => ({ - MMKV: jest.fn().mockImplementation(() => ({ - set: jest.fn(), - getString: jest.fn(), - delete: jest.fn(), - })), - useMMKVBoolean: jest.fn(() => [false, jest.fn()]), -})); +jest.mock('react-native-mmkv', () => { + const kv = new Map<string, string>(); + const mmkv = { + set: jest.fn((k: string, v: string) => void kv.set(k, String(v))), + getString: jest.fn((k: string) => kv.get(k) ?? null), + delete: jest.fn((k: string) => void kv.delete(k)), + }; + return { + MMKV: jest.fn().mockImplementation(() => mmkv), + useMMKVBoolean: jest.fn((key: string) => { + const setter = jest.fn((val: boolean) => kv.set(key, String(val))); + const current = kv.get(key) === 'true'; + return [current, setter] as const; + }), + }; +});jest-platform-setup.ts (2)
4-5: Prefer nullish coalescing in Platform.select mock.Avoid treating falsy (0, '') as absent; use ?? to match RN behavior more closely.
- select: jest.fn().mockImplementation((obj: any) => obj.ios || obj.default), + select: jest.fn().mockImplementation((obj: any) => obj?.ios ?? obj?.default),
11-16: Remove duplicate global Platform assignment.You already define the global via defineProperty; the extra assignment is redundant and can mask intent.
Object.defineProperty(global, 'Platform', { value: mockPlatform, writable: true, enumerable: true, configurable: true, }); -// Ensure Platform is available in the global scope for React Navigation and other libs -(global as any).Platform = mockPlatform;Also applies to: 22-22
src/app/(app)/__tests__/index.test.tsx (2)
92-99: Avoid double-mocking use-analytics; pick one strategy.There’s a top-level jest.mock here and a per-test jest.doMock later. Since Map is imported at file top, the per-test mock won’t take effect. Either remove this top-level mock and isolate modules in that test, or keep this and assert via the shared mock.
-jest.mock('@/hooks/use-analytics', () => ({ - useAnalytics: () => ({ - trackEvent: jest.fn(), - }), -}));Example per-test isolation (outside this hunk) for the analytics test:
jest.isolateModules(() => { jest.doMock('@/hooks/use-analytics', () => ({ useAnalytics: () => ({ trackEvent: mockTrackEvent }) })); const MapIsolated = require('../index').default; const { unmount } = render(<MapIsolated />); // assertions... unmount(); });
139-149: Also assert cleanup via stopLocationUpdates.You already stop updates in afterEach; add an expectation to guarantee unmount cleanup.
For example, in the first test:
- // Clean up the component - unmount(); + // Clean up the component + unmount(); + expect(mockLocationService.stopLocationUpdates).toHaveBeenCalled();
📜 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 (25)
.github/workflows/react-native-cicd.yml(1 hunks)__mocks__/expo-av.ts(1 hunks)__mocks__/expo-device.ts(1 hunks)__mocks__/expo-location.ts(1 hunks)__mocks__/expo-navigation-bar.ts(1 hunks)__mocks__/expo-task-manager.ts(1 hunks)__mocks__/react-native-edge-to-edge.ts(1 hunks)jest-platform-setup.ts(1 hunks)jest-setup.ts(1 hunks)src/app/(app)/__tests__/index.test.tsx(13 hunks)src/components/notifications/__tests__/NotificationInbox.test.tsx(8 hunks)src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx(2 hunks)src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx(1 hunks)src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx(6 hunks)src/components/sidebar/__tests__/unit-sidebar-minimal.test.tsx(2 hunks)src/components/status/__tests__/status-bottom-sheet.test.tsx(3 hunks)src/components/ui/__tests__/focus-aware-status-bar.test.tsx(1 hunks)src/services/__tests__/push-notification.test.ts(3 hunks)src/stores/app/__tests__/location-store.test.ts(1 hunks)src/stores/calls/__tests__/detail-store.test.ts(1 hunks)src/stores/calls/__tests__/integration.test.ts(1 hunks)src/stores/calls/__tests__/store.test.ts(1 hunks)src/stores/contacts/__tests__/store.test.ts(1 hunks)src/stores/dispatch/__tests__/store.test.ts(1 hunks)src/stores/status/__tests__/store.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
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
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
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/stores/app/__tests__/location-store.test.tssrc/stores/contacts/__tests__/store.test.ts__mocks__/expo-device.tsjest-platform-setup.tssrc/stores/calls/__tests__/detail-store.test.tssrc/stores/calls/__tests__/store.test.tssrc/stores/calls/__tests__/integration.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/stores/dispatch/__tests__/store.test.tssrc/app/(app)/__tests__/index.test.tsx__mocks__/expo-location.ts__mocks__/react-native-edge-to-edge.tssrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/notifications/__tests__/NotificationInbox.test.tsx__mocks__/expo-task-manager.tsjest-setup.ts__mocks__/expo-navigation-bar.ts__mocks__/expo-av.tssrc/stores/status/__tests__/store.test.tssrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/stores/app/__tests__/location-store.test.tssrc/stores/contacts/__tests__/store.test.ts__mocks__/expo-device.tsjest-platform-setup.tssrc/stores/calls/__tests__/detail-store.test.tssrc/stores/calls/__tests__/store.test.tssrc/stores/calls/__tests__/integration.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/stores/dispatch/__tests__/store.test.tssrc/app/(app)/__tests__/index.test.tsx__mocks__/expo-location.ts__mocks__/react-native-edge-to-edge.tssrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/notifications/__tests__/NotificationInbox.test.tsx__mocks__/expo-task-manager.tsjest-setup.ts__mocks__/expo-navigation-bar.ts__mocks__/expo-av.tssrc/stores/status/__tests__/store.test.tssrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit inference engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/stores/app/__tests__/location-store.test.tssrc/stores/contacts/__tests__/store.test.tssrc/stores/calls/__tests__/detail-store.test.tssrc/stores/calls/__tests__/store.test.tssrc/stores/calls/__tests__/integration.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/stores/dispatch/__tests__/store.test.tssrc/app/(app)/__tests__/index.test.tsxsrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/notifications/__tests__/NotificationInbox.test.tsxsrc/stores/status/__tests__/store.test.tssrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/stores/app/__tests__/location-store.test.tssrc/stores/contacts/__tests__/store.test.tssrc/stores/calls/__tests__/detail-store.test.tssrc/stores/calls/__tests__/store.test.tssrc/stores/calls/__tests__/integration.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/stores/dispatch/__tests__/store.test.tssrc/app/(app)/__tests__/index.test.tsxsrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/notifications/__tests__/NotificationInbox.test.tsxsrc/stores/status/__tests__/store.test.tssrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/stores/app/__tests__/location-store.test.tssrc/stores/contacts/__tests__/store.test.tssrc/stores/calls/__tests__/detail-store.test.tssrc/stores/calls/__tests__/store.test.tssrc/stores/calls/__tests__/integration.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/stores/dispatch/__tests__/store.test.tssrc/app/(app)/__tests__/index.test.tsxsrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/notifications/__tests__/NotificationInbox.test.tsxsrc/stores/status/__tests__/store.test.tssrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
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, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use @rnmapbox/maps for maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
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
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use @rnmapbox/maps for maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/notifications/__tests__/NotificationInbox.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/notifications/__tests__/NotificationInbox.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Build and maintain shared Gluestack UI wrappers in src/components/ui
Files:
src/components/ui/__tests__/focus-aware-status-bar.test.tsx
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{ts,tsx} : Use react-native-mmkv for local storage
Applied to files:
src/stores/app/__tests__/location-store.test.tssrc/stores/contacts/__tests__/store.test.tssrc/stores/calls/__tests__/detail-store.test.tssrc/stores/calls/__tests__/store.test.tssrc/stores/calls/__tests__/integration.test.tssrc/stores/dispatch/__tests__/store.test.tssrc/stores/status/__tests__/store.test.ts
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
Applied to files:
jest-platform-setup.tssrc/app/(app)/__tests__/index.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
Applied to files:
jest-platform-setup.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/components/sidebar/__tests__/unit-sidebar-minimal.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/ui/__tests__/focus-aware-status-bar.test.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : Generate tests for all components, services, and logic; ensure tests run without errors
Applied to files:
src/app/(app)/__tests__/index.test.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use gluestack-ui icon component
Applied to files:
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Applied to files:
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx
🧬 Code graph analysis (4)
src/components/sidebar/__tests__/unit-sidebar-minimal.test.tsx (1)
src/components/sidebar/unit-sidebar.tsx (1)
SidebarUnitCard(21-78)
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx (3)
src/stores/app/core-store.ts (1)
useCoreStore(48-263)src/stores/units/store.ts (1)
useUnitsStore(16-31)src/components/settings/unit-selection-bottom-sheet.tsx (1)
UnitSelectionBottomSheet(28-187)
src/components/notifications/__tests__/NotificationInbox.test.tsx (1)
src/components/notifications/NotificationInbox.tsx (1)
NotificationInbox(26-340)
src/components/ui/__tests__/focus-aware-status-bar.test.tsx (1)
src/components/ui/focus-aware-status-bar.tsx (1)
FocusAwareStatusBar(9-63)
🔇 Additional comments (20)
src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx (1)
453-459: Verify ScrollView testID source.The test expects testID="scroll-view". Since this file no longer mocks ScrollView, ensure UnitSelectionBottomSheet sets that testID internally; otherwise this will fail intermittently across suites.
src/components/sidebar/__tests__/unit-sidebar-minimal.test.tsx (4)
31-41: LGTM on lucide icon mocks.Simple host-view replacements with testIDs are sufficient and deterministic.
69-71: LGTM on bottom sheet mock.Nulling the sheet keeps focus on parent interactions.
80-94: Good baseline render assertions.Covers default props plumbing without coupling to store state.
96-110: Button presence checks look good.Stable testIDs match the implementation snippet.
.github/workflows/react-native-cicd.yml (2)
150-150: Nice: Android-only guard for google-services.json.Prevents unnecessary steps on iOS.
122-134: Node 24 on Expo 53: verify support.Expo CLI/EAS occasionally lag latest Node majors. If instability appears, pin to a known-good LTS (e.g., 20/22).
Action: confirm Expo 53 compatibility with Node 24; otherwise downgrade.
src/components/notifications/__tests__/NotificationInbox.test.tsx (3)
244-250: LGTM: closed state returns null.Basic guard works as intended.
385-400: LGTM: missing unit/config returns null.Matches production component’s safety checks.
28-136: Stop mocking the SUT in NotificationInbox tests
Replace the mock component andjest.mock('../NotificationInbox')insrc/components/notifications/__tests__/NotificationInbox.test.tsxwith a direct import of the real component:-// Mock the actual NotificationInbox component to avoid deep dependency issues -jest.mock('../NotificationInbox', () => ({ - NotificationInbox: MockNotificationInbox, -})); - -// Import after mocking -const { NotificationInbox } = require('../NotificationInbox'); +// Import the real component (dependencies are mocked above) +import { NotificationInbox } from '../NotificationInbox';Render
<NotificationInbox isOpen onClose={...} />in your tests (mock only its hooks/dependencies) and adjust selectors/testIDs/text queries to match the real UI.src/services/__tests__/push-notification.test.ts (1)
94-100: Good pattern capturing the listener handler.Capturing the handler via the mocked addNotificationReceivedListener keeps the test deterministic. LGTM.
src/components/status/__tests__/status-bottom-sheet.test.tsx (1)
1-3177: Overall test flow and coverage look solid.The suite exercises destination flows, active call preselection, loading states, and submission behavior with clear expectations. Once the duplicate mocks are cleaned up, this is in great shape.
__mocks__/expo-location.ts (2)
1-16: Enums look fine for tests, but confirm callers don’t rely on exact numeric valuesIf production code compares enum numbers from Expo 53, mismatches could mask issues. If callers only check truthiness or use keys, you’re good.
66-68: LGTM: basic async stubs cover core scheduling APIsstart/stop/hasStarted mocks are sufficient and predictable for unit tests.
src/stores/contacts/__tests__/store.test.ts (3)
1-8: Verify Platform mock doesn’t diverge from global setupIf you also mock Platform in a global jest setup, double-mocking can cause inconsistencies. Prefer one source of truth.
10-18: MMKV mock is solid for unit testsStubbed set/getString/delete and useMMKVBoolean cover typical paths.
164-177: Expected error message may not match store implementationIf the store sets a generic error (e.g., 'Failed to fetch contacts'), this assertion is fine; if it uses a different message, the test will fail. Confirm and align.
src/stores/status/__tests__/store.test.ts (1)
40-58: Mocks look good.Location and roles store stubs cover the fields used in tests.
jest-platform-setup.ts (1)
19-19: Ensure this runs pre-module-load (setupFiles) or switch to jest.mock.jest.doMock is fine if this file is in setupFiles (not setupFilesAfterEnv). Otherwise, use jest.mock for hoisting.
Would you confirm jest-platform-setup.ts is listed under setupFiles so Platform is defined before any imports?
src/app/(app)/__tests__/index.test.tsx (1)
124-124: Scope fake timers to tests that need them or flush within tests.Using global fake timers with waitFor can cause subtle flakiness. Either enable fake timers only in tests that need them, or explicitly flush in those tests (act + runAllTimers).
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
Outdated
Show resolved
Hide resolved
src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx
Outdated
Show resolved
Hide resolved
| jest.mock('lucide-react-native'); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate jest.mock for 'lucide-react-native' will error
Mocking the same module twice can cause Jest hoist/duplication errors. Keep only the factory mock.
-jest.mock('lucide-react-native');
// Mock lucide icons to avoid SVG issues in tests
jest.mock('lucide-react-native', () => ({
Check: ({ size, className, testID, ...props }: any) => {
const React = require('react');
return React.createElement('Text', { testID: testID || 'check-icon', ...props }, 'Check');
},
}));Also applies to: 34-39
🤖 Prompt for AI Agents
In src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx around
lines 23-24 (and also lines 34-39), there are duplicate jest.mock calls for
'lucide-react-native' which will cause Jest hoist/duplication errors; remove the
redundant jest.mock instances and keep only a single factory mock declaration
(the one that provides the mocked implementations) at the top of the file so the
module is mocked once for the whole test file.
| jest.mock('@/stores/toast/store', () => ({ | ||
| useToastStore: jest.fn(), | ||
| })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove duplicate mocks for '@/stores/toast/store'.
Duplicate declarations are brittle; keep just one.
jest.mock('@/stores/toast/store', () => ({
useToastStore: jest.fn(),
}));
-
-jest.mock('@/stores/toast/store', () => ({
- useToastStore: jest.fn(),
-}));Also applies to: 242-245
🤖 Prompt for AI Agents
In src/components/status/__tests__/status-bottom-sheet.test.tsx around lines
214-216 and 242-245 there are duplicate jest.mock declarations for
'@/stores/toast/store'; remove the redundant mock declaration so only a single
jest.mock call remains, consolidating any mock implementation (e.g.
useToastStore jest.fn() setup) into that one declaration and update any
references in the test to use the consolidated mock.
| jest.mock('react-native', () => ({ | ||
| Platform: { | ||
| OS: 'ios', | ||
| select: jest.fn((obj) => obj.ios || obj.default), | ||
| }, | ||
| })); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Mock only Platform while preserving the rest of react-native exports.
Overriding the entire react-native module risks breaking any downstream imports that rely on other RN exports. Merge with the actual module and override Platform only.
-jest.mock('react-native', () => ({
- Platform: {
- OS: 'ios',
- select: jest.fn((obj) => obj.ios || obj.default),
- },
-}));
+jest.mock('react-native', () => {
+ const RN = jest.requireActual('react-native');
+ return {
+ ...RN,
+ Platform: {
+ ...RN.Platform,
+ OS: 'ios',
+ select: jest.fn((obj: Record<string, unknown>) => (obj?.ios ?? obj?.default)),
+ },
+ };
+});🤖 Prompt for AI Agents
In src/services/__tests__/push-notification.test.ts around lines 21 to 26, the
test currently replaces the entire react-native module which can break other
imports; instead call jest.requireActual('react-native') and spread its exports
into the mock, then override only the Platform export (OS set to 'ios' and
select mocked as needed). Replace the current jest.mock body with one that
merges the actual module and returns { ...actual, Platform: { OS: 'ios', select:
jest.fn((obj) => obj.ios ?? obj.default) } } so only Platform is mocked and all
other react-native exports remain intact.
|
Approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is approved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/components/ui/skeleton/index.tsx (2)
7-12: Add missing speed prop to the public propsspeed is destructured but not declared on ISkeletonProps, which forces unsafe typing elsewhere.
type ISkeletonProps = React.ComponentProps<typeof View> & VariantProps<typeof skeletonStyle> & { isLoaded?: boolean; startColor?: string; + speed?: number; };
21-65: Start/stop animation in effects; avoid creating Animated.Value/loops during renderAnimated.Value and loop() are created/started on every render, which can leak loops and cause jank. Persist the value via useRef and control the loop in useEffect with cleanup.
- const pulseAnim = new Animated.Value(1); - const customTimingFunction = Easing.bezier(0.4, 0, 0.6, 1); - const fadeDuration = 0.6; - const animationDuration = (fadeDuration * 10000) / speed; // Convert seconds to milliseconds - - const pulse = Animated.sequence([ - Animated.timing(pulseAnim, { - toValue: 1, // Start with opacity 1 - duration: animationDuration / 2, // Third of the animation duration - easing: customTimingFunction, - useNativeDriver: Platform.OS !== 'web', - }), - Animated.timing(pulseAnim, { - toValue: 0.75, - duration: animationDuration / 2, // Third of the animation duration - easing: customTimingFunction, - useNativeDriver: Platform.OS !== 'web', - }), - Animated.timing(pulseAnim, { - toValue: 1, - duration: animationDuration / 2, // Third of the animation duration - easing: customTimingFunction, - useNativeDriver: Platform.OS !== 'web', - }), - ]); + const pulseAnim = React.useRef(new Animated.Value(1)).current; + React.useEffect(() => { + const customTimingFunction = Easing.bezier(0.4, 0, 0.6, 1); + const fadeDuration = 0.6; + const animationDuration = (fadeDuration * 10000) / speed; // seconds → ms + const sequence = Animated.sequence([ + Animated.timing(pulseAnim, { + toValue: 1, + duration: animationDuration / 2, + easing: customTimingFunction, + useNativeDriver: Platform.OS !== 'web', + }), + Animated.timing(pulseAnim, { + toValue: 0.75, + duration: animationDuration / 2, + easing: customTimingFunction, + useNativeDriver: Platform.OS !== 'web', + }), + Animated.timing(pulseAnim, { + toValue: 1, + duration: animationDuration / 2, + easing: customTimingFunction, + useNativeDriver: Platform.OS !== 'web', + }), + ]); + const loop = Animated.loop(sequence); + if (!isLoaded) { + loop.start(); + return () => loop.stop(); + } else { + loop.stop(); + } + }, [isLoaded, speed, pulseAnim]);src/components/status/__tests__/status-bottom-sheet.test.tsx (1)
2686-2693: Fix waitFor misuse; assert the condition instead of passing a function.waitFor expects an assertion; the current callback returns a function reference and never waits.
-// Wait for the operation to complete -await waitFor(() => slowSaveUnitStatus); +// Wait for the operation to complete +await waitFor(() => expect(slowSaveUnitStatus).toHaveBeenCalled());src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx (1)
79-81: Ensure useToastStore mock returns the expected selector shape
In src/components/settings/tests/unit-selection-bottom-sheet-simple.test.tsx (L79–81), update the mock so it returns{ showToast }when called with a selector, for example:-jest.mock('@/stores/toast/store', () => ({ - useToastStore: jest.fn(), -})); +jest.mock('@/stores/toast/store', () => ({ + useToastStore: jest.fn((selector) => + typeof selector === 'function' + ? selector({ showToast: jest.fn() }) + : undefined + ), +}));
♻️ Duplicate comments (6)
jest-setup.ts (1)
37-78: Stop deep-mocking Testing Library internals; use public configureHostComponentNames APIThe internal path is unstable, and jest.mock hoisting negates the try/catch intent. Configure via the public API and drop the internal mock.
-// Mock the host component names function to prevent testing library errors -// Check if the internal module exists (for pre-v13 compatibility) -try { - require.resolve('@testing-library/react-native/build/helpers/host-component-names'); - // If the internal module exists, mock it (pre-v13) - jest.mock('@testing-library/react-native/build/helpers/host-component-names', () => ({ - getHostComponentNames: jest.fn(() => ({ - text: 'Text', - view: 'View', - scrollView: 'ScrollView', - touchable: 'TouchableOpacity', - switch: 'Switch', - textInput: 'TextInput', - })), - configureHostComponentNamesIfNeeded: jest.fn(), - isHostText: jest.fn((element) => element?.type === 'Text' || element?._fiber?.type === 'Text' || (typeof element === 'object' && element?.props?.children && typeof element.props.children === 'string')), - isHostTextInput: jest.fn((element) => element?.type === 'TextInput' || element?._fiber?.type === 'TextInput'), - isHostImage: jest.fn((element) => element?.type === 'Image' || element?._fiber?.type === 'Image'), - isHostSwitch: jest.fn((element) => element?.type === 'Switch' || element?._fiber?.type === 'Switch'), - isHostScrollView: jest.fn((element) => element?.type === 'ScrollView' || element?._fiber?.type === 'ScrollView'), - isHostModal: jest.fn((element) => element?.type === 'Modal' || element?._fiber?.type === 'Modal'), - })); -} catch (error) { - // Module doesn't exist (v13+), try to use the public API if available - try { - const { configureHostComponentNames } = require('@testing-library/react-native'); - // Configure host component names using the public API (v13+) - if (configureHostComponentNames) { - configureHostComponentNames({ - text: 'Text', - view: 'View', - scrollView: 'ScrollView', - touchable: 'TouchableOpacity', - switch: 'Switch', - textInput: 'TextInput', - }); - } - } catch (publicApiError) { - // If neither internal nor public API is available, log a warning but continue - console.warn('Unable to configure host component names for @testing-library/react-native. Tests may fail if they rely on component type detection.'); - } -} +// Configure host component names via public API (v13+); silently ignore if unavailable (older versions) +try { + const { configureHostComponentNames } = require('@testing-library/react-native'); + if (configureHostComponentNames) { + configureHostComponentNames({ + text: 'Text', + view: 'View', + scrollView: 'ScrollView', + touchable: 'TouchableOpacity', + switch: 'Switch', + textInput: 'TextInput', + }); + } +} catch { + // no-op for older @testing-library/react-native versions +}src/components/status/__tests__/status-bottom-sheet.test.tsx (2)
185-192: Consolidated offline-event-manager mock looks correct.Addresses previous duplicate-mock feedback. No further action needed.
194-199: Store mocks are consistent and single-sourced now.Previous duplicate core/roles/toast mocks were removed; this is the right pattern.
Also applies to: 200-210, 212-219
src/services/__tests__/push-notification.test.ts (1)
21-26: Mock only Platform while preserving the rest of react-native exports.Overriding the whole module can break other imports; spread the actual module and override Platform.
-jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn((obj) => obj.ios ?? obj.default), - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { + ...RN.Platform, + OS: 'ios', + select: jest.fn((obj: Record<string, unknown>) => (obj?.ios ?? obj?.default)), + }, + }; +});src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx (1)
1-6: Single react-i18next mock returning keys aligns with assertions.This removes the prior conflict and keeps tests stable across locales.
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx (1)
32-38: LGTM: lucide-react-native is mocked once (duplicate removed).Previous duplication is resolved; a single factory mock remains. Good fix.
🧹 Nitpick comments (35)
src/components/ui/bottomsheet/index.tsx (2)
39-39: Tighten RefObject type (no need for| nullon the generic).
RefObject<T>’scurrentis alreadyT | null. UsingReact.RefObject<GorhomBottomSheet | null>is redundant and mildly confusing.- bottomSheetRef: React.RefObject<GorhomBottomSheet | null>; + bottomSheetRef: React.RefObject<GorhomBottomSheet>;
166-178: Don’t swallow consumeronKeyDown; call through and fix deps.If a caller passes
onKeyDown, it’s currently overridden. Forward it after handling Escape and add it to hook deps.const keyDownHandlers = useMemo(() => { return Platform.OS === 'web' ? { - onKeyDown: (e: React.KeyboardEvent) => { + onKeyDown: (e: React.KeyboardEvent) => { if (e.key === 'Escape') { e.preventDefault(); handleClose(); return; } + // pass through to consumer handler if provided + // @ts-ignore (RNW typing differences) + props.onKeyDown?.(e); }, } : {}; - }, [handleClose]); + }, [handleClose, props.onKeyDown]);src/services/bluetooth-audio.service.ts (1)
42-42: Prefer ReturnType for cross-env timer IDsThis avoids DOM vs Node timer mismatches and keeps typings portable across RN, web, and Jest.
Apply this diff:
- private scanTimeout: number | null = null; + private scanTimeout: ReturnType<typeof setTimeout> | null = null;Optionally align
connectionTimeoutto the same pattern for consistency (outside this hunk).src/app/login/index.tsx (1)
71-71: Remove no-op any-cast spreads on Modal/Drawer componentsThe
{...({} as any)}hack hides type issues and violates our “avoid any” guideline. It appears in multiple places:
- src/app/(app)/_layout.tsx:235
- src/app/login/index.tsx:71
- src/components/roles/roles-modal.tsx:78
- src/components/notifications/NotificationInbox.tsx:317
- src/components/calls/full-screen-image-modal.tsx:120
- src/components/push-notification/push-notification-modal.tsx:107
Remove these spreads and replace them with explicit, typed prop objects, for example:
const modalProps = { isOpen, onClose, size: 'full', } satisfies React.ComponentProps<typeof Modal>; <Modal {...modalProps}>…</Modal>Likewise for
<Drawer>, usesatisfies React.ComponentProps<typeof Drawer>.src/components/calls/full-screen-image-modal.tsx (1)
120-120: Avoid any-cast prop spread on ModalSame rationale as login: remove
{...({} as any)}and prefer typed props withsatisfies.Apply this diff:
- <Modal isOpen={isOpen} onClose={onClose} size="full" {...({} as any)}> + <Modal isOpen={isOpen} onClose={onClose} size="full">Optional: the double-tap handler wraps worklet logic in
runOnJS. You can execute the zoom reset directly in the worklet to avoid JS thread hops.src/hooks/use-signalr-lifecycle.ts (1)
34-35: Use ReturnType for timers
Switch both timers touseRef<ReturnType<typeof setTimeout> | null>(null).- const backgroundTimer = useRef<number | null>(null); - const resumeTimer = useRef<number | null>(null); + const backgroundTimer = useRef<ReturnType<typeof setTimeout> | null>(null); + const resumeTimer = useRef<ReturnType<typeof setTimeout> | null>(null);Apply this pattern to other
number | nulltimer fields for consistency.src/components/push-notification/push-notification-modal.tsx (1)
107-107: Drop any-cast spread; type Modal props explicitlyRemove
{...({} as any)}and rely on a typed props object (satisfies) to keep TS strict without suppressions.Apply this diff:
- <Modal isOpen={isOpen} onClose={handleClose} size="md" {...({} as any)}> + <Modal isOpen={isOpen} onClose={handleClose} size="md">src/components/ui/drawer/index.tsx (1)
33-36: Replace ts-ignore with ts-expect-error and add a typed interop helperSuppressing errors with ts-ignore hides future type fixes. Use ts-expect-error and centralize the interop to keep it intentional and discoverable.
-// @ts-ignore - Motion component type compatibility issue -cssInterop(AnimatedPressable, { className: 'style' }); -// @ts-ignore - Motion component type compatibility issue -cssInterop(Motion.View, { className: 'style' }); +// @ts-expect-error Motion component type compatibility issue (Legend/Motion x NativeWind) +cssInterop(AnimatedPressable, { className: 'style' }); +// @ts-expect-error Motion component type compatibility issue (Legend/Motion x NativeWind) +cssInterop(Motion.View, { className: 'style' });Optionally, move these to a single module (e.g., src/types/css-interop.ts) and import it once to avoid repetition across files.
src/lib/i18n/index.tsx (1)
12-12: Prefer languageTag fallback to preserve region when availableUsing languageCode drops region (e.g., en-US → en). If you plan region-specific strings, prefer languageTag with a safe fallback.
- lng: getLanguage() || Localization.getLocales()[0]?.languageCode || 'en', + lng: getLanguage() || Localization.getLocales()[0]?.languageTag || Localization.getLocales()[0]?.languageCode || 'en',src/hooks/use-map-signalr-updates.ts (1)
15-15: Type timer handle as ReturnType for cross-env safetyUsing number works in RN/browser, but ReturnType is portable across Jest/Node and RN without casting.
- const debounceTimer = useRef<number | null>(null); + const debounceTimer = useRef<ReturnType<typeof setTimeout> | null>(null);src/components/notifications/NotificationInbox.tsx (1)
317-317: Remove any-spread hack; it disables prop type-checking on ModalSpreading {} as any masks real typing issues and weakens safety with no runtime benefit.
- <Modal isOpen={showDeleteConfirmModal} onClose={() => setShowDeleteConfirmModal(false)} {...({} as any)}> + <Modal isOpen={showDeleteConfirmModal} onClose={() => setShowDeleteConfirmModal(false)}>src/app/(app)/_layout.tsx (1)
235-235: Avoid {...({} as any)} on Drawer; keep strong prop checksThis pattern suppresses useful TS diagnostics and can hide real prop issues.
- <Drawer isOpen={isOpen} onClose={() => setIsOpen(false)} {...({} as any)}> + <Drawer isOpen={isOpen} onClose={() => setIsOpen(false)}>src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx (1)
15-24: Prefer partial override via requireActual to avoid wiping RN exportsMocking the entire module risks missing exports used by the SUT. Extend the real module instead.
-jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), - }, - ScrollView: ({ children, ...props }: any) => { - const React = require('react'); - return React.createElement('View', { testID: 'scroll-view', ...props }, children); - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { + ...RN.Platform, + OS: 'ios', + select: jest.fn().mockImplementation((obj) => obj.ios || obj.default), + }, + ScrollView: ({ children, ...props }: any) => { + const React = require('react'); + return React.createElement('View', { testID: 'scroll-view', ...props }, children); + }, + }; +});src/components/status/__tests__/status-bottom-sheet.test.tsx (7)
791-801: Avoid brittle findAllByType; assert disabled via the button parent.Relying on internal node types is fragile. Use the text node’s parent (TouchableOpacity mock) to inspect accessibilityState.
-// Find all button elements and check the submit button's disabled state -const buttons = screen.getAllByTestId('button'); -const submitButton = buttons.find(button => { - try { - const textElements = button.findAllByType('Text'); - return textElements.some((text: any) => text.props.children === 'Submit'); - } catch (e) { - return false; - } -}); -expect(submitButton?.props.accessibilityState?.disabled).toBe(true); +const submitText = screen.getByText('Submit'); +const submitButton = submitText.parent; // TouchableOpacity in the mock +expect(submitButton?.props.accessibilityState?.disabled).toBe(true);
822-833: Same here: simplify submit enabled assertion.Mirror the approach above for the enabled case.
-// Find all button elements and check the submit button's disabled state -const buttons = screen.getAllByTestId('button'); -const submitButton = buttons.find(button => { - try { - const textElements = button.findAllByType('Text'); - return textElements.some((text: any) => text.props.children === 'Submit'); - } catch (e) { - return false; - } -}); -expect(submitButton?.props.accessibilityState?.disabled).toBe(false); +const submitText = screen.getByText('Submit'); +const submitButton = submitText.parent; +expect(submitButton?.props.accessibilityState?.disabled).toBe(false);
170-177: Icon mock is fine. Consider centralizing in mocks for reuse.Optional: move this lucide mock to mocks/lucide-react-native.ts to DRY across suites.
112-126: Stable nativewind/i18n mocks.Mocks look good and deterministic. Consider promoting them to jest-setup or mocks to reduce per-file boilerplate.
Also applies to: 117-126
1146-1153: Clarify “Next” triggers submit in this flow.Given Note: 0 and Detail: 2, “Next” finalizes. Add a short inline comment to avoid future regressions if copy changes.
2252-2254: Comment mismatch with Note value.Note: 2 corresponds to “required” elsewhere; the comment says “optional”.
-Note: 2, // Note optional +Note: 2, // Note required
1-4: Optional: centralize i18n translation map.You re-wire useTranslation per-test. Consider a shared mock returning t(key, opts) with common keys to avoid duplicating dictionaries across suites.
I can extract a mocks/react-i18next.ts compatible with all current suites.
Also applies to: 220-228
types/gluestack-ui.d.ts (3)
1-8: Import React types explicitly to avoid ambient dependency.Referencing React.ReactNode without an import relies on global type leakage. Importing React types makes this file self-contained.
/** * Type declarations to handle Gluestack UI v2 compatibility issues * These declarations resolve type mismatches between packages */ +import type * as React from 'react';Also applies to: 22-26, 34-38, 40-44, 46-50
6-8: Prefer merging declarations once per module for readability.You can merge the createX declarations and props interfaces in single blocks per module to cut duplication. Functional behavior is identical.
Also applies to: 22-26, 34-38, 40-44, 46-50
52-56: TVConfig any index is fine short-term; consider a minimal shape later.If you touch tv config, we can tighten it to known keys to regain some type-safety.
I can draft a minimal TVConfig aligned with tailwind-variants’ public types.
src/services/__tests__/push-notification.test.ts (1)
83-92: Inline simulator bypasses service handler; consider asserting via the service.You mock pushNotificationService.getInstance().handleNotificationReceived but never invoke/assert it. Either remove that mock or drive assertions through it to couple tests to the real handler contract.
-const mockNotificationHandler = jest.fn(); +const mockNotificationHandler = jest.fn(); // ... later -// Test the notification handling logic directly -const simulateNotificationReceived = (notification: Notifications.Notification): void => { +// Option A: Proxy to the service handler +const simulateNotificationReceived = (notification: Notifications.Notification): void => { const data = notification.request.content.data; // ... - if (data && data.eventCode && typeof data.eventCode === 'string') { - const notificationData = { - eventCode: data.eventCode as string, - title: notification.request.content.title || undefined, - body: notification.request.content.body || undefined, - data, - }; - usePushNotificationModalStore.getState().showNotificationModal(notificationData); - } + if (data && typeof data.eventCode === 'string') { + require('../push-notification') + .pushNotificationService + .getInstance() + .handleNotificationReceived(notification); + } };Also applies to: 129-147
__mocks__/expo-location.ts (1)
32-44: Optional: make heading consistently number | null.getCurrentPositionAsync uses heading: null while watchPositionAsync sets heading: 0. Aligning these reduces test branching.
- heading: null, + heading: 0,src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx (1)
89-159: UI mocks are lean and consistent.Good for avoiding SVG/native deps; consider centralizing in mocks to DRY.
Also applies to: 161-173
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx (9)
4-15: Streamline i18n mock and avoid rebuilding the map per call.Compute the dynamic message outside the translations map and use nullish coalescing.
Apply this diff:
- t: jest.fn((key: string, options?: any) => { - const translations: { [key: string]: string } = { + t: jest.fn((key: string, options?: any) => { + if (key === 'settings.unit_selected_successfully') { + return `${options?.unitName || 'Unit'} selected successfully`; + } + const translations: Record<string, string> = { 'settings.select_unit': 'Select Unit', 'settings.current_unit': 'Current Unit', 'settings.no_units_available': 'No units available', 'common.cancel': 'Cancel', - 'settings.unit_selected_successfully': `${options?.unitName || 'Unit'} selected successfully`, 'settings.unit_selection_failed': 'Failed to select unit. Please try again.', }; - return translations[key] || key; + return translations[key] ?? key; }),
40-149: Consider centralizing UI component mocks to DRY up tests.These repeated gluestack UI mocks are good, but moving them to a shared test util or Jest setup will reduce duplication across suites and keep individual tests focused on behavior.
169-171: Drop fragile typeof assertion for memoized component.React may change typeof for memoized components; asserting displayName is sufficient.
Apply this diff:
- // React.memo returns an object, not a function - expect(typeof UnitSelectionBottomSheet).toBe('object'); - expect(UnitSelectionBottomSheet.displayName).toBe('UnitSelectionBottomSheet'); + // Verify identity via displayName rather than relying on typeof + expect(UnitSelectionBottomSheet.displayName).toBe('UnitSelectionBottomSheet');
182-211: Avoid shadowing top-level mock refs within Import Test.Local variables reuse names from the module-scope mocks, which is confusing. Rename locals.
Apply this diff:
- const mockUseCoreStore = require('@/stores/app/core-store').useCoreStore as jest.MockedFunction<any>; - const mockUseUnitsStore = require('@/stores/units/store').useUnitsStore as jest.MockedFunction<any>; - const mockUseToastStore = require('@/stores/toast/store').useToastStore as jest.MockedFunction<any>; - const mockUseRolesStore = require('@/stores/roles/store').useRolesStore; + const coreStoreMockLocal = require('@/stores/app/core-store').useCoreStore as jest.MockedFunction<any>; + const unitsStoreMockLocal = require('@/stores/units/store').useUnitsStore as jest.MockedFunction<any>; + const toastStoreMockLocal = require('@/stores/toast/store').useToastStore as jest.MockedFunction<any>; + const rolesStoreModule = require('@/stores/roles/store').useRolesStore; @@ - mockUseCoreStore.mockReturnValue({ + coreStoreMockLocal.mockReturnValue({ @@ - mockUseUnitsStore.mockReturnValue({ + unitsStoreMockLocal.mockReturnValue({ @@ - mockUseRolesStore.getState = jest.fn(() => ({ + rolesStoreModule.getState = jest.fn(() => ({ @@ - mockUseToastStore.mockImplementation((selector: any) => { + toastStoreMockLocal.mockImplementation((selector: any) => {
311-315: Ensure roles store auto-mock exposes getState.Hardening for environments where the automock doesn’t preserve Zustand’s static members.
Apply this diff:
// Mock the roles store - (useRolesStore.getState as jest.Mock).mockReturnValue({ + if (!(useRolesStore as any).getState) { + (useRolesStore as any).getState = jest.fn(); + } + (useRolesStore.getState as jest.Mock).mockReturnValue({ fetchRolesForUnit: mockFetchRolesForUnit, });
399-406: Use testID for Cancel to decouple from translated text.More robust against i18n changes.
Apply this diff:
- const cancelButton = screen.getByText('Cancel'); + const cancelButton = screen.getByTestId('cancel-button'); fireEvent.press(cancelButton);
417-423: Await async side-effects (roles + toast, and close) to avoid flakiness.Wrap expectations in waitFor.
Apply this diff:
- expect(mockFetchRolesForUnit).toHaveBeenCalledWith('2'); - expect(mockShowToast).toHaveBeenCalledWith('success', 'Ladder 1 selected successfully'); + await waitFor(() => { + expect(mockFetchRolesForUnit).toHaveBeenCalledWith('2'); + expect(mockShowToast).toHaveBeenCalledWith('success', 'Ladder 1 selected successfully'); + expect(mockProps.onClose).toHaveBeenCalled(); + });
472-478: Await async side-effects on roles fetch failure.Wrap in waitFor so the assertion doesn’t race the promise chain.
Apply this diff:
- expect(mockFetchRolesForUnit).toHaveBeenCalledWith('2'); - expect(mockShowToast).toHaveBeenCalledWith('error', 'Failed to select unit. Please try again.'); + await waitFor(() => { + expect(mockFetchRolesForUnit).toHaveBeenCalledWith('2'); + expect(mockShowToast).toHaveBeenCalledWith('error', 'Failed to select unit. Please try again.'); + });
497-516: Make the “loading prevents close” test deterministic with fake timers.Avoid real sleeps to reduce flakes and speed up the suite.
Apply this diff:
- it('does not close when loading', async () => { - // Make setActiveUnit slow to simulate loading state + it('does not close when loading', async () => { + // Make setActiveUnit slow to simulate loading state + jest.useFakeTimers(); @@ - // onClose should not be called while loading - await new Promise((resolve) => setTimeout(resolve, 50)); // Wait a bit but not long enough for the async operation - expect(mockProps.onClose).not.toHaveBeenCalled(); + // onClose should not be called while loading + act(() => { + jest.advanceTimersByTime(50); + }); + expect(mockProps.onClose).not.toHaveBeenCalled(); + jest.useRealTimers();
📜 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 (24)
__mocks__/expo-location.ts(1 hunks)jest-setup.ts(1 hunks)src/app/(app)/_layout.tsx(1 hunks)src/app/login/index.tsx(1 hunks)src/components/calls/full-screen-image-modal.tsx(1 hunks)src/components/notifications/NotificationInbox.tsx(1 hunks)src/components/push-notification/push-notification-modal.tsx(1 hunks)src/components/roles/roles-modal.tsx(1 hunks)src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx(1 hunks)src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx(1 hunks)src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx(6 hunks)src/components/sidebar/__tests__/unit-sidebar-minimal.test.tsx(2 hunks)src/components/status/__tests__/status-bottom-sheet.test.tsx(4 hunks)src/components/ui/bottomsheet/index.tsx(2 hunks)src/components/ui/drawer/index.tsx(1 hunks)src/components/ui/skeleton/index.tsx(1 hunks)src/hooks/use-map-signalr-updates.ts(1 hunks)src/hooks/use-signalr-lifecycle.ts(1 hunks)src/lib/i18n/index.tsx(2 hunks)src/services/__tests__/push-notification.test.ts(15 hunks)src/services/bluetooth-audio.service.ts(1 hunks)src/services/offline-event-manager.service.ts(1 hunks)tsconfig.json(1 hunks)types/gluestack-ui.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/services/offline-event-manager.service.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/sidebar/tests/unit-sidebar-minimal.test.tsx
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (e.g., isFetchingData, handleUserInput)
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Use Expo SecureStore for sensitive data
Use zustand for state management
Use react-query for data fetching and caching
Use react-i18next for internationalization
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
Use React Navigation for handling navigation and deep linking
Handle errors gracefully and provide user feedback
Use Expo's SecureStore for sensitive data
Use zustand for state management
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/services/bluetooth-audio.service.tssrc/app/login/index.tsxsrc/components/ui/skeleton/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/push-notification/push-notification-modal.tsxsrc/hooks/use-map-signalr-updates.tssrc/components/calls/full-screen-image-modal.tsxsrc/components/roles/roles-modal.tsxsrc/hooks/use-signalr-lifecycle.tssrc/lib/i18n/index.tsxsrc/app/(app)/_layout.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxtypes/gluestack-ui.d.tssrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/ui/bottomsheet/index.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx__mocks__/expo-location.tssrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxjest-setup.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/services/bluetooth-audio.service.tssrc/app/login/index.tsxsrc/components/ui/skeleton/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/push-notification/push-notification-modal.tsxsrc/hooks/use-map-signalr-updates.tssrc/components/calls/full-screen-image-modal.tsxsrc/components/roles/roles-modal.tsxsrc/hooks/use-signalr-lifecycle.tssrc/lib/i18n/index.tsxtsconfig.jsonsrc/app/(app)/_layout.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxtypes/gluestack-ui.d.tssrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/ui/bottomsheet/index.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx__mocks__/expo-location.tssrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsxjest-setup.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and hooks over class components
Use PascalCase for React component names (e.g., UserProfile, ChatScreen)
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, windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Optimize images using react-native-fast-image
Use React Navigation for navigation and deep linking with best practices
Wrap all user-facing text in t() from react-i18next
Use react-hook-form for form handling
Use @rnmapbox/maps for maps or vehicle navigation
Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Use the conditional operator (?:) for conditional rendering instead of &&
**/*.tsx: Use functional components and hooks over class components
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
Optimize image handling using react-native-fast-image
Wrap all user-facing text in t() from react-i18next
Ensure support for dark mode and light mode
Ensure the app is accessible following WCAG for mobile
Use react-hook-form for form handling
Use @rnmapbox/maps for maps and navigation
Use lucide-react-native for icons directly in markup; do not us...
Files:
src/app/login/index.tsxsrc/components/ui/skeleton/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/push-notification/push-notification-modal.tsxsrc/components/calls/full-screen-image-modal.tsxsrc/components/roles/roles-modal.tsxsrc/lib/i18n/index.tsxsrc/app/(app)/_layout.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/ui/bottomsheet/index.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
{components/ui/**/*.{ts,tsx},**/*.tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Files:
src/app/login/index.tsxsrc/components/ui/skeleton/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/push-notification/push-notification-modal.tsxsrc/components/calls/full-screen-image-modal.tsxsrc/components/roles/roles-modal.tsxsrc/lib/i18n/index.tsxsrc/app/(app)/_layout.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/ui/bottomsheet/index.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
src/components/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Build and maintain shared Gluestack UI wrappers in src/components/ui
Files:
src/components/ui/skeleton/index.tsxsrc/components/ui/drawer/index.tsxsrc/components/ui/bottomsheet/index.tsx
tsconfig.json
📄 CodeRabbit inference engine (.cursorrules)
Enable strict typing in tsconfig.json
Files:
tsconfig.json
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit inference engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/services/__tests__/push-notification.test.tssrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to {components/ui/**/*.{ts,tsx},**/*.tsx} : Use gluestack-ui consistently; if no component exists in components/ui, style via StyleSheet.create or styled-components
Applied to files:
src/components/ui/drawer/index.tsxtypes/gluestack-ui.d.ts
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use gluestack-ui for styling; if no component exists in src/components/ui, style via StyleSheet.create or styled-components
Applied to files:
src/components/ui/drawer/index.tsxtypes/gluestack-ui.d.ts
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{ts,tsx} : Use react-i18next for internationalization
Applied to files:
src/lib/i18n/index.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to jest.config.@(js|ts) : Provide Jest configuration
Applied to files:
tsconfig.json
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated components
Applied to files:
tsconfig.jsonsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated components
Applied to files:
tsconfig.jsonsrc/components/settings/__tests__/unit-selection-bottom-sheet.test.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsxsrc/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to src/components/ui/**/*.{ts,tsx} : Build and maintain shared Gluestack UI wrappers in src/components/ui
Applied to files:
types/gluestack-ui.d.ts
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Wrap all user-facing text in t() from react-i18next
Applied to files:
src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx
📚 Learning: 2025-08-12T03:34:25.992Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-12T03:34:25.992Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use gluestack-ui icon component
Applied to files:
src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx
📚 Learning: 2025-08-12T03:33:40.238Z
Learnt from: CR
PR: Resgrid/Unit#0
File: .cursorrules:0-0
Timestamp: 2025-08-12T03:33:40.238Z
Learning: Applies to **/*.tsx : Use lucide-react-native for icons directly in markup; do not use the gluestack-ui icon component
Applied to files:
src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx
🧬 Code graph analysis (3)
src/lib/i18n/index.tsx (1)
src/lib/i18n/utils.tsx (1)
getLanguage(19-19)
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx (3)
src/stores/app/core-store.ts (1)
useCoreStore(48-263)src/stores/units/store.ts (1)
useUnitsStore(16-31)src/components/settings/unit-selection-bottom-sheet.tsx (1)
UnitSelectionBottomSheet(28-187)
src/services/__tests__/push-notification.test.ts (1)
src/stores/push-notification/store.ts (1)
usePushNotificationModalStore(31-96)
⏰ 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 (11)
tsconfig.json (1)
32-32: Include jest-platform-setup.ts in Jest setup configuration
• In jest.config.js’s setupFilesAfterEnv array (currently['<rootDir>/jest-setup.ts']), add'<rootDir>/jest-platform-setup.ts'so the global Platform mock is loaded.⛔ Skipped due to learnings
Learnt from: CR PR: Resgrid/Unit#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-12T03:34:25.992Z Learning: Applies to jest.config.@(js|ts) : Provide Jest configurationLearnt from: CR PR: Resgrid/Unit#0 File: .github/copilot-instructions.md:0-0 Timestamp: 2025-08-12T03:34:25.992Z Learning: Applies to **/*.{test,spec}.{ts,tsx} : Create and use Jest tests for all generated componentsLearnt from: CR PR: Resgrid/Unit#0 File: .cursorrules:0-0 Timestamp: 2025-08-12T03:33:40.238Z Learning: Applies to **/*.{test.ts,test.tsx,spec.ts,spec.tsx} : Create and use Jest tests to validate all generated componentssrc/components/roles/roles-modal.tsx (1)
128-133: Wrap user-facing text with i18n t()Button labels are localized—nice. Confirm all modal text (including errors) uses t() consistently across the app.
src/lib/i18n/index.tsx (1)
1-1: LGTM: switch to namespace import for expo-localizationThis is compatible with Expo 53+ typings.
src/components/settings/__tests__/server-url-bottom-sheet-simple.test.tsx (1)
15-24: LGTM: Single react-native mock resolves prior Platform mock conflictsConsolidating to one top-level mock is cleaner and avoids hoisting/resolution issues noted earlier.
src/components/status/__tests__/status-bottom-sheet.test.tsx (1)
3108-3157: Good coverage for overflow scenarios.Verifies button visibility with large lists; helpful against regressions.
types/gluestack-ui.d.ts (1)
58-66: Global HTMLAttributes augmentation: confirm DOM lib is included.This relies on DOM typings being present. If tsconfig excludes "dom", gate this behind a conditional types import or move into RN-web-only env.
Would you like me to scan tsconfig.json compilerOptions.lib and surface whether "dom" is enabled?
src/services/__tests__/push-notification.test.ts (1)
12-19: Expanded mocks look correct for Expo 53.Permissions/device/token/Android enums coverage is helpful.
Also applies to: 28-52
__mocks__/expo-location.ts (1)
45-76: Good: one-shot timer prevents leaked handles.The switch from setInterval to setTimeout with a removable subscription is the right trade-off for deterministic tests.
src/components/settings/__tests__/unit-selection-bottom-sheet-simple.test.tsx (2)
9-12: Platform-only mock is scoped correctly.Mocking react-native/Libraries/Utilities/Platform avoids clobbering RN exports.
317-346: Happy path and failure path both covered.Covers roles fetch chaining and toast/reporting; nice.
Also applies to: 348-372
src/components/settings/__tests__/unit-selection-bottom-sheet.test.tsx (1)
480-495: LGTM: concurrency guard behavior is covered.The test validates the ref-guard preventing duplicate selections. Nice.
| bottomSheetRef: { current: null as any }, | ||
| handleClose: () => { }, | ||
| handleOpen: () => { }, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove any from context default; keep types sound.
as any suppresses type safety and can hide ref typing issues. Cast the literal to the precise RefObject type instead.
- bottomSheetRef: { current: null as any },
+ bottomSheetRef: { current: null } as React.RefObject<GorhomBottomSheet>,Optionally, consider a safer pattern (outside this hunk): initialize the context as createContext<Ctx | null>(null) and expose a useBottomSheetContext() that throws if missing. This eliminates sentinel casts altogether.
📝 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.
| bottomSheetRef: { current: null as any }, | |
| handleClose: () => { }, | |
| handleOpen: () => { }, | |
| }); | |
| bottomSheetRef: { current: null } as React.RefObject<GorhomBottomSheet>, | |
| handleClose: () => { }, | |
| handleOpen: () => { }, | |
| }); |
🤖 Prompt for AI Agents
In src/components/ui/bottomsheet/index.tsx around lines 44 to 47, remove the
unsafe "as any" cast on the context default and replace it with a precise
RefObject type (e.g., React.RefObject<HTMLElement | null>) so bottomSheetRef is
typed as { current: HTMLElement | null }; ensure you import React/RefObject if
necessary and keep handleClose/handleOpen typed as () => void. Optionally
(outside this hunk) consider switching the context to createContext<Ctx |
null>(null) and adding a useBottomSheetContext() hook that throws when the
context is missing to avoid sentinel casts entirely.
Summary by CodeRabbit
Bug Fixes
Refactor
Tests
Chores