Conversation
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces Sentry error tracking integration, refactors the map screen from a side-menu drawer architecture to a tab-based layout, and adds comprehensive POI filtering with custom marker rendering. It also updates dependency versions, adds translations across multiple languages, and expands data models to support POI images. ChangesSentry Error Tracking Integration
Map Navigation Refactoring
POI Enhancement & Filtering
Minor Updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes The PR spans three independent, substantial feature areas (Sentry integration, map layout refactoring, and POI enhancement with filtering) across 40+ files. Logic density is moderate-to-high in the POI components (custom SVG rendering, filtering logic, icon resolution), requiring separate reasoning for each subsystem. The map refactoring touches core screen architecture with navigation wiring. Multiple data model updates and comprehensive translation additions add to scope diversity. Large repetitive sections (translation updates across nine languages) reduce per-file review burden, but the heterogeneity of feature types and integration points demands careful verification of interdependencies and intent.
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 (1)
src/app/(app)/messages.tsx (1)
397-401:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse ternary instead of
&&for conditional rendering.The FAB block uses
&&chaining for conditional rendering. Per coding guidelines, conditional rendering must use? :(never&&) since&&can render unintended falsy values like0/''in React Native.♻️ Proposed fix
- {!isSelectionMode && canUserCreateMessages && ( - <Fab placement="bottom right" size="lg" onPress={() => handleOpenCompose('fab')} testID="messages-compose-fab"> - <FabIcon as={MessageSquarePlus} size="lg" /> - </Fab> - )} + {!isSelectionMode && canUserCreateMessages ? ( + <Fab placement="bottom right" size="lg" onPress={() => handleOpenCompose('fab')} testID="messages-compose-fab"> + <FabIcon as={MessageSquarePlus} size="lg" /> + </Fab> + ) : null}The same pattern applies to line 320 (
{isLoading && filteredMessages.length === 0 && <Loading />}). As per coding guidelines: "Conditional rendering: use? :— never&&."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/messages.tsx around lines 397 - 401, Replace the conditional rendering that uses && with explicit ternary expressions: for the FAB block, change the expression that depends on isSelectionMode and canUserCreateMessages so it returns the <Fab> element when the condition is true and null/empty when false (locate the fragment using isSelectionMode, canUserCreateMessages, handleOpenCompose and testID "messages-compose-fab"); similarly update the loading block that uses isLoading && filteredMessages.length === 0 to use a ternary that renders <Loading /> when true and null/empty when false (locate isLoading, filteredMessages and Loading). Ensure no other logic changes—only replace the && chains with ? : returning the component or null.
🧹 Nitpick comments (9)
src/components/maps/poi-filter-bottom-sheet.tsx (1)
42-44: 💤 Low valueRedundant
handleDonewrapper.
handleDoneonly forwardsonClose(). You can passonClosedirectly to the Done button to avoid creating an extrauseCallback.♻️ Proposed simplification
- const handleDone = useCallback(() => { - onClose(); - }, [onClose]); @@ - <Button className="flex-1 bg-primary-600 dark:bg-primary-500" onPress={handleDone}> + <Button className="flex-1 bg-primary-600 dark:bg-primary-500" onPress={onClose}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/maps/poi-filter-bottom-sheet.tsx` around lines 42 - 44, The handleDone wrapper simply calls onClose and can be removed: delete the useCallback-defined handleDone function and any dependency referencing it, then pass the existing onClose directly to the Done button's onPress/onClick prop (where handleDone was used). Also remove the now-unused useCallback import if applicable and any references to handleDone.src/components/maps/poi-list-panel.tsx (1)
142-148: 💤 Low valueInline arrow handlers create new functions per render.
Both the clear-search slot's
onPressand the input'sonChangeText(when wrapped in arrows elsewhere) recreate handlers on every render. Hoist them withuseCallbackfor consistency with the rest of the component (which already memoizesopenFilterSheet,closeFilterSheet, etc.) and to avoid unnecessary re-renders of memoized children.♻️ Suggested cleanup
+ const clearSearchQuery = useCallback(() => setSearchQuery(''), []); @@ - <InputField placeholder={t('poi.search_placeholder', 'Search POIs...')} value={searchQuery} onChangeText={setSearchQuery} /> - {searchQuery ? ( - <InputSlot className="pr-3" onPress={() => setSearchQuery('')}> + <InputField placeholder={t('poi.search_placeholder', 'Search POIs...')} value={searchQuery} onChangeText={setSearchQuery} /> + {searchQuery ? ( + <InputSlot className="pr-3" onPress={clearSearchQuery}> <InputIcon as={XIcon} /> </InputSlot> ) : null}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/maps/poi-list-panel.tsx` around lines 142 - 148, The inline arrow handlers for clearing and updating the search recreate functions each render; wrap the setter calls in memoized callbacks using React.useCallback and replace the inline onPress and onChangeText with these callbacks (e.g. create const handleClearSearch = useCallback(() => setSearchQuery(''), [setSearchQuery]) and const handleChangeSearch = useCallback((val) => setSearchQuery(val), [setSearchQuery]) then pass handleClearSearch to InputSlot.onPress and handleChangeSearch to InputField.onChangeText) so memoized children like InputSlot/InputField and other memoized callbacks remain stable.src/components/maps/poi-icon.tsx (1)
1-2: ⚡ Quick winUse named imports from
lucide-react-nativeinstead of namespace import.While recent versions of lucide-react-native have improved how namespace imports are handled, the official documentation still recommends direct named imports like
import { Activity } from 'lucide-react-native'. Switching fromimport * as LucideIconsto named imports ensures better tree-shaking support across all bundler configurations and aligns with best practices. This is especially important when only a subset of icons is used.♻️ Proposed change
-import type { LucideIcon } from 'lucide-react-native'; -import * as LucideIcons from 'lucide-react-native'; +import type { LucideIcon } from 'lucide-react-native'; +import { + Antenna, Armchair, ArrowDown, Baby, Beer, Bell, Bike, Building, Building2, Bus, + Car, Church, Circle, CircleParking, Clapperboard, Coffee, Croissant, Cross, Dices, + DoorOpen, Droplets, Dumbbell, FerrisWheel, Fish, Flag, Flame, Flower, Footprints, + Fuel, Gavel, Gem, Globe, GraduationCap, Hammer, Home, Hospital, Hotel, Info, Key, + Landmark, Library, Mail, MapPin, Monitor, Mountain, Music, Paintbrush, Palette, + PawPrint, PillBottle, Phone, Plane, Route, Scale, School, Scissors, Shield, + ShieldAlert, ShieldCheck, Ship, Shirt, ShoppingBasket, ShoppingCart, Square, + Stethoscope, Store, Tent, TrainFront, TreePine, Truck, Utensils, Warehouse, Waves, + Wind, Wrench, Zap, +} from 'lucide-react-native'; @@ -const LUCIDE_ICON_MAP: Record<string, LucideIcon> = { +const LUCIDE_ICON_MAP: Record<string, LucideIcon> = { // Map marker icons - 'map-pin': LucideIcons.MapPin, + 'map-pin': MapPin, // ...etc };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/maps/poi-icon.tsx` around lines 1 - 2, The file currently uses a namespace import "import * as LucideIcons" which prevents optimal tree-shaking; replace the namespace import with direct named imports from 'lucide-react-native' for each icon used (keep the type import "LucideIcon" if needed), and update usages that reference LucideIcons.SomeIcon to refer directly to the named symbol (or create an explicit icon map of named imports if dynamic lookup is required); ensure only the icons actually used in this component are imported to improve bundling.src/constants/poi-marker-shapes.ts (2)
69-77: 💤 Low valueEmpty-string guard is redundant.
The
if (normalized.length === 0)branch is unreachable in practice becausePOI_MARKER_PATHS['']isundefinedand the?? MAP_PIN_PATHfallback below handles the empty-string case identically. The two-line return can be removed for slightly clearer code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/constants/poi-marker-shapes.ts` around lines 69 - 77, The empty-string guard in getPoiMarkerShapePath is redundant; compute normalized as you already do and replace the two-branch logic with a single return that looks up POI_MARKER_PATHS[normalized] and falls back to MAP_PIN_PATH (i.e., remove the if (normalized.length === 0) branch and use POI_MARKER_PATHS[normalized] ?? MAP_PIN_PATH instead), keeping the normalized computation from markerShape intact.
51-57: 💤 Low valueTighten typing of
POI_MARKER_PATHS.Using
Record<string, string>discards thePoiMarkerShapeunion and prevents the compiler from catching missing keys when new shapes are added to the type. ConsiderRecord<PoiMarkerShape, string>(looking up with a normalized string is still safe via the?? MAP_PIN_PATHfallback ingetPoiMarkerShapePath).♻️ Proposed refactor
-export const POI_MARKER_PATHS: Record<string, string> = { +export const POI_MARKER_PATHS: Record<PoiMarkerShape, string> = { MAP_PIN: MAP_PIN_PATH, SHIELD: SHIELD_PATH, ROUTE: ROUTE_PATH, SQUARE: SQUARE_PATH, SQUARE_ROUNDED: SQUARE_ROUNDED_PATH, -} as const; +};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/constants/poi-marker-shapes.ts` around lines 51 - 57, POI_MARKER_PATHS is typed too loosely (Record<string,string>) which loses the PoiMarkerShape union and prevents compile-time checks; change its type to Record<PoiMarkerShape, string> so the compiler enforces all shape keys, update the declaration of POI_MARKER_PATHS accordingly, and keep existing runtime lookup in getPoiMarkerShapePath (with its ?? MAP_PIN_PATH fallback) so normalized string lookups remain safe.src/components/maps/map-pins.tsx (1)
48-71: ⚡ Quick winAvoid inline arrow handlers inside the map iteration.
Both
<PoiPinMarker onPress={() => onPinPress?.(pin)} />(Line 54) and<PinMarker ... onPress={() => onPinPress?.(pin)} />(Line 68) allocate a fresh closure for every pin on every render, defeating any memoization on the marker children. Consider extracting a small memoized child (e.g.,MapPinItem/PoiPinItem) that receivespinand a stableonPinPressand binds the press internally withuseCallback.As per coding guidelines: "Avoid anonymous functions in
renderItemor event handlers to prevent re-renders".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/maps/map-pins.tsx` around lines 48 - 71, The inline arrow handlers passed to PoiPinMarker and PinMarker (onPress={() => onPinPress?.(pin)}) create new closures each render and break memoization; extract a memoized child component (e.g., MapPinItem and PoiPinItem) that accepts the pin and the stable onPinPress prop, use React.memo for the child and useCallback inside it to create a stable press handler that calls onPinPress(pin), then replace the inline handlers in the map iteration with onPress={childOnPress} by rendering those new components (reference PoiPinMarker, PinMarker, and the new MapPinItem/PoiPinItem wrapper names when locating code).src/components/maps/poi-pin-marker.tsx (1)
47-47: 💤 Low valueConsider a more stable
testID.
testID={poi-marker-${title}}will collide for POIs with duplicate titles and produce odd IDs when titles contain whitespace or special chars. If a stable identifier is available (e.g., passidthrough props), prefer it for test selectors.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/maps/poi-pin-marker.tsx` at line 47, The testID on the TouchableOpacity uses a fragile template testID={`poi-marker-${title}`} which can collide or include unsafe characters; update the POI pin component to accept and prefer a stable identifier prop (e.g., id or poiId) and use that for the TouchableOpacity testID (fall back to a sanitized title only if id is not provided). Modify the component prop list and the TouchableOpacity instance (symbol: TouchableOpacity, prop: testID, prop names: title and id) so test consumers receive testID like `poi-marker-${id}` and ensure any string used is safe (no whitespace/special chars) before assignment.src/app/(app)/__tests__/map.test.tsx (2)
327-340: 💤 Low valueRedundant
mockTrackEvent.mockReset()calls.
mockTrackEvent.mockReset()is invoked five times back-to-back; only one is needed (andjest.clearAllMocks()already clears it). Looks like an accidental duplication.♻️ Proposed cleanup
beforeEach(() => { jest.clearAllMocks(); mockRouterPush.mockReset(); mockTrackEvent.mockReset(); - mockTrackEvent.mockReset(); - mockTrackEvent.mockReset(); - mockTrackEvent.mockReset(); - mockTrackEvent.mockReset();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/__tests__/map.test.tsx around lines 327 - 340, Remove the redundant repeated mockTrackEvent.mockReset() calls in the beforeEach block—jest.clearAllMocks() already clears mocks, so either keep a single mockTrackEvent.mockReset() or remove them all; specifically edit the beforeEach that currently calls jest.clearAllMocks(), mockRouterPush.mockReset(), and multiple mockTrackEvent.mockReset() lines to eliminate the duplicate mockTrackEvent.mockReset() invocations while preserving the mockUseAnalytics.mockReturnValue({ trackEvent: mockTrackEvent }) setup.
356-374: 💤 Low valueTwo near-identical landscape tests.
renders in landscape mode(Line 356) andhandles landscape mode correctly(Line 534) now perform the same setup (mock landscape dimensions) and assert the same single thing (home-map-containeris rendered). Consider consolidating into one test to reduce noise and execution time.Also applies to: 534-552
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/__tests__/map.test.tsx around lines 356 - 374, Two tests ("renders in landscape mode" and "handles landscape mode correctly") both mock useWindowDimensions to landscape and only assert that HomeMap renders 'home-map-container'; consolidate by keeping one of these tests (e.g., "renders in landscape mode") and removing the duplicate, or change the second to assert a different behavior if intended. Update the test that remains to mock useWindowDimensions via the same mock (jest.requireMock('react-native').useWindowDimensions), render <HomeMap />, await the async load with waitFor on 'map-pins', and assert getByTestId('home-map-container'); remove the other near-identical test (or expand it to cover the side-menu behavior in _layout.tsx if that was the original intent).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/_layout.tsx`:
- Around line 82-85: The transaction filter currently inside beforeSend
(checking event.type === 'transaction' and the contexts.trace.op/data.route
guard) will never run for transactions; move that logic into the
beforeSendTransaction callback instead and remove it from beforeSend: locate the
existing beforeSend implementation and extract the navigation transaction guard
(the event.type === 'transaction' && event.contexts?.trace?.op === 'navigation'
&& !event.contexts?.trace?.data?.route check) into a new or existing
beforeSendTransaction handler so transactions are properly filtered by Sentry;
keep any other error-event filtering in beforeSend unchanged.
In `@src/app/call/`[id]/index.tsx:
- Line 777: The tab bar is forced non-scrollable which causes up to 7 tabs from
renderTabs() to be squashed on narrow portrait screens; update the SharedTabs
usage to allow scrolling — e.g., set the scrollable prop to true or make it
conditional (use isLandscape or the number of items produced by renderTabs()) so
that in portrait or when tab count is high the SharedTabs receives
scrollable={true} and in wide/landscape you can keep scrollable={false}.
In `@src/lib/poi.ts`:
- Around line 38-40: The code uses the nullish coalescing expression iconField =
marker.PoiImage ?? marker.ImagePath which treats an empty string as a present
value and prevents falling back to marker.ImagePath; update the selection logic
for iconField to treat empty/whitespace strings as absent (e.g. use a truthy
check or trim check) so that when marker.PoiImage is '' or only whitespace you
instead use marker.ImagePath before calling
iconField.toLowerCase().startsWith('map-icon-'); modify the assignment where
iconField is set (referencing marker.PoiImage and marker.ImagePath) accordingly.
In `@src/lib/utils.ts`:
- Around line 56-58: The getAvatarUrl function currently concatenates raw userId
into the query string which can produce malformed URLs if userId contains
reserved characters; update getAvatarUrl to URL-encode the userId (using
encodeURIComponent) before appending it to getBaseApiUrl() + '/Avatars/Get?id='
so the resulting URL is safely escaped and does not introduce extra query
parameters or break the path.
---
Outside diff comments:
In `@src/app/`(app)/messages.tsx:
- Around line 397-401: Replace the conditional rendering that uses && with
explicit ternary expressions: for the FAB block, change the expression that
depends on isSelectionMode and canUserCreateMessages so it returns the <Fab>
element when the condition is true and null/empty when false (locate the
fragment using isSelectionMode, canUserCreateMessages, handleOpenCompose and
testID "messages-compose-fab"); similarly update the loading block that uses
isLoading && filteredMessages.length === 0 to use a ternary that renders
<Loading /> when true and null/empty when false (locate isLoading,
filteredMessages and Loading). Ensure no other logic changes—only replace the &&
chains with ? : returning the component or null.
---
Nitpick comments:
In `@src/app/`(app)/__tests__/map.test.tsx:
- Around line 327-340: Remove the redundant repeated mockTrackEvent.mockReset()
calls in the beforeEach block—jest.clearAllMocks() already clears mocks, so
either keep a single mockTrackEvent.mockReset() or remove them all; specifically
edit the beforeEach that currently calls jest.clearAllMocks(),
mockRouterPush.mockReset(), and multiple mockTrackEvent.mockReset() lines to
eliminate the duplicate mockTrackEvent.mockReset() invocations while preserving
the mockUseAnalytics.mockReturnValue({ trackEvent: mockTrackEvent }) setup.
- Around line 356-374: Two tests ("renders in landscape mode" and "handles
landscape mode correctly") both mock useWindowDimensions to landscape and only
assert that HomeMap renders 'home-map-container'; consolidate by keeping one of
these tests (e.g., "renders in landscape mode") and removing the duplicate, or
change the second to assert a different behavior if intended. Update the test
that remains to mock useWindowDimensions via the same mock
(jest.requireMock('react-native').useWindowDimensions), render <HomeMap />,
await the async load with waitFor on 'map-pins', and assert
getByTestId('home-map-container'); remove the other near-identical test (or
expand it to cover the side-menu behavior in _layout.tsx if that was the
original intent).
In `@src/components/maps/map-pins.tsx`:
- Around line 48-71: The inline arrow handlers passed to PoiPinMarker and
PinMarker (onPress={() => onPinPress?.(pin)}) create new closures each render
and break memoization; extract a memoized child component (e.g., MapPinItem and
PoiPinItem) that accepts the pin and the stable onPinPress prop, use React.memo
for the child and useCallback inside it to create a stable press handler that
calls onPinPress(pin), then replace the inline handlers in the map iteration
with onPress={childOnPress} by rendering those new components (reference
PoiPinMarker, PinMarker, and the new MapPinItem/PoiPinItem wrapper names when
locating code).
In `@src/components/maps/poi-filter-bottom-sheet.tsx`:
- Around line 42-44: The handleDone wrapper simply calls onClose and can be
removed: delete the useCallback-defined handleDone function and any dependency
referencing it, then pass the existing onClose directly to the Done button's
onPress/onClick prop (where handleDone was used). Also remove the now-unused
useCallback import if applicable and any references to handleDone.
In `@src/components/maps/poi-icon.tsx`:
- Around line 1-2: The file currently uses a namespace import "import * as
LucideIcons" which prevents optimal tree-shaking; replace the namespace import
with direct named imports from 'lucide-react-native' for each icon used (keep
the type import "LucideIcon" if needed), and update usages that reference
LucideIcons.SomeIcon to refer directly to the named symbol (or create an
explicit icon map of named imports if dynamic lookup is required); ensure only
the icons actually used in this component are imported to improve bundling.
In `@src/components/maps/poi-list-panel.tsx`:
- Around line 142-148: The inline arrow handlers for clearing and updating the
search recreate functions each render; wrap the setter calls in memoized
callbacks using React.useCallback and replace the inline onPress and
onChangeText with these callbacks (e.g. create const handleClearSearch =
useCallback(() => setSearchQuery(''), [setSearchQuery]) and const
handleChangeSearch = useCallback((val) => setSearchQuery(val), [setSearchQuery])
then pass handleClearSearch to InputSlot.onPress and handleChangeSearch to
InputField.onChangeText) so memoized children like InputSlot/InputField and
other memoized callbacks remain stable.
In `@src/components/maps/poi-pin-marker.tsx`:
- Line 47: The testID on the TouchableOpacity uses a fragile template
testID={`poi-marker-${title}`} which can collide or include unsafe characters;
update the POI pin component to accept and prefer a stable identifier prop
(e.g., id or poiId) and use that for the TouchableOpacity testID (fall back to a
sanitized title only if id is not provided). Modify the component prop list and
the TouchableOpacity instance (symbol: TouchableOpacity, prop: testID, prop
names: title and id) so test consumers receive testID like `poi-marker-${id}`
and ensure any string used is safe (no whitespace/special chars) before
assignment.
In `@src/constants/poi-marker-shapes.ts`:
- Around line 69-77: The empty-string guard in getPoiMarkerShapePath is
redundant; compute normalized as you already do and replace the two-branch logic
with a single return that looks up POI_MARKER_PATHS[normalized] and falls back
to MAP_PIN_PATH (i.e., remove the if (normalized.length === 0) branch and use
POI_MARKER_PATHS[normalized] ?? MAP_PIN_PATH instead), keeping the normalized
computation from markerShape intact.
- Around line 51-57: POI_MARKER_PATHS is typed too loosely
(Record<string,string>) which loses the PoiMarkerShape union and prevents
compile-time checks; change its type to Record<PoiMarkerShape, string> so the
compiler enforces all shape keys, update the declaration of POI_MARKER_PATHS
accordingly, and keep existing runtime lookup in getPoiMarkerShapePath (with its
?? MAP_PIN_PATH fallback) so normalized string lookups remain safe.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 79f74ed6-e0d7-45b4-b79a-9dd6d8511fce
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (34)
jest-setup.tspackage.jsonsrc/app/(app)/__tests__/map.test.tsxsrc/app/(app)/map.tsxsrc/app/(app)/messages.tsxsrc/app/_layout.tsxsrc/app/call/[id]/index.tsxsrc/components/maps/map-panel.tsxsrc/components/maps/map-pins.tsxsrc/components/maps/pin-marker.tsxsrc/components/maps/poi-filter-bottom-sheet.tsxsrc/components/maps/poi-icon.tsxsrc/components/maps/poi-list-panel.tsxsrc/components/maps/poi-pin-marker.tsxsrc/components/ui/shared-tabs.tsxsrc/constants/map-icons.tssrc/constants/poi-icon-mapping.tssrc/constants/poi-marker-shapes.tssrc/lib/logging/index.tsxsrc/lib/poi.tssrc/lib/utils.tssrc/models/v4/mapping/getMapDataAndMarkersData.tssrc/models/v4/mapping/poiLayerData.tssrc/models/v4/mapping/poiResultData.tssrc/models/v4/mapping/poiTypeResultData.tssrc/translations/ar.jsonsrc/translations/de.jsonsrc/translations/en.jsonsrc/translations/es.jsonsrc/translations/fr.jsonsrc/translations/it.jsonsrc/translations/pl.jsonsrc/translations/sv.jsonsrc/translations/uk.json
| beforeSend(event) { | ||
| // Filter out problematic navigation transactions that might cause timestamp errors | ||
| if (event.contexts?.trace?.op === 'navigation' && !event.contexts?.trace?.data?.route) { | ||
| if (event.type === 'transaction' && event.contexts?.trace?.op === 'navigation' && !event.contexts?.trace?.data?.route) { | ||
| return null; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In @sentry/react-native8.x, are transaction events filtered throughbeforeSendorbeforeSendTransaction?
💡 Result:
In @sentry/react-native 8.x, transaction events are filtered through beforeSendTransaction, not beforeSend. The beforeSend callback filters error events from the JavaScript layer, while beforeSendTransaction specifically filters transaction events.[1][2] Official documentation explicitly shows beforeSendTransaction as the configuration option for transaction filtering in React Native SDK, including code examples for use in Sentry.init().[1] This behavior has been consistent since earlier versions (e.g., 4.9.0) and carries over to 8.x with no breaking changes noted in release notes or migration guides affecting it.[3][4][5]
Citations:
- 1: https://docs.sentry.io/platforms/react-native/configuration/filtering/
- 2: https://docs.sentry.io/platforms/react-native/enriching-events/event-processors/
- 3: https://github.com/getsentry/sentry-react-native/releases/tag/8.0.0
- 4: https://docs.sentry.io/platforms/react-native/migration/v7-to-v8/
- 5: https://github.com/getsentry/sentry-react-native/blob/main/CHANGELOG.md
🏁 Script executed:
cat -n src/app/_layout.tsx | sed -n '75,95p'Repository: Resgrid/Responder
Length of output: 1947
Use beforeSendTransaction for transaction filtering.
Line 82 currently filters event.type === 'transaction' inside beforeSend, but according to official Sentry documentation for @sentry/react-native, transaction events are filtered through beforeSendTransaction, not beforeSend. The beforeSend callback only filters error events, so this transaction guard will never apply. Move this filter to beforeSendTransaction:
Suggested fix
Sentry.init({
dsn: Env.SENTRY_DSN,
debug: __DEV__,
tracesSampleRate: __DEV__ ? 1.0 : 0.2,
profilesSampleRate: __DEV__ ? 1.0 : 1.0,
sendDefaultPii: false,
enableAppHangTracking: true,
enableWatchdogTerminationTracking: true,
integrations: [
navigationIntegration,
],
enableNativeFramesTracking: true,
- beforeSend(event) {
- // Filter out problematic navigation transactions that might cause timestamp errors
- if (event.type === 'transaction' && event.contexts?.trace?.op === 'navigation' && !event.contexts?.trace?.data?.route) {
- return null;
- }
- return event;
- },
+ beforeSendTransaction(event) {
+ if (event.contexts?.trace?.op === 'navigation' && !event.contexts?.trace?.data?.route) {
+ return null;
+ }
+ return event;
+ },
});📝 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.
| beforeSend(event) { | |
| // Filter out problematic navigation transactions that might cause timestamp errors | |
| if (event.contexts?.trace?.op === 'navigation' && !event.contexts?.trace?.data?.route) { | |
| if (event.type === 'transaction' && event.contexts?.trace?.op === 'navigation' && !event.contexts?.trace?.data?.route) { | |
| return null; | |
| Sentry.init({ | |
| dsn: Env.SENTRY_DSN, | |
| debug: __DEV__, | |
| tracesSampleRate: __DEV__ ? 1.0 : 0.2, | |
| profilesSampleRate: __DEV__ ? 1.0 : 1.0, | |
| sendDefaultPii: false, | |
| enableAppHangTracking: true, | |
| enableWatchdogTerminationTracking: true, | |
| integrations: [ | |
| navigationIntegration, | |
| ], | |
| enableNativeFramesTracking: true, | |
| beforeSendTransaction(event) { | |
| if (event.contexts?.trace?.op === 'navigation' && !event.contexts?.trace?.data?.route) { | |
| return null; | |
| } | |
| return event; | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/_layout.tsx` around lines 82 - 85, The transaction filter currently
inside beforeSend (checking event.type === 'transaction' and the
contexts.trace.op/data.route guard) will never run for transactions; move that
logic into the beforeSendTransaction callback instead and remove it from
beforeSend: locate the existing beforeSend implementation and extract the
navigation transaction guard (the event.type === 'transaction' &&
event.contexts?.trace?.op === 'navigation' &&
!event.contexts?.trace?.data?.route check) into a new or existing
beforeSendTransaction handler so transactions are properly filtered by Sentry;
keep any other error-event filtering in beforeSend unchanged.
| export function getAvatarUrl(userId: string) { | ||
| return Env.BASE_API_URL + Env.RESGRID_API_URL + '/Avatars/Get?id=' + userId; | ||
| return getBaseApiUrl() + '/Avatars/Get?id=' + userId; | ||
| } |
There was a problem hiding this comment.
Encode userId before appending it to the query string.
At Line 57, raw concatenation can produce malformed URLs (or unintended extra query params) when userId has reserved characters.
Suggested fix
export function getAvatarUrl(userId: string) {
- return getBaseApiUrl() + '/Avatars/Get?id=' + userId;
+ return getBaseApiUrl() + '/Avatars/Get?id=' + encodeURIComponent(userId);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/utils.ts` around lines 56 - 58, The getAvatarUrl function currently
concatenates raw userId into the query string which can produce malformed URLs
if userId contains reserved characters; update getAvatarUrl to URL-encode the
userId (using encodeURIComponent) before appending it to getBaseApiUrl() +
'/Avatars/Get?id=' so the resulting URL is safely escaped and does not introduce
extra query parameters or break the path.
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
1 participant
Summary by CodeRabbit
New Features
Improvements