Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (1)
📝 WalkthroughWalkthroughThis PR introduces comprehensive Points of Interest (POI) support across the application, including new API endpoints for fetching POI data and scheduled calls, new mobile screens for POI browsing and scheduled call management, integration of POI layers on maps with visibility controls, POI destination selection in call creation/editing flows, and POI-based destination assignment for personnel and unit status in the dispatch console. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PoisScreen as Pois Screen
participant PoisStore as Pois Store
participant API as POI API
participant Map as Map Component
User->>PoisScreen: Open POIs Screen
PoisScreen->>PoisStore: fetchPoiTypes()
PoisStore->>API: GET /Mapping/GetPoiTypes
API-->>PoisStore: PoiTypeResultData[]
PoisStore-->>PoisScreen: Update poiTypes
PoisScreen->>PoisStore: fetchPois()
PoisStore->>API: GET /Mapping/GetPois
API-->>PoisStore: PoiResultData[]
PoisStore-->>PoisScreen: Update pois list
User->>PoisScreen: Select POI Type / Search
PoisScreen->>PoisScreen: Filter & Sort POIs
PoisScreen->>User: Display filtered list
User->>PoisScreen: Tap POI Card
PoisScreen->>PoisStore: fetchPoi(poiId)
PoisStore->>API: GET /Mapping/GetPoi/{id}
API-->>PoisStore: PoiResultData detail
PoisStore-->>PoisScreen: Update selectedPoi
PoisScreen->>User: Show POI Detail Screen
sequenceDiagram
actor User
participant CallEditScreen as Call Edit Screen
participant DispatchAPI as Dispatch API
participant CallStore as Calls Store
participant CallAPI as Calls API
User->>CallEditScreen: Open Edit Call
CallEditScreen->>DispatchAPI: getNewCallData()
DispatchAPI-->>CallEditScreen: NewCallFormResult (DestinationPois[])
CallEditScreen->>CallEditScreen: Populate POI dropdown
User->>CallEditScreen: Select Destination POI
CallEditScreen->>CallEditScreen: Set destinationPoiId in form
User->>CallEditScreen: Submit Form
CallEditScreen->>CallAPI: updateCall(createCallRequest)
Note over CallAPI: Includes DestinationPoiId
CallAPI-->>CallStore: Call updated
CallStore-->>User: Navigate back
sequenceDiagram
actor User
participant Map as Map Screen
participant MapStore as Map Data Store
participant MapAPI as Mapping API
participant MapComponent as Map Component
User->>Map: Navigate to Map
Map->>MapAPI: getMapDataAndMarkers()
MapAPI-->>Map: MapDataAndMarkersData (includes PoiLayers[])
Map->>Map: Extract PoiLayers
Map->>Map: Initialize visiblePoiLayerIds (all enabled)
Map->>Map: Combine customLayers + PoiLayers
Map->>MapComponent: Render with combinedLayers
MapComponent->>MapComponent: Filter pins by POI visibility
MapComponent->>User: Display map with POI markers
User->>Map: Toggle POI Layer Visibility
Map->>Map: Update visiblePoiLayerIds
Map->>MapComponent: Re-filter visibleMapPins
MapComponent->>User: Update rendered pins
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/app/call/[id].web.tsx (1)
443-446:⚠️ Potential issue | 🟡 MinorStale keyboard shortcut hint.
The handler at lines 143-147 supports keys
1–7(and tabs include up tocheckinwhen enabled), but the displayed tip still says1-5. Update the translated hint and default to match the actual range.- {t('call_detail.keyboard_shortcuts', 'Tip: Press 1-5 to switch tabs, Ctrl+E to edit, Escape to go back')} + {t('call_detail.keyboard_shortcuts', 'Tip: Press 1-7 to switch tabs, Ctrl+E to edit, Escape to go back')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/`[id].web.tsx around lines 443 - 446, The keyboard shortcut hint string passed to t('call_detail.keyboard_shortcuts', ...) is stale (shows "1-5") while the key handler supports 1–7 and tabs can include 'checkin'; update the default translation to "Tip: Press 1-7 to switch tabs, Ctrl+E to edit, Escape to go back" or, better, compute the upper bound dynamically from the actual tabs array (the same source used by the key handler) and inject that number into the translated string so the displayed range matches the handler; modify the call to t(...) and the shortcut hint Text usage (referencing styles.shortcutHint, styles.shortcutText, isDark) accordingly.src/app/call/[id]/edit.tsx (1)
272-280:⚠️ Potential issue | 🟠 MajorSend the call type ID here, not the display name.
This screen still posts
type?.Name, even though the form is hydrated fromcall.Typeby matching onIdand the web editor in this PR now submitstype?.Id. Native edits can silently change or clear the type on save.Proposed fix
await useCallDetailStore.getState().updateCall({ callId: callId!, name: data.name, nature: data.nature, priority: priority?.Id || 0, - type: type?.Name || '', + type: type?.Id || '', note: data.note, destinationPoiId: data.destinationPoiId ? Number(data.destinationPoiId) : null,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/`[id]/edit.tsx around lines 272 - 280, The form currently sends type?: Name to updateCall which loses the actual type identifier; change the payload sent by useCallDetailStore.getState().updateCall so it includes the call type Id (e.g., type?.Id or null/0 as appropriate) instead of type?.Name — locate the updateCall invocation in edit.tsx that sets properties (callId, name, nature, priority, type, note, destinationPoiId, address) and replace the type field with a typeId (or the proper parameter name expected by updateCall) populated from type?.Id to ensure native edits persist the type correctly.src/translations/ar.json (1)
649-656:⚠️ Potential issue | 🟡 MinorRemove the duplicate
dispatched_resourceskey in this object.JSON keeps only the last duplicate key, so one value here is silently discarded. That makes this translation block brittle and easy to mis-edit later.
Proposed fix
"show_all": "إظهار الكل", "view_poi_details": "عرض تفاصيل نقطة الاهتمام", - "dispatched_resources": "المرسلون", "hide_all": "إخفاء الكل"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/ar.json` around lines 649 - 656, There are two identical keys "dispatched_resources" in the translations object which causes one value to be silently dropped; remove the duplicate so only a single "dispatched_resources" entry remains, keep the correct Arabic value you intend to use, and verify the surrounding keys ("view_call_details", "layers", "no_layers", "show_all", "view_poi_details", "hide_all") remain unchanged to ensure no accidental reordering or typos.src/app/(app)/map.web.tsx (1)
292-300:⚠️ Potential issue | 🟠 MajorCache
getMapPinSummary(pin)result and sanitize HTML content.
getMapPinSummary(pin)runs twice per marker (line 295). Additionally,pin.Title,Address,Note,PoiTypeName, andInfoWindowContentare interpolated directly intosetHTML()without escaping—if these fields contain untrusted data from the API, this creates an HTML injection risk.Suggested fix
+ const summary = getMapPinSummary(pin); + const escapeHtml = (text: string) => { + const div = document.createElement('div'); + div.textContent = text; + return div.innerHTML; + }; const popup = new mapboxgl.Popup({ offset: 25 }).setHTML( `<div style="padding: 8px;"> - <h3 style="margin: 0 0 8px 0; font-weight: 600;">${pin.Title}</h3> - ${getMapPinSummary(pin) ? `<p style="margin: 0 0 8px 0; font-size: 12px;">${getMapPinSummary(pin)}</p>` : ''} + <h3 style="margin: 0 0 8px 0; font-weight: 600;">${escapeHtml(pin.Title)}</h3> + ${summary ? `<p style="margin: 0 0 8px 0; font-size: 12px;">${escapeHtml(summary)}</p>` : ''}Or use a library like
DOMPurifyif HTML tags must be preserved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/map.web.tsx around lines 292 - 300, Cache the result of getMapPinSummary(pin) into a local variable so it’s only computed once and avoid calling it twice when building the popup; also never interpolate raw fields (pin.Title, pin.Address, pin.Note, pin.PoiTypeName, pin.InfoWindowContent) directly into mapboxgl.Popup().setHTML — escape/encode these values for HTML or sanitize the assembled HTML using a library like DOMPurify before calling setHTML to prevent injection. Ensure you reference getMapPinSummary(pin) for caching and sanitize output used in setHTML.
🧹 Nitpick comments (24)
src/models/v4/mapping/poiTypesResult.ts (1)
1-3: Run ESLint autofix to remove the blank line between the two imports.ESLint's
simple-import-sort/importsrule treatsBaseV4RequestandPoiTypeResultDataas part of the same relative-import group, so the blank line on line 2 is flagged. Same nit applies topoisResult.tsandpoiResult.ts.Proposed fix
import { BaseV4Request } from '../baseV4Request'; - import { PoiTypeResultData } from './poiTypeResultData';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/mapping/poiTypesResult.ts` around lines 1 - 3, Remove the extra blank line between the two relative imports so ESLint's simple-import-sort/imports groups them together; update the import block containing BaseV4Request and PoiTypeResultData (and apply the same change to similar files poisResult.ts and poiResult.ts) by running ESLint autofix or manually deleting the empty line so the imports are contiguous.src/models/v4/dispatch/newCallFormResultData.ts (1)
23-24: Consider co-locating destination POI fields with related call/dispatch state.Minor consistency nit: in
GetSetUnitStateResultDatathe equivalent fields are declared asDestinationPoisthenPoiTypes, while here they're declared asPoiTypesthenDestinationPois. Keeping the ordering identical across both result models makes the parallel between the two payloads easier to scan and reduces the chance of accidental field-mapping bugs in code that handles both.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/dispatch/newCallFormResultData.ts` around lines 23 - 24, The two public fields in NewCallFormResultData are declared as PoiTypes then DestinationPois which is the opposite order of the equivalent fields in GetSetUnitStateResultData; reorder the declarations in the NewCallFormResultData class so DestinationPois appears before PoiTypes (i.e., declare public DestinationPois: PoiResultData[] = []; then public PoiTypes: PoiTypeResultData[] = [];) to match GetSetUnitStateResultData and keep field ordering consistent.src/components/pois/poi-card.tsx (1)
22-22: Avoid anonymous handler inPressable.onPress.
() => onPress(poi)allocates a new function on every render and defeats memoization (especially relevant since this card is rendered inside theFlatListon the POIs screen). UseuseCallbackto bindpoi.As per coding guidelines: "Avoid anonymous functions in
renderItemor event handlers to prevent re-renders".Proposed change
-export const PoiCard: React.FC<PoiCardProps> = ({ poi, onPress }) => { +export const PoiCard: React.FC<PoiCardProps> = React.memo(({ poi, onPress }) => { const { t } = useTranslation(); + const handlePress = React.useCallback(() => onPress(poi), [onPress, poi]); const primaryText = getPoiPrimaryDisplayText(poi); const secondaryText = getPoiSecondaryDisplayText(poi); const accentStyle = StyleSheet.flatten([styles.accent, { backgroundColor: poi.Color || '#2563eb' }]); return ( - <Pressable style={styles.card} onPress={() => onPress(poi)}> + <Pressable style={styles.card} onPress={handlePress}> @@ -}; +}); +PoiCard.displayName = 'PoiCard';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/pois/poi-card.tsx` at line 22, Replace the inline anonymous handler in the Pressable (onPress={() => onPress(poi)}) with a stable callback created using useCallback: add a local handler (e.g., handlePress) defined with useCallback that calls the passed prop onPress(poi) and use that handler in the Pressable's onPress; this ensures the component (poi-card / Pressable) doesn't create a new function each render and preserves memoization when used inside FlatList.src/models/v4/calls/callResultData.ts (1)
39-40: Optional: tightenProtocols/UdfValuestyping.
unknown[]is fine as a placeholder, but if these arrays carry structured payloads from the API they'll be awkward to consume without narrowing. Consider introducing dedicated interfaces (orRecord<string, unknown>[]) once the schema is known.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/calls/callResultData.ts` around lines 39 - 40, Protocols and UdfValues are typed as unknown[] which makes downstream usage painful; replace unknown[] with a more specific type (e.g., create interfaces like Protocol and UdfValue or use Record<string, unknown>[]) and update their declarations in CallResultData (properties Protocols and UdfValues in callResultData.ts) so callers can access expected fields without repetitive narrowing; if the exact schema is unknown, prefer Record<string, unknown>[] as an interim type and add TODO comments to refine once API schema is available.src/lib/__tests__/poi-display.test.ts (1)
5-25: Add coverage for trimming,null/undefined, andgetPoiSecondaryDisplayText.The current suite covers happy paths but not:
- Whitespace-only fields being treated as empty (the
trim()behavior is the whole point ofgetTrimmedValue).Name/Address/etc. beingnull/undefined(the helper signature allowsstring | null).getPoiSecondaryDisplayTextis not tested at all although it is exported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/__tests__/poi-display.test.ts` around lines 5 - 25, Add unit tests that cover trimming and null/undefined inputs and include tests for getPoiSecondaryDisplayText: specifically, call getPoiPrimaryDisplayText, getPoiSelectionLabel, getPoiDestinationOptionLabel, and getPoiSearchValue with fields that are whitespace-only (to assert trim behavior via getTrimmedValue) and with Name/Address/Note set to null or undefined to assert they are treated as empty strings, and add tests exercising getPoiSecondaryDisplayText for combinations (e.g., Name present with Address secondary, Name absent with Address primary, and fallback to Note/PoiTypeName) to ensure expected secondary-label behavior.src/components/calls/call-card.web.tsx (1)
33-33: Consider extracting destinationText into a shared helper.This same
DestinationTypeName: DestinationNameformatting is duplicated insrc/components/calls/call-card.tsx(lines 39–45). A small shared formatter (e.g.,formatCallDestination(call)in@/lib/utilsor alongside the call models) would prevent the two variants from drifting if the format ever needs to change.♻️ Sketch
// src/lib/calls.ts import type { CallResultData } from '@/models/v4/calls/callResultData'; export const formatCallDestination = (call: CallResultData): string => { if (!call.DestinationName) return ''; return call.DestinationTypeName ? `${call.DestinationTypeName}: ${call.DestinationName}` : call.DestinationName; };Then in both
call-card.tsxandcall-card.web.tsx:- const destinationText = call.DestinationName ? (call.DestinationTypeName ? `${call.DestinationTypeName}: ${call.DestinationName}` : call.DestinationName) : ''; + const destinationText = formatCallDestination(call);Also applies to: 92-105
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/calls/call-card.web.tsx` at line 33, Duplicate destination formatting logic (constructing "DestinationTypeName: DestinationName") appears in call-card.web.tsx and call-card.tsx; extract it into a shared helper (e.g., formatCallDestination) and replace the inline expression in both files with a call to that helper. Create the helper (suggested name formatCallDestination) that accepts the call object (or CallResultData) and returns the formatted string using call.DestinationName and call.DestinationTypeName, export it from a shared module (e.g., src/lib/calls or utils), and update both components to import and use formatCallDestination wherever destinationText is computed.src/components/maps/unified-map-view.web.tsx (1)
297-305: Avoid double-invokinggetMapPinSummary(pin).
getMapPinSummary(pin)is computed twice for every pin's popup HTML. Hoist it once before building the template literal.♻️ Proposed fix
- // Create popup - const popup = new mapboxgl.Popup({ offset: 25 }).setHTML( - `<div style="padding: 8px;"> - <h3 style="margin: 0 0 8px 0; font-weight: 600;">${pin.Title}</h3> - ${getMapPinSummary(pin) ? `<p style="margin: 0 0 8px 0; font-size: 12px;">${getMapPinSummary(pin)}</p>` : ''} + // Create popup + const summary = getMapPinSummary(pin); + const popup = new mapboxgl.Popup({ offset: 25 }).setHTML( + `<div style="padding: 8px;"> + <h3 style="margin: 0 0 8px 0; font-weight: 600;">${pin.Title}</h3> + ${summary ? `<p style="margin: 0 0 8px 0; font-size: 12px;">${summary}</p>` : ''} <p style="margin: 0; font-size: 11px; color: `#666`;"> ${pin.Latitude.toFixed(6)}, ${pin.Longitude.toFixed(6)} </p> </div>` );Also consider HTML-escaping
pin.Titleandsummaryif they may contain markup, since they're injected viasetHTML.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/maps/unified-map-view.web.tsx` around lines 297 - 305, Hoist the result of getMapPinSummary(pin) into a local variable instead of calling it twice when building the popup HTML for new mapboxgl.Popup(...).setHTML(...): compute const summary = getMapPinSummary(pin) once, use summary in the template conditional and interpolation (alongside pin.Title, pin.Latitude, pin.Longitude), and replace the duplicate calls to getMapPinSummary(pin) with that variable; also consider HTML-escaping pin.Title and summary before injecting into setHTML to avoid markup injection.src/app/call/new/index.tsx (1)
236-238: Prefer the projectloggeroverconsole.error.The codebase already imports
@/lib/loggingin similar files (e.g.src/app/call/[id].web.tsx) and uses structuredlogger.error({ message, context }). Worth migrating this site as well — though not new in this PR, the freshly-added handler is a good place to start.- .catch((error) => { - console.error('Failed to load destination POIs:', error); - }) + .catch((error) => { + logger.error({ message: 'Failed to load destination POIs', context: { error } }); + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/new/index.tsx` around lines 236 - 238, Replace the raw console.error call in the promise .catch handler with the project's structured logger: import logger from '@/lib/logging' at the top of this module and call logger.error with an object payload (e.g., { message: 'Failed to load destination POIs', error, context: { /* any relevant ids or params available in this scope */ } }) instead of console.error; keep the same error variable name and include any local context you can (route params or handler identifiers) to aid debugging.src/app/(app)/pois.tsx (2)
123-137: Avoid the anonymousrenderItemandonPressclosures.
renderItemand theonPress={(poi) => router.push(...)}arrow are recreated on every render ofPois, defeating row memoization inFlatList. Hoist them withuseCallbacksoPoiCardinstances can memoize meaningfully.♻️ Suggested refactor
+ const handlePoiPress = useCallback((poi: PoiResultData) => { + router.push(`/poi/${poi.PoiId}` as Href); + }, []); + + const renderPoi = useCallback( + ({ item }: { item: PoiResultData }) => <PoiCard poi={item} onPress={handlePoiPress} />, + [handlePoiPress] + ); ... - <FlatList<PoiResultData> - testID="pois-list" - data={filteredPois} - keyExtractor={(item) => item.PoiId.toString()} - renderItem={({ item }) => <PoiCard poi={item} onPress={(poi) => router.push(`/poi/${poi.PoiId}` as Href)} />} + <FlatList<PoiResultData> + testID="pois-list" + data={filteredPois} + keyExtractor={keyExtractor} + renderItem={renderPoi}As per coding guidelines, "Avoid anonymous functions in
renderItemor event handlers to prevent re-renders".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/pois.tsx around lines 123 - 137, The anonymous renderItem and inline onPress arrow in the FlatList are recreated every render which prevents row memoization; hoist them into stable callbacks using React.useCallback: create a memoized renderItem callback (e.g., renderPoiItem) that returns <PoiCard poi={item} onPress={handlePoiPress} /> and a separate memoized handler (e.g., handlePoiPress) that calls router.push(`/poi/${poi.PoiId}` as Href); update the FlatList to use renderItem={renderPoiItem} and remove the inline onPress so PoiCard instances can memoize properly.
8-15: Minor: ESLint import-sort failure and barrel inconsistency.CI flagged
simple-import-sort/importsfor this file (lines 1-18). Also,FocusAwareStatusBaris imported via the@/components/uibarrel here while other screens (e.g.scheduled-calls.tsx) import it from@/components/ui/focus-aware-status-bar— pick one for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/pois.tsx around lines 8 - 15, The imports in pois.tsx are mis-ordered (triggering simple-import-sort) and the FocusAwareStatusBar is imported from the ui barrel inconsistent with other screens; reorder and group imports to satisfy the linter (external packages, absolute aliases, then local UI components) and replace the barrel import of FocusAwareStatusBar with its direct module import (the focus-aware-status-bar export) to match other files; update the import statement for FocusAwareStatusBar and adjust import ordering so ESLint simple-import-sort/imports passes.src/app/(app)/scheduled-calls.tsx (1)
109-114: Avoid anonymous functions inrenderItem.The inline arrow defines a new
renderItemand a newonPressclosure on every render, defeatingFlatListmemoization for cards. Per the coding guidelines, prefer a stable, memoized callback (and a stableonPressper row, e.g., auseCallback-bound handler that takes the call as an argument).♻️ Suggested refactor
+ const handleCallPress = useCallback((callId: string) => { + router.push(`/call/${callId}` as Href); + }, []); + + const renderCallItem = useCallback( + ({ item }: { item: CallResultData }) => ( + <ScheduledCallCard + call={item} + priority={callPriorities.find((p) => p.Id === item.Priority)} + dispatches={callDispatchesMap[item.CallId]} + isLoadingDispatches={loadingDispatchIds.has(item.CallId)} + onPress={handleCallPress} + /> + ), + [callPriorities, callDispatchesMap, loadingDispatchIds, handleCallPress] + ); ... - <FlatList<CallResultData> - ... - renderItem={({ item }: { item: CallResultData }) => ( - <Pressable onPress={() => router.push(`/call/${item.CallId}` as Href)}> - <ScheduledCallCard ... /> - </Pressable> - )} + <FlatList<CallResultData> + ... + renderItem={renderCallItem}(
Pressablecan move intoScheduledCallCardsoonPresscarries the call id without a closure.)As per coding guidelines, "Avoid anonymous functions in
renderItemor event handlers to prevent re-renders".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/scheduled-calls.tsx around lines 109 - 114, The inline renderItem and Pressable onPress create new closures each render; refactor by extracting a stable memoized render handler and a stable onPress callback: create a useCallback-based onPress function (e.g. handlePress(callId) using router.push) and a memoized renderItem function (e.g. renderScheduledCall) that reads callPriorities, callDispatchesMap and loadingDispatchIds and returns <ScheduledCallCard ... /> (move Pressable into ScheduledCallCard or have renderScheduledCall wrap ScheduledCallCard with a non-allocating onPress reference that calls handlePress), ensuring renderItem and the per-row onPress are stable and the component is memoized.src/stores/dispatch/personnel-actions-store.ts (1)
203-221: Optional: collapse the two destination branches into a single resolution.
respondingToandrespondingToTypeare computed via two parallel if/else-if chains over the same(statusDestinationType, selected*)state. Folding them into one block reduces duplication and keeps the two values impossible to drift apart.♻️ Sketch
- let respondingTo = ''; - if (statusDestinationType === 'call' && statusSelectedCall) { respondingTo = statusSelectedCall.CallId; } - else if (statusDestinationType === 'station' && statusSelectedStation) { respondingTo = statusSelectedStation.GroupId; } - else if (statusDestinationType === 'poi' && statusSelectedPoi) { respondingTo = statusSelectedPoi.PoiId.toString(); } - - let respondingToType: number | null = null; - if (statusDestinationType === 'call' && statusSelectedCall) { respondingToType = DestinationEntityType.Call; } - else if (statusDestinationType === 'station' && statusSelectedStation) { respondingToType = DestinationEntityType.Station; } - else if (statusDestinationType === 'poi' && statusSelectedPoi) { respondingToType = DestinationEntityType.Poi; } + let respondingTo = ''; + let respondingToType: number | null = null; + if (statusDestinationType === 'call' && statusSelectedCall) { + respondingTo = statusSelectedCall.CallId; + respondingToType = DestinationEntityType.Call; + } else if (statusDestinationType === 'station' && statusSelectedStation) { + respondingTo = statusSelectedStation.GroupId; + respondingToType = DestinationEntityType.Station; + } else if (statusDestinationType === 'poi' && statusSelectedPoi) { + respondingTo = statusSelectedPoi.PoiId.toString(); + respondingToType = DestinationEntityType.Poi; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/dispatch/personnel-actions-store.ts` around lines 203 - 221, Collapse the two parallel branches that set respondingTo and respondingToType into one unified block so they cannot diverge: inspect statusDestinationType and the corresponding selected entity (statusSelectedCall, statusSelectedStation, statusSelectedPoi) in a single if/else-if or switch, and inside each branch assign both respondingTo (string) and respondingToType (number | null) together (use CallId/GroupId/PoiId.toString() and DestinationEntityType.Call/Station/Poi), otherwise set respondingTo = '' and respondingToType = null; update the existing variables respondingTo and respondingToType in that combined block to preserve current types and behavior.src/components/status/status-bottom-sheet.tsx (1)
359-378: Replace numeric magic values withCustomStateDetailTypeenum.
getStatusDetailDescriptionswitches on raw numbers 1–7, butCustomStateDetailTypeis already imported elsewhere in the POI flow (e.g.,destination-helpers.test.ts) and used to express the same concepts. Using the enum would improve readability and prevent drift if values change.♻️ Suggested refactor
+ // Add CustomStateDetailType to imports from '@/lib/destination-helpers' const getStatusDetailDescription = (detail: number) => { switch (detail) { - case 1: + case CustomStateDetailType.Stations: return t('status.station_destination_enabled'); - case 2: + case CustomStateDetailType.Calls: return t('status.call_destination_enabled'); - case 3: + case CustomStateDetailType.CallsAndStations: return t('status.both_destinations_enabled'); - case 4: + case CustomStateDetailType.Pois: return t('status.poi_destination_enabled'); - case 5: + case CustomStateDetailType.CallsAndPois: return t('status.calls_and_pois_destination_enabled'); - case 6: + case CustomStateDetailType.StationsAndPois: return t('status.stations_and_pois_destination_enabled'); - case 7: + case CustomStateDetailType.CallsStationsAndPois: return t('status.all_destinations_enabled'); default: return ''; } };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/status/status-bottom-sheet.tsx` around lines 359 - 378, getStatusDetailDescription currently switches on raw numbers (1–7); replace those numeric case values with the corresponding CustomStateDetailType enum members (e.g., CustomStateDetailType.StationDestination, .CallDestination, etc.) so the switch uses enum symbols instead of magic numbers. Update the import usage if needed to ensure CustomStateDetailType is referenced in this file, keep the same return strings for each case, and leave the default branch returning an empty string unchanged.src/stores/calls/scheduled-store.ts (1)
28-68: LGTM on the new scheduled-calls store.Loading/error/lastFetched lifecycle is handled cleanly, errors are logged with context, and
getPriorityForCallcorrectly delegates to the calls store. One optional consideration: iffetchScheduledCallscan be triggered concurrently (e.g., screen focus + pull-to-refresh), you may want to short-circuit whenisLoadingis already true to avoid redundant API calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/calls/scheduled-store.ts` around lines 28 - 68, The fetchScheduledCalls implementation should short-circuit when a fetch is already in progress: at the start of useScheduledCallsStore's fetchScheduledCalls, check the current store state (useScheduledCallsStore.getState().isLoading) and immediately return if true to prevent concurrent API calls; otherwise proceed to set isLoading and continue as before. Ensure you reference and update the same state keys (isLoading, error, scheduledCalls, lastFetched) and keep existing try/catch logging behavior intact.src/lib/__tests__/destination-helpers.test.ts (2)
45-49: Avoidas anyin tests.Per coding guidelines, prefer precise types over
any. Cast toPoiResultData(orPartial<PoiResultData> as PoiResultData) so the test fixture stays type-checked if the model changes.As per coding guidelines: "Avoid using
any; strive for precise types".♻️ Suggested change
+import { type PoiResultData } from '../../models/v4/mapping/poiResultData'; @@ selectedPoi: { PoiId: 42, - } as any, + } as PoiResultData,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/__tests__/destination-helpers.test.ts` around lines 45 - 49, Replace the use of the unsafe cast "as any" in the test fixture with a precise type: cast the selectedPoi object to PoiResultData (or use Partial<PoiResultData> as PoiResultData) so the test stays type-checked if the model changes; update the selectedPoi assignment in the test (the object with PoiId: 42) to use one of these precise casts instead of "as any".
17-56: Consider broadening test coverage for negative cases.The current suite covers happy paths but doesn't assert behavior for non-POI inputs (e.g.,
isPoiDestinationType(DestinationEntityType.Call)should befalse,getDefaultDestinationTab(CustomStateDetailType.Stations)should be'stations',isCallMarker(MapMarkerEntityType.Poi)should befalse). Adding a few negative assertions would harden against regressions in the bidirectional mappings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/__tests__/destination-helpers.test.ts` around lines 17 - 56, Add negative test assertions to harden the mappings and tab defaults: extend the test file to assert that isPoiDestinationType(DestinationEntityType.Call) returns false, that getDefaultDestinationTab(CustomStateDetailType.Stations) returns 'stations' (and optionally that getDefaultDestinationTab(null/undefined) still returns 'calls' to cover fallback), and that isCallMarker(MapMarkerEntityType.Poi) returns false; also consider adding the inverse mapping check like getDestinationSelectionTypeValue('call') === DestinationEntityType.Call to cover both directions. These assertions should reference the existing helpers getDefaultDestinationTab, isPoiDestinationType, isCallMarker, and getDestinationSelectionTypeValue to locate the code to modify.src/stores/status/store.ts (1)
4-14: Imports could be tidier.
getSetUnitStatusData(line 4) andgetAllGroups(line 5) are placed beforeDestinationSelectionType(line 6), butsaveUnitStatus(line 7) sits after them. Other files in this PR have already been flagged forsimple-import-sortviolations — running the autofix here would prevent the same CI failures from creeping in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/status/store.ts` around lines 4 - 14, Imports in this file are not sorted by the project's simple-import-sort rules: reorder the imports so external and internal groups are alphabetically sorted and related types/functions are grouped (ensure getSetUnitStatusData, getAllGroups, DestinationSelectionType, saveUnitStatus and the other type imports follow the same ordering used across the repo); best fix is to run the project's autofix (eslint --fix or simple-import-sort --write) or manually reorder the imports to match the established import grouping and alphabetical order.src/components/dispatch-console/personnel-actions-panel.tsx (1)
656-662: Inconsistent i18n namespaces for POI strings.The POI tab label uses
t('menu.pois')and the empty state usest('status.no_pois_available'), while sibling Calls/Stations tabs uset('dispatch.calls')/t('dispatch.stations')and the stations empty state usest('dispatch.personnel_actions.no_stations_available'). Consider relocating the POI keys underdispatch.*(e.g.,dispatch.poisanddispatch.personnel_actions.no_pois_available) for consistency and easier translation maintenance.As per coding guidelines: "Ensure all text is wrapped in
t()fromreact-i18nextfor translations with the dictionary files stored insrc/translations".Also applies to: 718-718
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/personnel-actions-panel.tsx` around lines 656 - 662, The POI labels use inconsistent i18n keys; replace usages of t('menu.pois') and t('status.no_pois_available') with namespaced keys under dispatch (e.g., t('dispatch.pois') for the tab label and t('dispatch.personnel_actions.no_pois_available') for the empty state) so they match the Calls/Stations pattern; update the translation JSON/YAML in src/translations accordingly and ensure the components referencing destinationConfig.showPois, setDestinationTab, destinationTab and availablePois use the new keys.src/components/maps/pin-marker.tsx (1)
26-28: Avoid computing POI marker styles for non-POI pins.
poiMarkerStyle,poiMarkerDotStyle, andgetMapMarkerColor(pin)are evaluated on every render even whenisPoiMapPinis false. Consider gating these behind theisPoiMapPincheck (e.g., compute them inside the JSX ternary or via auseMemokeyed onisPoiMapPin/pin/size) to avoid unnecessary work for the common non-POI path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/maps/pin-marker.tsx` around lines 26 - 28, poiMarkerStyle, poiMarkerDotStyle, poiMarkerSize and getMapMarkerColor(pin) are being computed on every render even when isPoiMapPin is false; move these computations behind the isPoiMapPin check to avoid work on the common non-POI path — either compute them inline in the JSX ternary that renders the POI marker or wrap the calculations in a useMemo that depends on [isPoiMapPin, pin, size] (referencing poiMarkerSize, poiMarkerStyle, poiMarkerDotStyle, and getMapMarkerColor) so they only run when needed.src/stores/pois/store.ts (1)
105-114: Redundant guard after the cache assignment.After line 109 sets
selectedPoitocachedPoi, theget().selectedPoi?.PoiId === poiIdcheck on line 112 is always true whenevercachedPoiis truthy, so the third clause adds no extra protection. The expression simplifies toif (!forceRefresh && cachedPoi) return;. Not a bug, just dead logic that could mislead future readers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/pois/store.ts` around lines 105 - 114, In fetchPoi, the guard that checks get().selectedPoi?.PoiId === poiId after you already set selectedPoi to cachedPoi is redundant; simplify the condition to return early when a cached POI exists and forceRefresh is false (i.e., replace the current if (!forceRefresh && get().selectedPoi?.PoiId === poiId && cachedPoi) return; with a check that uses cachedPoi directly). Update the logic in fetchPoi (and related uses of getPoiById/selectedPoi/PoiId) so the early-return is based on cachedPoi and forceRefresh only.src/components/dispatch-console/unit-actions-panel.tsx (2)
176-195: Effect re-fetches whenselectedUnitreference changes even for the same unit.The effect's dependency on the whole
selectedUnitobject will re-run whenever the upstream store/prop produces a new reference (e.g., on unrelated unit metadata updates), causing redundantgetSetUnitStatusDataAPI calls. Depending onselectedUnit?.UnitIdinstead pins the fetch to identity that actually matters.♻️ Suggested change
- if (selectedUnit) { - loadOptions(); - } - }, [selectedUnit, setAvailableStatuses, setAvailableCalls, setAvailableStations, setAvailablePois, setIsLoadingOptions]); + if (selectedUnit) { + loadOptions(); + } + }, [selectedUnit?.UnitId, setAvailableStatuses, setAvailableCalls, setAvailableStations, setAvailablePois, setIsLoadingOptions]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/unit-actions-panel.tsx` around lines 176 - 195, The effect currently depends on the entire selectedUnit object causing ref-change re-fetches; change the dependency array to depend only on selectedUnit?.UnitId and keep the existing early-return logic so loadOptions runs only when UnitId is present; retain calls to getSetUnitStatusData and the setters (setAvailableStatuses, setAvailableCalls, setAvailableStations, setAvailablePois, setIsLoadingOptions) inside the effect but remove them from the dependency array so the fetch is triggered only when the unit identity (selectedUnit?.UnitId) changes.
99-101: Avoidpoi!non-null assertion; render label only when present.
getPoiSelectionLabel(poi!)will passnullat runtime ifDestinationSheetOptionis ever invoked withtype="poi"and noitem. Although current call sites always pass anitem, this assertion silently disables TypeScript's null check and is fragile. Prefer explicit narrowing:♻️ Suggested change
- {isNone ? label : isCall ? `#${call?.Number} - ${call?.Name}` : isPoi ? getPoiSelectionLabel(poi!) : station?.Name} + {isNone ? label : isCall ? `#${call?.Number} - ${call?.Name}` : isPoi ? (poi ? getPoiSelectionLabel(poi) : '') : station?.Name}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/dispatch-console/unit-actions-panel.tsx` around lines 99 - 101, The expression uses a non-null assertion on poi (poi!) inside the JSX conditional; replace that with an explicit null check and only call getPoiSelectionLabel when poi is defined—i.e., in the ternary branch for isPoi, return getPoiSelectionLabel(poi) only if poi is truthy, otherwise fall back to label (or another safe value); update the conditional around isNone/isCall/isPoi to reference getPoiSelectionLabel and poi together to avoid silencing TypeScript and prevent runtime null usage in getPoiSelectionLabel.src/lib/destination-helpers.ts (1)
88-99:getDestinationTypeLabelis currently the identity function for valid inputs.Each case returns the exact same string literal as the input
DestinationSelectionType, so this helper is effectively(type) => typeplus a'none'fallback. If the intent is to provide localized/human-readable labels, route these throught()(e.g.,t('destinations.types.call')); otherwise the helper can be removed and call sites can use the type directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/destination-helpers.ts` around lines 88 - 99, The getDestinationTypeLabel function is effectively an identity mapping; either convert it to return localized strings or remove it and use the type directly. If you want localization, replace the literal returns in getDestinationTypeLabel with calls to the translation helper (e.g., t('destinations.types.call'), t('destinations.types.station'), t('destinations.types.poi')) and import/receive the i18n t function as needed (or accept t as a parameter) so callers get human-readable labels; otherwise delete getDestinationTypeLabel and update all call sites to use the raw DestinationSelectionType value directly and remove the helper import.src/components/pois/poi-detail-screen.tsx (1)
27-31: Avoid full-store destructuring and inline object selectors to prevent extra re-renders.Two related issues:
- Line 27 destructures from the un-selected
usePoisStore()call, which subscribes this screen to every change in the POI store (e.g.,pois,destinationPois,poiTypes, allisLoading*flags). Any unrelated update will re-render the entire detail screen.- Line 28–31's
useLocationStoreselector returns a fresh{ latitude, longitude }object on every invocation, so without a custom equality function it will re-render on everyuseLocationStoreset call regardless of whether lat/lng changed.♻️ Suggested refactor
- const { selectedPoi, isLoadingDetail, detailError, fetchPoi, resetSelectedPoi } = usePoisStore(); - const userLocation = useLocationStore((state) => ({ - latitude: state.latitude, - longitude: state.longitude, - })); + const selectedPoi = usePoisStore((s) => s.selectedPoi); + const isLoadingDetail = usePoisStore((s) => s.isLoadingDetail); + const detailError = usePoisStore((s) => s.detailError); + const fetchPoi = usePoisStore((s) => s.fetchPoi); + const resetSelectedPoi = usePoisStore((s) => s.resetSelectedPoi); + const userLatitude = useLocationStore((s) => s.latitude); + const userLongitude = useLocationStore((s) => s.longitude);Then update the
handleRoutecall site to passuserLatitude/userLongitudedirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/pois/poi-detail-screen.tsx` around lines 27 - 31, Right now the component subscribes to the entire POI store and creates a fresh object for location on every render; fix by replacing the full destructure of usePoisStore() with specific selectors (e.g., call usePoisStore(state => state.selectedPoi), usePoisStore(state => state.isLoadingDetail), usePoisStore(state => state.detailError), usePoisStore(state => state.fetchPoi), usePoisStore(state => state.resetSelectedPoi)) so the component only subscribes to the exact fields it needs, and replace the inline object selector for useLocationStore with two stable selectors (e.g., userLatitude = useLocationStore(s => s.latitude) and userLongitude = useLocationStore(s => s.longitude)) and update the handleRoute call site to pass userLatitude/userLongitude directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53b402ca-ca7b-4ad3-8bfe-72375f8ed1a8
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (62)
package.jsonsrc/api/calls/calls.tssrc/api/dispatch/dispatch.tssrc/api/mapping/mapping.tssrc/api/personnel/personnelStatuses.tssrc/app/(app)/map.tsxsrc/app/(app)/map.web.tsxsrc/app/(app)/pois.tsxsrc/app/(app)/scheduled-calls.tsxsrc/app/call/[id].tsxsrc/app/call/[id].web.tsxsrc/app/call/[id]/edit.tsxsrc/app/call/[id]/edit.web.tsxsrc/app/call/new/index.tsxsrc/app/call/new/index.web.tsxsrc/app/poi/[id].tsxsrc/app/poi/[id].web.tsxsrc/components/calls/call-card.tsxsrc/components/calls/call-card.web.tsxsrc/components/calls/scheduled-call-card.tsxsrc/components/dispatch-console/personnel-actions-panel.tsxsrc/components/dispatch-console/personnel-panel.tsxsrc/components/dispatch-console/unit-actions-panel.tsxsrc/components/dispatch-console/units-panel.tsxsrc/components/maps/map-pins.tsxsrc/components/maps/pin-detail-modal.tsxsrc/components/maps/pin-marker.tsxsrc/components/maps/unified-map-view.web.tsxsrc/components/pois/poi-card.tsxsrc/components/pois/poi-detail-screen.tsxsrc/components/sidebar/__tests__/side-menu.test.tsxsrc/components/sidebar/side-menu.tsxsrc/components/status/status-bottom-sheet.tsxsrc/lib/__tests__/destination-helpers.test.tssrc/lib/__tests__/poi-display.test.tssrc/lib/destination-helpers.tssrc/lib/map-markers.tssrc/lib/poi-display.tssrc/lib/poi-map-layers.tssrc/models/v4/calls/callResultData.tssrc/models/v4/dispatch/getSetUnitStateResultData.tssrc/models/v4/dispatch/newCallFormResultData.tssrc/models/v4/mapping/getMapDataAndMarkersData.tssrc/models/v4/mapping/poiLayerData.tssrc/models/v4/mapping/poiResult.tssrc/models/v4/mapping/poiResultData.tssrc/models/v4/mapping/poiTypeResultData.tssrc/models/v4/mapping/poiTypesResult.tssrc/models/v4/mapping/poisResult.tssrc/models/v4/personnelStatuses/getCurrentStatusResultData.tssrc/models/v4/personnelStatuses/savePersonStatusInput.tssrc/models/v4/personnelStatuses/savePersonsStatusesInput.tssrc/models/v4/unitStatus/saveUnitStatusInput.tssrc/models/v4/unitStatus/unitStatusResultData.tssrc/stores/calls/scheduled-store.tssrc/stores/dispatch/personnel-actions-store.tssrc/stores/dispatch/unit-actions-store.tssrc/stores/pois/store.tssrc/stores/status/store.tssrc/translations/ar.jsonsrc/translations/en.jsonsrc/translations/es.json
| useMapSignalRUpdates(setMapPins); | ||
|
|
||
| const syncPoiLayers = useCallback((nextPoiLayers: PoiLayerData[]) => { | ||
| setPoiLayers(nextPoiLayers); | ||
| setVisiblePoiLayerIds(createDefaultVisiblePoiLayerIds(nextPoiLayers)); | ||
| }, []); | ||
|
|
||
| const togglePoiLayer = useCallback((layerId: string) => { | ||
| setVisiblePoiLayerIds((currentLayerIds) => { | ||
| const nextLayerIds = new Set(currentLayerIds); | ||
|
|
||
| if (nextLayerIds.has(layerId)) { | ||
| nextLayerIds.delete(layerId); | ||
| } else { | ||
| nextLayerIds.add(layerId); | ||
| } | ||
|
|
||
| return nextLayerIds; | ||
| }); | ||
| }, []); | ||
|
|
||
| const showAllMapLayers = useCallback(() => { | ||
| showAllLayers(); | ||
| setVisiblePoiLayerIds(createDefaultVisiblePoiLayerIds(poiLayers)); | ||
| }, [poiLayers, showAllLayers]); | ||
|
|
||
| const hideAllMapLayers = useCallback(() => { | ||
| hideAllLayers(); | ||
| setVisiblePoiLayerIds(new Set()); | ||
| }, [hideAllLayers]); | ||
|
|
||
| const combinedLayers = useMemo( | ||
| () => [ | ||
| ...layers.map((layer) => ({ Id: layer.Id, Name: layer.Name, Color: layer.Color, kind: 'custom' as const })), | ||
| ...poiLayers.map((layer) => ({ Id: getPoiMapLayerId(layer.PoiTypeId), Name: layer.Name, Color: layer.Color, kind: 'poi' as const })), | ||
| ], | ||
| [layers, poiLayers] | ||
| ); | ||
|
|
||
| const visibleMapPins = useMemo(() => filterMapPinsByPoiLayers(mapPins, visiblePoiLayerIds), [mapPins, visiblePoiLayerIds]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm filterMapPinsByPoiLayers behavior for unknown POI ids and empty sets
fd -e ts poi-map-layers
rg -nP -C5 'export function (filterMapPinsByPoiLayers|createDefaultVisiblePoiLayerIds|getPoiMapLayerId)' --type=tsRepository: Resgrid/Dispatch
Length of output: 86
🏁 Script executed:
cat -n src/lib/poi-map-layers.tsRepository: Resgrid/Dispatch
Length of output: 970
🏁 Script executed:
rg -n 'useMapSignalRUpdates' --type ts -A 10Repository: Resgrid/Dispatch
Length of output: 20938
🏁 Script executed:
cat -n src/hooks/use-map-signalr-updates.ts | head -60Repository: Resgrid/Dispatch
Length of output: 2821
🏁 Script executed:
rg -n 'getMapDataAndMarkers' --type ts -B 2 -A 5 src/api | head -40Repository: Resgrid/Dispatch
Length of output: 1629
🏁 Script executed:
rg -n 'GetMapDataAndMarkersResult' --type ts -A 10 src/models | head -50Repository: Resgrid/Dispatch
Length of output: 356
🏁 Script executed:
cat -n src/app/\(app\)/map.tsx | sed -n '225,240p'Repository: Resgrid/Dispatch
Length of output: 795
🏁 Script executed:
cat -n src/hooks/use-map-signalr-updates.ts | sed -n '49,95p'Repository: Resgrid/Dispatch
Length of output: 2060
🏁 Script executed:
cat -n src/app/\(app\)/map.tsx | sed -n '195,235p'Repository: Resgrid/Dispatch
Length of output: 1877
🏁 Script executed:
rg -n 'class MapDataAndMarkersData' --type ts -A 15 src/modelsRepository: Resgrid/Dispatch
Length of output: 1383
🏁 Script executed:
cat -n src/app/\(app\)/map.tsx | sed -n '73,78p'Repository: Resgrid/Dispatch
Length of output: 305
🏁 Script executed:
cat -n src/hooks/use-map-signalr-updates.ts | sed -n '60,75p'Repository: Resgrid/Dispatch
Length of output: 772
SignalR updates can introduce hidden POI markers.
useMapSignalRUpdates fetches the full map data (including PoiLayers) but only passes marker updates to its callback (setMapPins on line 75). If a SignalR update introduces a POI marker with a new type ID, it will be filtered out indefinitely by filterMapPinsByPoiLayers because:
- The new POI layer is never synced to
poiLayersstate - The new type ID never appears in
visiblePoiLayerIds - The marker is silently hidden with no UI control to display it
Consider updating useMapSignalRUpdates to accept a callback for POI layer updates as well, then invoke syncPoiLayers with the latest data when available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/map.tsx around lines 75 - 114, The SignalR hook
useMapSignalRUpdates currently only calls setMapPins so new POI types never
reach poiLayers or visiblePoiLayerIds; update useMapSignalRUpdates to accept a
second callback (e.g., onPoiLayersUpdate) and, where the hook receives full map
data including PoiLayers, call that callback with the fresh PoiLayers so the
component can call syncPoiLayers (or
setPoiLayers/createDefaultVisiblePoiLayerIds) to merge new layer definitions and
update visiblePoiLayerIds; ensure the component invocation of
useMapSignalRUpdates passes both setMapPins and syncPoiLayers so
filterMapPinsByPoiLayers can see newly introduced POI types.
| trackEvent('map_view_rendered', { | ||
| hasMapPins: mapPins.length > 0, | ||
| mapPinsCount: mapPins.length, | ||
| isMapLocked: location.isMapLocked, | ||
| theme: colorScheme || 'light', | ||
| layersCount: layers.length, | ||
| visibleLayersCount: visibleLayers.size, | ||
| layersCount: combinedLayers.length, | ||
| visibleLayersCount: visibleLayers.size + visiblePoiLayerIds.size, | ||
| }); | ||
| }, [trackEvent, mapPins.length, location.isMapLocked, colorScheme, layers.length, visibleLayers.size]); | ||
| }, [trackEvent, mapPins.length, location.isMapLocked, colorScheme, combinedLayers.length, visibleLayers.size, visiblePoiLayerIds.size]); |
There was a problem hiding this comment.
Run formatter to clear ESLint whitespace warnings.
The CI test check is flagging stray whitespace on line 274 and around lines 495–504 (Delete \·``). Run the formatter/autofix to clear these before merge.
🧰 Tools
🪛 GitHub Check: test
[warning] 274-274:
Delete ··
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/map.tsx around lines 274 - 282, There are stray whitespace
characters around the useEffect block that logs trackEvent (the call referencing
trackEvent, mapPins, combinedLayers, visibleLayers, visiblePoiLayerIds, and
location.isMapLocked); run the project's formatter or ESLint autofix (e.g.,
prettier or npm run format / eslint --fix) on src/app/(app)/map.tsx to remove
the extra spaces (delete the stray whitespace characters near the trackEvent
call and the surrounding lines) so the file passes CI formatting checks.
| useEffect(() => { | ||
| trackEvent('scheduled_calls_view_rendered', { | ||
| callsCount: scheduledCalls.length, | ||
| }); | ||
| }, [trackEvent, scheduledCalls.length]); | ||
|
|
||
| useEffect(() => { | ||
| const toFetch = scheduledCalls.filter((c) => !fetchedIdsRef.current.has(c.CallId)).map((c) => c.CallId); | ||
|
|
||
| if (toFetch.length === 0) return; | ||
|
|
||
| toFetch.forEach((id) => fetchedIdsRef.current.add(id)); | ||
|
|
||
| setLoadingDispatchIds((prev) => { | ||
| const next = new Set(prev); | ||
| toFetch.forEach((id) => next.add(id)); | ||
| return next; | ||
| }); | ||
|
|
||
| Promise.all( | ||
| toFetch.map((callId) => | ||
| getCallExtraData(callId) | ||
| .then((res) => ({ callId, dispatches: res?.Data?.Dispatches ?? ([] as DispatchedEventResultData[]) })) | ||
| .catch(() => ({ callId, dispatches: [] as DispatchedEventResultData[] })) | ||
| ) | ||
| ).then((results) => { | ||
| setCallDispatchesMap((prev) => { | ||
| const next = { ...prev }; | ||
| results.forEach(({ callId, dispatches }) => { | ||
| next[callId] = dispatches; | ||
| }); | ||
| return next; | ||
| }); | ||
| setLoadingDispatchIds((prev) => { | ||
| const next = new Set(prev); | ||
| toFetch.forEach((id) => next.delete(id)); | ||
| return next; | ||
| }); | ||
| }); | ||
| }, [scheduledCalls]); | ||
|
|
||
| const handleRefresh = useCallback(() => { | ||
| fetchedIdsRef.current = new Set(); | ||
| setCallDispatchesMap({}); | ||
| setLoadingDispatchIds(new Set()); | ||
| fetchScheduledCalls(); | ||
| }, [fetchScheduledCalls]); |
There was a problem hiding this comment.
Race condition and missing unmount guard in dispatch fetch effect.
Two related concerns:
- There is no abort/cleanup in the effect. If the screen unmounts while
Promise.allis in flight,setCallDispatchesMap/setLoadingDispatchIdswill fire on an unmounted component. handleRefreshresetsfetchedIdsRef.currentand clearscallDispatchesMap, but any in‑flightPromise.allfrom a prior render will still resolve and overwrite the freshly fetched data, producing stale results that match no current intent.
Consider an AbortController per effect run (passed into getCallExtraData if it supports signal) and an isCancelled flag checked before each setState. Alternatively, track an incrementing fetch token and ignore results whose token no longer matches.
♻️ Sketch
useEffect(() => {
const toFetch = scheduledCalls.filter((c) => !fetchedIdsRef.current.has(c.CallId)).map((c) => c.CallId);
if (toFetch.length === 0) return;
+ let cancelled = false;
toFetch.forEach((id) => fetchedIdsRef.current.add(id));
...
Promise.all(...).then((results) => {
+ if (cancelled) return;
setCallDispatchesMap((prev) => { ... });
setLoadingDispatchIds((prev) => { ... });
});
+ return () => { cancelled = true; };
}, [scheduledCalls]);📝 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.
| useEffect(() => { | |
| trackEvent('scheduled_calls_view_rendered', { | |
| callsCount: scheduledCalls.length, | |
| }); | |
| }, [trackEvent, scheduledCalls.length]); | |
| useEffect(() => { | |
| const toFetch = scheduledCalls.filter((c) => !fetchedIdsRef.current.has(c.CallId)).map((c) => c.CallId); | |
| if (toFetch.length === 0) return; | |
| toFetch.forEach((id) => fetchedIdsRef.current.add(id)); | |
| setLoadingDispatchIds((prev) => { | |
| const next = new Set(prev); | |
| toFetch.forEach((id) => next.add(id)); | |
| return next; | |
| }); | |
| Promise.all( | |
| toFetch.map((callId) => | |
| getCallExtraData(callId) | |
| .then((res) => ({ callId, dispatches: res?.Data?.Dispatches ?? ([] as DispatchedEventResultData[]) })) | |
| .catch(() => ({ callId, dispatches: [] as DispatchedEventResultData[] })) | |
| ) | |
| ).then((results) => { | |
| setCallDispatchesMap((prev) => { | |
| const next = { ...prev }; | |
| results.forEach(({ callId, dispatches }) => { | |
| next[callId] = dispatches; | |
| }); | |
| return next; | |
| }); | |
| setLoadingDispatchIds((prev) => { | |
| const next = new Set(prev); | |
| toFetch.forEach((id) => next.delete(id)); | |
| return next; | |
| }); | |
| }); | |
| }, [scheduledCalls]); | |
| const handleRefresh = useCallback(() => { | |
| fetchedIdsRef.current = new Set(); | |
| setCallDispatchesMap({}); | |
| setLoadingDispatchIds(new Set()); | |
| fetchScheduledCalls(); | |
| }, [fetchScheduledCalls]); | |
| useEffect(() => { | |
| const toFetch = scheduledCalls.filter((c) => !fetchedIdsRef.current.has(c.CallId)).map((c) => c.CallId); | |
| if (toFetch.length === 0) return; | |
| let cancelled = false; | |
| toFetch.forEach((id) => fetchedIdsRef.current.add(id)); | |
| setLoadingDispatchIds((prev) => { | |
| const next = new Set(prev); | |
| toFetch.forEach((id) => next.add(id)); | |
| return next; | |
| }); | |
| Promise.all( | |
| toFetch.map((callId) => | |
| getCallExtraData(callId) | |
| .then((res) => ({ callId, dispatches: res?.Data?.Dispatches ?? ([] as DispatchedEventResultData[]) })) | |
| .catch(() => ({ callId, dispatches: [] as DispatchedEventResultData[] })) | |
| ) | |
| ).then((results) => { | |
| if (cancelled) return; | |
| setCallDispatchesMap((prev) => { | |
| const next = { ...prev }; | |
| results.forEach(({ callId, dispatches }) => { | |
| next[callId] = dispatches; | |
| }); | |
| return next; | |
| }); | |
| setLoadingDispatchIds((prev) => { | |
| const next = new Set(prev); | |
| toFetch.forEach((id) => next.delete(id)); | |
| return next; | |
| }); | |
| }); | |
| return () => { cancelled = true; }; | |
| }, [scheduledCalls]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/scheduled-calls.tsx around lines 39 - 85, The fetch effect for
call dispatches can update state after unmount or after a refresh because
in‑flight Promise.all results are not cancelled or correlated to the current
run; update the effect to create a per-run identifier (e.g. fetchTokenRef) that
you increment at the start of the effect and also increment inside
handleRefresh, capture that token in the Promise.all then handler and only call
setCallDispatchesMap / setLoadingDispatchIds when the captured token still
equals fetchTokenRef.current; additionally create an AbortController for the run
(if getCallExtraData accepts a signal) and pass controller.signal into each
getCallExtraData call, and call controller.abort() in the effect cleanup to
cancel network requests on unmount/refresh—these changes involve the useEffect
that computes toFetch, fetchedIdsRef, getCallExtraData, setCallDispatchesMap,
setLoadingDispatchIds, and the handleRefresh callback.
| </Pressable> | ||
| )} | ||
| keyExtractor={(item: CallResultData) => item.CallId} | ||
| refreshControl={<RefreshControl refreshing={false} onRefresh={handleRefresh} />} |
There was a problem hiding this comment.
refreshing is hardcoded to false — pull-to-refresh spinner never appears.
The user can trigger handleRefresh, but the RefreshControl will never reflect the in‑progress state, so the gesture feels broken and a second pull can be initiated mid‑refetch.
🐛 Suggested fix
- refreshControl={<RefreshControl refreshing={false} onRefresh={handleRefresh} />}
+ refreshControl={<RefreshControl refreshing={isLoading} onRefresh={handleRefresh} />}If isLoading doesn't capture only the pull-to-refresh path, consider tracking a local isRefreshing state (as is done in pois.tsx) for an accurate indicator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/scheduled-calls.tsx at line 115, The RefreshControl is
hardcoded with refreshing={false} so the pull-to-refresh spinner never shows;
update the component to pass a real boolean tied to the refresh action (either
use existing isLoading if it only represents pull-to-refresh, or add a local
state like isRefreshing and set it true/false inside handleRefresh) so
RefreshControl's refreshing prop reflects the actual refresh lifecycle; locate
the RefreshControl usage and the handleRefresh function in scheduled-calls.tsx
and wire refreshing to isRefreshing (or isLoading) and ensure handleRefresh
toggles that state around the async fetch.
| {(call.ScheduledOn || call.ScheduledOnUtc) ? ( | ||
| <Box className="border-b border-outline-100 pb-2"> | ||
| <Text className="text-sm text-gray-500">{t('call_detail.scheduled_on')}</Text> | ||
| <Text className="font-medium text-amber-600 dark:text-amber-400"> | ||
| {format(new Date(call.ScheduledOn || call.ScheduledOnUtc), 'MMM d, yyyy h:mm a')} | ||
| </Text> | ||
| </Box> | ||
| ) : null} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the typing and any other consumers of ScheduledOn/ScheduledOnUtc to confirm
# whether these are guaranteed ISO strings or could be empty/non-ISO.
fd -t f 'callResultData.ts' | xargs -I{} cat {}
echo '---'
rg -nP -C2 '\bScheduledOn(Utc)?\b' --type=ts --type=tsxRepository: Resgrid/Dispatch
Length of output: 1687
🏁 Script executed:
# Read the file around lines 287-296 to see the full context
sed -n '280,305p' src/app/call/[id].tsx | cat -nRepository: Resgrid/Dispatch
Length of output: 1791
🏁 Script executed:
# Check for other date formatting patterns in the same file
rg 'format\(new Date' src/app/call/[id].tsx -A 2 -B 2Repository: Resgrid/Dispatch
Length of output: 738
🏁 Script executed:
# Check if there are any date validation utilities in the codebase
fd -t f '\.(ts|tsx)$' | xargs rg 'Number\.isNaN.*getTime|isValidDate|parseDate' | head -20Repository: Resgrid/Dispatch
Length of output: 2486
🏁 Script executed:
# Check the parseDateISOString implementation to understand its behavior
sed -n '1,100p' src/lib/utils.ts | grep -A 15 'parseDateISOString'Repository: Resgrid/Dispatch
Length of output: 42
🏁 Script executed:
# Also check formatDateForDisplay to see the complete pattern
sed -n '1,200p' src/lib/utils.ts | grep -A 10 'formatDateForDisplay'Repository: Resgrid/Dispatch
Length of output: 42
🏁 Script executed:
# Read the entire utils.ts file to see the date utility implementations
cat src/lib/utils.tsRepository: Resgrid/Dispatch
Length of output: 17257
Guard against invalid/empty date strings in both LoggedOn and ScheduledOn fields.
The format() calls at lines 288 and 289 lack validation and will fail if date strings are empty (the default) or malformed:
- Line 288:
format(new Date(call.LoggedOn), ...)is unguarded;LoggedOndefaults to''inCallResultData - Line 289:
format(new Date(call.ScheduledOn || call.ScheduledOnUtc), ...)is unguarded; both fields default to''
Calling format() on an invalid Date object will error. The codebase has an established pattern for this elsewhere: use parseDateISOString() + formatDateForDisplay() (seen in protocol-card, notes, dispatch-console). Consider applying that pattern here, or add validity checks before formatting.
The timezone concern from the original comment (local vs UTC semantics for ScheduledOn vs ScheduledOnUtc) remains valid—confirm with your backend that both fields use consistent ISO-8601 formatting with appropriate zone designators.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/call/`[id].tsx around lines 289 - 296, Guard the date formatting
calls for call.LoggedOn and call.ScheduledOn/ call.ScheduledOnUtc by converting
and validating the ISO strings before calling format: replace direct format(new
Date(...)) usages with the established helpers parseDateISOString(...) to obtain
a valid Date (or null) and then call formatDateForDisplay(...) (or only call
format if the Date is valid); update the JSX that renders LoggedOn and the
ScheduledOn block to render only when parseDateISOString(call.LoggedOn) or
parseDateISOString(call.ScheduledOn || call.ScheduledOnUtc) returns a valid date
(and keep using CallResultData defaults), and confirm you use the same helper
functions seen in protocol-card/notes/dispatch-console to preserve timezone/ISO
handling.
| expect(getPoiPrimaryDisplayText({ Name: 'Mercy Hospital', Address: '123 Main St', Note: 'ER entrance', PoiTypeName: 'Hospital' } as any)).toBe('Mercy Hospital'); | ||
| expect(getPoiPrimaryDisplayText({ Name: '', Address: '123 Main St', Note: 'ER entrance', PoiTypeName: 'Hospital' } as any)).toBe('123 Main St'); | ||
| expect(getPoiPrimaryDisplayText({ Name: '', Address: '', Note: 'ER entrance', PoiTypeName: 'Hospital' } as any)).toBe('ER entrance'); | ||
| expect(getPoiPrimaryDisplayText({ Name: '', Address: '', Note: '', PoiTypeName: 'Hospital' } as any)).toBe('Hospital'); | ||
| }); | ||
|
|
||
| it('builds selection labels from name and address when available', () => { | ||
| expect(getPoiSelectionLabel({ Name: 'Mercy Hospital', Address: '123 Main St', Note: '', PoiTypeName: 'Hospital' } as any)).toBe('Mercy Hospital - 123 Main St'); | ||
| expect(getPoiSelectionLabel({ Name: '', Address: '123 Main St', Note: '', PoiTypeName: 'Hospital' } as any)).toBe('123 Main St'); | ||
| }); | ||
|
|
||
| it('builds destination option labels with the type name prefix', () => { | ||
| expect(getPoiDestinationOptionLabel({ Name: 'Mercy Hospital', Address: '123 Main St', Note: '', PoiTypeName: 'Hospital' } as any)).toBe('Hospital: Mercy Hospital - 123 Main St'); | ||
| }); | ||
|
|
||
| it('builds a searchable lower-case value from all visible fields', () => { | ||
| expect(getPoiSearchValue({ Name: 'Mercy Hospital', Address: '123 Main St', Note: 'ER Entrance', PoiTypeName: 'Hospital' } as any)).toBe('mercy hospital 123 main st er entrance hospital'); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace as any with Partial<PoiResultData> (or the helper's Pick arg type).
The casts hide type errors and contradict project guidance to avoid any. Since each helper accepts a Pick<PoiResultData, …>, the literals will type-check directly without the cast.
As per coding guidelines: "Avoid using any; strive for precise types".
Suggested adjustment
-import { getPoiDestinationOptionLabel, getPoiPrimaryDisplayText, getPoiSearchValue, getPoiSelectionLabel } from '../poi-display';
+import { type PoiResultData } from '@/models/v4/mapping/poiResultData';
+import { getPoiDestinationOptionLabel, getPoiPrimaryDisplayText, getPoiSearchValue, getPoiSelectionLabel } from '../poi-display';
+
+type PoiDisplayInput = Pick<PoiResultData, 'Name' | 'Address' | 'Note' | 'PoiTypeName'>;
@@
- expect(getPoiPrimaryDisplayText({ Name: 'Mercy Hospital', Address: '123 Main St', Note: 'ER entrance', PoiTypeName: 'Hospital' } as any)).toBe('Mercy Hospital');
+ expect(getPoiPrimaryDisplayText({ Name: 'Mercy Hospital', Address: '123 Main St', Note: 'ER entrance', PoiTypeName: 'Hospital' } satisfies PoiDisplayInput)).toBe('Mercy Hospital');(Apply the same pattern to the other cases.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/__tests__/poi-display.test.ts` around lines 7 - 23, Tests use unsafe
casts "as any" which hide type errors; update the test literals passed to
getPoiPrimaryDisplayText, getPoiSelectionLabel, getPoiDestinationOptionLabel,
and getPoiSearchValue to use the proper typed shape instead of any (e.g.,
Partial<PoiResultData> or the specific Pick<> type the helper expects) so the
objects type-check without casting—replace each "as any" with the appropriate
Partial<PoiResultData> or Pick<PoiResultData, 'Name' | 'Address' | 'Note' |
'PoiTypeName'> to follow the project's no-any guideline.
| export const filterMapPinsByPoiLayers = (pins: MapMakerInfoData[], visiblePoiLayerIds: Set<string>) => { | ||
| return pins.filter((pin) => { | ||
| if (pin.Type !== MapMarkerEntityType.Poi || !pin.PoiTypeId) { | ||
| return true; | ||
| } | ||
|
|
||
| return visiblePoiLayerIds.has(getPoiMapLayerId(pin.PoiTypeId)); |
There was a problem hiding this comment.
Don't treat PoiTypeId = 0 as “missing”.
!pin.PoiTypeId bypasses layer filtering for any falsy numeric ID. If the backend ever emits 0, that POI can never be hidden with its layer. Check for null/undefined explicitly instead.
Proposed fix
export const filterMapPinsByPoiLayers = (pins: MapMakerInfoData[], visiblePoiLayerIds: Set<string>) => {
return pins.filter((pin) => {
- if (pin.Type !== MapMarkerEntityType.Poi || !pin.PoiTypeId) {
+ if (pin.Type !== MapMarkerEntityType.Poi || pin.PoiTypeId == null) {
return true;
}
return visiblePoiLayerIds.has(getPoiMapLayerId(pin.PoiTypeId));
});
};📝 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 filterMapPinsByPoiLayers = (pins: MapMakerInfoData[], visiblePoiLayerIds: Set<string>) => { | |
| return pins.filter((pin) => { | |
| if (pin.Type !== MapMarkerEntityType.Poi || !pin.PoiTypeId) { | |
| return true; | |
| } | |
| return visiblePoiLayerIds.has(getPoiMapLayerId(pin.PoiTypeId)); | |
| export const filterMapPinsByPoiLayers = (pins: MapMakerInfoData[], visiblePoiLayerIds: Set<string>) => { | |
| return pins.filter((pin) => { | |
| if (pin.Type !== MapMarkerEntityType.Poi || pin.PoiTypeId == null) { | |
| return true; | |
| } | |
| return visiblePoiLayerIds.has(getPoiMapLayerId(pin.PoiTypeId)); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/poi-map-layers.ts` around lines 12 - 18, The current filter in
filterMapPinsByPoiLayers treats pin.PoiTypeId with falsy check (!pin.PoiTypeId)
which incorrectly treats 0 as missing; change the check to explicitly test for
null/undefined (e.g., pin.PoiTypeId == null) so only absent IDs bypass layer
filtering; update the conditional that uses MapMarkerEntityType,
MapMakerInfoData, visiblePoiLayerIds and getPoiMapLayerId accordingly so POIs
with PoiTypeId === 0 are still filtered by visiblePoiLayerIds.
| public DestinationId: number | null = null; | ||
| public DestinationType: number | null = null; | ||
| public DestinationName: string = ''; | ||
| public DestinationAddress: string = ''; | ||
| public DestinationTypeName: string = ''; | ||
| public Note: string = ''; | ||
| public Latitude: string = ''; | ||
| public Longitude: string = ''; | ||
| public Latitude: number | null = null; | ||
| public Longitude: number | null = null; | ||
| public GroupName: string = ''; | ||
| public GroupId: string = ''; | ||
| public GroupId: number = 0; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find consumers of fields with potentially incompatible patterns.
rg -nP --type=ts -C2 '\.DestinationId\b'
rg -nP --type=ts -C2 '\.GroupId\b' -g '!**/*.test.ts'
rg -nP --type=ts -C2 '\bUnitStatusResultData\b'
# String-ish operations that would break against numeric fields:
rg -nP --type=ts -C1 'DestinationId\s*\.(startsWith|includes|toLowerCase|split)\b'
rg -nP --type=ts -C1 '(Latitude|Longitude)\s*\.(toString|startsWith|includes|split)\b'Repository: Resgrid/Dispatch
Length of output: 12331
Breaking type changes — multiple callsites have incompatible type mismatches.
Several public field types changed shape and not all consumers were updated:
DestinationId:string→number | nullGroupId:string→numberLatitude/Longitude:string→number | null
Specific issues found:
-
Type mismatch in station selection (
src/components/status/status-bottom-sheet.tsx:657):handleStationSelect(station.GroupId)passes a number to a function that declaresstationId: stringparameter. Line 95 then comparesGroupId (number) === stationId (string), which will never match. -
Incorrect fallback (
src/lib/destination-helpers.ts:152):selectedStation?.GroupId ?? ''returns either a number or empty string, breaking the return type contract. -
Similar mismatches in
src/components/dispatch-console/unit-actions-panel.tsx:237andpersonnel-actions-panel.tsx:269with the same pattern.
These are not just type violations—they cause runtime logic failures (e.g., station selection will silently fail due to number ≠ string comparison). All callsites require migration to numeric operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/models/v4/unitStatus/unitStatusResultData.ts` around lines 9 - 18, The
model changed several fields from string to numeric (DestinationId, GroupId,
Latitude, Longitude), so update all consumers to use numeric operations: change
the parameter type of handleStationSelect from stationId: string to stationId:
number and update its callers (e.g., the station selection at
src/components/status/status-bottom-sheet.tsx) to pass numbers; replace string
fallbacks like selectedStation?.GroupId ?? '' with a numeric-safe fallback
(null, 0, or undefined) in src/lib/destination-helpers.ts and ensure return
types reflect numbers; similarly update unit-actions-panel.tsx and
personnel-actions-panel.tsx callsites to accept/compare numbers (or explicitly
coerce via Number(...) at call boundary) so comparisons like GroupId ===
stationId are numeric-to-numeric and not number-to-string. Ensure any UI
bindings that expect strings serialize numbers with .toString() only where a
string is required.
| set({ | ||
| selectedPoi: nextPoi, | ||
| pois: nextPoi ? upsertPoi(currentPois, nextPoi) : currentPois, | ||
| destinationPois: nextPoi?.IsDestination ? upsertPoi(currentDestinationPois, nextPoi) : currentDestinationPois, | ||
| isLoadingDetail: false, | ||
| }); |
There was a problem hiding this comment.
Stale entries possible in destinationPois when IsDestination flips to false.
The upsert only adds/updates destinationPois when nextPoi.IsDestination is truthy, but it never removes a previously-cached POI whose IsDestination has since become false server-side. After such a transition, destinationPois will keep the stale POI until a full fetchDestinationPois(true) refresh runs.
♻️ Suggested change
- set({
- selectedPoi: nextPoi,
- pois: nextPoi ? upsertPoi(currentPois, nextPoi) : currentPois,
- destinationPois: nextPoi?.IsDestination ? upsertPoi(currentDestinationPois, nextPoi) : currentDestinationPois,
- isLoadingDetail: false,
- });
+ set({
+ selectedPoi: nextPoi,
+ pois: nextPoi ? upsertPoi(currentPois, nextPoi) : currentPois,
+ destinationPois: nextPoi
+ ? nextPoi.IsDestination
+ ? upsertPoi(currentDestinationPois, nextPoi)
+ : currentDestinationPois.filter((p) => p.PoiId !== nextPoi.PoiId)
+ : currentDestinationPois,
+ isLoadingDetail: false,
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/pois/store.ts` around lines 124 - 129, The current set call only
upserts into destinationPois when nextPoi.IsDestination is true, leaving stale
entries when a POI's IsDestination flips false; update the logic in the set
block that updates destinationPois (around selectedPoi/pois/destinationPois
usage) so that when nextPoi exists and nextPoi.IsDestination is false you remove
that POI from currentDestinationPois (e.g., filter out by POI id) instead of
returning currentDestinationPois unchanged; keep using upsertPoi when
IsDestination is true and ensure isLoadingDetail is still set to false.
| fetchDestinationData: async (unitId: string) => { | ||
| set({ isLoading: true, error: null }); | ||
| try { | ||
| // Fetch calls and groups (stations) in parallel | ||
| const [callsResponse, groupsResponse] = await Promise.all([getCalls(), getAllGroups()]); | ||
| const response = await getSetUnitStatusData(unitId); | ||
| const destinationData = response?.Data; | ||
|
|
||
| set({ | ||
| availableCalls: callsResponse.Data || [], | ||
| availableStations: groupsResponse.Data || [], | ||
| availableCalls: destinationData?.Calls || [], | ||
| availableStations: destinationData?.Stations || [], | ||
| availablePois: destinationData?.DestinationPois || [], | ||
| isLoading: false, | ||
| }); | ||
| } catch (error) { | ||
| set({ | ||
| error: 'Failed to fetch destination data', | ||
| isLoading: false, | ||
| }); | ||
| try { | ||
| const [callsResponse, groupsResponse] = await Promise.all([getCalls(), getAllGroups()]); | ||
|
|
||
| set({ | ||
| availableCalls: callsResponse.Data || [], | ||
| availableStations: groupsResponse.Data || [], | ||
| availablePois: [], | ||
| isLoading: false, | ||
| }); | ||
| } catch { | ||
| set({ | ||
| error: 'Failed to fetch destination data', | ||
| isLoading: false, | ||
| }); | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
Add logging and consider the impact of silently swallowing the original error.
The new primary→fallback flow swallows both error paths without logging. Compare with scheduled-store.ts, which logs failed fetches with context. This file already imports logger (line 8) — using it here would significantly aid debugging when the unit-scoped endpoint fails. Also, when both calls fail, the user-facing error: 'Failed to fetch destination data' masks both root causes.
🛡️ Suggested fix
} catch (error) {
+ logger.warn({
+ message: 'getSetUnitStatusData failed; falling back to getCalls/getAllGroups',
+ context: { unitId, error },
+ });
try {
const [callsResponse, groupsResponse] = await Promise.all([getCalls(), getAllGroups()]);
set({
availableCalls: callsResponse.Data || [],
availableStations: groupsResponse.Data || [],
availablePois: [],
isLoading: false,
});
- } catch {
+ } catch (fallbackError) {
+ logger.error({
+ message: 'Failed to fetch destination data (primary and fallback)',
+ context: { unitId, error, fallbackError },
+ });
set({
error: 'Failed to fetch destination data',
isLoading: false,
});
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/status/store.ts` around lines 84 - 113, In fetchDestinationData,
don’t silently swallow errors: when getSetUnitStatusData(unitId) fails, log the
original error and context using the existing logger (reference
fetchDestinationData and getSetUnitStatusData) before proceeding to the
fallback; likewise, if the Promise.all fallback (getCalls(), getAllGroups())
fails, log that error and include both error details in the final set({ error:
... }) so the user-facing error carries useful context (mirror the logging
pattern used in scheduled-store.ts), e.g., logger.error with descriptive
messages and the caught exceptions.
1 participant
Summary by CodeRabbit
New Features
Chores