CU-868ffcgaz Countly implementation #170
Conversation
|
Task linked: CU-868ffcgaz Update to Expo 53 |
WalkthroughReplaces Aptabase with Countly across the app: swaps dependencies and env keys, removes Aptabase provider/mocks/tests, adds Countly SDK, CountlyProvider, a new Countly-backed analytics service with retry/disable logic, updates hooks, tests, and docs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Layout as App Layout
participant Provider as CountlyProvider
participant Cfg as CountlyConfig
participant SDK as Countly SDK
participant Svc as countlyService
participant Env as Env
App->>Layout: Render
Layout->>Provider: Render with appKey/serverURL
Provider->>Svc: isAnalyticsDisabled()
alt Disabled
Provider-->>App: Render children (no init)
else Enabled
Provider->>Env: Resolve appKey/serverURL (props or Env)
alt Missing config
Provider-->>App: Render children (skip init)
else Config present
Provider->>Cfg: Build CountlyConfig(serverURL, appKey)
Provider->>SDK: initWithConfig(Cfg)
SDK-->>Provider: success / error
alt success
Provider-->>App: Render children
else error
Provider->>Svc: reset()
Provider-->>App: Render children
end
end
end
sequenceDiagram
autonumber
actor Feature as App Feature
participant Hook as useAnalytics
participant Svc as countlyService
participant SDK as Countly SDK
Feature->>Hook: obtain trackEvent()
Feature->>Svc: trackEvent(name, props)
alt Service disabled
Svc-->>Feature: skip (log)
else Enabled
Svc->>Svc: convert props -> segmentation (strings)
Svc->>SDK: events.recordEvent(name, segmentation, 1)
SDK-->>Svc: success / error
alt error
rect rgba(255,230,230,0.12)
note right of Svc: increment retryCount\ndisable after maxRetries\nschedule re-enable after timeout
Svc->>Svc: handleAnalyticsError()
end
else success
Svc-->>Feature: resolved
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
__mocks__/countly-sdk-react-native-bridge.ts (1)
6-16: Expand Countly mock to mirror real SDK surfaceVerified mocks/countly-sdk-react-native-bridge.ts is missing initWithConfig, isInitialized and a root-level recordEvent — add the following.
const mockCountly = { - init: jest.fn().mockResolvedValue(undefined), - start: jest.fn().mockResolvedValue(undefined), - enableCrashReporting: jest.fn().mockResolvedValue(undefined), + init: jest.fn().mockResolvedValue(undefined), + initWithConfig: jest.fn().mockResolvedValue(undefined), + isInitialized: jest.fn().mockResolvedValue(true), + start: jest.fn().mockResolvedValue(undefined), + enableCrashReporting: jest.fn().mockResolvedValue(undefined), + recordEvent: jest.fn().mockResolvedValue(undefined), events: { recordEvent: jest.fn().mockResolvedValue(undefined), }, }; export default mockCountly;Also add a subpath mock for CountlyConfig to avoid pulling the real module in tests:
// __mocks__/countly-sdk-react-native-bridge/CountlyConfig.ts export default class CountlyConfig { constructor(public serverUrl: string, public appKey: string) {} setLoggingEnabled = jest.fn().mockReturnThis(); enableCrashReporting = jest.fn().mockReturnThis(); // add any chainable config methods used in code under test }src/services/aptabase.service.ts (1)
58-66: Reset retry counter after successful event to prevent false disables.Without resetting on success, a couple of non-consecutive errors can eventually trip
maxRetriesand disable analytics.Apply this diff:
- Countly.events.recordEvent(eventName, segmentation, 1); + Countly.events.recordEvent(eventName, segmentation, 1); + // success path: clear transient error budget + this.retryCount = 0;
🧹 Nitpick comments (20)
package.json (1)
103-103: Pin Countly SDK minor and verify Expo 53 compatibility.Recommend pinning to a tilde to avoid unexpected minor upgrades, and double‑check NA support with Expo SDK 53 (RN 0.79 / React 19). Docs show RN 0.60+ is supported and events API is stable.
- "countly-sdk-react-native-bridge": "^25.4.0", + "countly-sdk-react-native-bridge": "~25.4.0",Refs: Expo 53 uses RN 0.79/React 19; Countly RN bridge 25.4 docs. (docs.expo.dev)
env.js (2)
94-96: Tighten validation for Countly env vars.Enforce URL shape and non‑empty key (or allow empty only in dev). This prevents misconfig at runtime.
- COUNTLY_APP_KEY: z.string(), - COUNTLY_SERVER_URL: z.string(), + COUNTLY_APP_KEY: z.string().min(1, 'COUNTLY_APP_KEY is required'), + COUNTLY_SERVER_URL: z + .string() + .refine((v) => v === '' || /^https?:\/\//.test(v), 'COUNTLY_SERVER_URL must be http(s) URL or empty for local dev'),Optionally make APP_KEY required only for production-like envs via a refine on APP_ENV.
128-130: Propagate stricter validation by documenting required .env keys per APP_ENV.Current defaults are empty strings; add a short comment or README table indicating required keys per env to avoid silent disablement.
src/services/analytics.service.ts (4)
5-7: Export the event props type for reuse across the app.Avoid redefining the same shape in hooks/components.
-interface AnalyticsEventProperties { +export interface AnalyticsEventProperties { [key: string]: string | number | boolean; }
84-93: Don’t coerce all segmentation values to strings (unless required).Countly accepts segmentation as an object; stringifying can lose numeric fidelity and hamper aggregations. Either pass through as provided or gate on real SDK requirements.
- private convertPropertiesToCountlyFormat(properties: AnalyticsEventProperties = {}): Record<string, string> { - const segmentation: Record<string, string> = {}; + private convertPropertiesToCountlyFormat(properties: AnalyticsEventProperties = {}): Record<string, string | number | boolean> { + const segmentation: Record<string, string | number | boolean> = {}; for (const [key, value] of Object.entries(properties)) { - segmentation[key] = String(value); + segmentation[key] = value; } return segmentation; }If docs mandate strings only for RN, keep current behavior and add a brief comment with a link. (support.countly.com)
10-14: Remove or use retryDelay; it’s currently dead config.The option is exposed but unused; either implement a timed retry/queue or drop it to avoid confusion.
Example minimal cleanup:
- retryDelay?: number; @@ - private retryDelay = 2000; @@ - this.retryDelay = options.retryDelay ?? 2000;Also applies to: 19-19, 28-28
98-98: Avoid any for errors; prefer unknown and normalize.Aligns with the TS guideline to avoid any.
- private handleAnalyticsError(error: any, eventName?: string, properties?: AnalyticsEventProperties): void { + private handleAnalyticsError(error: unknown, eventName?: string, properties?: AnalyticsEventProperties): void {And normalize message extraction once:
const message = error instanceof Error ? error.message : String(error);src/hooks/use-analytics.ts (1)
3-3: Reuse the exported AnalyticsEventProperties type from the service.Removes duplication and keeps types in sync.
-import { countlyService } from '@/services/analytics.service'; +import { countlyService } from '@/services/analytics.service'; +import type { AnalyticsEventProperties } from '@/services/analytics.service'; @@ -interface AnalyticsEventProperties { - [key: string]: string | number | boolean; -} +// (type re-used from service)Also applies to: 5-7, 15-17
src/app/_layout.tsx (1)
193-201: Gate analytics on both key and server URL; optionally keep disabled in devAvoid initializing when COUNTLY_SERVER_URL is missing and consider keeping analytics off in DEV to reduce noise.
Apply:
- <KeyboardProvider> - {Env.COUNTLY_APP_KEY ? ( + <KeyboardProvider> + {Env.COUNTLY_APP_KEY && Env.COUNTLY_SERVER_URL /* && !__DEV__ */ ? ( <CountlyProvider appKey={Env.COUNTLY_APP_KEY} serverURL={Env.COUNTLY_SERVER_URL}> {renderContent()} </CountlyProvider> ) : ( renderContent() )} </KeyboardProvider>src/components/common/__tests__/countly-provider.test.tsx (2)
11-12: Remove unused importact is imported but unused.
-import { render, act, waitFor } from '@testing-library/react-native'; +import { render, waitFor } from '@testing-library/react-native';
144-179: Strengthen AptabaseProviderWrapper assertionsThese tests currently assert only render. Add expectations to verify the wrapper passes serverURL/appKey correctly into CountlyConfig/init.
Example:
- it('should use environment server URL when not provided', () => { - render(<AptabaseProviderWrapper {...mockProps} />); - expect(true).toBe(true); - }); + it('should use environment server URL when not provided', async () => { + const { default: Countly } = require('countly-sdk-react-native-bridge'); + const CountlyConfig = require('countly-sdk-react-native-bridge/CountlyConfig'); + render(<AptabaseProviderWrapper {...mockProps} />); + await waitFor(() => { + expect(CountlyConfig).toHaveBeenCalledWith('https://mock-countly-server.com', 'test-app-key'); + expect(Countly.initWithConfig).toHaveBeenCalled(); + }); + }); - it('should prefer provided server URL over environment', () => { - const propsWithServer = { ...mockProps, serverURL: 'https://custom-server.com' }; - render(<AptabaseProviderWrapper {...propsWithServer} />); - expect(true).toBe(true); - }); + it('should prefer provided server URL over environment', async () => { + const { default: Countly } = require('countly-sdk-react-native-bridge'); + const CountlyConfig = require('countly-sdk-react-native-bridge/CountlyConfig'); + render(<AptabaseProviderWrapper {...mockProps} serverURL="https://custom-server.com" />); + await waitFor(() => { + expect(CountlyConfig).toHaveBeenCalledWith('https://custom-server.com', 'test-app-key'); + expect(Countly.initWithConfig).toHaveBeenCalled(); + }); + });src/hooks/__tests__/use-analytics.test.ts (1)
27-37: Rename test to reference Countly, not AptabaseUpdate description for clarity post-migration.
- it('should call aptabaseService.trackEvent with correct parameters', () => { + it('should call countlyService.trackEvent with correct parameters', () => {src/components/common/countly-provider.tsx (3)
9-16: Make appKey/serverURL optional to match fallback behaviorProps are required but implementation falls back to Env vars. Make them optional to simplify usage and reflect reality.
-interface CountlyProviderProps { - appKey: string; - serverURL: string; +interface CountlyProviderProps { + appKey?: string; + serverURL?: string;
57-61: Enable SDK logging in development via CountlyConfigHelpful during integration; harmless in prod when guarded.
- const config = new CountlyConfig(urlToUse, keyToUse).enableCrashReporting().setRequiresConsent(false); + const config = new CountlyConfig(urlToUse, keyToUse) + .enableCrashReporting() + .setRequiresConsent(false) + .setLoggingEnabled?.(__DEV__ === true);
96-102: Aptabase wrapper: allow appKey optional like CountlyProviderMirror optionality for parity and easier migration.
-interface AptabaseProviderWrapperProps { - appKey: string; - serverURL?: string; +interface AptabaseProviderWrapperProps { + appKey?: string; + serverURL?: string;src/services/aptabase.service.ts (2)
88-105: Avoidanyand normalize error handling.Use
unknownand safe narrowing to improve type safety and log accuracy.Apply this diff:
- private handleAnalyticsError(error: any, eventName?: string, properties?: AnalyticsEventProperties): void { + private handleAnalyticsError(error: unknown, eventName?: string, properties?: AnalyticsEventProperties): void { @@ - logger.error({ + const msg = error instanceof Error ? error.message : String(error); + logger.error({ message: 'Analytics tracking error', context: { - error: error.message || String(error), + error: msg, eventName, properties,
51-56: Don’t log raw event properties (possible PII).Limit logs to keys to reduce risk.
Apply this diff:
- logger.debug({ - message: 'Attempting to track Countly event', - context: { eventName, segmentation, originalProperties: properties }, - }); + logger.debug({ + message: 'Attempting to track Countly event', + context: { eventName, segmentationKeys: Object.keys(segmentation) }, + });src/components/common/aptabase-provider.tsx (3)
92-101: Clear the test-event timeout on unmount.Avoid a stray callback if the component unmounts quickly.
Apply this diff:
- setTimeout(() => { + const initEvtTimer = setTimeout(() => { @@ - }, 1000); + }, 1000); @@ - return () => { - // Cleanup if needed - Countly doesn't require explicit cleanup - }; + return () => { + // Cleanup timer for the test event + if (typeof initEvtTimer !== 'undefined') { + clearTimeout(initEvtTimer); + } + };Also applies to: 128-131
14-15: Consider aligning with the latest SDK by using CountlyConfig + initWithConfig.Modern docs prefer
initWithConfigand setting logging/crash on the config object. Optional, but future-proof. (support.countly.com)Example:
import CountlyConfig from 'countly-sdk-react-native-bridge/CountlyConfig'; const cfg = new CountlyConfig(urlToUse, keyToUse); cfg.setLoggingEnabled(true); cfg.enableCrashReporting(); await Countly.initWithConfig(cfg);
16-17:initializationFailedstate is never read.Remove it or surface it (e.g., to log once in render) to avoid dead state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
__mocks__/@aptabase/react-native.ts(0 hunks)__mocks__/countly-sdk-react-native-bridge.ts(1 hunks)docs/countly-migration.md(1 hunks)env.js(2 hunks)package.json(1 hunks)src/app/_layout.tsx(2 hunks)src/app/call/__tests__/[id].test.tsx(1 hunks)src/components/common/__tests__/aptabase-provider.test.tsx(0 hunks)src/components/common/__tests__/countly-provider.test.tsx(1 hunks)src/components/common/aptabase-provider.tsx(2 hunks)src/components/common/countly-provider.tsx(1 hunks)src/hooks/__tests__/use-analytics.test.ts(2 hunks)src/hooks/use-analytics.ts(1 hunks)src/services/__tests__/aptabase.service.test.ts(0 hunks)src/services/__tests__/countly.service.test.ts(1 hunks)src/services/analytics.service.ts(1 hunks)src/services/aptabase.service.ts(5 hunks)
💤 Files with no reviewable changes (3)
- src/services/tests/aptabase.service.test.ts
- mocks/@aptabase/react-native.ts
- src/components/common/tests/aptabase-provider.test.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (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/hooks/use-analytics.tssrc/services/__tests__/countly.service.test.tssrc/components/common/__tests__/countly-provider.test.tsx__mocks__/countly-sdk-react-native-bridge.tssrc/components/common/countly-provider.tsxsrc/services/aptabase.service.tssrc/services/analytics.service.tssrc/app/_layout.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/common/aptabase-provider.tsxsrc/hooks/__tests__/use-analytics.test.ts
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/hooks/use-analytics.tssrc/services/__tests__/countly.service.test.tssrc/components/common/__tests__/countly-provider.test.tsxenv.jspackage.json__mocks__/countly-sdk-react-native-bridge.tssrc/components/common/countly-provider.tsxsrc/services/aptabase.service.tsdocs/countly-migration.mdsrc/services/analytics.service.tssrc/app/_layout.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/common/aptabase-provider.tsxsrc/hooks/__tests__/use-analytics.test.ts
**/*.{test.ts,test.tsx,spec.ts,spec.tsx}
📄 CodeRabbit inference engine (.cursorrules)
Create and use Jest tests to validate all generated components
Files:
src/services/__tests__/countly.service.test.tssrc/components/common/__tests__/countly-provider.test.tsxsrc/app/call/__tests__/[id].test.tsxsrc/hooks/__tests__/use-analytics.test.ts
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/services/__tests__/countly.service.test.tssrc/components/common/__tests__/countly-provider.test.tsxsrc/app/call/__tests__/[id].test.tsxsrc/hooks/__tests__/use-analytics.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/services/__tests__/countly.service.test.tssrc/components/common/__tests__/countly-provider.test.tsxsrc/app/call/__tests__/[id].test.tsxsrc/hooks/__tests__/use-analytics.test.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/components/common/__tests__/countly-provider.test.tsxsrc/components/common/countly-provider.tsxsrc/app/_layout.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/common/aptabase-provider.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/common/__tests__/countly-provider.test.tsxsrc/components/common/countly-provider.tsxsrc/app/_layout.tsxsrc/app/call/__tests__/[id].test.tsxsrc/components/common/aptabase-provider.tsx
🧠 Learnings (1)
📚 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:
src/components/common/__tests__/countly-provider.test.tsxsrc/app/call/__tests__/[id].test.tsx
🧬 Code graph analysis (8)
src/hooks/use-analytics.ts (1)
src/services/analytics.service.ts (2)
trackEvent(36-79)countlyService(204-204)
src/services/__tests__/countly.service.test.ts (2)
src/lib/logging/index.tsx (1)
logger(80-80)src/services/analytics.service.ts (1)
countlyService(204-204)
src/components/common/__tests__/countly-provider.test.tsx (3)
src/components/common/countly-provider.tsx (2)
CountlyProvider(15-84)AptabaseProviderWrapper(96-102)src/services/analytics.service.ts (1)
countlyService(204-204)src/components/common/aptabase-provider.tsx (1)
AptabaseProviderWrapper(144-150)
src/components/common/countly-provider.tsx (4)
src/services/analytics.service.ts (1)
countlyService(204-204)src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)env.js (1)
Env(166-166)src/components/common/aptabase-provider.tsx (2)
CountlyProviderWrapper(14-135)AptabaseProviderWrapper(144-150)
src/services/aptabase.service.ts (2)
src/lib/logging/index.tsx (1)
logger(80-80)src/services/analytics.service.ts (1)
countlyService(204-204)
src/services/analytics.service.ts (2)
src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)src/services/aptabase.service.ts (1)
countlyService(194-194)
src/components/common/aptabase-provider.tsx (5)
src/components/common/countly-provider.tsx (2)
CountlyProviderWrapper(87-87)AptabaseProviderWrapper(96-102)src/lib/logging/index.tsx (2)
logger(80-80)error(74-76)env.js (1)
Env(166-166)src/services/analytics.service.ts (1)
countlyService(204-204)src/services/aptabase.service.ts (1)
countlyService(194-194)
src/hooks/__tests__/use-analytics.test.ts (3)
src/services/analytics.service.ts (1)
countlyService(204-204)src/services/aptabase.service.ts (1)
countlyService(194-194)src/hooks/use-analytics.ts (1)
useAnalytics(14-22)
⏰ 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 (7)
src/services/analytics.service.ts (1)
58-58: API usage LGTM: Countly.events.recordEvent(name, segmentation, count).This matches the official RN bridge API.
Ref: Countly RN docs (recordEvent examples). (support.countly.com)
src/app/call/__tests__/[id].test.tsx (1)
335-346: Countly mock shape matches service usage — good swap from AptabaseThe mocked default export exposes events.recordEvent, which aligns with countlyService.trackEvent. No issues spotted.
src/services/__tests__/countly.service.test.ts (2)
96-145: Robust retry/disable/re-enable flow — solid coverageMax-retry disable and timer-based recovery assertions look correct and deterministic with fake timers.
45-54: Incorrect — service already stringifies segmentation before calling Countlyanalytics.service.convertPropertiesToCountlyFormat converts all property values to strings and trackEvent passes that segmentation to Countly.events.recordEvent(eventName, segmentation, 1), so the test expectation is correct.
Likely an incorrect or invalid review comment.
src/hooks/__tests__/use-analytics.test.ts (1)
35-48: Expectation alignment — goodVerifies forwarding of both defined and undefined properties to service.
src/services/aptabase.service.ts (1)
58-59: SDK API usage looks correct — repo already pins a compatible Countly version.package.json has countly-sdk-react-native-bridge = ^25.4.0; Countly.events.recordEvent is used in src/services/aptabase.service.ts:58, src/services/analytics.service.ts:58, and src/components/common/aptabase-provider.tsx:96.
src/components/common/aptabase-provider.tsx (1)
93-101: No action required — Countly.events API is correct for the pinned SDK (^25.4.0).
package.json shows countly-sdk-react-native-bridge: ^25.4.0 (25.x), which supports the events namespace; Countly.events.recordEvent is valid.
| ### 2. Countly Provider Wrapper (`src/components/common/aptabase-provider.tsx`) | ||
|
|
||
| - **Purpose**: Initializes Countly SDK with error handling | ||
| - **Features**: | ||
| - Safe initialization with error recovery | ||
| - Uses the Countly service for error management | ||
| - Always renders children (no provider wrapper required) | ||
| - Configurable with app key and server URL | ||
| - Backward compatible `AptabaseProviderWrapper` export | ||
|
|
There was a problem hiding this comment.
Fix incorrect file references and init API in docs
Docs point to aptabase-provider.tsx and show Countly.init/start, but code uses countly-provider.tsx with CountlyConfig + initWithConfig. Correct to prevent developer confusion.
-### 2. Countly Provider Wrapper (`src/components/common/aptabase-provider.tsx`)
+### 2. Countly Provider (`src/components/common/countly-provider.tsx`)
@@
-- **Purpose**: Initializes Countly SDK with error handling
- - Safe initialization with error recovery
- - Uses the Countly service for error management
- - Always renders children (no provider wrapper required)
- - Configurable with app key and server URL
- - Backward compatible `AptabaseProviderWrapper` export
+-- **Purpose**: Initializes Countly SDK with error handling
+ - Uses CountlyConfig + Countly.initWithConfig
+ - Safe initialization with error recovery via countlyService
+ - Always renders children (no provider context required)
+ - Configurable with app key and server URL
+ - Backward compatible `AptabaseProviderWrapper` export📝 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.
| ### 2. Countly Provider Wrapper (`src/components/common/aptabase-provider.tsx`) | |
| - **Purpose**: Initializes Countly SDK with error handling | |
| - **Features**: | |
| - Safe initialization with error recovery | |
| - Uses the Countly service for error management | |
| - Always renders children (no provider wrapper required) | |
| - Configurable with app key and server URL | |
| - Backward compatible `AptabaseProviderWrapper` export | |
| ### 2. Countly Provider (`src/components/common/countly-provider.tsx`) | |
| - **Purpose**: Initializes Countly SDK with error handling | |
| - **Features**: | |
| - Uses CountlyConfig + Countly.initWithConfig | |
| - Safe initialization with error recovery via countlyService | |
| - Always renders children (no provider context required) | |
| - Configurable with app key and server URL | |
| - Backward compatible `AptabaseProviderWrapper` export |
🤖 Prompt for AI Agents
In docs/countly-migration.md around lines 27 to 36, update the incorrect file
and API references: replace references to "aptabase-provider.tsx" with
"countly-provider.tsx" and change the described initialization call from
Countly.init/start to the actual API used in code (use CountlyConfig and
initWithConfig). Also adjust the text about exports/compatibility to match the
real export names in countly-provider.tsx so the doc matches the implementation
and avoids developer confusion.
| - Replaced `@aptabase/react-native` with `countly-sdk-react-native-bridge` | ||
| - Updated service to convert properties to Countly segmentation format | ||
| - Enhanced provider initialization with crash reporting support | ||
| - Improved mock implementations for testing | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Align “Technical Changes” with actual initialization approach
Reflect modern initWithConfig usage rather than Countly.init/start in examples.
-- Enhanced provider initialization with crash reporting support
+- Enhanced provider initialization using CountlyConfig.initWithConfig with crash reporting enabled📝 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.
| - Replaced `@aptabase/react-native` with `countly-sdk-react-native-bridge` | |
| - Updated service to convert properties to Countly segmentation format | |
| - Enhanced provider initialization with crash reporting support | |
| - Improved mock implementations for testing | |
| - Replaced `@aptabase/react-native` with `countly-sdk-react-native-bridge` | |
| - Updated service to convert properties to Countly segmentation format | |
| - Enhanced provider initialization using CountlyConfig.initWithConfig with crash reporting enabled | |
| - Improved mock implementations for testing |
🤖 Prompt for AI Agents
In docs/countly-migration.md around lines 145 to 149, the "Technical Changes"
bullets currently mention provider initialization using Countly.init/start;
update the wording to reflect the modern initWithConfig usage instead: change
the initialization bullet to explicitly state migration to initWithConfig
(including crash reporting setup via the config), and adjust any example
references in that section to reference initWithConfig usage where appropriate
so the bullets align with the actual initialization approach shown elsewhere in
the document.
| // Initialize Countly with the modern React Native SDK pattern | ||
| Countly.init(keyToUse, urlToUse, null); | ||
| logger.info({ message: 'Countly.init() completed' }); | ||
|
|
||
| // Enable debugging for Countly SDK | ||
| Countly.setLoggingEnabled(true); | ||
| logger.info({ message: 'Countly logging enabled' }); | ||
|
|
||
| // Enable crash reporting | ||
| Countly.enableCrashReporting(); | ||
| logger.info({ message: 'Countly crash reporting enabled' }); | ||
|
|
||
| // Start Countly | ||
| Countly.start(); | ||
| logger.info({ message: 'Countly.start() completed' }); | ||
|
|
There was a problem hiding this comment.
Fix Countly init order, argument order, and await initialization.
- Arg order should be
(serverURL, appKey), not(appKey, serverURL). - Enable logging and crash reporting before init.
- Await
init(and guard withisInitialized) to avoid racingstart. (support.countly.com)
Apply this diff:
- // Initialize Countly with the modern React Native SDK pattern
- Countly.init(keyToUse, urlToUse, null);
- logger.info({ message: 'Countly.init() completed' });
-
- // Enable debugging for Countly SDK
- Countly.setLoggingEnabled(true);
- logger.info({ message: 'Countly logging enabled' });
-
- // Enable crash reporting
- Countly.enableCrashReporting();
- logger.info({ message: 'Countly crash reporting enabled' });
-
- // Start Countly
- Countly.start();
- logger.info({ message: 'Countly.start() completed' });
+ // Enable debugging for Countly SDK (before init to capture early logs)
+ Countly.setLoggingEnabled(true);
+ logger.info({ message: 'Countly logging enabled' });
+
+ // Enable crash reporting before init
+ Countly.enableCrashReporting();
+ logger.info({ message: 'Countly crash reporting enabled' });
+
+ // Initialize Countly (server URL first). Guard against double init.
+ if (!(await Countly.isInitialized())) {
+ await Countly.init(urlToUse, keyToUse);
+ logger.info({ message: 'Countly.init() completed' });
+ }
+
+ // Start Countly session tracking
+ Countly.start();
+ logger.info({ message: 'Countly.start() completed' });References:
- Minimal setup (
init(serverURL, appKey)+start) and param order. (support.countly.com) - Crash reporting must be enabled before init. (support.countly.com)
📝 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.
| // Initialize Countly with the modern React Native SDK pattern | |
| Countly.init(keyToUse, urlToUse, null); | |
| logger.info({ message: 'Countly.init() completed' }); | |
| // Enable debugging for Countly SDK | |
| Countly.setLoggingEnabled(true); | |
| logger.info({ message: 'Countly logging enabled' }); | |
| // Enable crash reporting | |
| Countly.enableCrashReporting(); | |
| logger.info({ message: 'Countly crash reporting enabled' }); | |
| // Start Countly | |
| Countly.start(); | |
| logger.info({ message: 'Countly.start() completed' }); | |
| // Enable debugging for Countly SDK (before init to capture early logs) | |
| Countly.setLoggingEnabled(true); | |
| logger.info({ message: 'Countly logging enabled' }); | |
| // Enable crash reporting before init | |
| Countly.enableCrashReporting(); | |
| logger.info({ message: 'Countly crash reporting enabled' }); | |
| // Initialize Countly (server URL first). Guard against double init. | |
| if (!(await Countly.isInitialized())) { | |
| await Countly.init(urlToUse, keyToUse); | |
| logger.info({ message: 'Countly.init() completed' }); | |
| } | |
| // Start Countly session tracking | |
| Countly.start(); | |
| logger.info({ message: 'Countly.start() completed' }); |
🤖 Prompt for AI Agents
In src/components/common/aptabase-provider.tsx around lines 76 to 91, Countly is
initialized with the wrong argument order, logging/crash reporting are enabled
after init, and init is not awaited which can race with start; change the call
to await Countly.init(serverURL, appKey, ...) using the correct (serverURL,
appKey) parameter order, move Countly.setLoggingEnabled(true) and
Countly.enableCrashReporting() to before the init call, await the init call and
then check Countly.isInitialized (or equivalent) before calling Countly.start()
to ensure start() runs only after successful initialization.
| export const countlyService = new CountlyService(); | ||
|
|
||
| // Keep the old export name for backward compatibility during migration | ||
| export const aptabaseService = countlyService; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Unify to a single CountlyService instance (avoid divergent state).
This file instantiates its own CountlyService while src/services/analytics.service.ts also exports a countlyService singleton. That creates two independent instances with different retryCount/isDisabled states. Re-export the canonical instance instead.
Apply this diff at the bottom:
-export const countlyService = new CountlyService();
-
-// Keep the old export name for backward compatibility during migration
-export const aptabaseService = countlyService;
+export { countlyService } from './analytics.service';
+export { countlyService as aptabaseService } from './analytics.service';Optionally, replace this file with a thin bridge:
// src/services/aptabase.service.ts
export { countlyService } from './analytics.service';
export { countlyService as aptabaseService } from './analytics.service';🏁 Script executed:
#!/bin/bash
# Verify only one CountlyService class/instance remains
rg -nP -C2 --type=ts '\bclass\s+CountlyService\b|export\s+const\s+countlyService\b' srcLength of output: 1011
Replace duplicate CountlyService in aptabase.service.ts with re-exports from analytics.service.ts
Both src/services/analytics.service.ts and src/services/aptabase.service.ts define class CountlyService and export separate countlyService instances, creating two independent instances with divergent state. Remove the duplicate instance in aptabase.service.ts and re-export the canonical instance from analytics.service.ts.
-export const countlyService = new CountlyService();
-
-// Keep the old export name for backward compatibility during migration
-export const aptabaseService = countlyService;
+export { countlyService } from './analytics.service';
+export { countlyService as aptabaseService } from './analytics.service';📝 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.
| export const countlyService = new CountlyService(); | |
| // Keep the old export name for backward compatibility during migration | |
| export const aptabaseService = countlyService; | |
| export { countlyService } from './analytics.service'; | |
| export { countlyService as aptabaseService } from './analytics.service'; |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/(app)/__tests__/index.test.tsx (2)
297-307: Same rerender issue (theme change test)Drop the second argument to fix TS2554.
- const { rerender, unmount } = render(<Map />, { wrapper: TestWrapper }); + const { rerender, unmount } = render(<Map />, { wrapper: TestWrapper }); @@ - rerender(<Map />, { wrapper: TestWrapper }); + rerender(<Map />);
190-201: Remove invalid second argument from rerender calls (fix TS2554 / CI failure)rerender from @testing-library/react-native accepts only the element; remove the
{ wrapper: TestWrapper }argument.
- src/app/(app)/tests/index.test.tsx — lines 200, 224, 306: change
rerender(<Map />, { wrapper: TestWrapper });→rerender(<Map />);
🧹 Nitpick comments (2)
src/app/(app)/index.tsx (1)
344-345: Prevent overlap with the bottom tab barThe button may still sit under the tab bar. Consider adding the tab bar height to the offset.
Apply within the selected range:
recenterButton: { position: 'absolute' as const, - bottom: 20 + insets.bottom, + bottom: 12 + insets.bottom + tabBarHeight, right: 20,And include
tabBarHeightin the deps:- }, [colorScheme, insets.bottom]); + }, [colorScheme, insets.bottom, tabBarHeight]);Outside this range, add:
import { useBottomTabBarHeight } from '@react-navigation/bottom-tabs'; // ... const tabBarHeight = useBottomTabBarHeight();Also applies to: 362-362
src/components/ui/focus-aware-status-bar.tsx (1)
20-33: Align navigation bar behavior with render-time hidingYou set Android NavigationBar to overlay but the rendered
<SystemBars>always hidesnavigationBar. Consider making them consistent (only hide whenhiddenis true).Outside this range, update render:
return isFocused && (Platform.OS === 'android' || Platform.OS === 'ios') ? <SystemBars style={colorScheme} hidden={{ statusBar: hidden, navigationBar: hidden }} /> : null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/app/(app)/__tests__/index.test.tsx(13 hunks)src/app/(app)/_layout.tsx(3 hunks)src/app/(app)/calls.tsx(2 hunks)src/app/(app)/index.tsx(4 hunks)src/components/common/aptabase-provider.tsx(0 hunks)src/components/ui/focus-aware-status-bar.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/common/aptabase-provider.tsx
🧰 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/components/ui/focus-aware-status-bar.tsxsrc/app/(app)/calls.tsxsrc/app/(app)/index.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/app/(app)/_layout.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/ui/focus-aware-status-bar.tsxsrc/app/(app)/calls.tsxsrc/app/(app)/index.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/app/(app)/_layout.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/components/ui/focus-aware-status-bar.tsxsrc/app/(app)/calls.tsxsrc/app/(app)/index.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/app/(app)/_layout.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/focus-aware-status-bar.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/ui/focus-aware-status-bar.tsxsrc/app/(app)/calls.tsxsrc/app/(app)/index.tsxsrc/app/(app)/__tests__/index.test.tsxsrc/app/(app)/_layout.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/app/(app)/__tests__/index.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/app/(app)/__tests__/index.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/app/(app)/__tests__/index.test.tsx
🧠 Learnings (3)
📚 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 rnmapbox/maps for maps and navigation
Applied to files:
src/app/(app)/index.tsxsrc/app/(app)/__tests__/index.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 rnmapbox/maps for maps or vehicle navigation
Applied to files:
src/app/(app)/index.tsxsrc/app/(app)/__tests__/index.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:
src/app/(app)/__tests__/index.test.tsx
🧬 Code graph analysis (1)
src/app/(app)/calls.tsx (1)
src/components/ui/focus-aware-status-bar.tsx (1)
FocusAwareStatusBar(9-68)
🪛 Biome (2.1.2)
src/app/(app)/__tests__/index.test.tsx
[error] 6-6: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 GitHub Check: test
src/app/(app)/__tests__/index.test.tsx
[failure] 200-200:
Expected 1 arguments, but got 2.
[failure] 224-224:
Expected 1 arguments, but got 2.
[failure] 306-306:
Expected 1 arguments, but got 2.
🪛 GitHub Actions: React Native CI/CD
src/app/(app)/__tests__/index.test.tsx
[error] 200-200: tsc --noemit: TypeScript error TS2554: Expected 1 arguments, but got 2.
🔇 Additional comments (6)
src/app/(app)/index.tsx (1)
8-8: Safe-area aware recenter button — nice improvementImporting and using
useSafeAreaInsets()to offset the recenter button is correct, and the dependency update ongetThemedStylesensures recomputation when insets change.Also applies to: 32-32, 344-345, 362-362
src/app/(app)/calls.tsx (1)
14-14: Using FocusAwareStatusBar in Calls screen looks goodImport and placement are correct and consistent with the edge-to-edge approach.
Also applies to: 88-89
src/app/(app)/_layout.tsx (1)
10-10: Safe-area aware tab bar padding/height — good callUsing
useSafeAreaInsets()and applying it totabBarStyle.paddingBottomandheightimproves ergonomics on devices with a home indicator.Also applies to: 44-45, 262-265
src/app/(app)/__tests__/index.test.tsx (3)
11-14: SafeArea mocking is appropriateMocking
react-native-safe-area-contextand providing stable insets will keep layout-dependent tests deterministic.
102-105: Wrapper setup is correct
TestWrapperensures the Map screen gets SafeArea context in tests.
160-169: Render with wrapper — goodUsing
{ wrapper: TestWrapper }on initial render is the right approach.Also applies to: 172-179
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (13)
src/app/(app)/__tests__/index.test.tsx (1)
316-338: Make the analytics test assertive and ensure the mock actually takes effectMove the analytics mock to module scope so the Map import uses the mocked hook, and assert trackEvent was called with the theme.
// top of src/app/(app)/__tests__/index.test.tsx export const mockTrackEvent = jest.fn(); jest.mock('@/hooks/use-analytics', () => ({ useAnalytics: () => ({ trackEvent: mockTrackEvent }), }));Replace the inline comment in the test with:
- // Note: The analytics tracking is tested indirectly since we can't easily mock it in this setup + expect(mockTrackEvent).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ theme: 'dark' }) +);If Map uses a specific event key, assert that exact string instead of expect.any(String).
src/app/(app)/__tests__/calls.test.tsx (12)
6-21: Partially mock react-native to avoid clobbering the module.Overriding the whole module can break other imports (ScrollView, Text, etc.). Prefer extending the real module and overriding only what you need.
-jest.mock('react-native', () => ({ - Platform: { - OS: 'ios', - }, - Pressable: ({ children, onPress, testID, ...props }: any) => ( - <button onClick={onPress} data-testid={testID} role="button" {...props}>{children}</button> - ), - RefreshControl: () => null, - View: ({ children, ...props }: any) => <div {...props}>{children}</div>, - StatusBar: { - setBackgroundColor: jest.fn(), - setTranslucent: jest.fn(), - setHidden: jest.fn(), - setBarStyle: jest.fn(), - }, -})); +jest.mock('react-native', () => { + const RN = jest.requireActual('react-native'); + return { + ...RN, + Platform: { ...RN.Platform, OS: 'ios' }, + Pressable: ({ children, onPress, testID, ...props }: any) => ( + <button onClick={onPress} data-testid={testID} role="button" {...props}>{children}</button> + ), + RefreshControl: () => null, + View: ({ children, ...props }: any) => <div {...props}>{children}</div>, + StatusBar: { + setBackgroundColor: jest.fn(), + setTranslucent: jest.fn(), + setHidden: jest.fn(), + setBarStyle: jest.fn(), + }, + }; +});
92-98: Make CallCard pressable in the mock to enable real navigation assertions.Allow tests to simulate presses instead of asserting tautologies.
-jest.mock('@/components/calls/call-card', () => ({ - CallCard: ({ call }: any) => ( - <div data-testid={`call-card-${call.CallId}`}> - {call.Nature} - </div> - ), -})); +jest.mock('@/components/calls/call-card', () => ({ + CallCard: ({ call, onPress }: any) => ( + <button + onClick={onPress} + onPress={onPress} + data-testid={`call-card-${call.CallId}`} + > + {call.Nature} + </button> + ), +}));
154-169: Expose a stable testID and map onChange to onChangeText to test search filtering.jest.mock('@/components/ui/input', () => ({ Input: ({ children, ...props }: any) => <div {...props}>{children}</div>, - InputField: ({ value, onChangeText, placeholder, ...props }: any) => ( + InputField: ({ value, onChangeText, placeholder, testID = 'search-input', ...props }: any) => ( <input + data-testid={testID} value={value} - onChange={(e) => onChangeText(e.target.value)} + onChange={(e) => onChangeText?.(e.target.value)} placeholder={placeholder} role="textbox" {...props} /> ),
238-247: Assert the FAB actually renders (no tautologies).- // Check if the component renders without crashing and FAB is present - const htmlContent = tree.toJSON(); - expect(htmlContent).toBeTruthy(); - - // Since we can see the button in debug output, let's just verify the mock is working - expect(mockSecurityStore.canUserCreateCalls).toBe(true); + // Assert FAB presence explicitly + expect(screen.getByTestId('fab')).toBeTruthy();
268-275: Assert FAB absence directly.- const buttons = screen.queryAllByRole('button'); - // Only the search clear button should be present, not the FAB - expect(buttons.length).toBeLessThanOrEqual(1); + expect(screen.queryByTestId('fab')).toBeNull();
296-306: Assert rendered call cards instead of store shape.- // Verify that the mock calls data is set up correctly - expect(mockCallsStore.calls).toHaveLength(2); - expect(mockCallsStore.calls[0].CallId).toBe('call-1'); - expect(mockCallsStore.calls[1].CallId).toBe('call-2'); - - // The component should render without errors when calls are present - expect(true).toBe(true); + expect(screen.getByTestId('call-card-call-1')).toBeTruthy(); + expect(screen.getByTestId('call-card-call-2')).toBeTruthy();
308-317: Simulate card press and assert navigation.- // Verify the data setup - expect(mockCallsStore.calls[0].CallId).toBe('call-1'); - - // Since we can see in HTML output that buttons are rendered correctly, - // this test verifies the component renders the call data properly - expect(router.push).toBeDefined(); + fireEvent.press(screen.getByTestId('call-card-call-1')); + expect(router.push).toHaveBeenCalled();
319-329: Exercise search filtering behavior.- // Verify that the component renders with search functionality - // From HTML output we can see the input is there with correct placeholder - expect(mockCallsStore.calls).toHaveLength(2); - - // Test would verify search functionality but due to React Native Testing Library - // query limitations with HTML mocks, we verify setup instead - expect(true).toBe(true); + fireEvent.change(screen.getByTestId('search-input'), { + target: { value: 'Emergency Call 2' }, + }); + expect(screen.queryByTestId('call-card-call-1')).toBeNull(); + expect(screen.getByTestId('call-card-call-2')).toBeTruthy();
333-345: Assert loading indicator explicitly.- // Verify that the loading state is set correctly - expect(mockCallsStore.isLoading).toBe(true); - - // From HTML output we can see "calls.loading" text is rendered - expect(true).toBe(true); + expect(screen.getByTestId('loading')).toBeTruthy();
346-357: Assert error zero-state explicitly.- // Verify error state is set - expect(mockCallsStore.error).toBe('Network error'); - - // From HTML output we can see error text is rendered - expect(true).toBe(true); + expect(screen.getByTestId('error-state')).toBeTruthy();
359-371: Assert empty zero-state explicitly.- // Verify zero state setup - expect(mockCallsStore.calls).toHaveLength(0); - - // From HTML output we can see zero state text is rendered - expect(true).toBe(true); + expect(screen.getByTestId('zero-state')).toBeTruthy();
249-259: Simulate FAB press and assert navigation is called (use 'new-call-fab' and '/call/new/').
File: src/app/(app)/tests/calls.test.tsx — update the "navigates to new call screen when FAB is pressed" test.- // Verify that the router push function exists (it will be called when FAB is pressed) - expect(router.push).toBeDefined(); - - // The test should pass if the component renders without errors - expect(true).toBe(true); + fireEvent.press(screen.getByTestId('new-call-fab')); + expect(router.push).toHaveBeenCalled(); + // Optionally assert exact route: + // expect(router.push).toHaveBeenCalledWith('/call/new/');
♻️ Duplicate comments (1)
src/app/(app)/__tests__/index.test.tsx (1)
190-190: Good fix: rerender now reuses the existing wrapper contextRemoving the wrapper option from rerender resolves the “Expected 1 arguments” error and matches RTL’s API. Nice.
If you want to DRY this, add a tiny helper:
const renderMap = () => render(<MapScreen />, { wrapper: TestWrapper });Also applies to: 200-200, 216-216, 224-224, 297-297, 306-306
🧹 Nitpick comments (2)
src/app/(app)/__tests__/index.test.tsx (1)
4-4: SafeArea setup works; consider standard mock to reduce redundancyYou both mock SafeAreaProvider to pass-through and wrap with SafeAreaProvider. It’s harmless but redundant. Prefer the official jest mock for clarity.
Use:
-jest.mock('react-native-safe-area-context', () => ({ - SafeAreaProvider: ({ children }: { children: React.ReactNode }) => children, - useSafeAreaInsets: () => ({ top: 44, bottom: 34, left: 0, right: 0 }), -})); +jest.mock('react-native-safe-area-context', () => + require('react-native-safe-area-context/jest/mock') +);Then keep TestWrapper if you want explicit intent, or drop it entirely since the mock already supplies a provider.
Also applies to: 11-14, 102-105
src/app/(app)/__tests__/calls.test.tsx (1)
381-394: Make analytics assertion resilient with waitFor.- it('tracks view rendered event with correct parameters', () => { + it('tracks view rendered event with correct parameters', async () => { const mockCalls = [{ CallId: 'call-1', Nature: 'Test' }]; mockCallsStore.calls = mockCalls; useCallsStore.mockReturnValue(mockCallsStore); render(<CallsScreen />); - expect(mockAnalytics.trackEvent).toHaveBeenCalledWith('calls_view_rendered', { - callsCount: 1, - hasSearchQuery: false, - }); + await waitFor(() => + expect(mockAnalytics.trackEvent).toHaveBeenCalledWith('calls_view_rendered', { + callsCount: 1, + hasSearchQuery: false, + }) + ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/app/(app)/__tests__/calls.test.tsx(2 hunks)src/app/(app)/__tests__/index.test.tsx(11 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names (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/app/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/index.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/app/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/index.test.tsx
**/*
📄 CodeRabbit inference engine (.cursorrules)
Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)
Files:
src/app/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/index.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/app/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/index.test.tsx
**/__tests__/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/app/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/index.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/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/index.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Create and use Jest tests for all generated components
Files:
src/app/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/index.test.tsx
🧠 Learnings (3)
📚 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 rnmapbox/maps for maps and navigation
Applied to files:
src/app/(app)/__tests__/index.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 rnmapbox/maps for maps or vehicle navigation
Applied to files:
src/app/(app)/__tests__/index.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:
src/app/(app)/__tests__/index.test.tsx
🪛 Biome (2.1.2)
src/app/(app)/__tests__/index.test.tsx
[error] 6-6: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (4)
src/app/(app)/__tests__/calls.test.tsx (4)
179-185: Good, async-safe nav bar mocks.
186-189: Nativewind color scheme mock looks fine.
190-193: Edge-to-edge SystemBars mock is sufficient for tests.
195-197: FocusAwareStatusBar mock OK.
| jest.mock('@react-navigation/native', () => ({ | ||
| useFocusEffect: jest.fn((callback: () => void) => { | ||
| const React = require('react'); | ||
| React.useEffect(callback, []); | ||
| }), | ||
| useIsFocused: jest.fn(() => true), | ||
| })); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Preserve other @react-navigation/native exports and wire cleanup correctly.
Exporting only two hooks can break consumers; also model cleanup via useEffect wrapper.
-jest.mock('@react-navigation/native', () => ({
- useFocusEffect: jest.fn((callback: () => void) => {
- const React = require('react');
- React.useEffect(callback, []);
- }),
- useIsFocused: jest.fn(() => true),
-}));
+jest.mock('@react-navigation/native', () => {
+ const actual = jest.requireActual('@react-navigation/native');
+ const { useEffect } = require('react');
+ return {
+ ...actual,
+ useFocusEffect: jest.fn((callback: () => void | (() => void)) => {
+ useEffect(() => callback(), []);
+ }),
+ useIsFocused: jest.fn(() => true),
+ };
+});📝 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.
| jest.mock('@react-navigation/native', () => ({ | |
| useFocusEffect: jest.fn((callback: () => void) => { | |
| const React = require('react'); | |
| React.useEffect(callback, []); | |
| }), | |
| useIsFocused: jest.fn(() => true), | |
| })); | |
| jest.mock('@react-navigation/native', () => { | |
| const actual = jest.requireActual('@react-navigation/native'); | |
| const { useEffect } = require('react'); | |
| return { | |
| ...actual, | |
| useFocusEffect: jest.fn((callback: () => void | (() => void)) => { | |
| useEffect(() => callback(), []); | |
| }), | |
| useIsFocused: jest.fn(() => true), | |
| }; | |
| }); |
🤖 Prompt for AI Agents
In src/app/(app)/__tests__/calls.test.tsx around lines 200 to 206, the mock
replaces the entire @react-navigation/native module with only two hooks and
doesn't simulate effect cleanup; update the mock to import the real module via
jest.requireActual and spread its exports, then override only useFocusEffect and
useIsFocused; implement useFocusEffect as a jest.fn that uses React.useEffect to
call the provided callback and if that callback returns a cleanup function,
return that cleanup from the useEffect so teardown is modeled correctly, and
keep useIsFocused mocked to return true.
| import React from 'react'; | ||
| import { SafeAreaProvider } from 'react-native-safe-area-context'; | ||
|
|
||
| import Map from '../index'; |
There was a problem hiding this comment.
Fix linter error: imported Map shadows global Map
Rename the component variable to avoid Biome’s noShadowRestrictedNames error and future confusion.
Apply:
- import Map from '../index';
+ import MapScreen from '../index';And update usages:
- const { unmount } = render(<Map />, { wrapper: TestWrapper });
+ const { unmount } = render(<MapScreen />, { wrapper: TestWrapper });
- const { rerender, unmount } = render(<Map />, { wrapper: TestWrapper });
+ const { rerender, unmount } = render(<MapScreen />, { wrapper: TestWrapper });
- rerender(<Map />);
+ rerender(<MapScreen />);Also applies to: 161-161, 172-172, 190-190, 200-200, 216-216, 224-224, 241-241, 257-257, 275-275, 297-297, 306-306, 330-330
🧰 Tools
🪛 Biome (2.1.2)
[error] 6-6: Do not shadow the global "Map" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In src/app/(app)/__tests__/index.test.tsx around line 6 (and also update
occurrences at lines 161, 172, 190, 200, 216, 224, 241, 257, 275, 297, 306,
330), the imported identifier Map shadows the global Map which triggers Biome’s
noShadowRestrictedNames linter error; rename the import to a non-conflicting
name (for example MapComponent or MapView) in the import statement and update
all local usages within this test file to that new name, then run the
linter/tests to confirm the no-shadow error is resolved.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit
New Features
Refactor
Chores
Documentation
Tests