Conversation
This comment has been minimized.
This comment has been minimized.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe pull request expands incident command workflows with new APIs, models, board editing, reopening, tactical maps, notes, attachments, needs, objectives, incident history, offline queue management, weather error handling, file-name sanitization, UI shell updates, tests, and translations. ChangesIncident command workflows
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)src/translations/ar.jsonTraceback (most recent call last): src/translations/de.jsonTraceback (most recent call last): src/translations/es.jsonTraceback (most recent call last):
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| const response = await getWeatherAlertApi.get<WeatherAlertResult>({ | ||
| alertId: encodeURIComponent(alertId), | ||
| }); | ||
| const response = await getWeatherAlertEndpoint(alertId).get<WeatherAlertResult>(); |
There was a problem hiding this comment.
Unhandled promise rejection risk identified on line 23, as the awaited async call lacks a try/catch block. Enclose the call in a try/catch, log the error with context such as alertId, and map low-level rejections to an application-level error to satisfy Rule [1].
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/api/weather-alerts/weather-alerts.ts:
Line 23:
Unhandled promise rejection risk identified on line 23, as the awaited async call lacks a try/catch block. Enclose the call in a try/catch, log the error with context such as alertId, and map low-level rejections to an application-level error to satisfy Rule [1].
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const response = await getWeatherAlertApi.get<WeatherAlertResult>({ | ||
| alertId: encodeURIComponent(alertId), | ||
| }); | ||
| const response = await getWeatherAlertEndpoint(alertId).get<WeatherAlertResult>(); |
There was a problem hiding this comment.
Unhandled network exception on line 23 violates Rule [30] by omitting a try/catch for the external GET request. Enclose the call in a try/catch, include the alertId and endpoint in the error context, and map low-level errors to a domain-appropriate error type.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/api/weather-alerts/weather-alerts.ts:
Line 23:
Unhandled network exception on line 23 violates Rule [30] by omitting a try/catch for the external GET request. Enclose the call in a try/catch, include the alertId and endpoint in the error context, and map low-level errors to a domain-appropriate error type.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
This comment has been minimized.
This comment has been minimized.
| }; | ||
|
|
||
| /** Most recent command for a call across ALL statuses — lets the app detect a prior ended command and offer reopen. */ | ||
| export const getCommandForCall = async (callId: string | number) => { |
There was a problem hiding this comment.
Missing @returns {Promise<Type>} JSDoc on the async function prevents callers from knowing the resolve type and rejection conditions. Add @returns {Promise<IncidentCommandResult>} to the JSDoc comment.
Kody rule violation: Document async/Promise behavior and errors
Prompt for LLM
File src/api/incidentCommand/incidentCommand.ts:
Line 70:
Missing `@returns {Promise<Type>}` JSDoc on the async function prevents callers from knowing the resolve type and rejection conditions. Add `@returns {Promise<IncidentCommandResult>}` to the JSDoc comment.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| {/* Prior ended command found — reopen it (with a reason) or start fresh */} | ||
| <ReopenCommandSheet | ||
| isOpen={reopenPrompt !== null} | ||
| onClose={() => setReopenPrompt(null)} |
There was a problem hiding this comment.
Inline arrow functions or .bind() calls inside JSX props, such as onClose={() => setReopenPrompt(null)}, create new function instances on every render, impacting component performance. Move function definitions outside the render method to prevent unnecessary re-renders.
Kody rule violation: Avoid using .bind() or arrow functions in JSX props
Prompt for LLM
File src/app/(app)/calls.tsx:
Line 202:
Inline arrow functions or `.bind()` calls inside JSX props, such as `onClose={() => setReopenPrompt(null)}`, create new function instances on every render, impacting component performance. Move function definitions outside the render method to prevent unnecessary re-renders.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const incidentCommandId = priorCommandPrompt.IncidentCommandId; | ||
| setPriorCommandPrompt(null); | ||
| const ok = await useCommandStore.getState().reopenCommandForCall(call.CallId, incidentCommandId, reason || null); | ||
| showToast(ok ? 'success' : 'error', ok ? t('command.reopen_success') : t('command.reopen_error')); |
There was a problem hiding this comment.
Inline string literals 'success' and 'error' act as toast type discriminators without a centralized source of truth, increasing typo risks. Define a ToastType enum or constants to represent the finite set of severity types.
Kody rule violation: Use enums instead of magic strings
Prompt for LLM
File src/app/call/[id].tsx:
Line 164:
Inline string literals `'success'` and `'error'` act as toast type discriminators without a centralized source of truth, increasing typo risks. Define a `ToastType` enum or constants to represent the finite set of severity types.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| const command = board.Command; | ||
| const isEnded = command.Status !== 0; |
There was a problem hiding this comment.
Null pointer dereference occurs when accessing .Status on command derived from board.Command without a null check, causing a runtime TypeError if the field is undefined. Add an early return guard or use optional chaining like command?.Status to prevent the crash.
Kody rule violation: Add null checks before accessing properties
Prompt for LLM
File src/app/incident/[id].tsx:
Line 93:
Null pointer dereference occurs when accessing `.Status` on `command` derived from `board.Command` without a null check, causing a runtime `TypeError` if the field is undefined. Add an early return guard or use optional chaining like `command?.Status` to prevent the crash.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } | ||
|
|
||
| const command = board.Command; | ||
| const isEnded = command.Status !== 0; |
There was a problem hiding this comment.
Magic number 0 compares against a status field without self-documenting its domain concept, despite the codebase using enums like IncidentNeedStatus.Met. Define or use an existing enum like CommandStatus.Active to clarify the status check.
Kody rule violation: Replace magic numbers with named constants
Prompt for LLM
File src/app/incident/[id].tsx:
Line 93:
Magic number `0` compares against a status field without self-documenting its domain concept, despite the codebase using enums like `IncidentNeedStatus.Met`. Define or use an existing enum like `CommandStatus.Active` to clarify the status check.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| * Sync queue inspector: every queued offline write with its status, attempts, and error, plus | ||
| * retry / cancel per event and sync-now / retry-failed / clear actions for the whole queue. | ||
| */ | ||
| export default function OfflineQueue() { |
There was a problem hiding this comment.
Default exports on OfflineQueue reduce refactor safety and grepability, violating Rule 77. Switch to a named export like export function OfflineQueue(), unless the framework strictly requires a default export for route entries.
Kody rule violation: Avoid default exports
Prompt for LLM
File src/app/settings/offline-queue.tsx:
Line 42:
Default exports on `OfflineQueue` reduce refactor safety and grepability, violating Rule 77. Switch to a named export like `export function OfflineQueue()`, unless the framework strictly requires a default export for route entries.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const events = useMemo(() => [...queuedEvents].sort((a, b) => b.createdAt - a.createdAt), [queuedEvents]); | ||
| const pendingCount = useMemo(() => queuedEvents.filter((event) => event.status === QueuedEventStatus.PENDING || event.status === QueuedEventStatus.PROCESSING).length, [queuedEvents]); | ||
| const failedCount = useMemo(() => queuedEvents.filter((event) => event.status === QueuedEventStatus.FAILED).length, [queuedEvents]); | ||
| const completedCount = useMemo(() => queuedEvents.filter((event) => event.status === QueuedEventStatus.COMPLETED).length, [queuedEvents]); |
There was a problem hiding this comment.
Performance overhead arises as lines 61–63 independently traverse queuedEvents with separate useMemo hooks to compute completedCount and related counts. Consolidate these into a single useMemo using a reduce or forEach to tally all counts in one pass, satisfying Rule 16.
Kody rule violation: Use computed/derived properties for repeated calculations
Prompt for LLM
File src/app/settings/offline-queue.tsx:
Line 63:
Performance overhead arises as lines 61–63 independently traverse `queuedEvents` with separate `useMemo` hooks to compute `completedCount` and related counts. Consolidate these into a single `useMemo` using a `reduce` or `forEach` to tally all counts in one pass, satisfying Rule 16.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| setEstimatedEndOn(new Date(Date.now() + hours * 60 * 60 * 1000).toISOString()); | ||
| }, []); | ||
|
|
||
| const setterFor = useCallback((slot: LocationSlot) => (slot === 'commandPost' ? setCommandPost : slot === 'staging' ? setStaging : setRehab), []); |
There was a problem hiding this comment.
Shared raw string literals like 'commandPost' and 'staging' in equality comparisons violate Rule 7 by lacking a single source of truth. Centralize these identifiers using a const object or enum like LOCATION_SLOT.
Kody rule violation: Centralize string constants
Prompt for LLM
File src/components/command/command-details-sheet.tsx:
Line 148:
Shared raw string literals like `'commandPost'` and `'staging'` in equality comparisons violate Rule 7 by lacking a single source of truth. Centralize these identifiers using a `const` object or enum like `LOCATION_SLOT`.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| const fileUri = `${documentDirectory}${attachment.FileName}`; | ||
| await writeAsStringAsync(fileUri, base64, { encoding: EncodingType.Base64 }); |
There was a problem hiding this comment.
Unvalidated path sink in attachment.FileName allows server-stored values containing slashes or .. segments to write outside the intended documentDirectory before writeAsStringAsync. Sanitize the input to a basename and explicitly prefix the directory to prevent path traversal.
const safeName = attachment.FileName.replace(/^.*[\\/]/, '').replace(/\.\./g, '');
const fileUri = `${documentDirectory}${safeName}`;
await writeAsStringAsync(fileUri, base64, { encoding: EncodingType.Base64 });Prompt for LLM
File src/components/command/incident-files-section.tsx:
Line 72 to 73:
Unvalidated path sink in `attachment.FileName` allows server-stored values containing slashes or `..` segments to write outside the intended `documentDirectory` before `writeAsStringAsync`. Sanitize the input to a basename and explicitly prefix the directory to prevent path traversal.
Suggested Code:
const safeName = attachment.FileName.replace(/^.*[\\/]/, '').replace(/\.\./g, '');
const fileUri = `${documentDirectory}${safeName}`;
await writeAsStringAsync(fileUri, base64, { encoding: EncodingType.Base64 });
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Exception swallowing in parseAnnotationFeature silently catches JSON parse errors, malformed GeoJSON, and type errors while returning null, violating Rule [31] and making production issues impossible to diagnose. Log the error with structured context before returning null to expose corrupt annotation data failures.
Kody rule violation: Avoid empty catch blocks
Prompt for LLM
File src/components/command/incident-map-layers.tsx:
Line 21 to 23:
Exception swallowing in `parseAnnotationFeature` silently catches JSON parse errors, malformed GeoJSON, and type errors while returning `null`, violating Rule [31] and making production issues impossible to diagnose. Log the error with structured context before returning `null` to expose corrupt annotation data failures.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| return t('command.incident_maps_updated_by', { name: resolveUserName?.(map.UpdatedByUserId) ?? map.UpdatedByUserId, when: new Date(map.UpdatedOn).toLocaleString() }); | ||
| } | ||
| if (map.CreatedByUserId) { | ||
| return t('command.incident_maps_created_by', { name: resolveUserName?.(map.CreatedByUserId) ?? map.CreatedByUserId, when: new Date(map.CreatedOn).toLocaleString() }); |
There was a problem hiding this comment.
Logic error causes new Date(map.CreatedOn) to evaluate to epoch (1970-01-01) when CreatedOn is null, because the if statement only guards map.CreatedByUserId. Extend the condition to check map.CreatedOn or coerce the date value safely.
Kody rule violation: Add null checks to prevent NullReferenceException
Prompt for LLM
File src/components/command/incident-maps-section.tsx:
Line 64:
Logic error causes `new Date(map.CreatedOn)` to evaluate to epoch (1970-01-01) when `CreatedOn` is null, because the `if` statement only guards `map.CreatedByUserId`. Extend the condition to check `map.CreatedOn` or coerce the date value safely.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| fetchUpdates(needId) | ||
| .then((rows) => { | ||
| if (!cancelled) { | ||
| setUpdates(rows); | ||
| } | ||
| }) | ||
| .finally(() => { | ||
| if (!cancelled) { | ||
| setIsLoadingUpdates(false); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Unhandled promise rejection in fetchUpdates(needId) leaves the UI in an inconsistent state where the loading spinner never clears on error, violating Rule [1] requiring error guards for async operations. Append a .catch handler to log errors, reset the loading state, and prevent potential app crashes.
Kody rule violation: Handle async operations with proper error handling
Prompt for LLM
File src/components/command/need-details-sheet.tsx:
Line 66 to 76:
Unhandled promise rejection in `fetchUpdates(needId)` leaves the UI in an inconsistent state where the loading spinner never clears on error, violating Rule [1] requiring error guards for async operations. Append a `.catch` handler to log errors, reset the loading state, and prevent potential app crashes.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| fetchUpdates(needId) | ||
| .then((rows) => { | ||
| if (!cancelled) { | ||
| setUpdates(rows); | ||
| } | ||
| }) | ||
| .finally(() => { | ||
| if (!cancelled) { | ||
| setIsLoadingUpdates(false); | ||
| } | ||
| }); |
There was a problem hiding this comment.
External network call fetchUpdates propagates failures silently without user feedback, violating Rule [29] which requires mapping to application-level errors. Wrap the call in a try/catch block or append a .catch handler to log the error context and surface the failure gracefully.
Kody rule violation: Add try-catch blocks for external calls
Prompt for LLM
File src/components/command/need-details-sheet.tsx:
Line 66 to 76:
External network call `fetchUpdates` propagates failures silently without user feedback, violating Rule [29] which requires mapping to application-level errors. Wrap the call in a `try/catch` block or append a `.catch` handler to log the error context and surface the failure gracefully.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| try { | ||
| const result = await getCommandBoardById(incidentCommandId); | ||
| // Same wire payload as GetCommandBoard — the two local IncidentCommandBoard interfaces only differ in optionality. | ||
| const board = (result?.Data ?? null) as unknown as IncidentCommandBoard | null; |
There was a problem hiding this comment.
Unsafe type casting using as unknown as on the API payload bypasses TypeScript's type checker and suppresses all validation, violating Rule 6. Unify the IncidentCommandBoard interfaces or validate the payload shape with a runtime type guard to handle divergent wire structures safely.
Kody rule violation: Use safe type casting with as operator
Prompt for LLM
File src/stores/command/board-store.ts:
Line 58:
Unsafe type casting using `as unknown as` on the API payload bypasses TypeScript's type checker and suppresses all validation, violating Rule 6. Unify the `IncidentCommandBoard` interfaces or validate the payload shape with a runtime type guard to handle divergent wire structures safely.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| let timeline: CommandLogEntry[] = []; | ||
| let attachments: IncidentAttachment[] = []; | ||
| try { | ||
| const [timelineResult, attachmentsResult] = await Promise.all([getCommandTimeline(callId), getIncidentAttachments(callId)]); |
There was a problem hiding this comment.
Best-effort fetches using Promise.all discard successful responses when any independent request fails, defeating the intent of partial failure tolerance. Replace Promise.all with Promise.allSettled and inspect each result status individually to handle independent failures gracefully, as recommended by Rule 137.
Kody rule violation: Use Promise.allSettled for batch operations with partial failures
Prompt for LLM
File src/stores/command/board-store.ts:
Line 64:
Best-effort fetches using `Promise.all` discard successful responses when any independent request fails, defeating the intent of partial failure tolerance. Replace `Promise.all` with `Promise.allSettled` and inspect each result status individually to handle independent failures gracefully, as recommended by Rule 137.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| * board; once the queued event completes, the refreshed server board contains the real row and the | ||
| * matching queue entry is gone, so the local row stops being carried. | ||
| */ | ||
| const preserveQueuedLocalRows = (callId: string, serverBoard: IncidentCommandBoard | null, previousBoard: IncidentCommandBoard | null | undefined): IncidentCommandBoard | null => { |
There was a problem hiding this comment.
Race condition identified in preserveQueuedLocalRows causes duplicate objectives, needs, nodes, and resources after reconnecting from offline, because refreshCommandBoard executes after the API call succeeds but before the event is marked COMPLETED in offline-event-manager.service.ts:595. Mark the queued event COMPLETED or dedupe carried local- rows against existing server rows before the post-processing board refresh to resolve the duplication.
// In offline-event-manager.service.ts, mark the event complete BEFORE the post-save refresh so the
// local row is no longer carried once the server already has the real row:
// await saveObjective(...);
// store.updateEventStatus(event.id, QueuedEventStatus.COMPLETED); // moved before refresh
// await this.refreshCommandBoard(event.data.callId);
// (the API call already succeeded, so refresh failure is purely cosmetic and self-heals on next sync)Prompt for LLM
File src/stores/command/store.ts:
Line 249:
Race condition identified in `preserveQueuedLocalRows` causes duplicate objectives, needs, nodes, and resources after reconnecting from offline, because `refreshCommandBoard` executes after the API call succeeds but before the event is marked `COMPLETED` in `offline-event-manager.service.ts:595`. Mark the queued event `COMPLETED` or dedupe carried `local-` rows against existing server rows before the post-processing board refresh to resolve the duplication.
Suggested Code:
// In offline-event-manager.service.ts, mark the event complete BEFORE the post-save refresh so the
// local row is no longer carried once the server already has the real row:
// await saveObjective(...);
// store.updateEventStatus(event.id, QueuedEventStatus.COMPLETED); // moved before refresh
// await this.refreshCommandBoard(event.data.callId);
// (the API call already succeeded, so refresh failure is purely cosmetic and self-heals on next sync)
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/weather-alerts/store.ts (1)
75-80: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve an explicit detail error state.
After clearing
selectedAlert, a failed request leaves itnulland only setsisLoadingDetailtofalse.src/app/weather-alert/[id].tsxthen renders the “no alerts” empty state for network/server failures. Add a detail error with retry feedback instead of treating failures as missing alerts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/weather-alerts/store.ts` around lines 75 - 80, Update the detail-loading action around getWeatherAlert to preserve an explicit error state when the request fails: clear any prior detail error before loading, then set a descriptive detail error in the catch path alongside isLoadingDetail: false. Update the [id] detail screen’s error handling to render retry feedback for that error instead of the “no alerts” empty state, while keeping successful response rendering unchanged.
🧹 Nitpick comments (11)
src/stores/command/__tests__/incidents-store.test.ts (2)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured store alias.
Replace
../incidents-storewith@/stores/command/incidents-store. As per coding guidelines,srcimports must use configured path aliases instead of relative imports when applicable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/command/__tests__/incidents-store.test.ts` at line 4, Update the import of useIncidentsStore in the incidents-store test to use the configured "`@/stores/command/incidents-store`" alias instead of the relative "../incidents-store" path.Source: Coding guidelines
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
import typeconsistently for type-only imports.Both changed files use inline
typespecifiers inside value imports, contrary to the repository import convention.
src/stores/command/__tests__/incidents-store.test.ts#L2-L2: replace the model import with a separateimport typedeclaration.src/components/command/lane-details-sheet.tsx#L12-L12: split model types from the value enum imports and useimport type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/stores/command/__tests__/incidents-store.test.ts` at line 2, Use standalone import type declarations for type-only model imports in src/stores/command/__tests__/incidents-store.test.ts at line 2 and src/components/command/lane-details-sheet.tsx at line 12; split the model types from value enum imports in lane-details-sheet.tsx while preserving the existing runtime imports.Source: Coding guidelines
src/components/command/lane-details-sheet.tsx (1)
83-86: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the linked-map edit flow.
Please extend the lane-details tests to cover initial
LinkedMapIdseeding, selecting a map, clearing it with “none,” and verifyingLinkedMapIdin theonSavepatch. As per coding guidelines, generate tests for new logic.Also applies to: 211-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/command/lane-details-sheet.tsx` around lines 83 - 86, Extend the lane-details test suite for the save flow around the component’s LinkedMapId handling: verify initial LinkedMapId seeding, selecting a map, clearing it with “none,” and that the onSave patch contains the expected LinkedMapId value. Cover both setting and clearing the linked map while preserving existing test behavior.Source: Coding guidelines
src/components/command/__tests__/needs-section.test.tsx (1)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the command component renders with
TestWrapper. These test suites callrenderdirectly without the shared test provider setup required by the coding guidelines.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/command/__tests__/needs-section.test.tsx` at line 45, Wrap the command component renders with the shared TestWrapper provider setup instead of rendering directly. Update the render calls in src/components/command/__tests__/needs-section.test.tsx:45, src/components/command/__tests__/notes-section.test.tsx:46, src/components/command/__tests__/objective-details-sheet.test.tsx:48, and src/components/command/__tests__/reopen-command-sheet.test.tsx:29, preserving each test’s existing component props and assertions.Source: Coding guidelines
src/components/command/__tests__/notes-section.test.tsx (1)
8-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove
anyfrom command test mocks.Type the mocked React component props in the affected tests so the TypeScript strict guideline is not violated:
src/components/command/__tests__/notes-section.test.tsx: mockGlobe/Lockprops andCustomBottomSheetprops.src/components/command/__tests__/objective-details-sheet.test.tsx: mockCustomBottomSheetprops plusAlertDialog,passthrough, and alert-dialog component props.src/components/command/__tests__/reopen-command-sheet.test.tsx: mockCustomBottomSheetprops.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/command/__tests__/notes-section.test.tsx` around lines 8 - 18, Remove all any types from the command test mocks. In src/components/command/__tests__/notes-section.test.tsx lines 8-18, type Globe/Lock and CustomBottomSheet props; apply the corresponding prop types to CustomBottomSheet, AlertDialog, passthrough, and alert-dialog component mocks in src/components/command/__tests__/objective-details-sheet.test.tsx lines 8-23; and type CustomBottomSheet props in src/components/command/__tests__/reopen-command-sheet.test.tsx lines 8-10, preserving each mock’s existing behavior.Source: Coding guidelines
src/app/(app)/incidents.tsx (2)
54-59: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winWire
refreshingtoisLoadingfor pull-to-refresh feedback.
refreshControlhardcodesrefreshing={false}, so pulling to refresh never shows the native spinner even thoughisLoadingis already selected from the store.♻️ Proposed fix
- refreshControl={<RefreshControl refreshing={false} onRefresh={fetchIncidents} />} + refreshControl={<RefreshControl refreshing={isLoading} onRefresh={fetchIncidents} />}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/incidents.tsx around lines 54 - 59, Update the FlatList refreshControl in the incidents screen to pass the existing isLoading state to RefreshControl’s refreshing prop instead of hardcoding false, while preserving fetchIncidents as the onRefresh handler.
69-78: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFilter chips lack accessible semantics.
The active/all filter
Pressables have noaccessibilityRole/accessibilityLabel, so screen readers won't announce them as toggle buttons or reflect current selection state. Flagging here; see consolidated comment for a shared fix withincident-card.tsx.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/`(app)/incidents.tsx around lines 69 - 78, Update the active and all filter Pressables in the incidents filter HStack to expose button semantics, localized accessibility labels, and their current selected state through the appropriate accessibility props. Keep the existing includeClosed state and visual behavior unchanged, and apply the same shared accessibility treatment used for the related incident-card controls.src/components/ui/side-drawer.tsx (1)
94-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePanel styling uses hardcoded hex colors instead of semantic Tailwind tokens.
The panel background (
#111827/#ffffff) and backdrop (#000) are hardcoded rather than using NativeWind dark:/light: classes.As per coding guidelines, "Support both dark and light modes using
useColorScheme()and use semantic Tailwind color tokens instead of hardcoded hex values."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ui/side-drawer.tsx` around lines 94 - 113, Update the Animated.View styling in the side drawer to replace the hardcoded panel background and backdrop colors with semantic NativeWind dark/light color tokens. Preserve the existing useColorScheme-based dark/light behavior and apply the tokens through the component’s supported styling mechanism.Source: Coding guidelines
src/components/command/incident-card.tsx (2)
1-1: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd accessible semantics to custom
Pressablecontrols. Both the incident card and the active/all filter chips are tappablePressableelements with onlytestID, noaccessibilityRole/accessibilityLabel, so screen reader users won't get a coherent announcement of what the control is or its state.
src/components/command/incident-card.tsx#L44-44: addaccessibilityRole="button"and anaccessibilityLabelsummarizing the incident (name, status, duration) on the outerPressable.src/app/(app)/incidents.tsx#L69-78: addaccessibilityRole="button"andaccessibilityState={{ selected }}(or equivalentaccessibilityLabel) to each filter chipPressableso the active/all toggle state is announced.As per coding guidelines, "Follow mobile WCAG practices, use semantic components and accessible labels, and maintain sufficient contrast in both color schemes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/command/incident-card.tsx` at line 1, Add accessible semantics to the tappable controls: update the outer Pressable in the incident card component with accessibilityRole="button" and a label summarizing the incident name, status, and duration; update each filter-chip Pressable in the incidents screen with accessibilityRole="button" and accessibilityState reflecting whether that chip is selected.Source: Coding guidelines
44-44: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCard
Pressablelacks accessible semantics.No
accessibilityRole/accessibilityLabelon the tappable card container, so screen readers must piece together disjoint text/icon nodes. Flagging here; see consolidated comment for a shared fix withincidents.tsx's filter chips.As per coding guidelines, "Follow mobile WCAG practices, use semantic components and accessible labels, and maintain sufficient contrast in both color schemes."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/command/incident-card.tsx` at line 44, Add accessible semantics to the card Pressable in the incident-card component by assigning an appropriate accessibilityRole and a meaningful accessibilityLabel that describes the card action/content, consolidating the label from its visible text rather than requiring screen readers to traverse child nodes.Source: Coding guidelines
src/api/weather-alerts/weather-alerts.ts (1)
35-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse a single endpoint definition instead of rebuilding the path string.
The
/WeatherAlerts/GetWeatherAlert/${encodeURIComponent(alertId)}literal is built here (Line 36) and again insidegetWeatherAlertEndpoint(Line 12). If the path ever changes, the logged/error-contextendpointcan silently drift from the actually-requested URL.♻️ Suggested consolidation
-// GetWeatherAlert uses a path parameter, so the endpoint is created per call -const getWeatherAlertEndpoint = (alertId: string) => createApiEndpoint(`/WeatherAlerts/GetWeatherAlert/${encodeURIComponent(alertId)}`); +// GetWeatherAlert uses a path parameter, so the endpoint is created per call +const weatherAlertPath = (alertId: string) => `/WeatherAlerts/GetWeatherAlert/${encodeURIComponent(alertId)}`;export const getWeatherAlert = async (alertId: string) => { - const endpoint = `/WeatherAlerts/GetWeatherAlert/${encodeURIComponent(alertId)}`; + const endpoint = weatherAlertPath(alertId); try { - const response = await getWeatherAlertEndpoint(alertId).get<WeatherAlertResult>(); + const response = await createApiEndpoint(endpoint).get<WeatherAlertResult>(); return response.data;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/api/weather-alerts/weather-alerts.ts` around lines 35 - 45, Update getWeatherAlert to reuse the endpoint definition from getWeatherAlertEndpoint instead of rebuilding the encoded path locally. Ensure the endpoint value logged and passed to WeatherAlertFetchError matches the URL actually requested, eliminating the duplicate path construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/incidentCommand/incidentCommand.ts`:
- Around line 118-129: Update addIncidentAttachment to keep using the raw
api.post multipart request but remove the manually supplied Content-Type header,
allowing Axios/React Native to generate the FormData boundary automatically.
In `@src/components/command/__tests__/incident-map-card.test.tsx`:
- Line 1: Wrap every component render in each listed test suite with the shared
TestWrapper provider: import TestWrapper and pass it as the render wrapper in
incident-map-card.test.tsx, incident-maps-section.test.tsx,
incident-weather-section.test.tsx, need-details-sheet.test.tsx, and
need-entity-picker.test.tsx. Update all render calls in each file consistently.
- Line 21: Replace broad any props with narrow interfaces in the mock
components: type passthrough in
src/components/command/__tests__/incident-map-card.test.tsx at lines 21-21, and
type the bottom-sheet and alert-dialog mock props in
src/components/command/__tests__/incident-maps-section.test.tsx at lines 14-21
and src/components/command/__tests__/need-details-sheet.test.tsx at lines 9-21.
Include only the props each mock reads or forwards, such as testID, children,
and relevant component-specific callbacks or values.
In `@src/components/command/__tests__/need-entity-picker.test.tsx`:
- Around line 35-40: Update the NeedEntityPicker test after the unit selection
to rerender the controlled component with the unit entity as selected before
switching to the group tab. Then select the group option and assert onChange was
last called with both the existing unit and newly selected group entities,
preserving the controlled selected-state behavior.
In `@src/components/command/command-details-sheet.tsx`:
- Around line 103-114: Update the useEffect initialization in
command-details-sheet so fields are populated from a snapshot captured when
isOpen transitions to true, rather than the live command prop on every command
change. Keep manual edits intact during board refreshes, while still resetting
the form from the current command when the sheet is opened.
In `@src/components/command/incident-files-section.tsx`:
- Line 153: Update the file-delete confirmation button in the incident files
section to use a files-specific translation key, replacing the map-specific
incident_map_delete label while preserving the existing translation pattern and
dialog behavior.
In `@src/components/command/incident-weather-section.tsx`:
- Around line 42-47: Update the alert-fetch error path around setAlerts and the
catch block to surface failures through useToastStore.showToast, while retaining
the existing logger.warn call and loading cleanup. Add the
command.incident_weather_error translation key to every locale so both initial
failures and manual refresh errors provide user feedback.
In `@src/components/command/lane-details-sheet.tsx`:
- Around line 214-224: Update the lane linked-map buttons in the map selection
rendering to expose their selected state through accessibility semantics. Add an
accessibilityState selected value to the “none” button when linkedMapId is null
and to each mapped button when linkedMapId matches map.IncidentMapId, preserving
the existing visual variants and press behavior.
In `@src/lib/utils.ts`:
- Around line 435-440: Ensure sanitizeFileName only returns safe fallback values
by sanitizing or validating the fallback before using it when the input name is
empty. Update the call site in src/lib/utils.ts lines 435-440 and the
server-derived fallback usage in src/components/calls/call-files-modal.tsx lines
114-116, preserving safe literal fallbacks while preventing file.Id-derived
values from being mounted unsafely under documentDirectory.
In `@src/stores/command/__tests__/incidents-store.test.ts`:
- Around line 13-18: The test fixtures in summaries are missing the required
DepartmentId field and rely on unknown casts that bypass type checking. Add
DepartmentId to each IncidentCommandSummary fixture and type listResult directly
as Awaited<ReturnType<typeof getCommandList>>, removing the unknown casts so
contract changes are caught by the tests.
In `@src/stores/command/store.ts`:
- Around line 1294-1298: Declare the missing Maps property on
IncidentCommandBoard as an optional IncidentMap array, matching the board.Maps
usage in saveIncidentMapEntry and deleteIncidentMapEntry. Keep the existing map
update behavior unchanged and ensure the composite snapshot type consistently
represents this field.
In `@src/translations/ar.json`:
- Around line 823-852: Translate all remaining English incident-related values
in src/translations/ar.json lines 823-852, src/translations/de.json lines
823-852, src/translations/es.json lines 823-852, src/translations/fr.json lines
823-852, and src/translations/it.json lines 823-852, including commander,
establish, loading, no_command, and resources. Keep every translation key
identical to en.json and preserve the existing locale structure.
In `@src/translations/pl.json`:
- Around line 823-852: Translate all newly added English incident labels in the
incidents sections of src/translations/pl.json lines 823-852,
src/translations/sv.json lines 823-852, and src/translations/uk.json lines
823-852 into the respective locale languages, including commander,
command-establishment states, empty states, resources, title, and other
remaining English values; preserve the existing translation keys and JSON
structure.
---
Outside diff comments:
In `@src/stores/weather-alerts/store.ts`:
- Around line 75-80: Update the detail-loading action around getWeatherAlert to
preserve an explicit error state when the request fails: clear any prior detail
error before loading, then set a descriptive detail error in the catch path
alongside isLoadingDetail: false. Update the [id] detail screen’s error handling
to render retry feedback for that error instead of the “no alerts” empty state,
while keeping successful response rendering unchanged.
---
Nitpick comments:
In `@src/api/weather-alerts/weather-alerts.ts`:
- Around line 35-45: Update getWeatherAlert to reuse the endpoint definition
from getWeatherAlertEndpoint instead of rebuilding the encoded path locally.
Ensure the endpoint value logged and passed to WeatherAlertFetchError matches
the URL actually requested, eliminating the duplicate path construction.
In `@src/app/`(app)/incidents.tsx:
- Around line 54-59: Update the FlatList refreshControl in the incidents screen
to pass the existing isLoading state to RefreshControl’s refreshing prop instead
of hardcoding false, while preserving fetchIncidents as the onRefresh handler.
- Around line 69-78: Update the active and all filter Pressables in the
incidents filter HStack to expose button semantics, localized accessibility
labels, and their current selected state through the appropriate accessibility
props. Keep the existing includeClosed state and visual behavior unchanged, and
apply the same shared accessibility treatment used for the related incident-card
controls.
In `@src/components/command/__tests__/needs-section.test.tsx`:
- Line 45: Wrap the command component renders with the shared TestWrapper
provider setup instead of rendering directly. Update the render calls in
src/components/command/__tests__/needs-section.test.tsx:45,
src/components/command/__tests__/notes-section.test.tsx:46,
src/components/command/__tests__/objective-details-sheet.test.tsx:48, and
src/components/command/__tests__/reopen-command-sheet.test.tsx:29, preserving
each test’s existing component props and assertions.
In `@src/components/command/__tests__/notes-section.test.tsx`:
- Around line 8-18: Remove all any types from the command test mocks. In
src/components/command/__tests__/notes-section.test.tsx lines 8-18, type
Globe/Lock and CustomBottomSheet props; apply the corresponding prop types to
CustomBottomSheet, AlertDialog, passthrough, and alert-dialog component mocks in
src/components/command/__tests__/objective-details-sheet.test.tsx lines 8-23;
and type CustomBottomSheet props in
src/components/command/__tests__/reopen-command-sheet.test.tsx lines 8-10,
preserving each mock’s existing behavior.
In `@src/components/command/incident-card.tsx`:
- Line 1: Add accessible semantics to the tappable controls: update the outer
Pressable in the incident card component with accessibilityRole="button" and a
label summarizing the incident name, status, and duration; update each
filter-chip Pressable in the incidents screen with accessibilityRole="button"
and accessibilityState reflecting whether that chip is selected.
- Line 44: Add accessible semantics to the card Pressable in the incident-card
component by assigning an appropriate accessibilityRole and a meaningful
accessibilityLabel that describes the card action/content, consolidating the
label from its visible text rather than requiring screen readers to traverse
child nodes.
In `@src/components/command/lane-details-sheet.tsx`:
- Around line 83-86: Extend the lane-details test suite for the save flow around
the component’s LinkedMapId handling: verify initial LinkedMapId seeding,
selecting a map, clearing it with “none,” and that the onSave patch contains the
expected LinkedMapId value. Cover both setting and clearing the linked map while
preserving existing test behavior.
In `@src/components/ui/side-drawer.tsx`:
- Around line 94-113: Update the Animated.View styling in the side drawer to
replace the hardcoded panel background and backdrop colors with semantic
NativeWind dark/light color tokens. Preserve the existing useColorScheme-based
dark/light behavior and apply the tokens through the component’s supported
styling mechanism.
In `@src/stores/command/__tests__/incidents-store.test.ts`:
- Line 4: Update the import of useIncidentsStore in the incidents-store test to
use the configured "`@/stores/command/incidents-store`" alias instead of the
relative "../incidents-store" path.
- Line 2: Use standalone import type declarations for type-only model imports in
src/stores/command/__tests__/incidents-store.test.ts at line 2 and
src/components/command/lane-details-sheet.tsx at line 12; split the model types
from value enum imports in lane-details-sheet.tsx while preserving the existing
runtime imports.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 68cfe77c-2de9-4a29-92a5-cf0df896640c
📒 Files selected for processing (72)
src/api/incidentCommand/incidentCommand.tssrc/api/weather-alerts/weather-alerts.tssrc/app/(app)/__tests__/calls.test.tsxsrc/app/(app)/__tests__/command.test.tsxsrc/app/(app)/_layout.tsxsrc/app/(app)/calls.tsxsrc/app/(app)/command.tsxsrc/app/(app)/incidents.tsxsrc/app/(app)/settings.tsxsrc/app/call/[id].tsxsrc/app/call/__tests__/[id].security.test.tsxsrc/app/call/__tests__/[id].test.tsxsrc/app/command-map/[callId].tsxsrc/app/incident/[id].tsxsrc/app/settings/__tests__/offline-queue.test.tsxsrc/app/settings/offline-queue.tsxsrc/components/calls/call-files-modal.tsxsrc/components/command/__tests__/incident-card.test.tsxsrc/components/command/__tests__/incident-map-card.test.tsxsrc/components/command/__tests__/incident-maps-section.test.tsxsrc/components/command/__tests__/incident-weather-section.test.tsxsrc/components/command/__tests__/need-details-sheet.test.tsxsrc/components/command/__tests__/need-entity-picker.test.tsxsrc/components/command/__tests__/needs-section.test.tsxsrc/components/command/__tests__/notes-section.test.tsxsrc/components/command/__tests__/objective-details-sheet.test.tsxsrc/components/command/__tests__/reopen-command-sheet.test.tsxsrc/components/command/command-details-sheet.tsxsrc/components/command/incident-card.tsxsrc/components/command/incident-files-section.tsxsrc/components/command/incident-map-card.tsxsrc/components/command/incident-map-layers.tsxsrc/components/command/incident-maps-section.tsxsrc/components/command/incident-weather-section.tsxsrc/components/command/lane-details-sheet.tsxsrc/components/command/need-details-sheet.tsxsrc/components/command/need-entity-picker.tsxsrc/components/command/need-entity-status-list.tsxsrc/components/command/needs-section.tsxsrc/components/command/notes-section.tsxsrc/components/command/objective-details-sheet.tsxsrc/components/command/objectives-section.tsxsrc/components/command/reopen-command-sheet.tsxsrc/components/ui/__tests__/side-drawer.test.tsxsrc/components/ui/bottom-sheet.tsxsrc/components/ui/side-drawer.tsxsrc/components/weather-alerts/severity-filter-tabs.tsxsrc/lib/__tests__/sanitize-file-name.test.tssrc/lib/incident-command-utils.tssrc/lib/utils.tssrc/models/offline-queue/queued-event.tssrc/models/v4/incidentCommand/incidentCommand.tssrc/models/v4/incidentCommand/incidentCommandBoard.tssrc/models/v4/incidentCommand/incidentCommandModels.tssrc/services/__tests__/offline-event-manager.service.test.tssrc/services/offline-event-manager.service.tssrc/stores/command/__tests__/incidents-store.test.tssrc/stores/command/__tests__/store-preserve-local.test.tssrc/stores/command/__tests__/store.test.tssrc/stores/command/board-store.tssrc/stores/command/incidents-store.tssrc/stores/command/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
| export const addIncidentAttachment = async (incidentCommandId: string, visibility: number, description: string | null, file: { uri: string; name: string; type: string }) => { | ||
| const formData = new FormData(); | ||
| formData.append('IncidentCommandId', incidentCommandId); | ||
| formData.append('Visibility', String(visibility)); | ||
| if (description) { | ||
| formData.append('Description', description); | ||
| } | ||
| formData.append('File', { uri: file.uri, name: file.name, type: file.type } as unknown as Blob); | ||
| // The shared endpoint helper pins JSON; multipart needs its own content type, so use the raw client. | ||
| const response = await api.post<IncidentAttachmentResult>('/IncidentCommand/AddAttachment', formData, { headers: { 'Content-Type': 'multipart/form-data' } }); | ||
| return response.data; | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
axios React Native FormData multipart/form-data boundary missing when Content-Type set manually
💡 Result:
When working with FormData in React Native using Axios, you should not manually set the Content-Type header [1][2]. Doing so often overrides the system's ability to automatically generate the necessary multipart/form-data boundary, which causes the server to reject the request [3][4]. The runtime environment (in this case, React Native's networking layer) is responsible for appending the correct boundary parameter to the Content-Type header [1][2]. ### Recommended Solution Remove any manual Content-Type header configuration from your Axios request. When you pass a FormData object as the data property, Axios is designed to handle the header automatically [1][2]. Example: javascript // Correct way: Let Axios/React Native handle the header const formData = new FormData(); formData.append('key', 'value'); await axios.post('/your-endpoint', formData); ### Why Issues Occur 1. Manual Override: If you manually set Content-Type: multipart/form-data, Axios may pass this header directly to the underlying networking layer without the required boundary parameter [3][4]. Servers expect the boundary to be explicitly defined in the header string (e.g., multipart/form-data; boundary=----WebKitFormBoundary...) to parse the request body correctly [4][5]. 2. Default Header Conflicts: In some React Native versions, Axios may attempt to apply a default application/x-www-form-urlencoded header to requests. If this is not properly cleared, it can conflict with the FormData body, causing errors in React Native's NetworkingModule (especially on Android) [6][7]. ### If You Still Face Issues If your request is failing despite not setting the header manually, ensure you are using an up-to-date version of Axios. Recent patches (such as those in early 2026) have specifically addressed issues where the default Content-Type was not being correctly cleared for FormData in React Native, which previously caused crashes on Android [6][7]. If you are on an older version and cannot upgrade, a common workaround used to be setting the header to false or using a transformRequest function to ensure the Content-Type header is removed: javascript // Potential workaround for older versions if automatic handling fails await axios.post('/your-endpoint', formData, { headers: { 'Content-Type': false, // Prevents Axios from setting a default header }, }); [7]
Citations:
- 1: https://github.com/axios/axios/blob/e435384f/docs/pages/advanced/multipart-form-data-format.md
- 2: https://axios.rest/pages/advanced/multipart-form-data-format
- 3: Axios fetch adapter: Missing boundary when Content-Type is set manually with FormData axios/axios#7054
- 4: Axios does not append form boundary for multipart requests in node environment axios/axios#5379
- 5: FormData request header doesn't include boundary axios/axios#4631
- 6: fix: clear RN FormData content type axios/axios#10898
- 7: FormData requests crash on RN Android due to incorrect default Content-Type axios/axios#10895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find relevant files and package versions"
fd -a 'incidentCommand\.ts$|package\.json$|package-lock\.json$|yarn\.lock$|pnpm-lock\.yaml$' . | sed 's#^\./##'
echo
echo "Inspect incidentCommand.ts around target"
file="$(fd 'incidentCommand\.ts$' . | head -n 1)"
if [ -n "${file:-}" ]; then
sed -n '1,180p' "$file" | cat -n
fi
echo
echo "Package manifest axios/expo versions"
for f in package.json package-lock.json yarn.lock pnpm-lock.yaml; do
[ -f "$f" ] && { echo "--- $f"; grep -nE '"axios"|"axios-adapter"|"expo|react-native-network-adapter|react-native-networking|FormData|React Native' "$f" | head -n 80; }
done
echo
echo "Search for duplicate addIncidentAttachment or API helpers"
rg -n "addIncidentAttachment|createApiEndpoint|createCachedApiEndpoint|setHeader|Content-Type|FormData" -g '*.ts' -g '*.json' .Repository: Resgrid/IC
Length of output: 11277
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Inspect Axios version metadata from yarn.lock"
node - <<'JS'
const fs = require('fs');
const lock = fs.readFileSync('yarn.lock', 'utf8');
const markers = lock.split('\n\n').filter(s => s.includes('axios@npm:') || s.includes('axios@patch:'));
console.log(JSON.stringify(markers.slice(0, 4).map(m => ({ head: m.split('\n').slice(0, 8).join('\n') })), null, 2));
JS
echo
echo "Locate vendored Axios adapter/source in node_modules if present"
if [ -d node_modules/axios/lib/adapters ]; then
fd -d 2 . node_modules/axios/lib/adapters -x sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,220p" "$1" | cat -n' sh {}
else
echo "node_modules/axios/lib/adapters not present"
fi
echo
echo "Search axios source in node_modules for FormData / Content-Type handling if present"
if [ -d node_modules/axios ]; then
rg -n "FormData|Content-Type|boundary|multipart" node_modules/axios -g !node_modules/axios/dist -g !node_modules/axios/lib/helpers -g !node_modules/axios/lib/defaults | head -n 120
fiRepository: Resgrid/IC
Length of output: 391
Let Axios set the multipart Content-Type boundary for FormData.
Manually setting Content-Type: multipart/form-data can override Axios/React Native’s automatic multipart/form-data; boundary=... header, which may make the upload appear empty or be rejected by the server. Remove the explicit header while still using the raw axios client for the multipart request.
Possible fix
- // The shared endpoint helper pins JSON; multipart needs its own content type, so use the raw client.
- const response = await api.post<IncidentAttachmentResult>('/IncidentCommand/AddAttachment', formData, { headers: { 'Content-Type': 'multipart/form-data' } });
+ // The shared endpoint helper pins JSON; let the client set multipart + boundary for FormData.
+ const response = await api.post<IncidentAttachmentResult>('/IncidentCommand/AddAttachment', formData);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const addIncidentAttachment = async (incidentCommandId: string, visibility: number, description: string | null, file: { uri: string; name: string; type: string }) => { | |
| const formData = new FormData(); | |
| formData.append('IncidentCommandId', incidentCommandId); | |
| formData.append('Visibility', String(visibility)); | |
| if (description) { | |
| formData.append('Description', description); | |
| } | |
| formData.append('File', { uri: file.uri, name: file.name, type: file.type } as unknown as Blob); | |
| // The shared endpoint helper pins JSON; multipart needs its own content type, so use the raw client. | |
| const response = await api.post<IncidentAttachmentResult>('/IncidentCommand/AddAttachment', formData, { headers: { 'Content-Type': 'multipart/form-data' } }); | |
| return response.data; | |
| }; | |
| export const addIncidentAttachment = async (incidentCommandId: string, visibility: number, description: string | null, file: { uri: string; name: string; type: string }) => { | |
| const formData = new FormData(); | |
| formData.append('IncidentCommandId', incidentCommandId); | |
| formData.append('Visibility', String(visibility)); | |
| if (description) { | |
| formData.append('Description', description); | |
| } | |
| formData.append('File', { uri: file.uri, name: file.name, type: file.type } as unknown as Blob); | |
| // The shared endpoint helper pins JSON; let the client set multipart + boundary for FormData. | |
| const response = await api.post<IncidentAttachmentResult>('/IncidentCommand/AddAttachment', formData); | |
| return response.data; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/incidentCommand/incidentCommand.ts` around lines 118 - 129, Update
addIncidentAttachment to keep using the raw api.post multipart request but
remove the manually supplied Content-Type header, allowing Axios/React Native to
generate the FormData boundary automatically.
| @@ -0,0 +1,122 @@ | |||
| import { fireEvent, render, waitFor } from '@testing-library/react-native'; | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Wrap these component suites in TestWrapper.
src/components/command/__tests__/incident-map-card.test.tsx#L1-L1: import and useTestWrapperfor each render.src/components/command/__tests__/incident-maps-section.test.tsx#L1-L1: import and useTestWrapperfor each render.src/components/command/__tests__/incident-weather-section.test.tsx#L1-L1: import and useTestWrapperfor each render.src/components/command/__tests__/need-details-sheet.test.tsx#L1-L1: import and useTestWrapperfor each render.src/components/command/__tests__/need-entity-picker.test.tsx#L1-L1: import and useTestWrapperfor each render.
As per coding guidelines, “use TestWrapper for providers.”
📍 Affects 5 files
src/components/command/__tests__/incident-map-card.test.tsx#L1-L1(this comment)src/components/command/__tests__/incident-maps-section.test.tsx#L1-L1src/components/command/__tests__/incident-weather-section.test.tsx#L1-L1src/components/command/__tests__/need-details-sheet.test.tsx#L1-L1src/components/command/__tests__/need-entity-picker.test.tsx#L1-L1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/command/__tests__/incident-map-card.test.tsx` at line 1, Wrap
every component render in each listed test suite with the shared TestWrapper
provider: import TestWrapper and pass it as the render wrapper in
incident-map-card.test.tsx, incident-maps-section.test.tsx,
incident-weather-section.test.tsx, need-details-sheet.test.tsx, and
need-entity-picker.test.tsx. Update all render calls in each file consistently.
Source: Coding guidelines
| jest.mock('@/components/maps/mapbox', () => { | ||
| const React = require('react'); | ||
| const { View } = require('react-native'); | ||
| const passthrough = (name: string) => (props: any) => React.createElement(View, { ...props, testID: props.testID ?? `mock-${name}` }, props.children); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace any in mock component props with narrow interfaces.
src/components/command/__tests__/incident-map-card.test.tsx#L21-L21: type the passthrough component props precisely.src/components/command/__tests__/incident-maps-section.test.tsx#L14-L21: type bottom-sheet and alert-dialog mock props precisely.src/components/command/__tests__/need-details-sheet.test.tsx#L9-L21: type bottom-sheet and alert-dialog mock props precisely.
As per coding guidelines, “never use any, and prefer precise types and interfaces.”
📍 Affects 3 files
src/components/command/__tests__/incident-map-card.test.tsx#L21-L21(this comment)src/components/command/__tests__/incident-maps-section.test.tsx#L14-L21src/components/command/__tests__/need-details-sheet.test.tsx#L9-L21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/command/__tests__/incident-map-card.test.tsx` at line 21,
Replace broad any props with narrow interfaces in the mock components: type
passthrough in src/components/command/__tests__/incident-map-card.test.tsx at
lines 21-21, and type the bottom-sheet and alert-dialog mock props in
src/components/command/__tests__/incident-maps-section.test.tsx at lines 14-21
and src/components/command/__tests__/need-details-sheet.test.tsx at lines 9-21.
Include only the props each mock reads or forwards, such as testID, children,
and relevant component-specific callbacks or values.
Source: Coding guidelines
| fireEvent.press(getByTestId(`need-entity-option-${NeedEntityKind.Unit}-5`)); | ||
| expect(onChange).toHaveBeenCalledWith([{ kind: NeedEntityKind.Unit, id: '5', name: 'Engine 5' }]); | ||
|
|
||
| fireEvent.press(getByTestId(`need-entity-tab-${NeedEntityKind.Group}`)); | ||
| fireEvent.press(getByTestId(`need-entity-option-${NeedEntityKind.Group}-7`)); | ||
| expect(onChange).toHaveBeenLastCalledWith([{ kind: NeedEntityKind.Group, id: '7', name: 'Station 2' }]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Rerender after selecting the unit.
NeedEntityPicker is controlled by selected; onChange does not mutate it. The group assertion therefore tests [group], not cross-kind accumulation. Rerender with the selected unit before pressing the group tab and expect both entities.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/command/__tests__/need-entity-picker.test.tsx` around lines 35
- 40, Update the NeedEntityPicker test after the unit selection to rerender the
controlled component with the unit entity as selected before switching to the
group tab. Then select the group option and assert onChange was last called with
both the existing unit and newly selected group entities, preserving the
controlled selected-state behavior.
| useEffect(() => { | ||
| if (isOpen) { | ||
| setName(command?.Name ?? ''); | ||
| setEstablishedOn(command?.EstablishedOn ?? null); | ||
| setEstimatedEndOn(command?.EstimatedEndOn ?? null); | ||
| setImportantInformation(command?.ImportantInformation ?? ''); | ||
| setCommandPost(locationFrom(command?.CommandPostLocationText, command?.CommandPostLatitude, command?.CommandPostLongitude)); | ||
| setStaging(locationFrom(command?.StagingLocationText, command?.StagingLatitude, command?.StagingLongitude)); | ||
| setRehab(locationFrom(command?.RehabLocationText, command?.RehabLatitude, command?.RehabLongitude)); | ||
| setPickerTarget(null); | ||
| } | ||
| }, [command, isOpen]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how CommandDetailsSheet receives `command` and whether it can change mid-edit.
rg -nP -C3 'CommandDetailsSheet' --type=tsxRepository: Resgrid/IC
Length of output: 180
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo files containing CommandDetailsSheet:"
rg -n -C3 'CommandDetailsSheet' -g '*.{ts,tsx}'
echo
echo "File outline:"
ast-grep outline src/components/command/command-details-sheet.tsx || true
echo
echo "Relevant file section:"
sed -n '1,180p' src/components/command/command-details-sheet.tsxRepository: Resgrid/IC
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find file"
fd -a 'command-details-sheet' . || true
fd -a 'command-details-sheet.tsx' . || true
echo
echo "Find usages"
rg -n -C3 'CommandDetailsSheet|command-details-sheet' . || true
echo
echo "List candidate command files"
fd -a 'command' src | head -100Repository: Resgrid/IC
Length of output: 5777
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "component component-details-sheet relevant sections"
sed -n '1,280p' src/components/command/command-details-sheet.tsx
echo
echo "call site command.tsx around sheet prop"
sed -n '800,860p' 'src/app/(app)/command.tsx'
echo
echo "state that supplies boardState.board.Command"
rg -n -C4 'boardState\.board|setBoardState|incidentCommand|Command =|Command:' 'src/app/(app)/command.tsx' | sed -n '1,220p'Repository: Resgrid/IC
Length of output: 28790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Command store definition/refreshBoard:"
rg -n -C5 'refreshBoard|boards|activeCallId|addBoard|updateBoard|boards:|setBoards|setActiveCallId' src/stores 'src/app/(app)/command.tsx' | sed -n '1,260p'
echo
echo "Any background auto-refresh polling/intervals"
rg -n -C3 'setInterval|refreshBoard\(|useInterval|background|fetchBoard|fetch.*Board' src/app src/stores src/api/incidentCommand src/api/command | sed -n '1,240p'Repository: Resgrid/IC
Length of output: 34378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate command store files"
fd -a 'store' src/stores/command | sort
echo
echo "Read command store with line numbers (limited candidate matches)"
rg -n -C10 'export const useCommandStore|const useCommandStore|refreshBoard|setBoards|boards:|type .*CommandBoard|interface .*CommandBoard' src/stores/command | sed -n '1,320p'
candidate=$(fd -a '.*store.*\.ts$' src/stores/command | sed -n '1,5p')
for f in $candidate; do
echo "--- $f ---"
wc -l "$f"
sed -n '1,260p' "$f"
doneRepository: Resgrid/IC
Length of output: 50366
Snapshot command when opening the sheet so manual edits aren’t overwritten.
boardState.board comes directly from the command store, and refreshBoard(...) replaces it with the fresh server response. Since this sheet uses command in its effect dependencies, any manual change to the Command props (including a board refresh) re-applies those fields and can drop unsaved edits. Capture the command state on isOpen and re-populate from that capture, or persist local edits across board refreshes before saving.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/command/command-details-sheet.tsx` around lines 103 - 114,
Update the useEffect initialization in command-details-sheet so fields are
populated from a snapshot captured when isOpen transitions to true, rather than
the live command prop on every command change. Keep manual edits intact during
board refreshes, while still resetting the form from the current command when
the sheet is opened.
| export function sanitizeFileName(fileName: string | null | undefined, fallback = 'attachment'): string { | ||
| const base = (fileName ?? '') | ||
| .replace(/^.*[\\/]/, '') | ||
| .replace(/\.\./g, '') | ||
| .trim(); | ||
| return base.length > 0 ? base : fallback; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- files ---\n'
git ls-files | rg '(^|/)src/(lib/utils\.ts|components/calls/call-files-modal\.tsx)$|src/lib/utils\.ts$|src/components/calls/call-files-modal\.tsx$' || true
printf '\n--- src/lib/utils.ts outline around sanitizeFileName ---\n'
ast-grep outline src/lib/utils.ts --match sanitizeFileName --view expanded || true
sed -n '400,460p' src/lib/utils.ts | cat -n
printf '\n--- src/components/calls/call-files-modal.tsx outline/context ---\n'
ast-grep outline src/components/calls/call-files-modal.tsx --view compact || true
sed -n '1,160p' src/components/calls/call-files-modal.tsx | cat -n
printf '\n--- all sanitizeFileName usages ---\n'
rg -n "sanitizeFileName\(" src || trueRepository: Resgrid/IC
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- files ---'
git ls-files | rg '(^|/)src/(lib/utils\.ts|components/calls/call-files-modal\.tsx)$|src/lib/utils\.ts$|src/components/calls/call-files-modal\.tsx$' || true
printf '%s\n' ''
printf '%s\n' '--- src/lib/utils.ts outline around sanitizeFileName ---'
ast-grep outline src/lib/utils.ts --match sanitizeFileName --view expanded || true
sed -n '400,460p' src/lib/utils.ts | cat -n
printf '%s\n' ''
printf '%s\n' '--- src/components/calls/call-files-modal.tsx outline/context ---'
ast-grep outline src/components/calls/call-files-modal.tsx --view compact || true
sed -n '1,160p' src/components/calls/call-files-modal.tsx | cat -n
printf '%s\n' ''
printf '%s\n' '--- all sanitizeFileName usages ---'
rg -n "sanitizeFileName\(" src || trueRepository: Resgrid/IC
Length of output: 11078
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- sanitizeFileName tests ---'
cat -n src/lib/__tests__/sanitize-file-name.test.ts
printf '%s\n' ''
printf '%s\n' '--- behavioral probe for sanitizeFileName candidates ---'
node - <<'JS'
function sanitizeFileName(fileName, fallback = 'attachment') {
const base = (fileName ?? '')
.replace(/^.*[\\\/]/, '')
.replace(/\.\./g, '')
.trim();
return base.length > 0 ? base : fallback;
}
const cases = [
['../../etc/passwd', `file_${'../..'}'edBad'`],
['...', `../../path/e${'..secret'}xploit`],
[null, 'file_./..evil'],
['/.\\..//', '__'],
];
for (const [input, fallback] of cases) {
console.log(JSON.stringify(sanitizeFileName(input, fallback)));
}
JSRepository: Resgrid/IC
Length of output: 1488
Ensure only safe fallback values are used.
Sanitizing the computed base name is not enough when the caller passes server-derived fallback strings such as file_${file.Id}; sanitizeFileName() already returns arbitrary fallbacks unchanged. Restrict fallbacks to safe literals or validate/sanitize them in the same way as the file name before mounting them under documentDirectory.
📍 Affects 2 files
src/lib/utils.ts#L435-L440(this comment)src/components/calls/call-files-modal.tsx#L114-L116
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/utils.ts` around lines 435 - 440, Ensure sanitizeFileName only
returns safe fallback values by sanitizing or validating the fallback before
using it when the input name is empty. Update the call site in src/lib/utils.ts
lines 435-440 and the server-derived fallback usage in
src/components/calls/call-files-modal.tsx lines 114-116, preserving safe literal
fallbacks while preventing file.Id-derived values from being mounted unsafely
under documentDirectory.
| const summaries = [ | ||
| { IncidentCommandId: 'ic-1', CallId: 5, Status: 0, EstablishedOn: '2026-07-01T10:00:00Z', AssignedUnitCount: 2, AssignedPersonnelCount: 3 }, | ||
| { IncidentCommandId: 'ic-2', CallId: 6, Status: 1, EstablishedOn: '2026-06-01T10:00:00Z', ClosedOn: '2026-06-01T14:00:00Z', AssignedUnitCount: 0, AssignedPersonnelCount: 0 }, | ||
| ] as unknown as IncidentCommandSummary[]; | ||
|
|
||
| const bundleResult = { Data: fakeBundle } as unknown as Awaited<ReturnType<typeof getBundle>>; | ||
| const listResult = { Data: summaries } as unknown as Awaited<ReturnType<typeof getCommandList>>; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep the fixtures type-safe.
IncidentCommandSummary requires DepartmentId, but the unknown casts allow incomplete summaries and response payloads to compile. Add the required fields and type the result directly so API contract changes fail in tests.
Also applies to: 49-49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/command/__tests__/incidents-store.test.ts` around lines 13 - 18,
The test fixtures in summaries are missing the required DepartmentId field and
rely on unknown casts that bypass type checking. Add DepartmentId to each
IncidentCommandSummary fixture and type listResult directly as
Awaited<ReturnType<typeof getCommandList>>, removing the unknown casts so
contract changes are caught by the tests.
| mutateBoard(set, get, callId, (board) => { | ||
| const maps = board.Maps ?? []; | ||
| const exists = maps.some((m) => m.IncidentMapId === saved.IncidentMapId); | ||
| return { ...board, Maps: exists ? maps.map((m) => (m.IncidentMapId === saved.IncidentMapId ? saved : m)) : [...maps, saved] }; | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether IncidentCommandBoard declares a Maps field.
fd -t f incidentCommandBoard.ts src/models | xargs -I{} sh -c 'echo "== {} =="; sed -n "1,80p" {}'
echo "--- Maps references in the board interface ---"
rg -nP -C2 '\bMaps\b' src/models/v4/incidentCommand/incidentCommandBoard.ts src/models/v4/incidentCommand/incidentCommandModels.tsRepository: Resgrid/IC
Length of output: 6895
Declare Maps on IncidentCommandBoard or update it consistently.
IncidentCommandBoard defines command-snapshot fields but no Maps, while saveIncidentMapEntry and deleteIncidentMapEntry read/write board.Maps. Either add Maps?: IncidentMap[]; to the composite snapshot interface or adjust these operations to target the model that actually owns maps.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/stores/command/store.ts` around lines 1294 - 1298, Declare the missing
Maps property on IncidentCommandBoard as an optional IncidentMap array, matching
the board.Maps usage in saveIncidentMapEntry and deleteIncidentMapEntry. Keep
the existing map update behavior unchanged and ensure the composite snapshot
type consistently represents this field.
2 participants
PR Description: RIC-T40 Fixes
This pull request addresses several fixes related to the weather alerts feature:
Changes
1. Fixed Weather Alert Detail API Call
The
getWeatherAlertfunction was updated to pass thealertIdas a path parameter (/WeatherAlerts/GetWeatherAlert/{alertId}) rather than as a query parameter. This corrects how the endpoint is constructed for retrieving a specific weather alert by ID.2. Improved Weather Alert Detail Loading State
When fetching a new weather alert's details, the store now clears the previously loaded
selectedAlert(sets it tonull) before beginning the fetch. This prevents stale data from being displayed while a new alert detail is loading.3. Fixed Severity Filter Tabs Layout
Added
grow-0class to the severity filter tabs ScrollView to prevent it from expanding unnecessarily, fixing a layout issue with the filter tabs.Summary by CodeRabbit