Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (lower.includes('.m3u8')) return FeedFormat.HLS; | ||
| if (lower.includes('.mpd')) return FeedFormat.DASH; | ||
| if (lower.startsWith('rtsp://')) return FeedFormat.RTSP; | ||
| if (lower.includes('youtube.com') || lower.includes('youtu.be')) return FeedFormat.YouTubeLive; |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds check‑in timers with OS indicators (iOS Live Activity, Android notifications), weather alerts (models, store, UI, SignalR), call video feeds (API, models, store, UI, player), POI support (models, store, UI), numerous stores/hooks/services, offline check‑in queuing, broad i18n additions, and Expo/native config + dependency updates. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App UI (CallDetail / Sidebar)
participant Store as CheckInTimerStore
participant PollHook as useCheckInTimerPolling
participant API as CheckIn API
participant Native as iOS LiveActivity
participant Notifee as Android Notification Service
participant Offline as OfflineEventManager
UI->>Store: startPolling(callId, interval)
Store->>PollHook: subscribe/status refresh
PollHook->>API: GET /CheckIn/Statuses?callId
API-->>PollHook: statuses
PollHook->>Store: set timerStatuses
PollHook->>Native: start/update Live Activity (iOS)
PollHook->>Notifee: start/update notification (Android)
UI->>Store: performCheckIn(input)
Store->>API: POST /CheckIn
alt API online success
API-->>Store: success
Store->>PollHook: refresh statuses
Store-->>UI: return "success"
else API offline / network error
Store->>Offline: queueCheckInEvent(payload)
Store-->>UI: return "queued"
end
UI->>Native: end Live Activity (on stop/unmount)
UI->>Notifee: stop notification (on stop/unmount)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/app/call/new/index.tsx (1)
2-2:⚠️ Potential issue | 🟠 MajorRemove unused
@testing-library/react-nativeimport from production code.Line 2 imports
renderfrom@testing-library/react-native, but the function is never used. Test utilities should not appear in shipped code—this unnecessarily bloats the production bundle and may cause runtime issues in environments where test dependencies are absent. Remove the import.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/new/index.tsx` at line 2, The file imports the test helper "render" from "@testing-library/react-native" but never uses it; remove the unused import statement (the `render` import from `@testing-library/react-native`) from the module (index.tsx) so test utilities are not bundled into production; after removal, run the linter/build to confirm no remaining references to `render` or other test-only imports.src/app/(app)/index.tsx (1)
1-29:⚠️ Potential issue | 🟠 MajorFix import sort to unblock CI.
ESLint
simple-import-sort/importsis failing on this file (per pipeline failure logs). Lines 28-29 adduseWeatherAlertsStore(from@/stores/...) beforeWeatherAlertBanner(from@/components/...), breaking the existing grouping/ordering convention.🔧 Proposed fix
import MapPins from '@/components/maps/map-pins'; import Mapbox from '@/components/maps/mapbox'; import PinDetailModal from '@/components/maps/pin-detail-modal'; import { StopMarker } from '@/components/routes/stop-marker'; import { FocusAwareStatusBar } from '@/components/ui/focus-aware-status-bar'; +import { WeatherAlertBanner } from '@/components/weather-alerts/weather-alert-banner'; import { useAnalytics } from '@/hooks/use-analytics'; ... import { useRoutesStore } from '@/stores/routes/store'; import { useToastStore } from '@/stores/toast/store'; -import { useWeatherAlertsStore } from '@/stores/weather-alerts/store'; -import { WeatherAlertBanner } from '@/components/weather-alerts/weather-alert-banner'; +import { useWeatherAlertsStore } from '@/stores/weather-alerts/store';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/index.tsx around lines 1 - 29, Reorder the imports so they satisfy simple-import-sort: move the WeatherAlertBanner import to come before useWeatherAlertsStore (i.e. ensure '@/components/...' imports appear before '@/stores/...' imports), and then run the linter; specifically adjust the import block containing WeatherAlertBanner and useWeatherAlertsStore so WeatherAlertBanner is listed earlier and the grouped/import ordering convention (external packages, then app components, hooks, libs, models, services, stores) is preserved.
♻️ Duplicate comments (1)
src/components/call-video-feeds/feed-format-utils.ts (1)
63-63:⚠️ Potential issue | 🟡 MinorUse hostname matching for YouTube detection.
includes('youtube.com')matches anywhere in the URL, sohttps://malicious.example/?ref=youtube.com/...is mis-classified asYouTubeLiveand routed to the YouTube branch in the player. Parse the URL and check the hostname against an allowlist instead.🛡️ Proposed fix
- if (lower.includes('youtube.com') || lower.includes('youtu.be')) return FeedFormat.YouTubeLive; + try { + const host = new URL(url).hostname.toLowerCase(); + if (host === 'youtube.com' || host.endsWith('.youtube.com') || host === 'youtu.be') { + return FeedFormat.YouTubeLive; + } + } catch { + // not a valid absolute URL; fall through + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/call-video-feeds/feed-format-utils.ts` at line 63, The current string includes check mis-classifies URLs; update the YouTube detection in the function that returns FeedFormat.YouTubeLive to parse the input as a URL (using new URL(...)) and match URL.hostname against an allowlist (e.g. "youtube.com", "www.youtube.com", "m.youtube.com", and "youtu.be") or use endsWith(".youtube.com") for subdomains; wrap parsing in a try/catch and only return FeedFormat.YouTubeLive when the hostname matches the allowlist, otherwise fall back to the existing non-YouTube logic (do not rely on lower.includes('youtube.com') or similar).
🟡 Minor comments (24)
src/models/v4/destinations/destinationEntityTypes.ts-1-6 (1)
1-6:⚠️ Potential issue | 🟡 MinorFile naming convention violation.
The file name
destinationEntityTypes.tsuses camelCase, but the project guidelines require lowercase-hyphenated names. Please rename this file todestination-entity-types.ts.As per coding guidelines, "Directory and file names should be lowercase and hyphenated (e.g., user-profile, chat-screen)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/destinations/destinationEntityTypes.ts` around lines 1 - 6, Rename the file containing the DestinationEntityTypes enum from camelCase to the project's lowercase-hyphenated convention: change destinationEntityTypes.ts to destination-entity-types.ts and update any imports that reference DestinationEntityTypes (e.g., import { DestinationEntityTypes } ...) so they point to the new filename; ensure the exported symbol name DestinationEntityTypes and its enum members remain unchanged.src/models/v4/checkIn/checkInRecordResultData.ts-3-3 (1)
3-3:⚠️ Potential issue | 🟡 Minor
CallIdshould be typed asstring, notnumber—inconsistent with other models.Every other ID model (
CallResultData,CallFileResultData,CallVideoFeedResultData) declaresCallId: string. The code atcheck-in-sidebar-widget.tsx:28converts it withparseInt(activeCall.CallId, 10), confirming it arrives as a string from the backend. Note:callNoteResultData.tshas the identical issue.- public CallId: number = 0; + public CallId: string = '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/checkIn/checkInRecordResultData.ts` at line 3, Change the CallId property types from number to string in the affected model(s): update CallId on the CheckInRecordResultData class (where currently declared as "public CallId: number = 0;") to be a string with a string default (e.g., ""), and make the same change in callNoteResultData.ts where CallId is incorrectly typed; this aligns these models with CallResultData/CallFileResultData/CallVideoFeedResultData and avoids needless parseInt usage elsewhere like in check-in-sidebar-widget.tsx.src/translations/fr.json-118-118 (1)
118-118:⚠️ Potential issue | 🟡 Minor
minutes_agotranslation is missing the temporal "ago" marker, making it inconsistent with all other languages.The French translation
"min"lacks the temporal marker that other languages include: English"min ago", Spanish"min atrás", Italian"min fa", Polish"min temu", Swedish"min sedan", German"Min. her". The phrase renders as{minutesAgo} min(e.g., "5 min"), which is ambiguous—it could indicate duration or time elapsed rather than explicitly conveying the "ago" meaning.Align with the pattern used by other languages by adding a temporal suffix to match their approach (e.g., following Romance language conventions similar to Spanish).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/fr.json` at line 118, Update the minutes_ago translation so it explicitly includes the temporal "ago" marker; replace the current "minutes_ago" value ("min") with a French phrasing that conveys elapsed time (e.g., "il y a {minutesAgo} min") so the rendered string becomes "il y a 5 min" instead of just "5 min". Ensure the change is applied to the "minutes_ago" JSON entry so consumers using the minutes_ago key get the temporal suffix.src/services/check-in-notification.service.ts-50-56 (1)
50-56:⚠️ Potential issue | 🟡 MinorCountdown interval keeps re-displaying every second forever, even after the timer reaches zero.
Math.max(0, this.currentSeconds - 1)clamps at 0, butsetIntervalcontinues firing anddisplayNotificationkeeps re-issuingnotifee.displayNotificationindefinitely with the same0:00body. That's wasted work and continual system-notification updates untilstopNotificationis called externally. Consider stopping the countdown when reaching zero (or transitioning to an "Overdue" steady state without a per-second redraw).🛠️ Proposed adjustment
this.countdownInterval = setInterval(async () => { - this.currentSeconds = Math.max(0, this.currentSeconds - 1); - await this.displayNotification(callName, callNumber, timerName); + if (this.currentSeconds <= 0) { + this.stopCountdown(); + return; + } + this.currentSeconds -= 1; + await this.displayNotification(callName, callNumber, timerName); }, 1000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/check-in-notification.service.ts` around lines 50 - 56, The countdown interval in check-in-notification.service keeps firing after currentSeconds reaches 0; modify the setInterval callback in the method that starts the timer so that after decrementing currentSeconds you detect when it has reached 0, call stopCountdown() (or clearInterval(this.countdownInterval)) and then call displayNotification one final time (or transition to the "Overdue" state) instead of continuing to schedule per-second updates; update the logic around this.countdownInterval, stopCountdown(), currentSeconds, and displayNotification to ensure the interval is cleared as soon as currentSeconds === 0 so no further notifee.displayNotification calls occur.src/stores/call-video-feeds/store.ts-37-44 (1)
37-44:⚠️ Potential issue | 🟡 MinorGuard against non-array
Databefore spreading.If the backend returns
null/undefinedforresult.Data(e.g., empty payload or auth edge case),[...result.Data].sort(...)will throw aTypeErrorand the catch will surface it as a confusing "is not iterable" error instead of an empty list. The rest of the codebase consistently usesArray.isArray(...)guards (seesrc/stores/calls/store.ts).- const result = await getCallVideoFeeds(callId); - const sorted = [...result.Data].sort((a, b) => a.SortOrder - b.SortOrder); - set({ feeds: sorted, isLoadingFeeds: false }); + const result = await getCallVideoFeeds(callId); + const data = Array.isArray(result.Data) ? result.Data : []; + const sorted = [...data].sort((a, b) => a.SortOrder - b.SortOrder); + set({ feeds: sorted, isLoadingFeeds: false });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/call-video-feeds/store.ts` around lines 37 - 44, The code assumes result.Data is iterable before sorting, causing a TypeError when it's null/undefined; update the fetch handling in the async block that calls getCallVideoFeeds to guard result.Data with Array.isArray and default to an empty array before spreading/sorting (e.g., const items = Array.isArray(result.Data) ? result.Data : []; const sorted = [...items].sort(...)), then call set({ feeds: sorted, isLoadingFeeds: false }) and keep the existing error handling that sets feedsError and logs via logger.error; ensure you reference the same state keys (feeds, isLoadingFeeds, feedsError) so callers relying on an empty array still work.src/api/dispatch/dispatch.ts-20-22 (1)
20-22:⚠️ Potential issue | 🟡 MinorRemove the unused
getSetUnitStatefunction.The search found zero callers of
getSetUnitStatein the codebase. All internal code imports and usesgetSetUnitStatusDatadirectly. Since this is an internal application with no external consumers, this unused wrapper should be removed entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/api/dispatch/dispatch.ts` around lines 20 - 22, Remove the unused wrapper function getSetUnitState: delete the exported async function getSetUnitState which simply forwards to getSetUnitStatusData, since there are no callers and internal code imports getSetUnitStatusData directly; ensure no external exports reference getSetUnitState and update any export lists or barrel files if necessary so only getSetUnitStatusData remains exported.src/lib/poi-utils.ts-95-101 (1)
95-101:⚠️ Potential issue | 🟡 MinorUntranslated user-visible fallback string.
The
`Type ${poiTypeId}`fallback at Line 98 will be rendered to users (it's used asgroup.titleindestination-poi-selector.tsxand elsewhere) but isn't translated. Since this is a pure utility and can't calluseTranslationdirectly, consider either:
- Accepting an optional
defaultTitle?: string(or formatter) parameter so callers can passt('pois.unknown_type', { id: poiTypeId }), or- Returning the raw
poiTypeIdand letting callers format the title.As per coding guidelines: "Wrap all user-visible text in
t()fromreact-i18nextfor internationalization."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/poi-utils.ts` around lines 95 - 101, The fallback title `Type ${poiTypeId}` used when getPoiTypeName returns falsy is user-visible and must be localized; update the utility that builds group objects (the code mapping [...groups.entries()] to { poiTypeId, title, items }) to accept an optional parameter (e.g. defaultTitle?: string | (id: string) => string) or a formatter function, and replace the hardcoded ``Type ${poiTypeId}`` with a call to that parameter (if provided) or else return the raw poiTypeId so callers can format it; then update callers such as destination-poi-selector.tsx to pass t('pois.unknown_type', { id: poiTypeId }) or a formatter that calls t(), ensuring all user-visible titles are produced by react-i18next rather than in this pure utility.src/stores/pois/store.ts-56-58 (1)
56-58:⚠️ Potential issue | 🟡 MinorHardcoded English error strings won't honor app locale.
The error messages stored in
state.error('Failed to fetch POI types','Failed to fetch destination POIs','Failed to fetch POIs','Failed to fetch POI') are raw English. If the UI rendersstate.errordirectly, non-English users will see English text. Prefer storing a translation key (or an error code) and let the consumer pass it throught().As per coding guidelines: "Ensure all text is wrapped in
t()fromreact-i18nextfor translations".Also applies to: 79-81, 107-108, 137-138
🤖 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 56 - 58, Replace hardcoded English error messages stored in state.error with translation keys or error codes so the UI can call t() and render localized text: update the four set(...) calls (the ones setting error to 'Failed to fetch POI types', 'Failed to fetch destination POIs', 'Failed to fetch POIs', and 'Failed to fetch POI') to assign keys like 'errors.fetch_poi_types', 'errors.fetch_destination_pois', 'errors.fetch_pois', 'errors.fetch_poi' (or numeric codes) and preserve isLoading/state behavior; ensure consumers read state.error and pass it through i18next's t() rather than assuming a literal message.src/stores/pois/store.ts-111-122 (1)
111-122:⚠️ Potential issue | 🟡 Minor
Number(poiId)silently producesNaNfor non-numeric input.If
poiIdarrives as a non-numeric string (e.g., a deep-link param),Number(poiId)becomesNaN, the cache lookup returnsundefined, andgetPoi(NaN)is dispatched to the API. Consider validating early and short-circuiting:🛡️ Suggested guard
fetchPoi: async (poiId: number | string, force = false) => { const normalizedPoiId = Number(poiId); + if (!Number.isFinite(normalizedPoiId)) { + set({ selectedPoi: null, error: 'Invalid POI id' }); + return null; + } const cachedPoi = get().poiDetails[normalizedPoiId];🤖 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 111 - 122, fetchPoi currently uses Number(poiId) which yields NaN for non-numeric input and then calls getPoi(NaN); update fetchPoi to validate normalizedPoiId after conversion (Number(poiId)) using Number.isFinite or !Number.isNaN and short-circuit if invalid by setting error (set({ error: new Error('Invalid poiId') })) and clearing isLoadingDetail, returning early (and do not call getPoi), or alternatively try a safe numeric parse (e.g., parseInt) before validation; refer to the fetchPoi function, the normalizedPoiId variable, the poiDetails cache lookup, and the getPoi call when implementing this guard so you never dispatch getPoi with NaN and you set selectedPoi/error state appropriately.src/stores/pois/store.ts-43-110 (1)
43-110:⚠️ Potential issue | 🟡 MinorTTL guard re-fetches forever when the API legitimately returns an empty list.
fetchPoiTypes,fetchDestinationPois, andfetchAllPoiDataall use<list>.length > 0as part of theisFreshcheck. If the backend returns[](e.g., the org has no POI types or no destination POIs),lastFetchedAtis updated but the next call still seeslength === 0→isFresh = false→ it re-fetches every time the consumer calls the action. Track freshness vialastFetchedAt(and/or a separate "ever fetched" flag) independently of list length.♻️ Suggested approach
- const { poiTypes, lastFetchedAt } = get(); - const isFresh = poiTypes.length > 0 && Date.now() - lastFetchedAt < STORE_TTL_MS; + const { lastFetchedAt } = get(); + const isFresh = lastFetchedAt > 0 && Date.now() - lastFetchedAt < STORE_TTL_MS;🤖 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 43 - 110, The TTL guard currently uses "<list>.length > 0" so empty API results never mark the store as fresh; update fetchPoiTypes, fetchDestinationPois, and fetchAllPoiData to determine freshness only from lastFetchedAt (and whether it has ever been set) rather than list length — e.g., compute isFresh as lastFetchedAt > 0 && Date.now() - lastFetchedAt < STORE_TTL_MS; keep setting lastFetchedAt = Date.now() on every successful fetch (even when the returned arrays are empty) so subsequent calls respect the TTL; modify the isFresh checks in fetchPoiTypes, fetchDestinationPois, and fetchAllPoiData accordingly.src/app/(app)/index.tsx-67-75 (1)
67-75:⚠️ Potential issue | 🟡 MinorEdge case: dismissal won't reset if alert count is unchanged.
useEffectonextremeAlerts.lengthonly resetsisBannerDismissedwhen the array length changes. If one extreme alert is removed and another added in the same update tick (count unchanged), the previously dismissed banner stays hidden even though the alert set has changed. Consider keying off the alerts identity (e.g., a stable hash ofWeatherAlertIds) if showing the banner for newly arrived alerts is required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/index.tsx around lines 67 - 75, The reset effect currently depends on extremeAlerts.length so dismissal won't reset when the set changes but count stays the same; update the logic to derive a stable identity for the alert set (e.g., a memoized key computed from extremeAlerts' WeatherAlertId values — sort and join or hash the IDs) and change the useEffect dependency from extremeAlerts.length to that memoized key; in short, create a memo (e.g., alertsKey) from extremeAlerts.map(a => a.WeatherAlertId) and call setIsBannerDismissed(false) inside the useEffect that depends on alertsKey so the banner resets whenever the actual alerts change, not just when the count changes.src/app/call/[id].tsx-324-338 (1)
324-338:⚠️ Potential issue | 🟡 MinorMinor: extra leading whitespace flagged by linter.
Static analysis is reporting trailing whitespace/extra leading spaces on lines 324–329 (Prettier
Delete ·). Re-indent these to match the surrounding<Box>siblings (which use 14-space indentation, not 15) so CI lint passes.🤖 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 324 - 338, The JSX block rendering the address/destination has one extra leading space on several lines causing a Prettier "Delete ·" lint failure; open the JSX around the Box/Text elements that reference call, destinationLabel, call.DestinationTypeName and call.DestinationAddress and re-indent the lines inside that conditional to match the surrounding siblings (use the same 14-space indentation used by other <Box> blocks) so there are no stray leading spaces; verify the indentation around the conditional render of destinationLabel and the nested Text that joins DestinationTypeName and DestinationAddress.src/app/call/[id].tsx-201-210 (1)
201-210:⚠️ Potential issue | 🟡 MinorAdd validation to prevent NaN from being passed to
startPolling.The
parseInt(call.CallId, 10)conversion is necessary sincecall.CallIdis a string andstartPollingexpects a number. However, ifCallIdis non-numeric (unlikely but possible),parseIntreturnsNaN, which is then passed tostartPollingandfetchTimerStatuseswith no validation. Add a guard to ensureCallIdis numeric before callingstartPolling:if (call?.CheckInTimersEnabled && call?.CallId) { const numericCallId = parseInt(call.CallId, 10); if (!isNaN(numericCallId)) { startPolling(numericCallId, 30000); } }The dependency array is correctly structured—it uses the extracted values (
call?.CheckInTimersEnabled,call?.CallId) rather than the object reference, so there is no polling restart thrashing when thecallobject is replaced with the same property values.🤖 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 201 - 210, The useEffect may pass NaN to startPolling when parseInt(call.CallId, 10) fails; update the effect around startPolling (in the block using startPolling, stopPolling, resetTimers) to first check call?.CallId exists, parse it to a number (e.g., numericCallId = parseInt(call.CallId, 10)) and only call startPolling(numericCallId, 30000) if !isNaN(numericCallId); keep the return cleanup calling stopPolling() and resetTimers() and leave the dependency array as-is.src/hooks/use-quick-check-in.ts-23-30 (1)
23-30:⚠️ Potential issue | 🟡 MinorUnsafe
UnitIdparsing —parseIntreturnsNaNfor non-numeric strings.
parseInt(activeUnit.UnitId, 10)will returnNaNifUnitIdis not strictly numeric (e.g., 'unit1', UUID, or alphanumeric). Test mocks in the codebase use this format (UnitId: 'unit1'), indicating IDs may not be purely numeric. NaN then propagates into the API payload and fails server-side validation or silently serializes as null.Either validate that
UnitIdis numeric before parsing with a guard likeNumber.isFinite(parsed), or changePerformCheckInInput.UnitIdto accept the raw string if the backend supports it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-quick-check-in.ts` around lines 23 - 30, The quickCheckIn function builds a PerformCheckInInput using parseInt(activeUnit.UnitId, 10) which can produce NaN for non-numeric IDs (e.g., 'unit1'), so update quickCheckIn to either validate and only use a numeric UnitId (parse -> Number.isFinite(parsed) and set UnitId to parsed when valid) or pass the raw string UnitId through if the backend accepts strings; adjust the CheckInType logic (activeUnit / CHECK_IN_TYPE_UNIT) accordingly so UnitId is only transformed when safe and otherwise assigned as the original activeUnit.UnitId string or omitted.src/components/check-in-timers/check-in-history-list.tsx-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorHardcoded "Unit:" / "User:" labels bypass i18n.
These user-visible strings should be wrapped in
t()so they translate across the seven supported languages.🌐 Proposed fix
- <Text className="text-xs text-gray-500">{item.UnitId ? `Unit: ${item.UnitId}` : `User: ${item.UserId}`}</Text> + <Text className="text-xs text-gray-500"> + {item.UnitId ? t('check_in.unit_label', { id: item.UnitId }) : t('check_in.user_label', { id: item.UserId })} + </Text>Note:
twould need to be threaded intorenderItem(e.g., moverenderIteminside the component or accepttas a closure).As per coding guidelines: "Wrap all user-facing text in t() from react-i18next".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/check-in-timers/check-in-history-list.tsx` at line 20, The hardcoded labels in the renderItem output ("Unit:" / "User:") bypass i18n; update the render so those strings are wrapped in t() from react-i18next (e.g., t('Unit') and t('User')) and interpolate the id values, and make sure t is available to renderItem by either moving renderItem inside the component that calls useTranslation() or passing t into renderItem as a closure/prop; update the Text line that renders item.UnitId/item.UserId to use the translated label via t() and preserve the existing conditional logic using item.UnitId and item.UserId.src/components/check-in-timers/check-in-history-list.tsx-33-39 (1)
33-39:⚠️ Potential issue | 🟡 MinorEmpty state likely uses the wrong translation key.
check_in.historyis the "History" heading label (used elsewhere as a section title). Showing it centered in an empty list reads as just "History" rather than an empty-state message like "No check-in history". Use a dedicated empty-state key (e.g.,check_in.no_history).🛠️ Proposed fix
if (records.length === 0) { return ( <Box className="p-4"> - <Text className="text-center text-gray-500">{t('check_in.history')}</Text> + <Text className="text-center text-gray-500">{t('check_in.no_history')}</Text> </Box> ); }Add the corresponding key to
src/translations/*.json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/check-in-timers/check-in-history-list.tsx` around lines 33 - 39, In check-in-history-list.tsx where the empty-state branch checks records.length === 0, replace the t('check_in.history') usage with a dedicated empty-state translation key such as t('check_in.no_history') and add that key/value to your translations JSON files (e.g., src/translations/*.json) for all supported locales; ensure keys are consistent across locales so the UI shows "No check-in history" (or localized equivalent) instead of the section title.src/components/check-in-timers/check-in-tab-content.tsx-66-71 (1)
66-71:⚠️ Potential issue | 🟡 MinorConfusing toggle label: heading and button both say "History".
When collapsed, the button reads
t('check_in.history')("History") right next to a heading also rendered ast('check_in.history'). Use a distinct show/hide label (e.g.,check_in.show_history/check_in.hide_history, or reusecommon.show/common.hide).🛠️ Proposed fix
- <Button variant="link" size="sm" onPress={() => setShowHistory(!showHistory)}> - <ButtonText>{showHistory ? t('common.close') : t('check_in.history')}</ButtonText> - </Button> + <Button variant="link" size="sm" onPress={() => setShowHistory((prev) => !prev)}> + <ButtonText>{showHistory ? t('check_in.hide_history') : t('check_in.show_history')}</ButtonText> + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/check-in-timers/check-in-tab-content.tsx` around lines 66 - 71, The heading and toggle Button in check-in-tab-content.tsx both render t('check_in.history'), causing confusion; update the ButtonText (the Button with onPress={() => setShowHistory(!showHistory)}) to use distinct translation keys based on the showHistory state (e.g., t('check_in.hide_history') when showHistory is true and t('check_in.show_history') or t('common.show')/t('common.hide') when false) so the Heading (Heading size="sm") remains "History" while the Button clearly indicates show vs hide; keep the existing setShowHistory toggle behavior unchanged.src/components/check-in-timers/check-in-tab-content.tsx-63-63 (1)
63-63:⚠️ Potential issue | 🟡 MinorReplace non-scrollable
FlatListwith.map()for this small, bounded list.A non-scrollable
FlatListloses virtualization benefits and produces aVirtualizedList nested inside ScrollViewwarning becauseCheckInTabContentis rendered as tab content within the parentScrollViewin the call detail screen. For the typical 3-item dataset intimerStatuses, usetimerStatuses.map(...)instead, or if virtualization becomes necessary, lift the list to be the scroll container itself usingListHeaderComponentto wrap the surrounding chrome (button, history section, bottom sheet).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/check-in-timers/check-in-tab-content.tsx` at line 63, The FlatList usage in CheckInTabContent (the line using FlatList with data={timerStatuses}, renderItem={renderTimerCard}, keyExtractor={keyExtractor}, scrollEnabled={false}) should be replaced with a simple timerStatuses.map(...) render because the list is small and non-scrollable; change the component to render timerStatuses.map((item, index) => <container key={keyExtractor(item, index)}>{renderTimerCard({ item, index })}</container>) (or call renderTimerCard with the same shape it expects) and remove the FlatList-specific props (scrollEnabled, removeClippedSubviews, maxToRenderPerBatch) — if you later need virtualization, lift the list to be the scroll container using ListHeaderComponent instead.src/components/sidebar/check-in-sidebar-widget.tsx-28-29 (1)
28-29:⚠️ Potential issue | 🟡 MinorGuard
useQuickCheckInhook invocation to only whenactiveCallis defined.The hook is called unconditionally with
callId = 0whenactiveCallis falsy (lines 28–29), even though the component returns null immediately after ifactiveCallis not present (lines 32–34). Move the hook call inside the guard condition or check thatactiveCallexists before invoking it to avoid unnecessary hook registration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/check-in-sidebar-widget.tsx` around lines 28 - 29, Currently useQuickCheckIn is invoked before we check activeCall; instead, move the activeCall guard up so the component returns null before any hooks are called. Concretely, check if (!activeCall) return null at the top of the component, then compute const callId = parseInt(activeCall.CallId, 10) and call const { quickCheckIn, isCheckingIn } = useQuickCheckIn(callId) after that guard; this keeps the hook invocation tied to an existing activeCall and avoids registering the hook when activeCall is absent.src/components/call-video-feeds/video-feed-form-sheet.tsx-156-181 (1)
156-181:⚠️ Potential issue | 🟡 MinorWire
Controller'sonBlurso RHF tracks touched/blurred state.The
Urlfield overridesonBlurto callhandleUrlBlur, but it never invokesfield.onBlurfromController, so react-hook-form never marks the field as touched. TheNamefield also hasrules={{ required: true }}but itsonBlurisn't wired either, and there's no error UI rendered for required violations — submitting an empty name silently fails validation with no user feedback.♻️ Proposed fix
- render={({ field: { onChange, value } }) => ( + render={({ field: { onChange, onBlur, value }, fieldState: { error } }) => ( <Box className="rounded-lg border border-outline-200 p-2"> - <TextInput value={value} onChangeText={onChange} placeholder={t('video_feeds.name')} /> + <TextInput value={value} onBlur={onBlur} onChangeText={onChange} placeholder={t('video_feeds.name')} /> </Box> + {error ? <Text className="text-xs text-red-500">{t('form.required')}</Text> : null} )}And for
Url:- render={({ field: { onChange, value } }) => ( + render={({ field: { onChange, onBlur, value } }) => ( <Box className="rounded-lg border border-outline-200 p-2"> - <TextInput value={value} onChangeText={onChange} onBlur={() => handleUrlBlur(value)} placeholder="https://" autoCapitalize="none" keyboardType="url" /> + <TextInput + value={value} + onChangeText={onChange} + onBlur={() => { onBlur(); handleUrlBlur(value); }} + placeholder="https://" + autoCapitalize="none" + keyboardType="url" + /> </Box> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/call-video-feeds/video-feed-form-sheet.tsx` around lines 156 - 181, The Controller render callbacks for the "Name" and "Url" fields are not calling the provided field.onBlur, so react-hook-form never marks them as touched and required validation feedback is not shown; update both Controller render functions (the ones with name "Name" and "Url") to call the controller's field.onBlur when the TextInput blurs (for "Url" call field.onBlur before/after invoking handleUrlBlur(value)), and add UI to render validation errors from formState.errors for these fields so required violations are visible to the user.src/components/routes/active-routes-list.tsx-81-83 (1)
81-83:⚠️ Potential issue | 🟡 MinorPull-to-refresh remounts the loader instead of refreshing the list.
if (isLoading) return <Loading />;runs on every refresh — including when the user triggersRefreshControl. TheFlatListunmounts mid-pull, theRefreshControlindicator disappears, and the user sees a full-screen spinner instead of an inline pull-refresh. Gate the early loader on first load only:♻️ Proposed fix
- if (isLoading) { + if (isLoading && routePlans.length === 0) { return <Loading text={t('routes.loading')} />; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/active-routes-list.tsx` around lines 81 - 83, The current early return in ActiveRoutesList using if (isLoading) return <Loading /> causes the loader to remount on every RefreshControl pull; update the condition to only show the full-screen Loading on first load (e.g., when isLoading is true AND there are no items yet) so the FlatList and its RefreshControl remain mounted during pull-to-refresh. Locate the isLoading check and replace it with a gate like "isLoading && routes.length === 0" (or a similar "hasLoadedOnce" flag) so the inline refresh indicator is preserved while subsequent loads still show the spinner only when the list is empty.src/stores/weather-alerts/store.ts-47-71 (1)
47-71:⚠️ Potential issue | 🟡 MinorInitialization can NPE if
WeatherAlertSettingsDatais missing.
settingsResponse.Datais typed/treated as non-null at line 49 and line 52, but if the backend ever returnsnull/undefinedData(timeout, partial response, etc.) the access tosettings.WeatherAlertsEnabledwill throw and be swallowed by the catch, leavingisInitialized: truewith stale state. Add a defensive null check before readingWeatherAlertsEnabled.🛡️ Suggested fix
const settingsResponse = await getWeatherAlertSettings(); - const settings = settingsResponse.Data; + const settings = settingsResponse?.Data ?? null; set({ settings }); - if (settings.WeatherAlertsEnabled) { + if (settings?.WeatherAlertsEnabled) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/weather-alerts/store.ts` around lines 47 - 71, The code assumes settingsResponse.Data is non-null before accessing settings.WeatherAlertsEnabled; update the initializer (the async block that calls getWeatherAlertSettings) to first check if settingsResponse?.Data is present and valid (e.g., const settings = settingsResponse?.Data ?? null) and only call settings.WeatherAlertsEnabled when settings is non-null; if Data is missing, call set({ error: 'Failed to initialize weather alerts', isLoading: false, isInitialized: false }) or provide a safe default settings object, and avoid setting isInitialized: true or touching alerts until you successfully fetch/validate settings; apply the same defensive null-check pattern around settingsResponse.Data and anywhere settings is used before calling getActiveAlerts or sortAlertsBySeverity to prevent NPEs.src/lib/weather-alert-utils.ts-51-88 (1)
51-88:⚠️ Potential issue | 🟡 MinorPolygon parser does not validate parsed JSON shape or numeric coordinates.
Two related robustness issues in
parsePolygonGeoJSON:
JSON.parse(polygonStr)can legitimately returnnull(e.g., the input string is"null"), at which pointparsed.typethrows — not caught by the outertry, since this is the outer try. It will fall through, but the same risk applies ifparsedis a primitive number/string with no.typereading boolean false; thennullis returned correctly. However anullparsed value will throw a TypeError on property access, which is caught — so behavior is "fall through to coordinate parsing" by accident. Add an explicit object check.- The coordinate-pair fallback uses
Number(...)without validating finiteness. Inputs like"abc,def"produce[NaN, NaN]pairs that pass thelength < 3check and are emitted as a Polygon Feature withNaNcoordinates. The closing-vertex checkfirst[0] !== last[0]always evaluates totrueforNaN, so a duplicateNaNvertex is appended. Downstream Mapbox rendering will silently fail or warn.🛡️ Suggested hardening
try { // Try parsing as GeoJSON first const parsed = JSON.parse(polygonStr); + if (!parsed || typeof parsed !== 'object') return null; if (parsed.type === 'Feature') return parsed; if (parsed.type === 'Polygon' || parsed.type === 'MultiPolygon') { return { type: 'Feature', properties: {}, geometry: parsed }; } return null; } catch { // Try parsing as coordinate pairs "lat,lng lat,lng ..." try { - const coords = polygonStr + const coords: number[][] = polygonStr .trim() .split(/\s+/) .map((pair) => { const [lat, lng] = pair.split(',').map(Number); return [lng, lat]; - }); + }) + .filter(([lng, lat]) => Number.isFinite(lng) && Number.isFinite(lat)); if (coords.length < 3) return null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/weather-alert-utils.ts` around lines 51 - 88, The parser parsePolygonGeoJSON should explicitly guard against non-object JSON results and validate numeric coordinates: after JSON.parse(polygonStr) verify parsed is an object (not null) and that parsed.type is a string before accessing it; for the coordinate-pairs fallback, split each pair into exactly two parts, convert with Number and require Number.isFinite(lat) && Number.isFinite(lng), discard any invalid pairs, ensure at least 3 valid coordinate pairs remain, and use numeric comparisons (not relying on !== which fails for NaN) to decide whether to close the ring (compare first[0] === last[0] && first[1] === last[1]); only then build and return the Feature/Polygon.src/models/v4/unitStatus/unitStatusResultData.ts-10-19 (1)
10-19:⚠️ Potential issue | 🟡 MinorInconsistent
GroupIdtype breaks downstream comparisons.
GroupId(and similar fields) now acceptsnumber | stringinunitStatusResultData, inconsistent with the rest of the codebase whereGroupIdis uniformlystring. This creates silent equality bugs: at line 318 instatus-bottom-sheet.tsx,stationId: stringis compared againstitem.GroupId: number | stringusing===, which will fail if types mismatch (e.g.,"123" === 123returns false).Align the type with the established pattern: use
stringexclusively to matchunitResultData,groupsResultData, and other models across the app. This eliminates type coercion risk for all consumers.🤖 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 10 - 19, The GroupId field in unitStatusResultData is typed as number | string which breaks strict comparisons (e.g., stationId: string === item.GroupId); change GroupId's type to string (remove number union) in the unitStatusResultData model so it matches unitResultData and groupsResultData, then run TS checks and update any call sites that assumed numeric GroupId (or explicitly cast there) — look for the GroupId property in the unitStatusResultData definition and the comparison in status-bottom-sheet.tsx to verify compatibility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad6024d0-446c-4ec7-92ea-6e2ef886e147
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (121)
app.config.tspackage.jsonplugins/withCheckInLiveActivity.jssrc/api/call-video-feeds/call-video-feeds.tssrc/api/calls/calls.tssrc/api/check-in-timers/check-in-timers.tssrc/api/dispatch/dispatch.tssrc/api/mapping/mapping.tssrc/api/weather-alerts/weather-alerts.tssrc/app/(app)/_layout.tsxsrc/app/(app)/index.tsxsrc/app/(app)/routes.tsxsrc/app/(app)/weather-alerts.tsxsrc/app/call/[id].tsxsrc/app/call/[id]/edit.tsxsrc/app/call/__tests__/[id].security.test.tsxsrc/app/call/__tests__/[id].test.tsxsrc/app/call/new/index.tsxsrc/app/routes/_layout.tsxsrc/app/routes/history/instance/[id].tsxsrc/app/routes/index.tsxsrc/app/routes/poi/[id].tsxsrc/app/weather-alert/[id].tsxsrc/components/call-video-feeds/__tests__/video-feed-card.test.tsxsrc/components/call-video-feeds/__tests__/video-feed-tab-content.test.tsxsrc/components/call-video-feeds/feed-format-utils.tssrc/components/call-video-feeds/video-feed-card.tsxsrc/components/call-video-feeds/video-feed-form-sheet.tsxsrc/components/call-video-feeds/video-feed-tab-content.tsxsrc/components/call-video-feeds/video-player-modal.tsxsrc/components/calls/call-card.tsxsrc/components/calls/destination-poi-selector.tsxsrc/components/check-in-timers/__tests__/check-in-bottom-sheet.test.tsxsrc/components/check-in-timers/__tests__/check-in-timer-card.test.tsxsrc/components/check-in-timers/check-in-bottom-sheet.tsxsrc/components/check-in-timers/check-in-history-list.tsxsrc/components/check-in-timers/check-in-tab-content.tsxsrc/components/check-in-timers/check-in-timer-card.tsxsrc/components/maps/__tests__/pin-actions.test.tsxsrc/components/maps/pin-detail-modal.tsxsrc/components/routes/active-routes-list.tsxsrc/components/routes/poi-card.tsxsrc/components/routes/poi-list-content.tsxsrc/components/routes/routes-home.tsxsrc/components/sidebar/__tests__/call-sidebar.test.tsxsrc/components/sidebar/check-in-sidebar-widget.tsxsrc/components/sidebar/sidebar-content.tsxsrc/components/status/__tests__/status-bottom-sheet.test.tsxsrc/components/status/__tests__/status-gps-debug.test.tsxsrc/components/status/__tests__/status-gps-integration-working.test.tsxsrc/components/status/__tests__/status-gps-integration.test.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/weather-alerts/__tests__/weather-alert-banner.test.tsxsrc/components/weather-alerts/__tests__/weather-alert-card.test.tsxsrc/components/weather-alerts/severity-filter-tabs.tsxsrc/components/weather-alerts/weather-alert-banner.tsxsrc/components/weather-alerts/weather-alert-card.tsxsrc/components/weather-alerts/weather-alert-detail-map.tsxsrc/hooks/__tests__/use-quick-check-in.test.tssrc/hooks/use-check-in-timer-polling.tssrc/hooks/use-quick-check-in.tssrc/lib/__tests__/poi-utils.test.tssrc/lib/native-modules/check-in-live-activity.tssrc/lib/poi-utils.tssrc/lib/weather-alert-utils.tssrc/models/offline-queue/queued-event.tssrc/models/v4/callVideoFeeds/callVideoFeedResult.tssrc/models/v4/callVideoFeeds/callVideoFeedResultData.tssrc/models/v4/callVideoFeeds/saveCallVideoFeedResult.tssrc/models/v4/calls/callResultData.tssrc/models/v4/checkIn/checkInRecordResult.tssrc/models/v4/checkIn/checkInRecordResultData.tssrc/models/v4/checkIn/checkInTimerStatusResult.tssrc/models/v4/checkIn/checkInTimerStatusResultData.tssrc/models/v4/checkIn/performCheckInResult.tssrc/models/v4/checkIn/resolvedCheckInTimerResult.tssrc/models/v4/checkIn/resolvedCheckInTimerResultData.tssrc/models/v4/customStatuses/customStateDetailTypes.tssrc/models/v4/destinations/destinationEntityTypes.tssrc/models/v4/dispatch/getSetUnitStateResultData.tssrc/models/v4/dispatch/newCallFormResultData.tssrc/models/v4/mapping/getMapDataAndMarkersData.tssrc/models/v4/mapping/poiResultData.tssrc/models/v4/mapping/poiResults.tssrc/models/v4/personnelStatuses/getCurrentStatusResultData.tssrc/models/v4/unitStatus/saveUnitStatusInput.tssrc/models/v4/unitStatus/unitStatusResultData.tssrc/models/v4/weatherAlerts/activeWeatherAlertsResult.tssrc/models/v4/weatherAlerts/weatherAlertEnums.tssrc/models/v4/weatherAlerts/weatherAlertResult.tssrc/models/v4/weatherAlerts/weatherAlertResultData.tssrc/models/v4/weatherAlerts/weatherAlertSettingsData.tssrc/models/v4/weatherAlerts/weatherAlertSettingsResult.tssrc/models/v4/weatherAlerts/weatherAlertZoneResultData.tssrc/models/v4/weatherAlerts/weatherAlertZonesResult.tssrc/services/__tests__/offline-event-manager-gps.test.tssrc/services/__tests__/offline-event-manager.service.test.tssrc/services/__tests__/push-notification.test.tssrc/services/check-in-notification.service.tssrc/services/offline-event-manager.service.tssrc/services/push-notification.tssrc/stores/call-video-feeds/__tests__/store.test.tssrc/stores/call-video-feeds/store.tssrc/stores/calls/store.tssrc/stores/check-in-timers/__tests__/store.test.tssrc/stores/check-in-timers/store.tssrc/stores/pois/store.tssrc/stores/signalr/signalr-store.tssrc/stores/status/__tests__/store.test.tssrc/stores/status/store.tssrc/stores/weather-alerts/__tests__/store.test.tssrc/stores/weather-alerts/store.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
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/translations/sv.json (1)
240-292:⚠️ Potential issue | 🟠 MajorMany new POI/destination keys are left in English in the Swedish dictionary.
These keys are placed under
calls,map,routes,status, andcheck_inbut their values are English strings, e.g.,"destination_poi": "Destination POI","loading_destination_pois": "Loading destination POIs...","no_destination_pois_available": "No destination POIs available","select_destination_poi": "Select Destination POI","pin_address": "Address","pin_note": "Note","pin_type": "POI Type","view_poi_details": "View POI Details","failed_to_open_poi_maps": "Failed to open maps for this POI","loading_poi": "Loading POI...","loading_pois": "Loading POIs...","no_pois": "No POIs","no_pois_description","no_pois_filtered_description","poi_*"keys at lines 710–724,"route_to_poi","routes_tab","search_pois","set_poi_destination","view_on_map", the"status.*pois*"keys at lines 866–891, and"check_in.notification_channel_*"at lines 328–329. Swedish users will see a mix of Swedish and English copy. Either translate them now or open a follow-up to do so before release; otherwise the locale silently degrades.Want me to draft Swedish translations for these strings as a follow-up patch?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/sv.json` around lines 240 - 292, The Swedish locale contains many untranslated POI/destination strings—update the keys such as "destination_poi", "loading_destination_pois", "no_destination_pois_available", "select_destination_poi", "pin_address", "pin_note", "pin_type", "view_poi_details", "failed_to_open_poi_maps", "loading_poi", "loading_pois", "no_pois", "no_pois_description", "no_pois_filtered_description", the "poi_*" keys, "route_to_poi", "routes_tab", "search_pois", "set_poi_destination", "view_on_map", the "status.*pois*" keys and "check_in.notification_channel_*" to Swedish equivalents (keep phrasing consistent with existing keys like "destination": "Destination" and "no_files": "Inga filer tillgängliga"), ensuring pluralization and placeholders match the English originals; either translate them inline in src/translations/sv.json or create a follow-up patch containing Swedish translations for all these keys so the locale is fully localized.src/translations/es.json (1)
147-902:⚠️ Potential issue | 🟡 MinorMany newly added Spanish translation keys still contain English values.
In this Spanish locale file, a sizeable portion of the new POI / destination / weather-related keys are left in English, which will surface untranslated UI to Spanish users:
call_detail.destination(147),calls.destination(240),calls.destination_poi(241),calls.destination_poi_none(242),calls.loading_destination_pois(260),calls.no_destination_pois_available(272),calls.select_destination_poi(292)check_in.notification_channel_description(328),check_in.notification_channel_name(329)map.pin_address(540),map.pin_note(542),map.pin_type(543),map.view_poi_details(547)routes.*:failed_to_open_poi_maps(676),loading_poi(683),loading_pois(684),no_pois(695),no_pois_description(696),no_pois_filtered_description(697),poi_address(710),poi_coordinates(711),poi_destination_enabled(714),poi_filter_all_types(715),poi_filter_placeholder(716),poi_not_found(717),poi_not_found_description(718),poi_note(719),poi_sort_display(720),poi_sort_placeholder(721),poi_sort_type(722),poi_type_unknown(723),pois_tab(724),route_to_poi(735),routes_tab(736),search_pois(739),set_poi_destination(741),view_on_map(770)status.*:calls_and_pois_destinations_enabled(866),calls_stations_pois_destinations_enabled(867),loading_pois(871),no_pois_available(874),poi_destination_enabled(880),pois_tab(881),stations_and_pois_destinations_enabled(890)Please provide proper Spanish translations for these keys (and audit other locale files in this PR for the same gap).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/es.json` around lines 147 - 902, Many newly added Spanish translation keys remain in English; update the Spanish locale JSON to provide proper Spanish translations for the listed keys (e.g., call_detail.destination, calls.destination, calls.destination_poi, calls.destination_poi_none, calls.loading_destination_pois, calls.no_destination_pois_available, calls.select_destination_poi, check_in.notification_channel_description, check_in.notification_channel_name, map.pin_address, map.pin_note, map.pin_type, map.view_poi_details, routes.failed_to_open_poi_maps, routes.loading_poi, routes.loading_pois, routes.no_pois, routes.no_pois_description, routes.no_pois_filtered_description, routes.poi_address, routes.poi_coordinates, routes.poi_destination_enabled, routes.poi_filter_all_types, routes.poi_filter_placeholder, routes.poi_not_found, routes.poi_not_found_description, routes.poi_note, routes.poi_sort_display, routes.poi_sort_placeholder, routes.poi_sort_type, routes.poi_type_unknown, routes.pois_tab, routes.route_to_poi, routes.routes_tab, routes.search_pois, routes.set_poi_destination, routes.view_on_map, status.calls_and_pois_destinations_enabled, status.calls_stations_pois_destinations_enabled, status.loading_pois, status.no_pois_available, status.poi_destination_enabled, status.pois_tab, status.stations_and_pois_destinations_enabled) by replacing the English strings with accurate Spanish equivalents and ensure pluralization/variables remain intact; after updating these keys, scan other changed locale files in the PR for similar untranslated English strings and apply equivalent translations.src/translations/pl.json (1)
147-292:⚠️ Potential issue | 🟠 MajorSeveral newly added keys are not translated to Polish.
Same issue as in
fr.json: many of the new POI/check-in/routes/map/status keys retain their English source text in this Polish dictionary. Polish users will see English in the corresponding flows. Untranslated keys include:
call_detail.destination(line 147) — should likely be "Cel" / "Miejsce docelowe"calls.destination,calls.destination_poi,calls.destination_poi_none,calls.loading_destination_pois,calls.no_destination_pois_available,calls.select_destination_poi(lines 240-242, 260, 272, 292)map.pin_address,map.pin_note,map.pin_type,map.view_poi_details(lines 540, 542, 543, 547)- All
routes.*poi*androutes.view_on_map/routes.routes_tab/routes.failed_to_open_poi_maps(lines 676, 683-684, 695-697, 710-724, 735-736, 739, 741, 770)status.calls_and_pois_destinations_enabled,status.calls_stations_pois_destinations_enabled,status.loading_pois,status.no_pois_available,status.poi_destination_enabled,status.pois_tab,status.stations_and_pois_destinations_enabled(lines 866, 867, 871, 874, 880, 881, 890)check_in.notification_channel_description,check_in.notification_channel_name(lines 328-329)Please provide Polish translations before release.
I'm happy to draft Polish translations for these keys if helpful.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/pl.json` around lines 147 - 292, Several new i18n keys in the Polish translations file are still in English; update the values for keys such as call_detail.destination, calls.destination, calls.destination_poi, calls.destination_poi_none, calls.loading_destination_pois, calls.no_destination_pois_available, calls.select_destination_poi, map.pin_address, map.pin_note, map.pin_type, map.view_poi_details, all routes.*poi* keys including routes.view_on_map, routes.routes_tab, routes.failed_to_open_poi_maps, status.* keys (e.g. status.calls_and_pois_destinations_enabled, status.calls_stations_pois_destinations_enabled, status.loading_pois, status.no_pois_available, status.poi_destination_enabled, status.pois_tab, status.stations_and_pois_destinations_enabled) and check_in.notification_channel_description/name — replace the English strings with correct Polish translations (e.g., "destination" -> "Cel" or "Miejsce docelowe", "No destination selected" -> "Brak wybranego miejsca docelowego", etc.), ensuring pluralization/context matches other nearby keys and keeping JSON key names unchanged.src/translations/fr.json (1)
240-292:⚠️ Potential issue | 🟠 MajorSeveral newly added keys are not translated to French.
A large portion of the new POI/check-in/routes/map/status keys still hold their English source text in this French dictionary. French users will see English UI in the corresponding flows. Notable untranslated values:
calls.destination_poi,calls.destination_poi_none,calls.loading_destination_pois,calls.no_destination_pois_available,calls.select_destination_poi(lines 241, 242, 260, 272, 292)map.pin_address,map.pin_note,map.pin_type,map.view_poi_details(lines 540, 542, 543, 547)routes.failed_to_open_poi_maps,routes.loading_poi,routes.loading_pois,routes.no_pois*,routes.poi_address,routes.poi_coordinates*,routes.poi_destination_enabled,routes.poi_filter_*,routes.poi_not_found*,routes.poi_note,routes.poi_sort_*,routes.poi_type_unknown,routes.pois_tab,routes.route_to_poi,routes.routes_tab,routes.search_pois,routes.set_poi_destination,routes.view_on_map(lines 676, 683-684, 695-697, 710-724, 735-736, 739, 741, 770)status.calls_and_pois_destinations_enabled,status.calls_stations_pois_destinations_enabled,status.loading_pois,status.no_pois_available,status.poi_destination_enabled,status.pois_tab,status.stations_and_pois_destinations_enabled(lines 866, 867, 871, 874, 880, 881, 890)check_in.notification_channel_description,check_in.notification_channel_name(lines 328-329)Please replace these with French translations before release.
I'm happy to draft French translations for these keys if helpful.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/translations/fr.json` around lines 240 - 292, Multiple newly added translation keys in the French JSON (e.g., calls.destination_poi, calls.destination_poi_none, calls.loading_destination_pois, calls.no_destination_pois_available, calls.select_destination_poi, map.pin_address, map.pin_note, map.pin_type, map.view_poi_details, routes.* keys like routes.loading_pois/routes.no_pois*/routes.poi_address/etc., status.* keys like status.loading_pois/status.no_pois_available, and check_in.notification_channel_name/description) are still in English; replace each English value with the appropriate French translation string in src/translations/fr.json so the UI is fully localized — update the values for the listed keys (and other POI/check-in/routes/map/status keys mentioned in the review) to French equivalents, keeping the key names unchanged and matching existing translation style and punctuation.
♻️ Duplicate comments (2)
src/translations/ar.json (1)
240-292:⚠️ Potential issue | 🟠 MajorSame untranslated-strings issue as
sv.json— many POI/destination/notification keys are English-only inar.json.Identical pattern to the Swedish file:
calls.destination*(lines 240–242, 260, 272, 292),call_detail.destination(line 147),check_in.notification_channel_*(lines 328–329),map.pin_*andmap.view_poi_details(lines 540–547),routes.failed_to_open_poi_maps/loading_poi*/no_pois*/poi_*/route_to_poi/routes_tab/search_pois/set_poi_destination/view_on_map(lines 676–770), andstatus.*pois*/status.poi_destination_enabled/status.stations_and_pois_destinations_enabled(lines 866–890) all carry English values in the Arabic dictionary. RTL Arabic users will see English mixed in. Either translate now or track in a follow-up before release.src/services/check-in-notification.service.ts (1)
85-91:⚠️ Potential issue | 🟡 MinorHardcoded English literals still leak into the notification body.
The past i18n round-trip moved most strings into
NotificationLabels, but two English fragments remain user-visible:
- Line 91:
` - ${timeStr} remaining [${statusLabel}]`hardcodes"remaining"and the bracket framing.- Line 85:
?? 'Check In'fallback is English.For a Spanish/French user, the body will render e.g.
"PAR - 2:30 remaining [OK]"instead of"PAR - 2:30 restante [OK]". Either addremainingSuffix(and optionallybodyFormat) toNotificationLabelsand have the polling hook supply translated strings, or pass the entire pre-formatted body in. Also drop the English fallback oncelabelsis required (it already is instartNotification).♻️ Suggested fix
export interface NotificationLabels { statusLabels: Record<string, string>; channelName: string; channelDescription: string; actionText: string; + remainingSuffix: string; // e.g. t('check_in.remaining') }- const statusLabel = this.currentLabels?.statusLabels[this.currentStatus] ?? this.currentStatus; - const actionText = this.currentLabels?.actionText ?? 'Check In'; + const statusLabel = this.currentLabels?.statusLabels[this.currentStatus] ?? this.currentStatus; + const actionText = this.currentLabels?.actionText ?? ''; + const remaining = this.currentLabels?.remainingSuffix ?? '';- body: `${timerName} - ${timeStr} remaining [${statusLabel}]`, + body: `${timerName} - ${timeStr} ${remaining} [${statusLabel}]`,As per coding guidelines: "Wrap all user-facing text in
t()fromreact-i18nextfor internationalization".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/check-in-notification.service.ts` around lines 85 - 91, The notification body still contains hardcoded English fragments: remove the English fallback ('Check In') from this.currentLabels?.actionText and stop assembling the string with the literal "remaining" and brackets inside the notifee.displayNotification call; instead extend NotificationLabels to include a translated remainingSuffix (or a full bodyFormat/bodyText) and have the polling hook / startNotification supply the fully localized pieces (e.g., currentLabels.actionText and currentLabels.remainingSuffix or currentLabels.bodyFormat) so that notifee.displayNotification uses only values from this.currentLabels (and drop the English fallback since labels are required).
🧹 Nitpick comments (14)
src/components/check-in-timers/check-in-timer-card.tsx (2)
62-92: Hardcoded gray/white colors won't adapt to dark mode.
text-gray-500,bg-gray-200, and the inlinecolor: 'white'onButtonTextare fixed in the light palette. Per coding guidelines, components must "Support both dark mode and light mode in styling". Prefer the gluestack tokens (e.g.,text-typography-500,bg-background-100) so the card renders correctly under the system theme — same fix should be applied to the inlinestyle={{ color: statusColor }}chip text wherestatusColor + '20'is used as a background tint (which is fine, but pair it with token-based foregrounds when status colors aren't applicable).As per coding guidelines: "Support both dark mode and light mode in styling".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/check-in-timers/check-in-timer-card.tsx` around lines 62 - 92, The card uses hardcoded light-mode classes and inline colors (e.g., "text-gray-500", "bg-gray-200", and inline style color: statusColor on the chip text), which breaks dark-mode support; update all hardcoded color tokens in the CheckInTimerCard render (components: Box, Text, HStack, the progress bar Box, and the chip using statusColor + '20') to use gluestack theme tokens (e.g., replace "text-gray-500" with "text-typography-500", "bg-gray-200" with "bg-background-100", and use a token-based foreground for the chip text instead of a hardcoded inline color), keep statusColor only for the tint background but pair it with a theme foreground token for accessibility and dark-mode compatibility (look for usages of timer.TargetName, timer.DurationMinutes, safeStatusLower, minutesAgo, localElapsed to locate the JSX block).
25-40: WrapCheckInTimerCardinReact.memoand consider one shared ticker for the list.Two related performance points when this card is rendered in a list (
check-in-tab-content.tsx):
- The component is not memoized, so any state change in the parent re-creates
onCheckInand re-runs the render, even thoughtimeris unchanged. As per coding guidelines,React.memo()should be used "for components with static props to prevent unnecessary re-renders" — this is a textbook fit.- Each card runs its own
setInterval(... 1000)(line 36–38). With N timers visible, you get N JS-thread timers per second just to do a+1/60increment. Lifting the ticker to the parent (or the store) and passinglocalElapseddown would let one tick update the whole list.♻️ Minimal memoization
-export const CheckInTimerCard: React.FC<CheckInTimerCardProps> = ({ timer, onCheckIn, showCheckInButton = true }) => { +const CheckInTimerCardComponent: React.FC<CheckInTimerCardProps> = ({ timer, onCheckIn, showCheckInButton = true }) => { ... -}; +}; + +export const CheckInTimerCard = React.memo(CheckInTimerCardComponent);As per coding guidelines: "Use
React.memo()for components with static props to optimize performance".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/check-in-timers/check-in-timer-card.tsx` around lines 25 - 40, The CheckInTimerCard component creates a per-instance setInterval and is not memoized, causing N timers and unnecessary re-renders; wrap the component export (CheckInTimerCard) in React.memo to avoid re-renders when props like timer haven't changed, and remove the internal ticker useEffect (the setInterval that updates localElapsed) so the component consumes a single shared ticker value passed from the parent/store (e.g., accept a prop like sharedElapsed or subscribe to a sharedTicker hook) and use that to derive displayed elapsed time instead of setLocalElapsed; keep the existing props (timer, onCheckIn, showCheckInButton) and ensure onCheckIn is stable (useCallback in parent) so memoization is effective.src/lib/weather-alert-utils.ts (1)
90-100:parseCenterLocation: deadtry/catchand missing range validation.Neither
String.prototype.splitnorNumber(...)throws, so thetry/catchhere is unreachable — the only failure mode the function actually has is theisNaNbranch. Also, lat/lon should be validated against their valid ranges (lat ∈ [-90, 90], lon ∈ [-180, 180]); otherwise pathological strings like"500,1000"parse as a "valid" location.♻️ Suggested fix
-export const parseCenterLocation = (centerStr: string): { latitude: number; longitude: number } | null => { - if (!centerStr) return null; - - try { - const [lat, lng] = centerStr.split(',').map(Number); - if (isNaN(lat) || isNaN(lng)) return null; - return { latitude: lat, longitude: lng }; - } catch { - return null; - } -}; +export const parseCenterLocation = (centerStr: string): { latitude: number; longitude: number } | null => { + if (!centerStr) return null; + const [lat, lng] = centerStr.split(',').map(Number); + if (!Number.isFinite(lat) || !Number.isFinite(lng)) return null; + if (lat < -90 || lat > 90 || lng < -180 || lng > 180) return null; + return { latitude: lat, longitude: lng }; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/weather-alert-utils.ts` around lines 90 - 100, The parseCenterLocation function currently has an unnecessary try/catch and lacks lat/lon range checks; remove the dead try/catch and instead validate that centerStr splits into exactly two numeric parts and that latitude ∈ [-90,90] and longitude ∈ [-180,180] (return null if any check fails). Locate parseCenterLocation and replace the try/catch block with explicit checks: split on ',', ensure two elements, use Number to parse, check !isNaN for both, then verify the numeric ranges before returning { latitude, longitude } or null on failure.src/stores/calls/store.ts (1)
100-108: Side-effect on sharedcallPriorities/callTypesslices.
fetchCallFormDataoverwrites the existingcallPrioritiesandcallTypesslices that are independently fetched byfetchCallPriorities/fetchCallTypes(and used elsewhere in the calls store). IfgetNewCallData()ever returns a smaller/older subset forPriorities/CallTypesthan the dedicated endpoints (or returns them in a different shape), other screens consuming these slices will silently regress until the next dedicated fetch.If the new-call form is the only consumer of these refreshed slices, consider isolating the form data into its own keys (e.g.,
callFormPriorities,callFormCallTypes) — or document thatgetNewCallDatais the canonical source and remove the now-duplicatedfetchCallTypeslength-based guard at lines 78–82 to avoid divergence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/calls/store.ts` around lines 100 - 108, fetchCallFormData currently overwrites the shared slices callPriorities and callTypes fetched independently by fetchCallPriorities/fetchCallTypes, which can cause silent regressions; change fetchCallFormData to store its values under isolated keys (e.g., callFormPriorities, callFormCallTypes, and set isCallFormDataLoaded) instead of writing callPriorities/callTypes, or alternatively remove the duplicated writes and rely on getNewCallData as the canonical source by deleting the assignments to callPriorities/callTypes in fetchCallFormData; update all references to these form-specific values to use the new callForm* keys and keep fetchCallPriorities/fetchCallTypes behavior unchanged.src/models/v4/mapping/getMapDataAndMarkersData.ts (1)
21-27:PoiTypeId?: number | nullis the onlynull-permitting field in this model.Every other optional field uses
field?: T = ''/0semantics (omit means "not provided"), butPoiTypeIdadds a third state vianull. Unless the API truly distinguishes "missing" vs "explicitly null" for this property, consider dropping the| nullso that consumers only need to handle one absence form (undefined). Otherwise nullish checks must use!= nulleverywhere instead of a simple optional check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/v4/mapping/getMapDataAndMarkersData.ts` around lines 21 - 27, The model field PoiTypeId currently allows a third state via "number | null" which is inconsistent with other optional fields; change the declaration in getMapDataAndMarkersData.ts from "PoiTypeId?: number | null = null" to a single optional number (e.g. "PoiTypeId?: number = 0") so consumers only handle undefined as absent, and update any call sites that relied on explicit null checks to use the simple optional/undefined check for PoiTypeId.plugins/withCheckInLiveActivity.js (2)
366-368: Idempotency check may miss repeat runs because group/file refs are still added before the target check.The early-return at line 366–368 only protects against re-creating the target, but earlier in the callback the Swift sources were already written to disk in Step 3 (and on subsequent prebuild runs the target check returns early before any project mutation). That part is fine. However, the file-on-disk write in Step 3 always overwrites the generated Swift sources unconditionally — if a future maintainer hand-edits any of the generated
.swift/.mfiles inios/, the next prebuild silently clobbers them. Worth at minimum a code comment stating that these files are plugin-owned and any edits must be made in the plugin templates, not inios/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/withCheckInLiveActivity.js` around lines 366 - 368, The plugin currently returns early using project.pbxTargetByName(WIDGET_NAME) but only after the Swift source files were already unconditionally written to disk, which allows prebuild to silently clobber any manual edits; fix by moving the target/idempotency check (the project.pbxTargetByName(WIDGET_NAME) / project.pbxTargetByName(`"${WIDGET_NAME}"`)) to before the Step 3 write of Swift sources so writes are skipped when the target already exists, or make the Step 3 writer conditional: only create files if they don't exist (check fs.existsSync) or write to a plugin-owned path and skip overwriting, and also add an explicit code comment near the Swift/M file generation function (referencing WIDGET_NAME and the Swift source write block) stating these generated files are plugin-owned and must not be edited in ios/ so maintainers understand the ownership and idempotency behavior.
195-200:Activity.request(attributes:contentState:)is deprecated since iOS 16.2.The deprecated initializer still works for the iOS 16.1+ deployment target, but it triggers a deprecation warning. The recommended path on iOS 16.2+ is
Activity.request(attributes:content:pushType:)usingActivityContent(state:staleDate:). Since you're generating this code from a plugin, you can keep iOS 16.1 minimum and gate the new API with#available(iOS 16.2, *)to avoid future-removal risk.♻️ Suggested update
- do { - let _ = try Activity.request(attributes: attributes, contentState: state) - resolve(true) - } catch { - reject("LIVE_ACTIVITY_ERROR", error.localizedDescription, error) - } + do { + if `#available`(iOS 16.2, *) { + let content = ActivityContent(state: state, staleDate: nil) + _ = try Activity.request(attributes: attributes, content: content, pushType: nil) + } else { + _ = try Activity.request(attributes: attributes, contentState: state) + } + resolve(true) + } catch { + reject("LIVE_ACTIVITY_ERROR", error.localizedDescription, error) + }A similar treatment applies to
activity.update(using:)on line 215. Replace with a conditional that usesActivityContent(state:staleDate:)andactivity.update(_:)on iOS 16.2+.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/withCheckInLiveActivity.js` around lines 195 - 200, Replace the deprecated Activity.request(attributes:contentState:) call with a runtime-gated implementation: if `#available`(iOS 16.2, *) call Activity.request(attributes:content:pushType:) using an ActivityContent constructed via ActivityContent(state:staleDate:), otherwise fall back to the existing Activity.request(attributes:contentState:) path; apply the same pattern for activity.update(using:)—use activity.update(_:) with ActivityContent(state:staleDate:) when `#available`(iOS 16.2, *), else keep activity.update(using:) for older OS versions, ensuring you reference the existing Activity.request(attributes:contentState:) and activity.update(using:) sites when making the change.src/app/weather-alert/[id].tsx (2)
23-23: UnuseduseRouterimport/binding.
routeris declared at line 23 but never referenced in the component. Remove it (and theuseRouterimport) to keep the file clean.♻️ Suggested change
-import { Stack, useLocalSearchParams, useRouter } from 'expo-router'; +import { Stack, useLocalSearchParams } from 'expo-router';- const { id } = useLocalSearchParams(); - const alertId = Array.isArray(id) ? id[0] : id; - const router = useRouter(); - const { t } = useTranslation(); + const { id } = useLocalSearchParams(); + const alertId = Array.isArray(id) ? id[0] : id; + const { t } = useTranslation();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/weather-alert/`[id].tsx at line 23, Remove the unused router binding by deleting the useRouter import and the line that declares const router = useRouter(); (remove references to useRouter and the local identifier router in the component) so there are no unused imports/variables; search for useRouter and the const router = useRouter() occurrence in the component (e.g., in the [id].tsx file) and remove both the import and the declaration.
155-162: Brittle enum-to-translation mapping via hardcoded ordered arrays.
['immediate', 'expected', 'future', 'past', 'unknown'][alert.Urgency]and['observed', 'likely', 'possible', 'unlikely', 'unknown'][alert.Certainty]assume a specific zero-indexed enum order. If the backend ever reorders, inserts, or returns a non-numeric value, the wrong translation will silently render with no warning. Prefer aRecord<EnumValue, string>keyed by the actual enum constants (or a helper like the existinggetSeverityTranslationKey).♻️ Suggested shape
// in src/lib/weather-alert-utils.ts const URGENCY_KEYS: Record<WeatherAlertUrgency, string> = { [WeatherAlertUrgency.Immediate]: 'weather_alerts.urgency.immediate', [WeatherAlertUrgency.Expected]: 'weather_alerts.urgency.expected', [WeatherAlertUrgency.Future]: 'weather_alerts.urgency.future', [WeatherAlertUrgency.Past]: 'weather_alerts.urgency.past', [WeatherAlertUrgency.Unknown]: 'weather_alerts.urgency.unknown', }; export const getUrgencyTranslationKey = (u: number) => URGENCY_KEYS[u as WeatherAlertUrgency] ?? 'weather_alerts.urgency.unknown';Then in this screen:
- <Text className="text-sm font-medium text-gray-900 dark:text-gray-100">{t(`weather_alerts.urgency.${['immediate', 'expected', 'future', 'past', 'unknown'][alert.Urgency] ?? 'unknown'}`)}</Text> + <Text className="text-sm font-medium text-gray-900 dark:text-gray-100">{t(getUrgencyTranslationKey(alert.Urgency))}</Text>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/weather-alert/`[id].tsx around lines 155 - 162, Replace the brittle array-index mapping with explicit lookup helpers: add functions like getUrgencyTranslationKey(u: number) and getCertaintyTranslationKey(c: number) (similar to getSeverityTranslationKey) in your weather-alert-utils module that map WeatherAlertUrgency/WeatherAlertCertainty enum values to translation keys (returning 'weather_alerts.urgency.unknown' / 'weather_alerts.certainty.unknown' as default), then update the JSX in the [id] component to call t(getUrgencyTranslationKey(alert.Urgency)) and t(getCertaintyTranslationKey(alert.Certainty)) instead of indexing into hardcoded arrays.src/stores/signalr/signalr-store.ts (1)
73-103: Optional: deduplicate the three weather-alert handlers and the event-name list.The
updateEventsarray (73–84) and themethodslist (92–103) are identical — extract a single shared constant to avoid drift. Similarly, the threeweatherAlertReceived/weatherAlertUpdated/weatherAlertExpiredhandlers (133–161) are structurally identical aside from the dispatched store action; a small helper would tighten this and make adding future events trivial.♻️ Suggested shape
const UPDATE_HUB_EVENTS = [ 'personnelStatusUpdated', 'personnelStaffingUpdated', 'unitStatusUpdated', 'callsUpdated', 'callAdded', 'callClosed', 'weatherAlertReceived', 'weatherAlertUpdated', 'weatherAlertExpired', 'onConnected', ] as const; const wireWeatherAlertEvent = ( event: 'weatherAlertReceived' | 'weatherAlertUpdated' | 'weatherAlertExpired', dispatch: (alertId: string) => void, ) => { signalRService.on(event, (message) => { set({ lastUpdateMessage: JSON.stringify(message), lastUpdateTimestamp: Date.now() }); const alertId = extractAlertId(message); if (alertId) dispatch(alertId); else logger.warn({ message: `${event}: could not extract alertId from message`, context: { message } }); }); };Also applies to: 133-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/signalr/signalr-store.ts` around lines 73 - 103, The updateEvents array and the methods list are duplicated; extract them into a single exported constant (e.g., UPDATE_HUB_EVENTS) and use that constant in both the removeAllListeners loop and the signalRService.connectToHubWithEventingUrl({ methods }) call to avoid drift. Also replace the three near-identical weather alert handlers with a small helper (e.g., wireWeatherAlertEvent) that calls signalRService.on(event, ...) and performs the common set({ lastUpdateMessage, lastUpdateTimestamp }), extracts the alert id (extractAlertId) and conditionally dispatches the appropriate store action (or logs a warning) so the handlers for 'weatherAlertReceived' | 'weatherAlertUpdated' | 'weatherAlertExpired' simply call that helper.src/app/call/[id]/edit.tsx (1)
44-44: Optional: storedestinationPoiIdasnumber | nullin the form to drop the string round-trip.The selector contract is
number | null, the API contract isnumber | null, but the form schema/state stores a string and you convert in three places (String(call.DestinationPoiId)on prefill,Number(value)on render,Number(data.destinationPoiId)on submit, andpoiId.toString()in the selector callback). Switching the schema toz.number().nullable().optional()and usingnullas the default would remove all four conversions and avoid edge cases like a falsy'0'string.♻️ Suggested change
- destinationPoiId: z.string().optional(), + destinationPoiId: z.number().nullable().optional(),- destinationPoiId: '', + destinationPoiId: null,- destinationPoiId: call.DestinationPoiId != null ? String(call.DestinationPoiId) : '', + destinationPoiId: call.DestinationPoiId ?? null,- destinationPoiId: data.destinationPoiId ? Number(data.destinationPoiId) : null, + destinationPoiId: data.destinationPoiId ?? null,- <Controller - control={control} - name="destinationPoiId" - render={({ field: { onChange, value } }) => ( - <DestinationPoiSelector - destinationPois={destinationPois} - poiTypes={poiTypes} - selectedPoiId={value ? Number(value) : null} - isLoading={callDataLoading && destinationPois.length === 0} - onChange={(poiId) => onChange(poiId != null ? poiId.toString() : '')} - /> - )} - /> + <Controller + control={control} + name="destinationPoiId" + render={({ field: { onChange, value } }) => ( + <DestinationPoiSelector + destinationPois={destinationPois} + poiTypes={poiTypes} + selectedPoiId={value ?? null} + isLoading={callDataLoading && destinationPois.length === 0} + onChange={onChange} + /> + )} + />Also applies to: 134-134, 172-172, 235-235, 625-637
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/call/`[id]/edit.tsx at line 44, Change the form schema for destinationPoiId from a string to a numeric nullable type (use z.number().nullable().optional() with default null) so form state holds number|null instead of string; update the prefill to assign call.DestinationPoiId directly (no String(...)), update the select/render logic to use the numeric value (no Number(value) conversion), update submit to send data.destinationPoiId directly (no Number(...)), and update the selector callback so it passes poiId (number|null) instead of calling poiId.toString(); the identifiers to change are the destinationPoiId schema declaration, the prefill using String(call.DestinationPoiId), the render conversion Number(value), the submit conversion Number(data.destinationPoiId), and the selector callback that currently uses poiId.toString().src/services/check-in-notification.service.ts (1)
61-67:updateNotificationsilently delays for up to ~1s.State is mutated but no re-render is triggered; the next interval tick (up to 1000 ms later) is what eventually surfaces the new
secondsRemaining/statusto the user. If a server-driven update jumps the timer significantly (e.g., reset after a check-in), the notification will keep counting down stale seconds for almost a full second before snapping. Consider awaitingdisplayNotification(...)here so updates are reflected immediately, with the storedcallName/callNumber/timerNamefromstartNotification.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/check-in-notification.service.ts` around lines 61 - 67, updateNotification currently only mutates state and waits for the next interval tick to update the UI; fix it by calling and awaiting your displayNotification(...) immediately after updating this.currentSeconds/this.currentStatus (use the stored identifiers set in startNotification: this.callName, this.callNumber, this.timerName) so the notification re-renders instantly; ensure you pass merged labels (merge this.currentLabels with the incoming statusLabels) and await the displayNotification promise before returning to preserve method semantics.src/components/routes/poi-list-content.tsx (2)
34-39: ReusecreatePoiTypeMapfrom@/lib/poi-utils.The same lookup is already exported from
src/lib/poi-utils.ts(and is reused elsewhere — e.g.status-bottom-sheet.tsxuses it). Inlining the reduce here duplicates logic and types(typeof poiTypes)[number]instead of the precisePoiTypeResultData.♻️ Suggested refactor
-import { filterPois, getPoiDisplayName, getPoiTypeName, isPoiDestinationEnabled, type PoiSortOption, sortPois } from '@/lib/poi-utils'; +import { createPoiTypeMap, filterPois, getPoiDisplayName, getPoiTypeName, isPoiDestinationEnabled, type PoiSortOption, sortPois } from '@/lib/poi-utils'; @@ - const poiTypesById = useMemo(() => { - return poiTypes.reduce<Record<number, (typeof poiTypes)[number]>>((accumulator, poiType) => { - accumulator[poiType.PoiTypeId] = poiType; - return accumulator; - }, {}); - }, [poiTypes]); + const poiTypesById = useMemo(() => createPoiTypeMap(poiTypes), [poiTypes]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/poi-list-content.tsx` around lines 34 - 39, Replace the inline reduce that builds poiTypesById with the shared utility createPoiTypeMap from '@/lib/poi-utils': import createPoiTypeMap, remove the manual reduce in the poiTypesById useMemo, and call createPoiTypeMap(poiTypes) inside the useMemo so the map uses the precise PoiTypeResultData typing and avoids duplicated logic; ensure the variable name poiTypesById remains the same and update the useMemo dependency to [poiTypes].
108-126: OptimizeFlatListand extract a stablerenderItem.The inline
renderItemarrow allocates a new function on every render andonPressinside it creates another closure per row. This violates the project guideline to avoid anonymous functions inrenderItem/ event handlers, and the list is also missing the recommended FlatList performance props (removeClippedSubviews,maxToRenderPerBatch,windowSize).♻️ Suggested refactor
+ const handlePoiPress = React.useCallback((poiId: number) => { + router.push({ pathname: '/routes/poi/[id]', params: { id: String(poiId) } }); + }, []); + + const keyExtractor = React.useCallback((item: PoiResultData) => String(item.PoiId), []); + + const renderPoi = React.useCallback( + ({ item }: { item: PoiResultData }) => ( + <PoiCard + poi={item} + poiTypeLabel={getPoiTypeName(item, poiTypesById) || t('routes.poi_type_unknown')} + displayName={getPoiDisplayName(item, poiTypesById)} + isDestinationEnabled={isPoiDestinationEnabled(item, poiTypesById)} + onPress={handlePoiPress} + /> + ), + [handlePoiPress, poiTypesById, t] + ); + <FlatList<PoiResultData> testID="pois-list" data={visiblePois} - keyExtractor={(item) => String(item.PoiId)} + keyExtractor={keyExtractor} refreshControl={<RefreshControl refreshing={isLoading} onRefresh={handleRefresh} />} - renderItem={({ item }) => ( - <PoiCard - poi={item} - poiTypeLabel={getPoiTypeName(item, poiTypesById) || t('routes.poi_type_unknown')} - displayName={getPoiDisplayName(item, poiTypesById)} - isDestinationEnabled={isPoiDestinationEnabled(item, poiTypesById)} - onPress={() => router.push({ pathname: '/routes/poi/[id]', params: { id: item.PoiId } })} - /> - )} + renderItem={renderPoi} + removeClippedSubviews + maxToRenderPerBatch={10} + windowSize={10} ListEmptyComponent={ <ZeroState heading={t('routes.no_pois')} description={searchQuery || selectedPoiTypeId !== 'all' ? t('routes.no_pois_filtered_description') : t('routes.no_pois_description')} icon={RefreshCcwDotIcon} /> } contentContainerStyle={{ paddingBottom: 20 }} />Note:
PoiCardwill need to accept anonPress: (poiId: number) => voidsignature (or partial-apply viauseCallbackper row) to avoid recreating the closure.Also note the cast to
String(item.PoiId)for the route param — Expo Router params should be strings; passing a raw number relies on implicit serialization.As per coding guidelines: "Avoid anonymous functions in renderItem or event handlers" and "Optimize FlatList with removeClippedSubviews, maxToRenderPerBatch, windowSize".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/routes/poi-list-content.tsx` around lines 108 - 126, The FlatList currently creates anonymous functions per render via the inline renderItem and per-row onPress closures; extract a stable renderItem using useCallback (e.g. memoized function referenced by FlatList.renderItem) and change PoiCard to accept onPress as onPress(poiId: number) => void so you can pass a prebound handler (or a memoized per-id callback) to avoid recreating closures; also add the recommended FlatList performance props removeClippedSubviews, maxToRenderPerBatch and windowSize, and ensure router.push param uses a string id (e.g. id: String(item.PoiId)) when building the navigation call inside your memoized renderItem.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01f7a5d0-33d6-4cb5-a32c-7c6200987296
📒 Files selected for processing (55)
plugins/withCheckInLiveActivity.jssrc/api/dispatch/dispatch.tssrc/app/(app)/index.tsxsrc/app/(app)/protocols.tsxsrc/app/(app)/weather-alerts.tsxsrc/app/call/[id].tsxsrc/app/call/[id]/edit.tsxsrc/app/call/new/index.tsxsrc/app/onboarding.tsxsrc/app/routes/poi/[id].tsxsrc/app/weather-alert/[id].tsxsrc/components/calls/__tests__/call-files-modal.test.tsxsrc/components/calls/__tests__/call-images-modal.test.tsxsrc/components/calls/call-files-modal.tsxsrc/components/calls/call-images-modal.tsxsrc/components/calls/destination-poi-selector.tsxsrc/components/check-in-timers/check-in-bottom-sheet.tsxsrc/components/check-in-timers/check-in-timer-card.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/routes/active-routes-list.tsxsrc/components/routes/poi-list-content.tsxsrc/components/settings/bluetooth-device-selection-bottom-sheet.tsxsrc/components/status/status-bottom-sheet.tsxsrc/components/ui/flat-list/index.tsxsrc/components/weather-alerts/severity-filter-tabs.tsxsrc/components/weather-alerts/weather-alert-banner.tsxsrc/components/weather-alerts/weather-alert-card.tsxsrc/components/weather-alerts/weather-alert-detail-map.tsxsrc/hooks/__tests__/use-quick-check-in.test.tssrc/hooks/use-check-in-timer-polling.tssrc/hooks/use-quick-check-in.tssrc/lib/poi-utils.tssrc/lib/weather-alert-utils.tssrc/models/v4/dispatch/newCallFormResultData.tssrc/models/v4/mapping/getMapDataAndMarkersData.tssrc/models/v4/mapping/poiResults.tssrc/services/check-in-notification.service.tssrc/services/push-notification.tssrc/services/push-notification.web.tssrc/stores/calls/store.tssrc/stores/check-in-timers/__tests__/store.test.tssrc/stores/check-in-timers/store.tssrc/stores/pois/store.tssrc/stores/signalr/signalr-store.tssrc/stores/status/store.tssrc/stores/weather-alerts/store.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
💤 Files with no reviewable changes (2)
- src/components/notifications/NotificationInbox.tsx
- src/app/(app)/protocols.tsx
✅ Files skipped from review due to trivial changes (5)
- src/components/settings/bluetooth-device-selection-bottom-sheet.tsx
- src/components/ui/flat-list/index.tsx
- src/components/calls/tests/call-images-modal.test.tsx
- src/app/(app)/index.tsx
- src/stores/status/store.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- src/models/v4/dispatch/newCallFormResultData.ts
- src/components/routes/active-routes-list.tsx
- src/components/weather-alerts/weather-alert-banner.tsx
- src/components/check-in-timers/check-in-bottom-sheet.tsx
- src/models/v4/mapping/poiResults.ts
- src/components/weather-alerts/severity-filter-tabs.tsx
- src/components/weather-alerts/weather-alert-card.tsx
- src/stores/pois/store.ts
- src/components/calls/destination-poi-selector.tsx
- src/api/dispatch/dispatch.ts
- src/app/call/new/index.tsx
- src/hooks/use-check-in-timer-polling.ts
- src/app/routes/poi/[id].tsx
- src/components/weather-alerts/weather-alert-detail-map.tsx
- src/hooks/use-quick-check-in.ts
- src/app/call/[id].tsx
- src/hooks/tests/use-quick-check-in.test.ts
- src/stores/check-in-timers/tests/store.test.ts
- src/translations/it.json
- src/lib/poi-utils.ts
- src/translations/uk.json
- src/translations/de.json
| fs.writeFileSync(path.join(appDir, 'CheckInTimerActivityManager.swift'), ACTIVITY_MANAGER_SWIFT); | ||
| fs.writeFileSync(path.join(appDir, 'CheckInTimerActivityBridge.m'), BRIDGE_OBJC); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the main-app bridging header / Swift presence assumption holds for this repo
fd -e swift . ios 2>/dev/null | head -20
fd 'Bridging-Header.h' ios 2>/dev/null
rg -n 'SWIFT_OBJC_BRIDGING_HEADER' ios 2>/dev/null
# Also check whether any other Expo plugin in this repo already wires Swift files into the main target
fd -e js plugins 2>/dev/nullRepository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# First, check the actual plugin file content around the flagged lines
head -n 350 plugins/withCheckInLiveActivity.js | tail -n 50Repository: Resgrid/Unit
Length of output: 2183
🏁 Script executed:
# Check the current ios directory structure
ls -la ios/ 2>/dev/null || echo "No ios directory"Repository: Resgrid/Unit
Length of output: 73
🏁 Script executed:
# Look for any existing pbxproj in the project
find . -name "*.pbxproj" -type d 2>/dev/null | head -5Repository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Check if there's any existing bridging header
find ios -name "*Bridging*" -o -name "*bridging*" 2>/dev/nullRepository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Look at the full plugin structure to understand how withXcodeProject is called
wc -l plugins/withCheckInLiveActivity.js
grep -n "withXcodeProject\|addSourceFile\|addTarget" plugins/withCheckInLiveActivity.js | head -20Repository: Resgrid/Unit
Length of output: 806
🏁 Script executed:
# Read more of the withXcodeProject callback to see if bridge files are added to main app target
sed -n '349,448p' plugins/withCheckInLiveActivity.jsRepository: Resgrid/Unit
Length of output: 4663
🏁 Script executed:
# Also check if the plugin uses any patterns to add files to the main app target
rg -A 5 "addSourceFile.*CheckInTimer" plugins/withCheckInLiveActivity.jsRepository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Verify the xcode npm package behavior with addSourceFile
rg -n "getFirstTarget\|addTarget.*app_extension" plugins/withCheckInLiveActivity.jsRepository: Resgrid/Unit
Length of output: 38
🏁 Script executed:
# Check if there are any other native files being written to the main app dir
grep -n "writeFileSync.*appDir" plugins/withCheckInLiveActivity.jsRepository: Resgrid/Unit
Length of output: 256
🏁 Script executed:
# Check the xcode npm package documentation or examples in node_modules
find . -name "xcode" -type d 2>/dev/null | head -5Repository: Resgrid/Unit
Length of output: 153
🏁 Script executed:
# Look for any examples of addSourceFile being used in the codebase
rg "addSourceFile" --type js -B 2 -A 2Repository: Resgrid/Unit
Length of output: 1416
Bridge files written to disk are never added to the main app's Xcode target — NativeModules.CheckInTimerActivityManager will be undefined at runtime.
CheckInTimerActivityManager.swift and CheckInTimerActivityBridge.m are written into ios/<AppName>/ at lines 340–341, but the withXcodeProject callback (lines 349–448) only adds source files to the widget target. The main app target's PBXSourcesBuildPhase is never touched, so on prebuild Xcode will not compile these files, and the React Native bridge module won't be exposed.
Add these files to the main app target by retrieving the main target UUID and using addSourceFile() with the same pattern applied to widget files. For example, after line 375 (after creating the widget target):
🔧 Add bridge files to main app target
const mainTarget = project.getFirstTarget();
const mainGroupKey = project.findPBXGroupKey({ name: appName }) ??
project.findPBXGroupKey({ path: appName });
const bridgeFiles = ['CheckInTimerActivityManager.swift', 'CheckInTimerActivityBridge.m'];
for (const filename of bridgeFiles) {
const filePath = `${appName}/${filename}`;
if (!project.hasFile(filePath)) {
project.addSourceFile(filePath, { target: mainTarget.uuid }, mainGroupKey);
}
}Also verify that the main app target has a Swift-Objective‑C bridging header configured (SWIFT_OBJC_BRIDGING_HEADER build setting). Expo's React Native template usually includes one by default, but you should confirm it exists before testing this plugin.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/withCheckInLiveActivity.js` around lines 340 - 341, The plugin writes
CheckInTimerActivityManager.swift and CheckInTimerActivityBridge.m into the app
folder but only adds sources to the widget target inside the withXcodeProject
callback; update withCheckInLiveActivity.js to also add these bridge files to
the main app target by retrieving the main target (project.getFirstTarget()) and
main group key (project.findPBXGroupKey({ name: appName }) or by path), check
project.hasFile(filePath) and call project.addSourceFile(filePath, { target:
mainTarget.uuid }, mainGroupKey) for each bridge file (same pattern used for
widget files), and finally ensure the main app target has
SWIFT_OBJC_BRIDGING_HEADER configured before testing.
| if (buildConfig) { | ||
| Object.assign(buildConfig.buildSettings, { | ||
| // Override the default addTarget placeholder (TargetName-Info.plist) | ||
| INFOPLIST_FILE: `"${WIDGET_NAME}/Info.plist"`, | ||
| SWIFT_VERSION: '"5.0"', | ||
| TARGETED_DEVICE_FAMILY: '"1,2"', | ||
| // ActivityKit requires iOS 16.1 or later | ||
| IPHONEOS_DEPLOYMENT_TARGET: '16.1', | ||
| SKIP_INSTALL: 'YES', | ||
| CODE_SIGN_STYLE: 'Automatic', | ||
| MARKETING_VERSION: '"1.0"', | ||
| CURRENT_PROJECT_VERSION: '1', | ||
| }); |
There was a problem hiding this comment.
Hardcoded MARKETING_VERSION / CURRENT_PROJECT_VERSION will diverge from the app over time.
Pinning the widget to MARKETING_VERSION = "1.0" and CURRENT_PROJECT_VERSION = 1 means every TestFlight/App Store submission will fail validation as soon as the main app bumps its version, because Apple requires the embedded extension's CFBundleShortVersionString and CFBundleVersion to match the host app. Mirror them from the host app's build settings (or from config.version / config.ios?.buildNumber) instead of pinning.
♻️ Suggested fix
- MARKETING_VERSION: '"1.0"',
- CURRENT_PROJECT_VERSION: '1',
+ MARKETING_VERSION: `"${config.version ?? '1.0.0'}"`,
+ CURRENT_PROJECT_VERSION: `"${config.ios?.buildNumber ?? '1'}"`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/withCheckInLiveActivity.js` around lines 425 - 437, The widget build
settings currently hardcode MARKETING_VERSION and CURRENT_PROJECT_VERSION;
instead fetch the host app's versions and assign them: read the host target's
build settings (or fallback to config.version and config.ios?.buildNumber) and
use those values when setting buildConfig.buildSettings.MARKETING_VERSION and
CURRENT_PROJECT_VERSION (keep the existing keys MARKETING_VERSION and
CURRENT_PROJECT_VERSION so locate where Object.assign(...) is applied to
buildConfig.buildSettings). If the host values are missing, fall back to the
config values or keep the original values; ensure values are stringified the
same way as other settings (e.g., quoted for MARKETING_VERSION and
unquoted/number for CURRENT_PROJECT_VERSION).
| const filteredAlerts = alerts.filter((alert) => { | ||
| if (severityFilter !== null && alert.Severity !== severityFilter) return false; | ||
| if (searchQuery) { | ||
| const query = searchQuery.toLowerCase(); | ||
| return alert.Event.toLowerCase().includes(query) || alert.Headline.toLowerCase().includes(query) || alert.AreaDescription.toLowerCase().includes(query); | ||
| } | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Search filter can throw on alerts with missing string fields.
alert.Event.toLowerCase(), alert.Headline.toLowerCase(), and alert.AreaDescription.toLowerCase() will throw Cannot read properties of undefined (reading 'toLowerCase') if any of those fields is missing/null on the payload (real-world CAP/weather alerts often have empty Headline or AreaDescription). A single bad record will crash the whole list render.
🛡️ Proposed fix
const filteredAlerts = alerts.filter((alert) => {
if (severityFilter !== null && alert.Severity !== severityFilter) return false;
if (searchQuery) {
const query = searchQuery.toLowerCase();
- return alert.Event.toLowerCase().includes(query) || alert.Headline.toLowerCase().includes(query) || alert.AreaDescription.toLowerCase().includes(query);
+ return (
+ (alert.Event ?? '').toLowerCase().includes(query) ||
+ (alert.Headline ?? '').toLowerCase().includes(query) ||
+ (alert.AreaDescription ?? '').toLowerCase().includes(query)
+ );
}
return true;
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(app)/weather-alerts.tsx around lines 44 - 51, filteredAlerts's
search branch can throw when alert.Event / alert.Headline /
alert.AreaDescription are null/undefined; update the alerts.filter callback (the
function used to compute filteredAlerts) to defensively coerce each field to a
string before calling toLowerCase (e.g. use optional chaining/nullish coalescing
like (alert.Event ?? '').toLowerCase()) for Event, Headline and AreaDescription
so missing fields become empty strings and the searchQuery check never throws;
keep the existing severityFilter logic unchanged.
| <Text className="text-xs text-gray-500"> | ||
| {t('check_in.last_check_in')}: {minutesAgo} {t('check_in.minutes_ago')} | ||
| </Text> | ||
| <Text className="text-xs text-gray-500"> | ||
| {Math.floor(localElapsed)}/{timer.DurationMinutes} {t('check_in.duration')} | ||
| </Text> |
There was a problem hiding this comment.
minutes_ago is computed from localElapsed, not from "since last check-in".
The label reads {t('check_in.last_check_in')}: {minutesAgo} {t('check_in.minutes_ago')} but minutesAgo is Math.floor(localElapsed) — the same value used as "elapsed". If localElapsed represents "minutes since timer started" rather than "minutes since the user's last check-in", this label is misleading: it would always equal the elapsed counter, never reset on a successful check-in until the next poll re-syncs timer.ElapsedMinutes.
If the API exposes a separate "last check-in age" (or you can derive it from timer.LastCheckInUtc), use that; otherwise the label and the value on the right ({Math.floor(localElapsed)}/{timer.DurationMinutes}) are duplicates and one can be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/check-in-timers/check-in-timer-card.tsx` around lines 86 - 91,
The "last check-in" label currently shows minutesAgo which is computed from
localElapsed (Math.floor(localElapsed)), duplicating the elapsed counter; update
the UI to show the true minutes-since-last-check-in by deriving it from
timer.LastCheckInUtc or timer.ElapsedMinutes (whichever accurately represents
last check-in) instead of localElapsed, e.g., compute minutesSinceLastCheckIn =
differenceBetweenNowAnd(timer.LastCheckInUtc) or use timer.ElapsedMinutes if
that is the server-side last-check-in age, and replace minutesAgo with that
value in the Text that renders t('check_in.last_check_in'); alternatively, if no
separate last-check-in value exists, remove the redundant "last check-in" line
so you don't display duplicate elapsed information.
| // Try parsing as coordinate pairs "lat,lng lat,lng ..." | ||
| try { | ||
| const coords = polygonStr | ||
| .trim() | ||
| .split(/\s+/) | ||
| .map((pair) => { | ||
| const [lat, lng] = pair.split(',').map(Number); | ||
| return [lng, lat]; | ||
| }); | ||
|
|
||
| if (coords.length < 3) return null; | ||
|
|
||
| // Close the polygon if needed | ||
| const first = coords[0]; | ||
| const last = coords[coords.length - 1]; | ||
| if (first[0] !== last[0] || first[1] !== last[1]) { | ||
| coords.push([...first]); | ||
| } | ||
|
|
||
| return { | ||
| type: 'Feature', | ||
| properties: {}, | ||
| geometry: { type: 'Polygon', coordinates: [coords] }, | ||
| }; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Coordinate-pair fallback can build a polygon containing NaN coordinates.
If polygonStr doesn't parse as JSON and contains a malformed pair (e.g., "40.1,-74.0 not_a_pair 40.2,-74.1"), pair.split(',').map(Number) yields [NaN, NaN] for the bad token, which silently flows into the closed polygon as [NaN, NaN] coordinates. Downstream Mapbox/turf consumers will then either render incorrectly or throw. Validate each pair and bail out (or filter the offender) instead of returning a polygon with NaN vertices.
🛡️ Suggested fix
const coords = polygonStr
.trim()
.split(/\s+/)
.map((pair) => {
const [lat, lng] = pair.split(',').map(Number);
return [lng, lat];
});
- if (coords.length < 3) return null;
+ const validCoords = coords.filter(([lng, lat]) => Number.isFinite(lng) && Number.isFinite(lat));
+ if (validCoords.length < 3) return null;
// Close the polygon if needed
- const first = coords[0];
- const last = coords[coords.length - 1];
+ const first = validCoords[0];
+ const last = validCoords[validCoords.length - 1];
if (first[0] !== last[0] || first[1] !== last[1]) {
- coords.push([...first]);
+ validCoords.push([...first]);
}
return {
type: 'Feature',
properties: {},
- geometry: { type: 'Polygon', coordinates: [coords] },
+ geometry: { type: 'Polygon', coordinates: [validCoords] },
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/weather-alert-utils.ts` around lines 60 - 87, The coordinate-pair
fallback in the polygon parsing branch currently maps tokens with
pair.split(',').map(Number) and can produce [NaN, NaN] entries; update the logic
in the parsing block that processes polygonStr so each token (from
polygonStr.trim().split(/\s+/)) is validated after splitting into [lat,lng]:
ensure both lat and lng parse to finite numbers (Number.isFinite) and skip or
abort when encountering invalid pairs, only include valid [lng, lat] entries in
coords, then require at least 3 valid coordinates before closing the polygon and
returning the GeoJSON Feature (maintain the existing closing check using
first/last and the returned object shape).
| // Check-in timers channel (silent updates) | ||
| await notifee.createChannel({ | ||
| id: 'check-in-timers', | ||
| name: 'Check-In Timers', | ||
| description: 'Timer notifications for call check-ins', | ||
| importance: AndroidImportance.LOW, | ||
| vibration: false, | ||
| }); |
There was a problem hiding this comment.
Hardcoded English channel name/description; also duplicates check-in-notification.service.ts channel creation.
Two concerns here:
-
i18n violation.
name: 'Check-In Timers'anddescription: 'Timer notifications for call check-ins'are user-visible strings on Android (shown in the system app notification settings) but are hardcoded English. The repo guideline mandates wrapping all user-visible text viat(). Since Android caches a channel's name/description after first creation, fixing this later will require an upgrade path — better to pass translated labels in now. -
Duplicate channel creation.
src/services/check-in-notification.service.tsalready lazily creates the samecheck-in-timerschannel viaensureChannel(...)using translatedNotificationLabels. Creating it here first with English values means the service's translated values won't apply on a clean install (whichever creation wins), defeating the i18n design that was specifically introduced in that service.
Recommend deleting this block here and letting the polling hook drive channel creation through checkInNotificationService with translated t('check_in.notification_channel_name') / t('check_in.notification_channel_description') keys (which already exist in src/translations/en.json).
♻️ Suggested change
- // Check-in timers channel (silent updates)
- await notifee.createChannel({
- id: 'check-in-timers',
- name: 'Check-In Timers',
- description: 'Timer notifications for call check-ins',
- importance: AndroidImportance.LOW,
- vibration: false,
- });
-As per coding guidelines: "Wrap all user-facing text in t() from react-i18next for internationalization".
📝 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.
| // Check-in timers channel (silent updates) | |
| await notifee.createChannel({ | |
| id: 'check-in-timers', | |
| name: 'Check-In Timers', | |
| description: 'Timer notifications for call check-ins', | |
| importance: AndroidImportance.LOW, | |
| vibration: false, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/push-notification.ts` around lines 71 - 78, This block creates
an Android notification channel with hardcoded English strings and duplicates
the channel that checkInNotificationService already creates; delete the
notifee.createChannel(...) block in src/services/push-notification.ts (the code
creating id 'check-in-timers') so the app relies on the existing
ensureChannel(...) flow in src/services/check-in-notification.service.ts which
uses translated labels (NotificationLabels) via
t('check_in.notification_channel_name') and
t('check_in.notification_channel_description'); ensure no other code path
pre-creates 'check-in-timers' with hardcoded text so the translated values from
the checkInNotificationService/polling hook will be used on first install.
| /** | ||
| * Convert VAPID key from base64 to Uint8Array | ||
| */ | ||
| private urlBase64ToUint8Array(base64String: string): Uint8Array { | ||
| private urlBase64ToArrayBuffer(base64String: string): ArrayBuffer { |
There was a problem hiding this comment.
Stale JSDoc — update to reflect ArrayBuffer return type.
The doc comment still says "Convert VAPID key from base64 to Uint8Array", but the helper was renamed and now returns an ArrayBuffer.
📝 Proposed fix
/**
- * Convert VAPID key from base64 to Uint8Array
+ * Convert VAPID key from URL-safe base64 to an ArrayBuffer
+ * suitable for `PushManager.subscribe`'s `applicationServerKey`.
*/
private urlBase64ToArrayBuffer(base64String: string): ArrayBuffer {📝 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.
| /** | |
| * Convert VAPID key from base64 to Uint8Array | |
| */ | |
| private urlBase64ToUint8Array(base64String: string): Uint8Array { | |
| private urlBase64ToArrayBuffer(base64String: string): ArrayBuffer { | |
| /** | |
| * Convert VAPID key from URL-safe base64 to an ArrayBuffer | |
| * suitable for `PushManager.subscribe`'s `applicationServerKey`. | |
| */ | |
| private urlBase64ToArrayBuffer(base64String: string): ArrayBuffer { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/push-notification.web.ts` around lines 298 - 301, Update the
stale JSDoc for the helper urlBase64ToArrayBuffer to accurately describe its
behavior and return type (ArrayBuffer) instead of "Uint8Array"; edit the comment
text above the method to say something like "Convert VAPID key from base64 to
ArrayBuffer" and ensure the `@returns` tag (if present) states ArrayBuffer so the
documentation matches the method signature and intent.
| const result = await getTimerStatuses(callId); | ||
| const sorted = [...result.Data].sort((a, b) => (STATUS_SEVERITY[a.Status] ?? 3) - (STATUS_SEVERITY[b.Status] ?? 3)); | ||
| set({ timerStatuses: sorted, isLoadingStatuses: false }); | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : 'Failed to fetch timer statuses'; | ||
| logger.error({ message: 'Failed to fetch timer statuses', context: { error, callId } }); | ||
| set({ statusError: message, isLoadingStatuses: false }); | ||
| } | ||
| }, | ||
|
|
||
| fetchResolvedTimers: async (callId: number) => { | ||
| try { | ||
| const result = await getTimersForCall(callId); | ||
| set({ resolvedTimers: result.Data }); |
There was a problem hiding this comment.
Guard against missing/non-array Data payloads in fetch responses.
[...result.Data].sort(...) on line 58 and set({ resolvedTimers: result.Data }) / set({ checkInHistory: result.Data }) on lines 70 and 80 will throw / pollute the store with undefined if the API returns an envelope where Data is null or absent (e.g., on a 200 with empty body). Other call-store actions in this repo guard with Array.isArray(...). Mirror that pattern here for consistency and resilience.
🛡️ Suggested fix
- const result = await getTimerStatuses(callId);
- const sorted = [...result.Data].sort((a, b) => (STATUS_SEVERITY[a.Status] ?? 3) - (STATUS_SEVERITY[b.Status] ?? 3));
- set({ timerStatuses: sorted, isLoadingStatuses: false });
+ const result = await getTimerStatuses(callId);
+ const data = Array.isArray(result.Data) ? result.Data : [];
+ const sorted = [...data].sort((a, b) => (STATUS_SEVERITY[a.Status] ?? 3) - (STATUS_SEVERITY[b.Status] ?? 3));
+ set({ timerStatuses: sorted, isLoadingStatuses: false });(and analogous Array.isArray(result.Data) ? result.Data : [] for fetchResolvedTimers and fetchCheckInHistory.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/check-in-timers/store.ts` around lines 57 - 70, fetch functions
assume result.Data is an array and will throw or set undefined when the API
returns null/absent Data; update fetchTimerStatuses, fetchResolvedTimers, and
fetchCheckInHistory to guard result.Data with Array.isArray(...) and default to
[] before using it (e.g., use an array fallback when spreading/sorting and when
calling set for resolvedTimers/checkInHistory), so calls to
getTimerStatuses/getTimersForCall no longer crash and the store never receives
undefined; keep existing symbols STATUS_SEVERITY, getTimerStatuses,
getTimersForCall, fetchTimerStatuses, fetchResolvedTimers, fetchCheckInHistory,
and set while applying the Array.isArray(result.Data) ? result.Data : []
pattern.
| const settingsResponse = await getWeatherAlertSettings(); | ||
| const settings = settingsResponse.Data; | ||
| set({ settings }); | ||
|
|
||
| if (settings.WeatherAlertsEnabled) { | ||
| const alertsResponse = await getActiveAlerts(); | ||
| const alerts = Array.isArray(alertsResponse.Data) ? sortAlertsBySeverity(alertsResponse.Data) : []; | ||
| set({ alerts }); | ||
| } |
There was a problem hiding this comment.
Guard against missing settings payload in init.
settings is derived from settingsResponse.Data and used immediately as settings.WeatherAlertsEnabled. If the API returns Data: null / undefined (or a network error ahead of it that yields a malformed envelope), this throws and falls into the generic catch as "Failed to initialize weather alerts" — masking the real cause and surfacing an init error to users even when the feature is simply unavailable.
🛡️ Suggested fix
const settingsResponse = await getWeatherAlertSettings();
- const settings = settingsResponse.Data;
- set({ settings });
+ const settings = settingsResponse.Data ?? null;
+ set({ settings });
- if (settings.WeatherAlertsEnabled) {
+ if (settings?.WeatherAlertsEnabled) {
const alertsResponse = await getActiveAlerts();📝 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.
| const settingsResponse = await getWeatherAlertSettings(); | |
| const settings = settingsResponse.Data; | |
| set({ settings }); | |
| if (settings.WeatherAlertsEnabled) { | |
| const alertsResponse = await getActiveAlerts(); | |
| const alerts = Array.isArray(alertsResponse.Data) ? sortAlertsBySeverity(alertsResponse.Data) : []; | |
| set({ alerts }); | |
| } | |
| const settingsResponse = await getWeatherAlertSettings(); | |
| const settings = settingsResponse.Data ?? null; | |
| set({ settings }); | |
| if (settings?.WeatherAlertsEnabled) { | |
| const alertsResponse = await getActiveAlerts(); | |
| const alerts = Array.isArray(alertsResponse.Data) ? sortAlertsBySeverity(alertsResponse.Data) : []; | |
| set({ alerts }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/weather-alerts/store.ts` around lines 48 - 56, The init flow uses
settingsResponse.Data directly (from getWeatherAlertSettings) and then reads
settings.WeatherAlertsEnabled which will throw if Data is null/undefined; update
the init logic around getWeatherAlertSettings/getActiveAlerts to defensively
handle a missing or malformed settings payload by: validate settingsResponse &&
settingsResponse.Data before assigning to settings (or default to an explicit
empty settings object), only check WeatherAlertsEnabled on a defined settings
object, and if missing, set settings to a safe default and skip fetching alerts
(or log a specific warning) instead of letting a TypeError bubble into the
generic "Failed to initialize weather alerts" catch; reference
getWeatherAlertSettings, settingsResponse.Data, settings.WeatherAlertsEnabled,
getActiveAlerts, and sortAlertsBySeverity to locate the code to change.
| handleAlertReceived: async (alertId: string) => { | ||
| try { | ||
| const response = await getWeatherAlert(alertId); | ||
| const newAlert = response.Data; | ||
| set((state) => { | ||
| const exists = state.alerts.some((a) => a.WeatherAlertId === newAlert.WeatherAlertId); | ||
| const updated = exists ? state.alerts.map((a) => (a.WeatherAlertId === newAlert.WeatherAlertId ? newAlert : a)) : [newAlert, ...state.alerts]; | ||
| return { alerts: sortAlertsBySeverity(updated) }; | ||
| }); | ||
| } catch (error) { | ||
| // Silently fail for SignalR handler | ||
| } | ||
| }, | ||
| handleAlertUpdated: async (alertId: string) => { | ||
| try { | ||
| const response = await getWeatherAlert(alertId); | ||
| const updatedAlert = response.Data; | ||
| set((state) => ({ | ||
| alerts: sortAlertsBySeverity(state.alerts.map((a) => (a.WeatherAlertId === alertId ? updatedAlert : a))), | ||
| })); | ||
| } catch (error) { | ||
| // Silently fail for SignalR handler | ||
| } | ||
| }, |
There was a problem hiding this comment.
Null-check response.Data in SignalR handlers before mutating alerts.
handleAlertReceived reads newAlert.WeatherAlertId without checking that response.Data exists — if the server returns a missing/expired alert, this NPEs and the error is silently swallowed (no state update, no log). handleAlertUpdated is worse: it replaces the matching alert with updatedAlert directly, so a null/undefined payload corrupts the list and the UI keys/severity sort.
🛡️ Suggested fix
handleAlertReceived: async (alertId: string) => {
try {
const response = await getWeatherAlert(alertId);
- const newAlert = response.Data;
+ const newAlert = response.Data;
+ if (!newAlert) {
+ return;
+ }
set((state) => {
const exists = state.alerts.some((a) => a.WeatherAlertId === newAlert.WeatherAlertId);
const updated = exists ? state.alerts.map((a) => (a.WeatherAlertId === newAlert.WeatherAlertId ? newAlert : a)) : [newAlert, ...state.alerts];
return { alerts: sortAlertsBySeverity(updated) };
});
} catch (error) {
// Silently fail for SignalR handler
}
},
handleAlertUpdated: async (alertId: string) => {
try {
const response = await getWeatherAlert(alertId);
- const updatedAlert = response.Data;
+ const updatedAlert = response.Data;
+ if (!updatedAlert) {
+ return;
+ }
set((state) => ({
alerts: sortAlertsBySeverity(state.alerts.map((a) => (a.WeatherAlertId === alertId ? updatedAlert : a))),
}));
} catch (error) {
// Silently fail for SignalR handler
}
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/weather-alerts/store.ts` around lines 96 - 119, Both SignalR
handlers (handleAlertReceived and handleAlertUpdated) assume response.Data is
present; null/undefined will cause a crash or corrupt the alerts array. Update
both handlers to check that response.Data (newAlert/updatedAlert) is non-null
before mutating state: in handleAlertReceived only add/replace when newAlert
exists (use WeatherAlertId from newAlert after the check) and in
handleAlertUpdated only map/replace when updatedAlert exists (otherwise leave
the existing alert unchanged); optionally log a warning when response.Data is
missing. Use the existing getWeatherAlert(), sortAlertsBySeverity(), alerts
array and WeatherAlertId identifiers when implementing these guards.
2 participants
Summary by CodeRabbit