CU-868ex18rd Migrating to countly #75
Conversation
WalkthroughMigrates analytics from Aptabase to Countly. Updates env keys and client env schema, replaces the Aptabase service with a new Countly-based analytics service, integrates analytics initialization into app startup, removes the Aptabase provider, adjusts mocks and tests accordingly, and adds a migration document. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App as App Startup
participant Init as AppInitializationService
participant Analyt as analyticsService
participant Countly as Countly SDK
App->>Init: initialize()
Note over Init: Idempotent startup
Init->>Analyt: initializeWithEnv()
Analyt->>Countly: initWithConfig(CountlyConfig)
Countly-->>Analyt: Promise resolved/rejected
alt init success
Analyt-->>Init: initialized
Init-->>App: continue startup
else init failure
Analyt-->>Init: error thrown
Note over Init: Log error, do not block startup
Init-->>App: continue startup
end
sequenceDiagram
autonumber
actor UI as Component/Hook
participant Hook as useAnalytics
participant Analyt as analyticsService
participant Countly as Countly SDK
UI->>Hook: track(eventName, props)
Hook->>Analyt: trackEvent(eventName, props)
alt service initialized and enabled
Analyt->>Countly: events.recordEvent({ key, segmentation })
Countly-->>Analyt: ack/error
alt record success
Analyt-->>Hook: resolve
else record error
Analyt->>Analyt: increment retryCount<br/>maybe disable temporarily
Analyt-->>Hook: reject
end
else not initialized or disabled
Analyt-->>Hook: no-op / warn
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/(app)/__tests__/map.test.tsx (1)
316-323: Reset window-dimension mock between tests to avoid orientation bleedmockReturnValue persists across tests; set the default every beforeEach.
beforeEach(() => { jest.clearAllMocks(); + const mockUseWindowDimensions = (jest.requireMock('react-native') as any).useWindowDimensions; + mockUseWindowDimensions.mockReturnValue({ width: 375, height: 812 });
🧹 Nitpick comments (17)
src/components/calendar/__tests__/compact-calendar-item.test.tsx (1)
148-162: Stabilize time-range assertion to avoid locale/timezone flakiness.The regex is permissive, but output can still vary with device locale/TZ. Consider pinning TZ in tests (e.g., set process.env.TZ = 'UTC' in jest.setup) or mocking your date formatter to a fixed locale/timezone so this never flakes on CI.
.env.development (1)
24-25: Fix dotenv lints (ordering and trailing newline).Place COUNTLY keys before SENTRY_DSN and add a newline at EOF.
-RESPOND_SENTRY_DSN= -RESPOND_COUNTLY_APP_KEY= -RESPOND_COUNTLY_URL= +RESPOND_COUNTLY_APP_KEY= +RESPOND_COUNTLY_URL= +RESPOND_SENTRY_DSN= +env.js (1)
95-97: Tighten validation for Countly env vars (URL format, prod requirements).Validate https URLs and require non-empty values in production/internal while allowing empty in dev/staging.
- COUNTLY_URL: z.string(), - COUNTLY_APP_KEY: z.string(), + COUNTLY_URL: z + .string() + .refine( + (v) => v === '' || /^https:\/\/.+/i.test(v), + 'COUNTLY_URL must be an https URL (or empty outside production).' + ), + COUNTLY_APP_KEY: z + .string() + .refine( + (v) => !(APP_ENV === 'production' || APP_ENV === 'internal') || v.trim().length > 0, + 'COUNTLY_APP_KEY is required in production/internal.' + ),__mocks__/countly-sdk-react-native-bridge/CountlyConfig.ts (1)
1-8: Add common fluent setters to future-proof the mock.If initialization starts calling additional setters (e.g., setServerURL/setAppKey), tests will fail. Add them returning this.
export default jest.fn().mockImplementation(() => ({ setLoggingEnabled: jest.fn().mockReturnThis(), enableCrashReporting: jest.fn().mockReturnThis(), setRequiresConsent: jest.fn().mockReturnThis(), giveConsent: jest.fn().mockReturnThis(), setLocation: jest.fn().mockReturnThis(), enableParameterTamperingProtection: jest.fn().mockReturnThis(), + setServerURL: jest.fn().mockReturnThis(), + setAppKey: jest.fn().mockReturnThis(), }));src/services/app-initialization.service.ts (2)
81-82: Nit: Clarify or remove placeholder comment.Either expand this comment with concrete next steps (e.g., background services you plan to init) or drop it.
84-99: End analytics session during cleanup to avoid dangling sessions.Since init is handled here, consider ending the session in cleanup.
@@ async cleanup(): Promise<void> { try { + // End analytics session + try { + analyticsService.endSession?.(); + } catch (_) { + // no-op + }If you prefer not to modify this block here, I can follow up with a small PR to wire it in.
src/services/analytics.service.ts (4)
121-123: Pass explicit count to recordEvent for SDK parityThe RN SDK examples pass an event count arg; make the call explicit to avoid signature drift across SDK versions. (support.countly.com)
- Countly.events.recordEvent(eventName, segmentation); + Countly.events.recordEvent(eventName, segmentation, 1);
11-16: Remove or use retryDelay; currently unusedretryDelay is configurable but never applied. Either implement actual backoff or drop the option/field to avoid confusion (docs/text mention “retry/backoff” but code only disables after strikes).
interface AnalyticsServiceOptions { maxRetries?: number; - retryDelay?: number; enableLogging?: boolean; disableTimeout?: number; } @@ private retryCount = 0; private maxRetries = 2; - private retryDelay = 2000; private enableLogging = true; @@ constructor(options: AnalyticsServiceOptions = {}) { this.maxRetries = options.maxRetries ?? 2; - this.retryDelay = options.retryDelay ?? 2000; this.enableLogging = options.enableLogging ?? true; this.disableTimeout = options.disableTimeout ?? 10 * 60 * 1000; }Also applies to: 19-35
53-55: Avoid hard-coding consent off; expose a toggleConsent requirement is disabled by default; explicitly setting setRequiresConsent(false) is redundant and makes future privacy toggling harder. Consider reading a boolean from Env or config to enable consents when needed (GDPR/CCPA). (support.countly.com)
208-211: Clear pending re-enable timer on resetA scheduled re-enable can fire after reset and flip state unexpectedly during tests or fast app restarts. Store the timer id and clear it in reset.
private errorThrottleMs = 30000; private isInitialized = false; + private reenableTimer: ReturnType<typeof setTimeout> | null = null; @@ - setTimeout(() => { - this.enableAnalytics(); - }, this.disableTimeout); + this.reenableTimer = setTimeout(() => { + this.enableAnalytics(); + this.reenableTimer = null; + }, this.disableTimeout); @@ public reset(): void { + if (this.reenableTimer) { + clearTimeout(this.reenableTimer); + this.reenableTimer = null; + } this.retryCount = 0; this.isDisabled = false; this.lastErrorTime = 0; this.isInitialized = false; }Also applies to: 248-254, 19-27
src/services/__tests__/analytics.service.test.ts (2)
129-136: Don’t reach into private state directlyAccessing private fields risks TS errors and brittle tests. Cast to any at the point of mutation or introduce a tiny test helper on the service.
- analyticsService['isDisabled'] = true; + (analyticsService as any).isDisabled = true;
329-339: Avoid hard-coding the disable timeout in testsRead it from the service to keep tests resilient to config changes.
- // Fast-forward time to trigger re-enable - jest.advanceTimersByTime(10 * 60 * 1000); + const minutes = analyticsService.getStatus().disableTimeoutMinutes; + jest.advanceTimersByTime(minutes * 60_000);ANALYTICS_MIGRATION.md (2)
1-1: Fix markdownlint: no trailing punctuation in headingRemove the colon to satisfy MD026.
-# Analytics Migration: Aptabase to Countly +# Analytics Migration – Aptabase to Countly
12-27: Docs vs code drift: backoff and session APIsDoc claims “exponential backoff” and “start, end, extend session,” but code implements strike-based disable and only endSession. Either update docs to match or extend the service.
src/app/(app)/__tests__/map.test.tsx (3)
2-2: Remove unusedactimportKeeps the test lean.
-import { fireEvent, render, screen, waitFor, act } from '@testing-library/react-native'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react-native';
112-128: Fire onCameraChanged in MapView mock to make recenter path testableTrigger a single user-interaction event on mount so the recenter button appears when unlocked. This removes the conditional no-op in the recenter test.
- MapView: ({ children, testID }: any) => { - const { View } = require('react-native'); - return <View testID={testID}>{children}</View>; - }, + MapView: ({ children, testID, onCameraChanged }: any) => { + const React = require('react'); + const { View } = require('react-native'); + React.useEffect(() => { + onCameraChanged?.({ properties: { isUserInteraction: true } }); + }, []); + return <View testID={testID}>{children}</View>; + },
585-621: Make the recenter test assertive (not conditional)With the MapView mock emitting a user interaction, the button should exist; remove the guard and assert the analytics call deterministically.
- const recenterButton = screen.queryByTestId('recenter-button'); - if (recenterButton) { - fireEvent.press(recenterButton); - - // Check analytics tracking for recenter action - expect(mockTrackEvent).toHaveBeenCalledWith('map_recentered', { - timestamp: expect.any(String), - isMapLocked: false, - zoomLevel: 12, - }); - } + fireEvent.press(screen.getByTestId('recenter-button')); + expect(mockTrackEvent).toHaveBeenCalledWith('map_recentered', { + timestamp: expect.any(String), + isMapLocked: false, + zoomLevel: 12, + });
📜 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 (20)
.env.development(1 hunks)ANALYTICS_MIGRATION.md(1 hunks)__mocks__/@aptabase/react-native.ts(0 hunks)__mocks__/countly-sdk-react-native-bridge/CountlyConfig.ts(1 hunks)__mocks__/countly-sdk-react-native-bridge/index.ts(1 hunks)env.js(2 hunks)package.json(1 hunks)src/app/(app)/__tests__/map.test.tsx(14 hunks)src/app/_layout.tsx(1 hunks)src/components/calendar/__tests__/calendar-item-details-sheet-minimal.test.tsx(0 hunks)src/components/calendar/__tests__/calendar-item-details-sheet.security.test.tsx(0 hunks)src/components/calendar/__tests__/compact-calendar-item.test.tsx(1 hunks)src/components/common/aptabase-provider.tsx(0 hunks)src/hooks/__tests__/use-analytics.test.ts(2 hunks)src/hooks/use-analytics.ts(1 hunks)src/services/__tests__/analytics.service.test.ts(1 hunks)src/services/__tests__/aptabase.service.test.ts(0 hunks)src/services/analytics.service.ts(1 hunks)src/services/app-initialization.service.ts(2 hunks)src/services/aptabase.service.ts(0 hunks)
💤 Files with no reviewable changes (6)
- src/services/tests/aptabase.service.test.ts
- src/components/calendar/tests/calendar-item-details-sheet-minimal.test.tsx
- src/components/calendar/tests/calendar-item-details-sheet.security.test.tsx
- mocks/@aptabase/react-native.ts
- src/components/common/aptabase-provider.tsx
- src/services/aptabase.service.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components and favor interfaces for props and state
Avoid using any; use precise types
Use React Navigation for navigation and deep linking following best practices
Handle errors gracefully and provide user feedback
Implement proper offline support (caching, queueing, retries)
Use Expo SecureStore for sensitive data storage
Use zustand for state management
Use react-hook-form for form handling
Use react-query for data fetching and caching
Use react-native-mmkv for local storage
Use axios for API requests
**/*.{ts,tsx}: Write concise, type-safe TypeScript code
Use camelCase for variable and function names
Use TypeScript for all components, favoring interfaces for props and state
Avoid using any; strive for precise types
Ensure support for dark mode and light mode
Handle errors gracefully and provide user feedback
Use react-query for data fetching
Use react-i18next for internationalization
Use react-native-mmkv for local storage
Use axios for API requests
Files:
src/services/analytics.service.ts__mocks__/countly-sdk-react-native-bridge/index.tssrc/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/services/__tests__/analytics.service.test.tssrc/services/app-initialization.service.tssrc/hooks/use-analytics.tssrc/app/_layout.tsx__mocks__/countly-sdk-react-native-bridge/CountlyConfig.tssrc/hooks/__tests__/use-analytics.test.tssrc/app/(app)/__tests__/map.test.tsx
src/**
📄 CodeRabbit inference engine (.cursorrules)
src/**: Organize files by feature, grouping related components, hooks, and styles
Directory and file names should be lowercase and hyphenated (e.g., user-profile)
Files:
src/services/analytics.service.tssrc/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/services/__tests__/analytics.service.test.tssrc/services/app-initialization.service.tssrc/hooks/use-analytics.tssrc/app/_layout.tsxsrc/hooks/__tests__/use-analytics.test.tssrc/app/(app)/__tests__/map.test.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursorrules)
**/*.tsx: Use functional components and React hooks instead of class components
Use PascalCase for React component names
Use React.FC for defining functional components with props
Minimize useEffect/useState usage and avoid heavy computations during render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Provide getItemLayout to FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers; define callbacks with useCallback or outside render
Use gluestack-ui for styling where available from components/ui; otherwise, style via StyleSheet.create or styled-components
Ensure responsive design across screen sizes and orientations
Use react-native-fast-image for image handling instead of the default Image where appropriate
Wrap all user-facing text in t() from react-i18next for translations
Support dark mode and light mode in UI components
Use @rnmapbox/maps for maps or navigation features
Use lucide-react-native for icons directly; do not use the gluestack-ui icon component
Use conditional rendering with the ternary operator (?:) instead of &&
**/*.tsx: Use functional components and hooks over class components
Ensure components are modular, reusable, and maintainable
Ensure all components are mobile-friendly, responsive, and support both iOS and Android
Use PascalCase for component names
Utilize React.FC for defining functional components with props
Minimize useEffect, useState, and heavy computations inside render
Use React.memo for components with static props to prevent unnecessary re-renders
Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, and windowSize
Use getItemLayout for FlatList when items have consistent size
Avoid anonymous functions in renderItem or event handlers to prevent re-renders
Ensure responsive design for different screen sizes and orientations
Optimize image handling using rea...
Files:
src/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/app/_layout.tsxsrc/app/(app)/__tests__/map.test.tsx
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{test,spec}.{ts,tsx}: Create Jest tests for all generated components, services, and logic
Ensure tests run without errors and fix failing tests
Files:
src/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/services/__tests__/analytics.service.test.tssrc/hooks/__tests__/use-analytics.test.tssrc/app/(app)/__tests__/map.test.tsx
src/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.test.{ts,tsx}: Create and use Jest tests to validate all generated components
Generate tests for all components, services, and logic; ensure tests run without errors
Files:
src/components/calendar/__tests__/compact-calendar-item.test.tsxsrc/services/__tests__/analytics.service.test.tssrc/hooks/__tests__/use-analytics.test.tssrc/app/(app)/__tests__/map.test.tsx
🧠 Learnings (3)
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps, mapping, or vehicle navigation
Applied to files:
src/app/(app)/__tests__/map.test.tsx
📚 Learning: 2025-08-21T02:46:42.743Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .cursorrules:0-0
Timestamp: 2025-08-21T02:46:42.743Z
Learning: Applies to **/*.tsx : Use rnmapbox/maps for maps or navigation features
Applied to files:
src/app/(app)/__tests__/map.test.tsx
📚 Learning: 2025-08-21T02:47:24.081Z
Learnt from: CR
PR: Resgrid/Responder#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-08-21T02:47:24.081Z
Learning: Applies to src/**/*.test.{ts,tsx} : Create and use Jest tests to validate all generated components
Applied to files:
src/app/(app)/__tests__/map.test.tsx
🧬 Code graph analysis (7)
src/services/analytics.service.ts (1)
env.js (1)
Env(170-170)
__mocks__/countly-sdk-react-native-bridge/index.ts (1)
src/services/analytics.service.ts (1)
endSession(320-339)
src/services/__tests__/analytics.service.test.ts (1)
src/services/analytics.service.ts (1)
analyticsService(342-342)
src/services/app-initialization.service.ts (1)
src/services/analytics.service.ts (1)
analyticsService(342-342)
src/hooks/use-analytics.ts (1)
src/services/analytics.service.ts (2)
trackEvent(97-134)analyticsService(342-342)
src/hooks/__tests__/use-analytics.test.ts (2)
src/services/analytics.service.ts (1)
analyticsService(342-342)src/hooks/use-analytics.ts (1)
useAnalytics(14-22)
src/app/(app)/__tests__/map.test.tsx (1)
src/app/(app)/map.tsx (1)
HomeMap(26-382)
🪛 dotenv-linter (3.3.0)
.env.development
[warning] 24-24: [UnorderedKey] The RESPOND_COUNTLY_APP_KEY key should go before the RESPOND_SENTRY_DSN key
(UnorderedKey)
[warning] 25-25: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 25-25: [UnorderedKey] The RESPOND_COUNTLY_URL key should go before the RESPOND_SENTRY_DSN key
(UnorderedKey)
🪛 markdownlint-cli2 (0.17.2)
ANALYTICS_MIGRATION.md
20-20: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ 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 (10)
src/components/calendar/__tests__/compact-calendar-item.test.tsx (1)
351-353: LGTM: assertion now targets Card directly.This matches the component change (Card without CardContent) and keeps the test intent clear.
package.json (1)
96-96: Countly RN bridge added — verify native build & remove Aptabase leftovers
- Findings:
@aptabase/react-nativeonly appears in ANALYTICS_MIGRATION.md (ANALYTICS_MIGRATION.md:9) — no runtime/package references found.- Findings:
countly-sdk-react-native-bridgeis added to package.json (line ~96); repo search returned no other references and no app.config.* plugin hooks mentioning countly.- Action required: Run EAS prebuild and iOS Pod install for Expo SDK 53 / RN 0.79.5 on both platforms and confirm both builds succeed; run a package check (yarn why / npm ls or rg) to ensure no Aptabase runtime deps remain and remove the doc “Reinstall: yarn add @aptabase/react-native” if Aptabase is intentionally removed.
src/app/_layout.tsx (1)
199-199: LGTM: Removed Aptabase wrapper; render tree is simplified.KeyboardProvider directly wraps content; analytics is now initialized via appInitializationService, which is appropriate.
env.js (1)
131-133: Ensure RESPOND_COUNTLY_APP_KEY and RESPOND_COUNTLY_URL are present and non-empty in all .env. files and CI secrets.*
- .env.development — both keys exist but are empty (lines 24–25).
- Scan found no other .env.* files; confirm production/.env.ci and CI secret injections contain non-empty values to avoid runtime "initialized=false".
__mocks__/countly-sdk-react-native-bridge/index.ts (1)
1-20: Solid Jest mock surface for Countly.Covers init, events, session, and init state; default export matches named API. Good for tests.
src/hooks/use-analytics.ts (1)
3-17: Migration to analyticsService looks good.The hook remains stable and memoized; passthrough to analyticsService is correct.
src/services/app-initialization.service.ts (1)
74-76: Good: Initialize analytics early in the startup sequence.Running analytics init before other services is appropriate and non-blocking given your try/catch.
src/hooks/__tests__/use-analytics.test.ts (1)
3-11: LGTM: hook wiring and mocks are cleanMocks the singleton correctly and exercises the public surface with and without props. Types are fine for a test context.
Also applies to: 14-15, 27-37, 39-48
src/services/__tests__/analytics.service.test.ts (1)
55-68: Initialization expectations align with SDKAsserting CountlyConfig(serverUrl, appKey) and initWithConfig once is correct for 25.x RN bridge. (support.countly.com)
ANALYTICS_MIGRATION.md (1)
8-11: Don't hard‑pin the SDK version in docs — reference package.json or enforce in CIpackage.json shows countly-sdk-react-native-bridge: ^25.4.0; ANALYTICS_MIGRATION.md (lines 8–11) is pinned to 25.4.0. Update the doc to '^25.4.0' or derive/inject the version from package.json (or add a CI check to fail on mismatch).
| - **Updated** `env.js` to support Countly variables: | ||
| - `RESPOND_COUNTLY_APP_KEY` (replaces `RESPOND_APTABASE_APP_KEY`) | ||
| - `RESPOND_COUNTLY_URL` (replaces `RESPOND_APTABASE_URL`) | ||
| - **Updated** `.env.development` with new variable names | ||
|
|
There was a problem hiding this comment.
Align env var names with implementation
Docs list RESPOND_COUNTLY_* keys, but the service currently reads COUNTLY_*; reconcile both sides and document the final names.
- - `RESPOND_COUNTLY_APP_KEY` (replaces `RESPOND_APTABASE_APP_KEY`)
- - `RESPOND_COUNTLY_URL` (replaces `RESPOND_APTABASE_URL`)
+ - `RESPOND_COUNTLY_APP_KEY` (or `COUNTLY_APP_KEY` for backward compat)
+ - `RESPOND_COUNTLY_URL` (or `COUNTLY_URL` for backward compat)
@@
-RESPOND_COUNTLY_APP_KEY=your_countly_app_key_here
-RESPOND_COUNTLY_URL=https://your-countly-server.com
+RESPOND_COUNTLY_APP_KEY=your_countly_app_key_here
+RESPOND_COUNTLY_URL=https://your-countly-server.com
+# Optionally support legacy names if present:
+# COUNTLY_APP_KEY=your_countly_app_key_here
+# COUNTLY_URL=https://your-countly-server.comAlso applies to: 79-83
🤖 Prompt for AI Agents
In ANALYTICS_MIGRATION.md around lines 34–38 and 79–83 the docs reference
RESPOND_COUNTLY_* env vars but the service reads COUNTLY_*; update the
documentation and any example .env entries to use the actual runtime names
(COUNTLY_APP_KEY and COUNTLY_URL) so names match implementation (or
alternatively change the service to read RESPOND_COUNTLY_* if you prefer that
convention) — make the change consistently in both places and update any mention
of replacing RESPOND_APTABASE_* to reflect the chosen final names.
| public async initializeWithEnv(): Promise<void> { | ||
| const appKey = Env.COUNTLY_APP_KEY; | ||
| const serverUrl = Env.COUNTLY_URL; | ||
|
|
||
| if (!appKey || !serverUrl) { | ||
| if (this.enableLogging) { | ||
| logger.warn({ | ||
| message: 'Analytics environment variables not configured, skipping initialization', | ||
| context: { hasAppKey: Boolean(appKey), hasServerUrl: Boolean(serverUrl) }, | ||
| }); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| return this.initialize(appKey, serverUrl); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix env key mismatch to avoid skipping initialization
Service reads COUNTLY_APP_KEY/COUNTLY_URL, while the migration docs instruct RESPOND_COUNTLY_APP_KEY/RESPOND_COUNTLY_URL. This will silently skip init in non-test environments. Support both (new first, fallback to old) or align names across code and docs.
Apply:
- public async initializeWithEnv(): Promise<void> {
- const appKey = Env.COUNTLY_APP_KEY;
- const serverUrl = Env.COUNTLY_URL;
+ public async initializeWithEnv(): Promise<void> {
+ const appKey = Env.RESPOND_COUNTLY_APP_KEY ?? Env.COUNTLY_APP_KEY;
+ const serverUrl = Env.RESPOND_COUNTLY_URL ?? Env.COUNTLY_URL;📝 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.
| public async initializeWithEnv(): Promise<void> { | |
| const appKey = Env.COUNTLY_APP_KEY; | |
| const serverUrl = Env.COUNTLY_URL; | |
| if (!appKey || !serverUrl) { | |
| if (this.enableLogging) { | |
| logger.warn({ | |
| message: 'Analytics environment variables not configured, skipping initialization', | |
| context: { hasAppKey: Boolean(appKey), hasServerUrl: Boolean(serverUrl) }, | |
| }); | |
| } | |
| return; | |
| } | |
| return this.initialize(appKey, serverUrl); | |
| } | |
| public async initializeWithEnv(): Promise<void> { | |
| const appKey = Env.RESPOND_COUNTLY_APP_KEY ?? Env.COUNTLY_APP_KEY; | |
| const serverUrl = Env.RESPOND_COUNTLY_URL ?? Env.COUNTLY_URL; | |
| if (!appKey || !serverUrl) { | |
| if (this.enableLogging) { | |
| logger.warn({ | |
| message: 'Analytics environment variables not configured, skipping initialization', | |
| context: { hasAppKey: Boolean(appKey), hasServerUrl: Boolean(serverUrl) }, | |
| }); | |
| } | |
| return; | |
| } | |
| return this.initialize(appKey, serverUrl); | |
| } |
🤖 Prompt for AI Agents
In src/services/analytics.service.ts around lines 77 to 92, the service
currently reads Env.COUNTLY_APP_KEY and Env.COUNTLY_URL but the migration docs
and environment use RESPOND_COUNTLY_APP_KEY / RESPOND_COUNTLY_URL; update the
code to prefer the new RESPOND_* keys and fall back to the old COUNTLY_* keys
(e.g., appKey = Env.RESPOND_COUNTLY_APP_KEY ?? Env.COUNTLY_APP_KEY; serverUrl =
Env.RESPOND_COUNTLY_URL ?? Env.COUNTLY_URL), keep the existing logging behavior
and context but ensure the hasAppKey/hasServerUrl booleans reflect the final
values checked, and then call initialize(appKey, serverUrl) as before when both
are present.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Summary by CodeRabbit
New Features
Chores
Documentation
Tests