Conversation
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 11 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds Weather Alerts and Scheduled Calls features: APIs, models, Zustand stores, widgets, screens, settings, grid/config hooks, SignalR handlers, wide i18n additions, and dependency upgrades; plus assorted UI/localization and minor refactors across the dashboard and supporting components. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as WeatherAlertsWidget
participant Store as useWeatherAlertsStore
participant API as Weather API
participant SignalR as SignalR Hub
participant UIList as WeatherAlerts Screen
UI->>Store: init() / fetchSettings()
Store->>API: GET /WeatherAlerts/GetSettings
API-->>Store: settings
Store->>API: GET /WeatherAlerts/GetActiveAlerts
API-->>Store: alerts[]
Store->>Store: sortAlerts(), setState
UIList->>Store: fetchActiveAlerts() (pull-to-refresh)
Store-->>UIList: alerts[] (render)
SignalR->>Store: weatherAlertReceived(alertId)
Store->>API: GET /WeatherAlerts/GetWeatherAlert?alertId=...
API-->>Store: alert
Store->>Store: prepend/replace, sort
Store-->>UI: updated alerts (re-render)
sequenceDiagram
participant WidgetUI as ScheduledCallsWidget
participant Store as useScheduledCallsStore
participant API as Calls API
participant Personnel as Personnel Store
participant Units as Units Store
WidgetUI->>Store: init()
Store->>API: GET /Calls/GetAllPendingScheduledCalls
API-->>Store: calls[]
Store->>API: GET /Calls/GetCallPriorities
API-->>Store: priorities[]
par per-call extra data
Store->>API: GET /Calls/GetCallExtraData?callId=X
API-->>Store: extraData(X)
end
Store->>Store: build callExtraDataMap, setState
WidgetUI->>Personnel: read personnel map
WidgetUI->>Units: read units map
WidgetUI->>WidgetUI: compute urgency colors, filter/sort, render table
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
CLAUDE.md-62-64 (1)
62-64:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced code block.
Line 62 uses an unlabeled fenced block, which violates markdownlint MD040. Use something like
```text(or```bashif preferred).Suggested fix
-``` +```text graph_add_memory(type="decision|task|next|fact|blocker", content="one sentence max 15 words", tags=["topic"], files=["relevant/file.ts"])</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@CLAUDE.mdaround lines 62 - 64, The fenced code block containing the Graph
helper call graph_add_memory(type="decision|task|next|fact|blocker",
content="one sentence max 15 words", tags=["topic"], files=["relevant/file.ts"])
is unlabeled and triggers markdownlint MD040; add a language tag to the opening
fence (for example usetext orbash) so the block is explicitly typed and
MD040 is satisfied, keeping the content unchanged.</details> </blockquote></details> <details> <summary>src/hooks/use-grid-config.ts-17-26 (1)</summary><blockquote> `17-26`: _⚠️ Potential issue_ | _🟡 Minor_ **Normalize the 600px breakpoint logic (Line 19 vs Line 25).** `width === 600` is treated as `phone` on web but `tablet` on native. This creates inconsistent default widget sizing at the same viewport width. <details> <summary>Suggested patch</summary> ```diff if (Platform.OS === 'web') { if (width > 1024) return 'desktop'; - if (width > 600) return 'tablet'; + if (width >= 600) return 'tablet'; return 'phone'; } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/hooks/use-grid-config.ts` around lines 17 - 26, The breakpoint for 600px is inconsistent between the web and native branches in use-grid-config (Platform.OS === 'web' branch uses width > 600 while the native branch uses width >= 600); make them consistent by changing the web branch to use width >= 600 so that width === 600 is treated as 'tablet' everywhere (update the conditional in the web branch where it currently reads width > 600). ``` </details> </blockquote></details> <details> <summary>src/stores/scheduledCalls/store.ts-90-92 (1)</summary><blockquote> `90-92`: _⚠️ Potential issue_ | _🟡 Minor_ **Inconsistent error handling compared to `init()`.** The `fetchScheduledCalls` catch block silently swallows the error without logging, whereas `init()` (line 59-63) logs the error with context. For consistency and debuggability, consider logging the error here as well. <details> <summary>🐛 Proposed fix</summary> ```diff - } catch { + } catch (error) { + logger.error({ + message: 'Failed to fetch scheduled calls', + context: { error }, + }); set({ error: 'Failed to fetch scheduled calls', isLoading: false }); } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/stores/scheduledCalls/store.ts` around lines 90 - 92, fetchScheduledCalls currently swallows exceptions in its catch block unlike init(), so update fetchScheduledCalls's catch to log the caught error with the same logger used in init (include error object and contextual message like "Failed to fetch scheduled calls") before calling set({ error: 'Failed to fetch scheduled calls', isLoading: false }); reference the fetchScheduledCalls function and the set call to locate the catch block and ensure the log mirrors init()'s error logging pattern. ``` </details> </blockquote></details> <details> <summary>app.config.ts-274-274 (1)</summary><blockquote> `274-274`: _⚠️ Potential issue_ | _🟡 Minor_ **Remove the redundant bare `@sentry/react-native` plugin entry.** Both `@sentry/react-native/expo` (lines 248–255) and `@sentry/react-native` (line 274) are equivalent—the bare package proxies to the `/expo` subpath. Using both is redundant and not recommended by Sentry. Keep the configured `/expo` variant with your organization and project settings; remove the bare entry at line 274. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@app.config.ts` at line 274, Remove the redundant bare plugin entry '@sentry/react-native' from the plugins array and keep only the configured '@sentry/react-native/expo' entry; locate the plugins list where both strings appear and delete the plain '@sentry/react-native' line so only '@sentry/react-native/expo' with your organization/project settings remains. ``` </details> </blockquote></details> <details> <summary>src/translations/ar.json-582-588 (1)</summary><blockquote> `582-588`: _⚠️ Potential issue_ | _🟡 Minor_ **Localize the new Arabic strings.** These entries are still English, so the Scheduled Calls and Weather Alerts screens will render mixed-language UI for Arabic users. Also applies to: 723-802 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/translations/ar.json` around lines 582 - 588, The Arabic locale contains untranslated English entries under the "scheduledCalls" object (keys: "activatesIn", "noScheduledCalls", "overdue", "scheduledTime", "title") and other untranslated strings in the weather alerts block (the entries referenced in the same file range). Replace each English value with the correct Arabic translations for those keys so the Scheduled Calls and Weather Alerts screens are fully localized; keep the JSON keys unchanged and only update the string values. ``` </details> </blockquote></details> <details> <summary>src/translations/es.json-582-588 (1)</summary><blockquote> `582-588`: _⚠️ Potential issue_ | _🟡 Minor_ **Localize the new Spanish strings.** These entries are still English, so the Scheduled Calls and Weather Alerts screens will render mixed-language UI for Spanish users. Also applies to: 723-802 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/translations/es.json` around lines 582 - 588, The Spanish translations file contains English text for the scheduled calls block and the weather alerts block; update the values for the keys under "scheduledCalls" (activatesIn, noScheduledCalls, overdue, scheduledTime, title) to proper Spanish phrases, and similarly translate all English strings in the "weatherAlerts" object (the block around the 723-802 region) so the UI shows fully localized Spanish text; keep the JSON keys unchanged and only replace the English string values with their Spanish equivalents. ``` </details> </blockquote></details> <details> <summary>src/stores/weatherAlerts/store.ts-76-84 (1)</summary><blockquote> `76-84`: _⚠️ Potential issue_ | _🟡 Minor_ **Potential duplicate alert if `handleAlertReceived` is called for an existing alert.** When a new alert is received via SignalR, this function prepends it to the list without checking if an alert with the same `WeatherAlertId` already exists. If the same alert ID is received twice (e.g., due to reconnection or duplicate events), duplicates will appear in the list. <details> <summary>🛡️ Proposed fix to prevent duplicates</summary> ```diff handleAlertReceived: async (alertId: string) => { try { const result = await getWeatherAlert(alertId); const { alerts } = get(); + // Avoid duplicates if alert already exists + if (alerts.some((a) => a.WeatherAlertId === alertId)) { + return; + } set({ alerts: sortAlerts([result.Data, ...alerts]) }); } catch (error) { logger.error({ message: 'Failed to handle received weather alert', context: { error, alertId } }); } }, ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/stores/weatherAlerts/store.ts` around lines 76 - 84, handleAlertReceived may prepend a duplicate alert when the same WeatherAlertId already exists; modify handleAlertReceived (which calls getWeatherAlert and set with sortAlerts) to first check the current alerts array (from get()) for an existing item with the same WeatherAlertId, and either skip adding if found or replace the existing item with the fresh result before calling set({ alerts: sortAlerts(...) }). Ensure comparison uses the WeatherAlertId field from result.Data and existing alerts to avoid duplicates on reconnection/duplicate events. ``` </details> </blockquote></details> <details> <summary>src/components/widgets/WeatherAlertsWidget.tsx-140-144 (1)</summary><blockquote> `140-144`: _⚠️ Potential issue_ | _🟡 Minor_ **Avoid `as any` type cast for route navigation.** The `as any` cast bypasses TypeScript's route type checking. Use the proper typed route or update the route types if this is a valid route. <details> <summary>🔧 Suggested fix</summary> ```diff const handlePress = () => { if (!isEditMode) { - router.push('/(app)/weather-alerts' as any); + router.push('/(app)/weather-alerts'); } }; ``` If TypeScript complains, ensure the route is properly typed in your route configuration rather than using `as any`. </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/widgets/WeatherAlertsWidget.tsx` around lines 140 - 144, The handlePress function is bypassing TypeScript route checks by using "as any" on router.push; remove the "as any" cast and call router.push('/(app)/weather-alerts') with proper typing, or update your route type definitions so '/(app)/weather-alerts' is a valid typed route. Specifically, change the router.push call in handlePress to use the typed push signature (or a typed UrlObject) and ensure your router/route config (types for AppRouterInstance or your route union) includes the '(app)/weather-alerts' route so no cast is necessary. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (14)</summary><blockquote> <details> <summary>src/stores/auth/store.tsx (1)</summary><blockquote> `237-245`: **Consider extracting the validation condition to eliminate duplication.** The validation `typeof rawExpiresIn === 'number' && rawExpiresIn > 0` is repeated on lines 238 and 239 (negated). Extracting it to a boolean variable would improve maintainability and adhere to DRY principles. <details> <summary>♻️ Proposed refactor to extract duplicate condition</summary> ```diff const now = new Date(); const rawExpiresIn = response.authResponse.expires_in; -const expiresInSeconds = typeof rawExpiresIn === 'number' && rawExpiresIn > 0 ? rawExpiresIn : 3600; -if (!(typeof rawExpiresIn === 'number' && rawExpiresIn > 0)) { +const isValidExpiresIn = typeof rawExpiresIn === 'number' && rawExpiresIn > 0; +const expiresInSeconds = isValidExpiresIn ? rawExpiresIn : 3600; +if (!isValidExpiresIn) { logger.warn({ message: 'LoginWithSso: expires_in missing or invalid; defaulting to 3600s', context: { expires_in: rawExpiresIn }, }); } const expiresOn = new Date(now.getTime() + expiresInSeconds * 1000).getTime().toString(); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/stores/auth/store.tsx` around lines 237 - 245, Extract the repeated validation into a well-named boolean (e.g., isValidExpiresIn) to avoid duplicate checks: evaluate isValidExpiresIn = typeof rawExpiresIn === 'number' && rawExpiresIn > 0, then use it to set expiresInSeconds (rawExpiresIn or default 3600) and to gate the logger.warn call; update the computation of expiresOn to use expiresInSeconds as before. This change touches the rawExpiresIn, expiresInSeconds, logger.warn and expiresOn logic in the LoginWithSso flow. ``` </details> </blockquote></details> <details> <summary>src/models/v4/weatherAlerts/weatherAlertSettingsData.ts (1)</summary><blockquote> `1-11`: **Align new `src` file/directory naming to lowercase-hyphenated convention.** The new weather-alerts model path uses camelCase segments. Please rename these newly added path segments to the project’s lowercase-hyphenated format to avoid introducing more mixed conventions. As per coding guidelines: `src/**/!(*.test|*.spec).{ts,tsx}` “Directory and file names should be lowercase and hyphenated (e.g., `user-profile`, `chat-screen`)”. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/models/v4/weatherAlerts/weatherAlertSettingsData.ts` around lines 1 - 11, The new interfaces WeatherAlertSeverityScheduleData and WeatherAlertSettingsData live in camelCase path segments; rename the directory segment "weatherAlerts" to "weather-alerts" and the file "weatherAlertSettingsData.ts" to "weather-alert-settings-data.ts" and update all imports/exports referencing these symbols to use the new lowercase-hyphenated names so project naming conventions remain consistent. ``` </details> </blockquote></details> <details> <summary>src/app/(app)/_layout.tsx (1)</summary><blockquote> `199-199`: **Consider breaking up this long Promise.all line for readability.** While functionally correct, the single-line `Promise.all` with 4 async calls is hard to scan. Breaking it across multiple lines would improve maintainability. <details> <summary>♻️ Suggested formatting</summary> ```diff - await Promise.all([useCoreStore.getState().fetchConfig(), useCallsStore.getState().fetchCalls(), useRolesStore.getState().fetchRoles(), useWeatherAlertsStore.getState().fetchActiveAlerts()]); + await Promise.all([ + useCoreStore.getState().fetchConfig(), + useCallsStore.getState().fetchCalls(), + useRolesStore.getState().fetchRoles(), + useWeatherAlertsStore.getState().fetchActiveAlerts(), + ]); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/_layout.tsx at line 199, The long single-line Promise.all call is hard to read; split it into multiple lines by assigning each async call (useCoreStore.getState().fetchConfig(), useCallsStore.getState().fetchCalls(), useRolesStore.getState().fetchRoles(), useWeatherAlertsStore.getState().fetchActiveAlerts()) into the Promise.all array on its own line (or into named constants first) so each call is on a separate line for readability and easier diffing, then await Promise.all([...]) as before. ``` </details> </blockquote></details> <details> <summary>src/stores/scheduledCalls/store.ts (1)</summary><blockquote> `39-57`: **Consider extracting duplicated extra data fetch logic.** The extra data fetching logic (lines 39-57 and 72-89) is nearly identical between `init()` and `fetchScheduledCalls()`. Extracting this into a private helper would reduce duplication and make future maintenance easier. <details> <summary>♻️ Proposed refactor to extract helper</summary> ```diff +const fetchExtraDataForCalls = async ( + calls: CallResultData[], + get: () => ScheduledCallsState, + set: (state: Partial<ScheduledCallsState>) => void +) => { + const fetchId = Date.now(); + set({ lastCallExtraDataFetchId: fetchId }); + if (calls.length > 0) { + const extraDataResults = await Promise.allSettled( + calls.map((call) => getScheduledCallExtraData(call.CallId)) + ); + if (get().lastCallExtraDataFetchId !== fetchId) return; + const newExtraDataMap: Record<string, CallExtraDataResultData> = {}; + extraDataResults.forEach((result, index) => { + if (result.status === 'fulfilled' && result.value?.Data) { + newExtraDataMap[calls[index].CallId] = result.value.Data; + } + }); + set({ callExtraDataMap: newExtraDataMap }); + } else { + if (get().lastCallExtraDataFetchId === fetchId) { + set({ callExtraDataMap: {} }); + } + } +}; ``` Then use in both actions: ```typescript // In init() after setting scheduledCalls await fetchExtraDataForCalls(calls, get, set); // In fetchScheduledCalls() after setting scheduledCalls await fetchExtraDataForCalls(calls, get, set); ``` </details> Also applies to: 72-89 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/stores/scheduledCalls/store.ts` around lines 39 - 57, Extract the duplicated extra-data fetch logic into a private helper, e.g., fetchExtraDataForCalls(calls, get, set): it should create fetchId = Date.now(), set lastCallExtraDataFetchId, call Promise.allSettled over calls.map(c => getScheduledCallExtraData(c.CallId)), bail out if get().lastCallExtraDataFetchId !== fetchId, build newExtraDataMap from fulfilled results (using CallId keys) and finally set callExtraDataMap (or {} if no calls) — then call this helper from both init() and fetchScheduledCalls() in place of the duplicated blocks to preserve existing behavior and state names (lastCallExtraDataFetchId, callExtraDataMap) and reuse getScheduledCallExtraData. ``` </details> </blockquote></details> <details> <summary>src/app/(app)/weather-alerts.tsx (1)</summary><blockquote> `103-105`: **Drop the `as any` navigation cast.** This opts out of route checking exactly where the new detail screen is wired up, so a pathname/param drift will no longer be caught at compile time. Prefer the typed `expo-router` route object here. As per coding guidelines, "Avoid using `any`; strive for precise types." <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/weather-alerts.tsx around lines 103 - 105, Remove the unsafe "as any" cast on the router.push call in the Pressable and use the strongly-typed route object from expo-router instead: update the call to router.push to pass a properly typed route (preserving pathname '/(app)/weather-alert-detail' and params.alertId = item.WeatherAlertId) and import/use the expo-router route/params type or your app's generated route types so TypeScript validates the pathname and params at compile time; specifically modify the router.push invocation near the Pressable component (the onPress handler) to use the typed route object rather than casting to any. ``` </details> </blockquote></details> <details> <summary>package.json (1)</summary><blockquote> `251-260`: **Stale version exclusions in `expo.install.exclude`.** The excluded versions reference older React Native ecosystem packages (e.g., `react-native@~0.76.6`, `react-native-reanimated@~3.16.1`) that no longer match the actual dependencies. These exclusions may be obsolete now that the project has upgraded to RN 0.81.5, reanimated 4.1.1, etc. Consider removing or updating this section to reflect the current dependency versions. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 251 - 260, The expo.install.exclude list in package.json contains stale entries (e.g., "react-native@~0.76.6", "react-native-reanimated@~3.16.1") that no longer match the project's current dependency versions; update or remove these exclusions under the expo.install.exclude key so they reflect the RN 0.81.5 and reanimated 4.1.1 upgrades (either delete obsolete package entries or replace the versions with current ones) and run a dependency install/test to verify nothing breaks. ``` </details> </blockquote></details> <details> <summary>src/stores/weatherAlerts/store.ts (2)</summary><blockquote> `26-35`: **Consider caching parsed dates for sorting performance.** The `sortAlerts` function creates new `Date` objects on every comparison. For large alert lists, this could impact performance. Consider pre-computing timestamps if performance becomes an issue. <details> <summary>♻️ Optional optimization</summary> ```diff const sortAlerts = (alerts: WeatherAlertResultData[]): WeatherAlertResultData[] => { - return [...alerts].sort((a, b) => { - // Sort by severity descending (Extreme=4 first) - if (b.Severity !== a.Severity) { - return b.Severity - a.Severity; - } - // Then by EffectiveUtc descending (newest first) - return new Date(b.EffectiveUtc).getTime() - new Date(a.EffectiveUtc).getTime(); - }); + return [...alerts] + .map((alert) => ({ alert, effectiveTime: new Date(alert.EffectiveUtc).getTime() })) + .sort((a, b) => { + if (b.alert.Severity !== a.alert.Severity) { + return b.alert.Severity - a.alert.Severity; + } + return b.effectiveTime - a.effectiveTime; + }) + .map(({ alert }) => alert); }; ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/stores/weatherAlerts/store.ts` around lines 26 - 35, The sortAlerts function repeatedly constructs Date objects during comparisons, hurting performance on large arrays; to fix, precompute numeric timestamps for each alert (e.g., map WeatherAlertResultData items to include an EffectiveUtcTs or a transient timestamp field) before sorting, then sort by Severity and the cached EffectiveUtcTs in the comparator, and finally return the sorted original alert objects (or strip the transient field) so callers of sortAlerts get WeatherAlertResultData[]; update sortAlerts and any helper mapping logic to use the cached timestamp rather than new Date(...) inside the comparator. ``` </details> --- `56-65`: **Missing error state update in `fetchAlertDetail`.** Unlike `fetchActiveAlerts` which sets an `error` state on failure, `fetchAlertDetail` only logs the error. Consider providing user feedback for detail fetch failures, or at minimum clearing `selectedAlert` to avoid showing stale data. <details> <summary>♻️ Proposed enhancement</summary> ```diff fetchAlertDetail: async (alertId: string) => { set({ isLoadingDetail: true }); try { const result = await getWeatherAlert(alertId); set({ selectedAlert: result.Data, isLoadingDetail: false }); } catch (error) { logger.error({ message: 'Failed to fetch weather alert detail', context: { error, alertId } }); - set({ isLoadingDetail: false }); + set({ selectedAlert: null, isLoadingDetail: false }); } }, ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/stores/weatherAlerts/store.ts` around lines 56 - 65, fetchAlertDetail currently logs failures but doesn't update store error state or clear stale selectedAlert; modify fetchAlertDetail to set isLoadingDetail: false and update the store's error (e.g., error: error) and clear selectedAlert (e.g., selectedAlert: null) inside the catch block so UI won't show stale details—keep the existing logger.error call and mirror the error-handling behavior used in fetchActiveAlerts. ``` </details> </blockquote></details> <details> <summary>src/components/widgets/ScheduledCallsWidget.tsx (2)</summary><blockquote> `44-59`: **Unstable function references in useEffect dependency array.** The dependency array includes `init`, `initPersonnel`, and `fetchUnits` which are store action references. While Zustand actions are typically stable, the pattern of including them as deps when they're destructured in the same render can cause unnecessary re-runs if the store reference changes. Consider using `useEffect` without these in the dependency array and relying on the store's stable references, or call them via `getState()`. Additionally, the API calls (`getAllGroups`, `getRecipients`) run in parallel with store inits but errors are silently swallowed. Consider adding error handling or using a combined loading state. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/widgets/ScheduledCallsWidget.tsx` around lines 44 - 59, Remove unstable action references from the useEffect dependency array: call init, initPersonnel, and fetchUnits once inside a useEffect with an empty dependency array (or fetch them via the store's getState() to guarantee stable references) instead of listing the destructured functions as deps; also add basic error handling for the parallel API calls getAllGroups and getRecipients by catching and logging errors (and still calling setGroups([]) / setRoles([]) on failure) so failures aren't silently swallowed — update handlers around getAllGroups, getRecipients, setGroups, and setRoles accordingly. ``` </details> --- `312-367`: **Missing i18n translations for user-facing text.** Several hardcoded English strings should be wrapped in `t()` for internationalization: - Line 314, 324, 333: `"Scheduled Calls"` (widget title) - Line 316: `"Failed to load"` - Line 361: `"No pending scheduled calls"` - Line 285: `"No address"` - Line 296: `"—"` (em-dash placeholder) As per coding guidelines: "Ensure all text is wrapped in `t()` from `react-i18next` for translations." <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/widgets/ScheduledCallsWidget.tsx` around lines 312 - 367, The widget uses hardcoded user-facing strings; update ScheduledCallsWidget to import and use the i18n hook (useTranslation/t) and wrap all visible text in t(...) including the WidgetContainer title ("Scheduled Calls"), the error text ("Failed to load"), the empty state ("No pending scheduled calls"), the "No address" placeholder (used in renderDataCell) and the em-dash placeholder ("—"). Ensure renderHeaderCell/renderDataCell return translated strings (call t inside those functions or pass t as a prop) so getUrgencyColor/filteredCalls/columnOrder logic stays unchanged and keys are meaningful (e.g. t('scheduledCalls.title') etc.). ``` </details> </blockquote></details> <details> <summary>src/models/v4/weatherAlerts/weatherAlertEnums.ts (1)</summary><blockquote> `65-71`: **Consider using i18n for `SEVERITY_LABELS`.** The coding guidelines require all text to be wrapped in `t()` from `react-i18next` for translations. These hardcoded labels will not be translatable for non-English users. Since these labels are used in UI components (`WeatherAlertsWidget.tsx`, `weather-alert-detail.tsx`), consider either: 1. Moving label resolution to the component level where `t()` is available, or 2. Storing translation keys here instead of literal strings. As per coding guidelines: "Ensure all text is wrapped in `t()` from `react-i18next` for translations." <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/models/v4/weatherAlerts/weatherAlertEnums.ts` around lines 65 - 71, SEVERITY_LABELS currently contains hardcoded English strings; change it so UI text is translatable by either (A) replacing the string values with translation keys (e.g., 'weatherAlert.severity.extreme') and update consumers (WeatherAlertsWidget.tsx, weather-alert-detail.tsx) to call t(SEVERITY_LABELS[severity]) before rendering, or (B) move the label resolution out of this file entirely and let the components map WeatherAlertSeverity -> t('...') themselves; update SEVERITY_LABELS or callers accordingly to ensure every displayed label is wrapped in t() from react-i18next. ``` </details> </blockquote></details> <details> <summary>src/components/widgets/WeatherAlertsWidget.tsx (2)</summary><blockquote> `91-120`: **AlertCard component could be memoized.** Since `AlertCard` receives props that may not change frequently (alert data, settings), wrapping it with `React.memo()` could prevent unnecessary re-renders when the parent updates. As per coding guidelines: "Use `React.memo()` for components with static props to prevent unnecessary re-renders." <details> <summary>♻️ Proposed enhancement</summary> ```diff -const AlertCard: React.FC<AlertCardProps> = ({ alert, isDark, showArea, showExpiry, fontSize }) => { +const AlertCard: React.FC<AlertCardProps> = React.memo(({ alert, isDark, showArea, showExpiry, fontSize }) => { // ... component body -}; +}); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/widgets/WeatherAlertsWidget.tsx` around lines 91 - 120, Wrap the AlertCard functional component with React.memo to avoid unnecessary re-renders when its props (alert, isDark, showArea, showExpiry, fontSize) haven't changed; locate the AlertCard declaration and replace its usage/export with a memoized version (e.g., const MemoizedAlertCard = React.memo(AlertCard) or export default React.memo(AlertCard)), optionally provide a custom props comparator if you need deep equality for the alert object. ``` </details> --- `28-42`: **Missing i18n translations for user-facing text.** Multiple hardcoded English strings need to be wrapped in `t()`: - `formatExpiry`: "Expired", "d remaining", "h", "m remaining" - Line 152: "Weather alerts not enabled" - Line 176: "No active alerts" - Line 196: "+{remaining} more alerts" - Widget title: "Weather Alerts" As per coding guidelines: "Ensure all text is wrapped in `t()` from `react-i18next` for translations." Also applies to: 146-181 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/components/widgets/WeatherAlertsWidget.tsx` around lines 28 - 42, The component WeatherAlertsWidget and its helper formatExpiry contain hardcoded English strings; import and use useTranslation from react-i18next (or accept a t prop) and replace all literal UI strings with t(...) calls and interpolation keys — e.g., in formatExpiry replace "Expired" with t('weatherAlerts.expired'), return t('weatherAlerts.remaining_days', { days }) for "d remaining", t('weatherAlerts.remaining_hours', { hours, mins }) for "h {mins}m remaining", and t('weatherAlerts.remaining_mins', { mins }) for "{m}m remaining"; in the JSX replace "Weather Alerts" (widget title), "Weather alerts not enabled", "No active alerts", and "+{remaining} more alerts" with t('weatherAlerts.title'), t('weatherAlerts.notEnabled'), t('weatherAlerts.noActive'), and t('weatherAlerts.moreAlerts', { remaining }) respectively; ensure you add appropriate i18n keys to your locale files and update any tests that assert text. ``` </details> </blockquote></details> <details> <summary>src/stores/widget-settings/scheduled-calls-settings-store.ts (1)</summary><blockquote> `1-4`: **Storage mechanism differs from main widget settings store.** This store uses `AsyncStorage` for persistence, while `src/stores/widget-settings/store.ts` uses `MMKV`. Looking at the context snippets, this matches the pattern of other individual widget settings stores (`calls-settings-store.ts`, `personnel-settings-store.ts`, `units-settings-store.ts`). While functionally fine, be aware of the two parallel persistence mechanisms in use. This appears to be an existing architectural pattern in the codebase. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/stores/widget-settings/scheduled-calls-settings-store.ts` around lines 1 - 4, The store currently imports AsyncStorage and uses it for persistence which diverges from the main widget settings store that uses MMKV; replace the AsyncStorage-based storage with the same MMKV-backed createJSONStorage pattern used in src/stores/widget-settings/store.ts so persistence is consistent. Specifically, remove the AsyncStorage import and change the persist/createJSONStorage configuration in scheduled-calls-settings-store.ts to use the MMKV instance and storage adapter (the same helper or factory used by the main store), keeping the existing zustand create/persist logic and store keys (e.g., the persisted store name) the same. Ensure you reference and reuse the MMKV storage helper used by other widget stores (instead of AsyncStorage) so scheduled calls persistence matches the rest of the widget-settings stores. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@src/app/(app)/configure.tsx:
- Around line 1121-1185: The sliders allow crossing thresholds; enforce
ascending order by clamping adjacent values in the setter and when persisting:
in the onChange handlers that call setFontSizes (keys
scheduledCallsRedThreshold, scheduledCallsYellowThreshold,
scheduledCallsGreenThreshold) clamp the new value against neighbors (e.g., red ≤
yellow ≤ green) using Math.min/Math.max so realtime UI cannot cross, and in
onChangeEnd (which calls updateScheduledCallsColumnSettings) either validate and
reject invalid combos or apply the same clamping before calling
updateScheduledCallsColumnSettings({
colorThresholdRedMinutes/...YellowMinutes/...GreenMinutes }) so persisted
settings remain ordered; also update the displayed Gray text that references
scheduledCallsSettings.colorThresholdGreenMinutes if you convert/clamp before
saving.- Around line 264-265: The new Weather Alerts and Scheduled Calls UI strings are
hardcoded; update all labels, headings, button text, helper text and
placeholders related to these features (e.g., the widget entries with keys
'weatherAlerts' and 'scheduledCalls' and any components/rendering code handling
WidgetType.WEATHER_ALERTS and WidgetType.SCHEDULED_CALLS between the noted
region) to use react-i18next translation calls (t()) instead of literal English.
Import and use the t function (e.g., via const { t } = useTranslation()) where
needed, replace each hardcoded string with t('your.translation.key') and add
matching keys to the src/translations dictionary files. Ensure keys exist for
tab labels, section headings, buttons, helper text and input placeholders so the
content is translatable.- Around line 40-42: The current maxW and maxH artificially cap widget size with
Math.min(..., 8/10) which overrides device-specific limits; change the
assignments so maxW uses gridConfig.maxWidgetWidth and maxH uses
gridConfig.maxWidgetHeight directly (remove the Math.min ceiling) so the grid
can utilize the full device-specific maxWidgetWidth and maxWidgetHeight values;
update or remove the associated comment explaining why no extra ceiling is
applied.In
@src/app/(app)/weather-alert-detail.tsx:
- Around line 16-42: Replace hardcoded English strings on the WeatherAlert
detail screen with translation keys using react-i18next: wrap visible strings
like the loading copy, section headers, and field labels in t('...') and move
enum label text into translation dictionaries, then build URGENCY_LABELS and
CERTAINTY_LABELS by calling t() for each key instead of hardcoding values (refer
to URGENCY_LABELS and CERTAINTY_LABELS), and ensure formatDateTime and any
inline labels in the component call t() where appropriate so all user-facing
text uses the locale dictionaries.- Around line 50-67: The component currently renders whatever selectedAlert
holds even if it doesn't match the requested alertId; update the effect and
render guard so we only consider selectedAlert when selectedAlert.WeatherAlertId
=== alertId: in the useEffect that runs on alertId change (and in the branch
where you call fetchAlertDetail), call useWeatherAlertsStore.setState({
selectedAlert: null }) before fetching so stale data is cleared, and change the
render condition to treat selectedAlert as valid only when
selectedAlert?.WeatherAlertId === alertId alongside isLoadingDetail; reference
the useEffect, fetchAlertDetail, useWeatherAlertsStore, selectedAlert,
isLoadingDetail, alerts, and alertId symbols when making these changes.In
@src/app/(app)/weather-alerts.tsx:
- Around line 63-65: The component currently reads alerts/isLoading from
useWeatherAlertsStore but ignores fetch errors (from fetchActiveAlerts) and
therefore shows the empty "No active alerts" state on network/API failures;
update the render logic to also read the store's error (e.g. error or
fetchError) from useWeatherAlertsStore and display an error UI when present:
show a concise error message, a Retry button that calls fetchActiveAlerts, and
only fall back to the "No active alerts" empty state when there is no error and
alerts is empty and isLoading is false; apply the same change to the other
empty-state render sites noted (around the other render blocks referenced) so
all empty states handle errors consistently.- Around line 26-57: The hardcoded strings in this file must be routed through
t() for translation: wrap the label strings in FILTERS and SORTS (e.g., FILTERS
constant and SORTS constant), any header/empty-state text used in the component,
and the outputs from formatDateTime and formatExpiry where they return
human-readable strings (update formatExpiry to return translated fragments like
'Expired', 'd', 'h', 'm', and 'remaining' via t()). Import and t from
react-i18next at the top, replace literal English strings with t('your.key')
calls using keys that match the project's src/translations dictionary, and
ensure the component uses those keys for header/title, card labels, filter/sort
options, and relative-time parts so all UI text is translatable.In
@src/components/calls/call-files-modal.tsx:
- Line 3: The import is using an internal path; in
src/components/calls/call-files-modal.tsx replace the internal import
"expo-file-system/src/legacy" with the public legacy entrypoint
"expo-file-system/legacy" for the imported symbols documentDirectory,
EncodingType, and writeAsStringAsync so the module uses the supported public API
and won't break on Expo updates.In
@src/components/calls/call-images-modal.tsx:
- Line 1: The import is using the internal path 'expo-file-system/src/legacy'
which can break on updates; update the import for EncodingType and
readAsStringAsync to use the public legacy entrypoint 'expo-file-system/legacy'
(i.e., replace the import source in the module that imports EncodingType and
readAsStringAsync in call-images-modal.tsx).In
@src/components/Dashboard.tsx:
- Around line 126-134: baseX and baseY are adding gridPadding while the
container already applies padding, resulting in double-offset widgets; update
the calculations in Dashboard so baseX = position.x * baseWidth and baseY =
position.y * baseHeight (remove the "+ gridPadding"), and keep the container
padding/height calculations as-is (adjust only if you opt instead to remove the
container padding); ensure you also remove any other places where individual
widget positions add gridPadding (search for usages of baseX/baseY or " +
gridPadding") so positioning, container height computation (the gridPadding * 2
usage), and overflow behavior remain consistent.In
@src/stores/signalr/signalr-store.ts:
- Around line 179-206: The three SignalR handlers (the signalRService.on
callbacks) currently call String(message) and pass it to
useWeatherAlertsStore.getState().handleAlertReceived/handleAlertUpdated/handleAlertExpired
and set(...) without validating the payload; change each handler to first
validate the incoming message with a type guard and/or regex (e.g., ensure
typeof message === 'string' and it matches the expected alert ID format or is a
non-empty string/UUID) and if invalid log a warning via logger.warn and return
early; only convert/use the message as alertId and call the corresponding
useWeatherAlertsStore.getState().handleAlert* and set({ lastUpdateMessage...,
lastUpdateTimestamp }) when the payload passes validation.In
@src/stores/widget-settings/store.ts:
- Around line 104-125: ScheduledCallsWidgetSettings is duplicated with
inconsistent defaults and missing properties (e.g., differing fontSize and
absent columnOrder) between this file and the dedicated
scheduled-calls-settings-store; remove the duplication by importing and
re-exporting the single source of truth: replace the local
ScheduledCallsWidgetSettings declaration with an import of
ScheduledCallsWidgetSettings from the dedicated scheduled-calls-settings-store
(or delete the other duplicate and export from here), ensure the shared type
includes columnOrder and the intended default values (aligned fontSize and
colorThresholds) so all consumers use the same interface.In
@src/types/widget.ts:
- Around line 103-104: The WIDGET_LABELS map currently contains hardcoded
English strings for WidgetType.WEATHER_ALERTS and WidgetType.SCHEDULED_CALLS;
change these to translation keys (e.g., 'widget.weatherAlerts',
'widget.scheduledCalls') or ensure the consumer calls t() when rendering (see
Dashboard component render at Dashboard.tsx where WIDGET_LABELS is used). Update
the entries in the WIDGET_LABELS constant (referencing WidgetType.WEATHER_ALERTS
and WidgetType.SCHEDULED_CALLS) to return keys instead of plain text, and/or
wrap the lookup in t(WIDGET_LABELS[widgetType]) at render time so all labels are
translated via react-i18next.
Minor comments:
In@app.config.ts:
- Line 274: Remove the redundant bare plugin entry '@sentry/react-native' from
the plugins array and keep only the configured '@sentry/react-native/expo'
entry; locate the plugins list where both strings appear and delete the plain
'@sentry/react-native' line so only '@sentry/react-native/expo' with your
organization/project settings remains.In
@CLAUDE.md:
- Around line 62-64: The fenced code block containing the Graph helper call
graph_add_memory(type="decision|task|next|fact|blocker", content="one sentence
max 15 words", tags=["topic"], files=["relevant/file.ts"]) is unlabeled and
triggers markdownlint MD040; add a language tag to the opening fence (for
example usetext orbash) so the block is explicitly typed and MD040 is
satisfied, keeping the content unchanged.In
@src/components/widgets/WeatherAlertsWidget.tsx:
- Around line 140-144: The handlePress function is bypassing TypeScript route
checks by using "as any" on router.push; remove the "as any" cast and call
router.push('/(app)/weather-alerts') with proper typing, or update your route
type definitions so '/(app)/weather-alerts' is a valid typed route.
Specifically, change the router.push call in handlePress to use the typed push
signature (or a typed UrlObject) and ensure your router/route config (types for
AppRouterInstance or your route union) includes the '(app)/weather-alerts' route
so no cast is necessary.In
@src/hooks/use-grid-config.ts:
- Around line 17-26: The breakpoint for 600px is inconsistent between the web
and native branches in use-grid-config (Platform.OS === 'web' branch uses width600 while the native branch uses width >= 600); make them consistent by
changing the web branch to use width >= 600 so that width === 600 is treated as
'tablet' everywhere (update the conditional in the web branch where it currently
reads width > 600).In
@src/stores/scheduledCalls/store.ts:
- Around line 90-92: fetchScheduledCalls currently swallows exceptions in its
catch block unlike init(), so update fetchScheduledCalls's catch to log the
caught error with the same logger used in init (include error object and
contextual message like "Failed to fetch scheduled calls") before calling set({
error: 'Failed to fetch scheduled calls', isLoading: false }); reference the
fetchScheduledCalls function and the set call to locate the catch block and
ensure the log mirrors init()'s error logging pattern.In
@src/stores/weatherAlerts/store.ts:
- Around line 76-84: handleAlertReceived may prepend a duplicate alert when the
same WeatherAlertId already exists; modify handleAlertReceived (which calls
getWeatherAlert and set with sortAlerts) to first check the current alerts array
(from get()) for an existing item with the same WeatherAlertId, and either skip
adding if found or replace the existing item with the fresh result before
calling set({ alerts: sortAlerts(...) }). Ensure comparison uses the
WeatherAlertId field from result.Data and existing alerts to avoid duplicates on
reconnection/duplicate events.In
@src/translations/ar.json:
- Around line 582-588: The Arabic locale contains untranslated English entries
under the "scheduledCalls" object (keys: "activatesIn", "noScheduledCalls",
"overdue", "scheduledTime", "title") and other untranslated strings in the
weather alerts block (the entries referenced in the same file range). Replace
each English value with the correct Arabic translations for those keys so the
Scheduled Calls and Weather Alerts screens are fully localized; keep the JSON
keys unchanged and only update the string values.In
@src/translations/es.json:
- Around line 582-588: The Spanish translations file contains English text for
the scheduled calls block and the weather alerts block; update the values for
the keys under "scheduledCalls" (activatesIn, noScheduledCalls, overdue,
scheduledTime, title) to proper Spanish phrases, and similarly translate all
English strings in the "weatherAlerts" object (the block around the 723-802
region) so the UI shows fully localized Spanish text; keep the JSON keys
unchanged and only replace the English string values with their Spanish
equivalents.
Nitpick comments:
In@package.json:
- Around line 251-260: The expo.install.exclude list in package.json contains
stale entries (e.g., "react-native@~0.76.6", "react-native-reanimated@~3.16.1")
that no longer match the project's current dependency versions; update or remove
these exclusions under the expo.install.exclude key so they reflect the RN
0.81.5 and reanimated 4.1.1 upgrades (either delete obsolete package entries or
replace the versions with current ones) and run a dependency install/test to
verify nothing breaks.In
@src/app/(app)/_layout.tsx:
- Line 199: The long single-line Promise.all call is hard to read; split it into
multiple lines by assigning each async call
(useCoreStore.getState().fetchConfig(), useCallsStore.getState().fetchCalls(),
useRolesStore.getState().fetchRoles(),
useWeatherAlertsStore.getState().fetchActiveAlerts()) into the Promise.all array
on its own line (or into named constants first) so each call is on a separate
line for readability and easier diffing, then await Promise.all([...]) as
before.In
@src/app/(app)/weather-alerts.tsx:
- Around line 103-105: Remove the unsafe "as any" cast on the router.push call
in the Pressable and use the strongly-typed route object from expo-router
instead: update the call to router.push to pass a properly typed route
(preserving pathname '/(app)/weather-alert-detail' and params.alertId =
item.WeatherAlertId) and import/use the expo-router route/params type or your
app's generated route types so TypeScript validates the pathname and params at
compile time; specifically modify the router.push invocation near the Pressable
component (the onPress handler) to use the typed route object rather than
casting to any.In
@src/components/widgets/ScheduledCallsWidget.tsx:
- Around line 44-59: Remove unstable action references from the useEffect
dependency array: call init, initPersonnel, and fetchUnits once inside a
useEffect with an empty dependency array (or fetch them via the store's
getState() to guarantee stable references) instead of listing the destructured
functions as deps; also add basic error handling for the parallel API calls
getAllGroups and getRecipients by catching and logging errors (and still calling
setGroups([]) / setRoles([]) on failure) so failures aren't silently swallowed —
update handlers around getAllGroups, getRecipients, setGroups, and setRoles
accordingly.- Around line 312-367: The widget uses hardcoded user-facing strings; update
ScheduledCallsWidget to import and use the i18n hook (useTranslation/t) and wrap
all visible text in t(...) including the WidgetContainer title ("Scheduled
Calls"), the error text ("Failed to load"), the empty state ("No pending
scheduled calls"), the "No address" placeholder (used in renderDataCell) and the
em-dash placeholder ("—"). Ensure renderHeaderCell/renderDataCell return
translated strings (call t inside those functions or pass t as a prop) so
getUrgencyColor/filteredCalls/columnOrder logic stays unchanged and keys are
meaningful (e.g. t('scheduledCalls.title') etc.).In
@src/components/widgets/WeatherAlertsWidget.tsx:
- Around line 91-120: Wrap the AlertCard functional component with React.memo to
avoid unnecessary re-renders when its props (alert, isDark, showArea,
showExpiry, fontSize) haven't changed; locate the AlertCard declaration and
replace its usage/export with a memoized version (e.g., const MemoizedAlertCard
= React.memo(AlertCard) or export default React.memo(AlertCard)), optionally
provide a custom props comparator if you need deep equality for the alert
object.- Around line 28-42: The component WeatherAlertsWidget and its helper
formatExpiry contain hardcoded English strings; import and use useTranslation
from react-i18next (or accept a t prop) and replace all literal UI strings with
t(...) calls and interpolation keys — e.g., in formatExpiry replace "Expired"
with t('weatherAlerts.expired'), return t('weatherAlerts.remaining_days', { days
}) for "d remaining", t('weatherAlerts.remaining_hours', { hours, mins }) for "h
{mins}m remaining", and t('weatherAlerts.remaining_mins', { mins }) for "{m}m
remaining"; in the JSX replace "Weather Alerts" (widget title), "Weather alerts
not enabled", "No active alerts", and "+{remaining} more alerts" with
t('weatherAlerts.title'), t('weatherAlerts.notEnabled'),
t('weatherAlerts.noActive'), and t('weatherAlerts.moreAlerts', { remaining })
respectively; ensure you add appropriate i18n keys to your locale files and
update any tests that assert text.In
@src/models/v4/weatherAlerts/weatherAlertEnums.ts:
- Around line 65-71: SEVERITY_LABELS currently contains hardcoded English
strings; change it so UI text is translatable by either (A) replacing the string
values with translation keys (e.g., 'weatherAlert.severity.extreme') and update
consumers (WeatherAlertsWidget.tsx, weather-alert-detail.tsx) to call
t(SEVERITY_LABELS[severity]) before rendering, or (B) move the label resolution
out of this file entirely and let the components map WeatherAlertSeverity ->
t('...') themselves; update SEVERITY_LABELS or callers accordingly to ensure
every displayed label is wrapped in t() from react-i18next.In
@src/models/v4/weatherAlerts/weatherAlertSettingsData.ts:
- Around line 1-11: The new interfaces WeatherAlertSeverityScheduleData and
WeatherAlertSettingsData live in camelCase path segments; rename the directory
segment "weatherAlerts" to "weather-alerts" and the file
"weatherAlertSettingsData.ts" to "weather-alert-settings-data.ts" and update all
imports/exports referencing these symbols to use the new lowercase-hyphenated
names so project naming conventions remain consistent.In
@src/stores/auth/store.tsx:
- Around line 237-245: Extract the repeated validation into a well-named boolean
(e.g., isValidExpiresIn) to avoid duplicate checks: evaluate isValidExpiresIn =
typeof rawExpiresIn === 'number' && rawExpiresIn > 0, then use it to set
expiresInSeconds (rawExpiresIn or default 3600) and to gate the logger.warn
call; update the computation of expiresOn to use expiresInSeconds as before.
This change touches the rawExpiresIn, expiresInSeconds, logger.warn and
expiresOn logic in the LoginWithSso flow.In
@src/stores/scheduledCalls/store.ts:
- Around line 39-57: Extract the duplicated extra-data fetch logic into a
private helper, e.g., fetchExtraDataForCalls(calls, get, set): it should create
fetchId = Date.now(), set lastCallExtraDataFetchId, call Promise.allSettled over
calls.map(c => getScheduledCallExtraData(c.CallId)), bail out if
get().lastCallExtraDataFetchId !== fetchId, build newExtraDataMap from fulfilled
results (using CallId keys) and finally set callExtraDataMap (or {} if no calls)
— then call this helper from both init() and fetchScheduledCalls() in place of
the duplicated blocks to preserve existing behavior and state names
(lastCallExtraDataFetchId, callExtraDataMap) and reuse
getScheduledCallExtraData.In
@src/stores/weatherAlerts/store.ts:
- Around line 26-35: The sortAlerts function repeatedly constructs Date objects
during comparisons, hurting performance on large arrays; to fix, precompute
numeric timestamps for each alert (e.g., map WeatherAlertResultData items to
include an EffectiveUtcTs or a transient timestamp field) before sorting, then
sort by Severity and the cached EffectiveUtcTs in the comparator, and finally
return the sorted original alert objects (or strip the transient field) so
callers of sortAlerts get WeatherAlertResultData[]; update sortAlerts and any
helper mapping logic to use the cached timestamp rather than new Date(...)
inside the comparator.- Around line 56-65: fetchAlertDetail currently logs failures but doesn't update
store error state or clear stale selectedAlert; modify fetchAlertDetail to set
isLoadingDetail: false and update the store's error (e.g., error: error) and
clear selectedAlert (e.g., selectedAlert: null) inside the catch block so UI
won't show stale details—keep the existing logger.error call and mirror the
error-handling behavior used in fetchActiveAlerts.In
@src/stores/widget-settings/scheduled-calls-settings-store.ts:
- Around line 1-4: The store currently imports AsyncStorage and uses it for
persistence which diverges from the main widget settings store that uses MMKV;
replace the AsyncStorage-based storage with the same MMKV-backed
createJSONStorage pattern used in src/stores/widget-settings/store.ts so
persistence is consistent. Specifically, remove the AsyncStorage import and
change the persist/createJSONStorage configuration in
scheduled-calls-settings-store.ts to use the MMKV instance and storage adapter
(the same helper or factory used by the main store), keeping the existing
zustand create/persist logic and store keys (e.g., the persisted store name) the
same. Ensure you reference and reuse the MMKV storage helper used by other
widget stores (instead of AsyncStorage) so scheduled calls persistence matches
the rest of the widget-settings stores.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `14838a6b-a548-426c-86a5-3c4e9c807b2f` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 5e6ce9e18acd75446c2df27515e594c8edb05e04 and acf0f0d8974508e4c222184c3cdb5ebe8db81604. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `yarn.lock` is excluded by `!**/yarn.lock`, `!**/*.lock` </details> <details> <summary>📒 Files selected for processing (39)</summary> * `.gitignore` * `CLAUDE.md` * `NOTICE` * `app.config.ts` * `package.json` * `src/api/scheduledCalls/scheduledCalls.ts` * `src/api/weatherAlerts/weatherAlerts.ts` * `src/app/(app)/_layout.tsx` * `src/app/(app)/configure.tsx` * `src/app/(app)/weather-alert-detail.tsx` * `src/app/(app)/weather-alerts.tsx` * `src/components/Dashboard.tsx` * `src/components/calls/auto-scrolling-dispatches.tsx` * `src/components/calls/call-files-modal.tsx` * `src/components/calls/call-images-modal.tsx` * `src/components/widgets/ScheduledCallsWidget.tsx` * `src/components/widgets/WeatherAlertsWidget.tsx` * `src/components/widgets/WidgetRenderer.tsx` * `src/hooks/use-grid-config.ts` * `src/models/v4/weatherAlerts/activeWeatherAlertsResult.ts` * `src/models/v4/weatherAlerts/weatherAlertEnums.ts` * `src/models/v4/weatherAlerts/weatherAlertResult.ts` * `src/models/v4/weatherAlerts/weatherAlertResultData.ts` * `src/models/v4/weatherAlerts/weatherAlertSettingsData.ts` * `src/models/v4/weatherAlerts/weatherAlertSettingsResult.ts` * `src/models/v4/weatherAlerts/weatherAlertZoneResultData.ts` * `src/models/v4/weatherAlerts/weatherAlertZonesResult.ts` * `src/stores/auth/store.tsx` * `src/stores/dashboard/store.ts` * `src/stores/scheduledCalls/store.ts` * `src/stores/signalr/signalr-store.ts` * `src/stores/weatherAlerts/store.ts` * `src/stores/widget-settings/index.ts` * `src/stores/widget-settings/scheduled-calls-settings-store.ts` * `src/stores/widget-settings/store.ts` * `src/translations/ar.json` * `src/translations/en.json` * `src/translations/es.json` * `src/types/widget.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/settings/language-item.tsx (1)
23-35:⚠️ Potential issue | 🟠 MajorLanguage labels are frozen after first render and may not update on locale switch.
langsis memoized with[], so translated labels are computed once. AftersetLanguage, option labels can remain in the old locale until remount. Uset(...)for labels and includetin memo dependencies (or remove the memo).🔧 Proposed fix
- const langs = React.useMemo( - () => [ - { label: translate('settings.english'), value: 'en' }, - { label: translate('settings.spanish'), value: 'es' }, - { label: translate('settings.swedish'), value: 'sv' }, - { label: translate('settings.german'), value: 'de' }, - { label: translate('settings.french'), value: 'fr' }, - { label: translate('settings.italian'), value: 'it' }, - { label: translate('settings.polish'), value: 'pl' }, - { label: translate('settings.ukrainian'), value: 'uk' }, - { label: translate('settings.arabic'), value: 'ar' }, - ], - [] - ); + const langs = React.useMemo<Array<{ label: string; value: Language }>>( + () => [ + { label: t('settings.english'), value: 'en' }, + { label: t('settings.spanish'), value: 'es' }, + { label: t('settings.swedish'), value: 'sv' }, + { label: t('settings.german'), value: 'de' }, + { label: t('settings.french'), value: 'fr' }, + { label: t('settings.italian'), value: 'it' }, + { label: t('settings.polish'), value: 'pl' }, + { label: t('settings.ukrainian'), value: 'uk' }, + { label: t('settings.arabic'), value: 'ar' }, + ], + [t] + );As per coding guidelines, “Ensure all text is wrapped in
t()fromreact-i18nextfor translations with the dictionary files stored insrc/translations.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/settings/language-item.tsx` around lines 23 - 35, The langs array is memoized with an empty dependency list so labels produced by translate(...) won't update when the locale changes; update the component to use the i18next t function for labels (replace translate(...) with t(...)) and include t in the React.useMemo dependency array (or remove useMemo entirely) so that langs is recomputed after calling setLanguage; target the langs constant and any usages of translate and setLanguage when making the change.
♻️ Duplicate comments (1)
src/app/(app)/weather-alert-detail.tsx (1)
33-34:⚠️ Potential issue | 🟡 MinorHardcoded 'N/A' string bypasses i18n.
The fallback string
'N/A'informatDateTimeis not translated. Use the existingcall_detail.not_availablekey for consistency.🌐 Suggested fix
const formatDateTime = (utc: string): string => { - if (!utc) return 'N/A'; + if (!utc) return t('call_detail.not_available'); return new Date(utc).toLocaleString(undefined, {Note: Since
formatDateTimeis defined outside the component, you'll need to either move it inside or passtas a parameter.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/weather-alert-detail.tsx around lines 33 - 34, The fallback 'N/A' in formatDateTime bypasses i18n; update formatDateTime to return the translated string using the existing key call_detail.not_available by either moving formatDateTime inside the React component so it can call t(...) directly or changing its signature to accept a t parameter (e.g., formatDateTime(utc: string, t: TFunction)): when utc is falsy return t('call_detail.not_available') and keep the rest of the formatting logic unchanged.
🧹 Nitpick comments (5)
src/app/(app)/_layout.tsx (1)
199-199: Gate resume-time alert fetch by weather-alert settings.Line 199 calls
fetchActiveAlerts()unconditionally on resume. SinceuseWeatherAlertsStore.init()already honorssettings?.WeatherAlertsEnabled, mirroring that guard here avoids unnecessary requests when alerts are disabled.♻️ Suggested refactor
- await Promise.all([useCoreStore.getState().fetchConfig(), useCallsStore.getState().fetchCalls(), useRolesStore.getState().fetchRoles(), useWeatherAlertsStore.getState().fetchActiveAlerts()]); + const weatherAlertsStore = useWeatherAlertsStore.getState(); + const shouldRefreshWeatherAlerts = weatherAlertsStore.settings?.WeatherAlertsEnabled !== false; + + await Promise.all([ + useCoreStore.getState().fetchConfig(), + useCallsStore.getState().fetchCalls(), + useRolesStore.getState().fetchRoles(), + ...(shouldRefreshWeatherAlerts ? [weatherAlertsStore.fetchActiveAlerts()] : []), + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/_layout.tsx at line 199, The resume logic currently calls useWeatherAlertsStore.getState().fetchActiveAlerts() unconditionally; change it to only call fetchActiveAlerts when weather alerts are enabled by checking the same flag used in useWeatherAlertsStore.init() (settings?.WeatherAlertsEnabled). Modify the Promise.all invocation to conditionally include useWeatherAlertsStore.getState().fetchActiveAlerts() (or skip it) based on settings?.WeatherAlertsEnabled so no network request happens when alerts are disabled.README.md (1)
117-128: Add a language specifier to the fenced code block.The code block at line 117 is missing a language identifier, which affects syntax highlighting and linting.
📝 Suggested fix
-``` +```text src/ ├── app/ # Expo Router screens (file-based routing)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 117 - 128, The fenced code block showing the project tree in README.md is missing a language specifier; update the opening fence from ``` to ```text (or another appropriate language like ```bash) so syntax highlighting and linting work correctly for the directory listing; locate the block that begins with the tree snippet (lines around the src/ ├── app/ entry) and change its opening fence to include the language identifier.src/app/(app)/weather-alerts.tsx (1)
100-103:SEVERITY_LABELSis still hardcoded English.The
SEVERITY_LABELSmap imported fromweatherAlertEnums.tscontains hardcoded English strings ('Extreme','Severe', etc.). Whiletranslate('common.unknown')is used as a fallback, the primary severity labels bypass i18n.Consider using translation keys for severity labels to maintain consistency with the rest of the i18n implementation.
♻️ Suggested approach
-const severityLabel = SEVERITY_LABELS[item.Severity as WeatherAlertSeverity] || translate('common.unknown'); +const severityLabel = translate(`weatherAlerts.severity.${SEVERITY_LABELS[item.Severity as WeatherAlertSeverity]?.toLowerCase()}`) || translate('common.unknown');Or create a mapping function that uses the existing
weatherAlerts.severity.*keys from en.json.🤖 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 100 - 103, SEVERITY_LABELS in weatherAlertEnums.ts is hardcoded English; update renderAlertCard to use i18n by replacing direct lookups of SEVERITY_LABELS[item.Severity] with a translation lookup (e.g., translate(`weatherAlerts.severity.${key}`)) or add a helper getSeverityLabel(severity: WeatherAlertSeverity) that maps severity enum -> translation key and calls translate; update references to SEVERITY_LABELS in renderAlertCard and ensure fallback still uses translate('common.unknown') so labels come from the en.json weatherAlerts.severity.* keys.src/app/(app)/configure.tsx (1)
446-487: Several existing widget tabs still have hardcoded English strings.The Weather, Units, Notes, and Time widget configuration sections contain hardcoded strings (e.g.,
"Weather Widget","Units","Options","24-Hour Format"). While these are pre-existing and not introduced by this PR, they should be addressed for full i18n compliance.As per coding guidelines: "Ensure all text is wrapped in
t()fromreact-i18nextfor translations."Also applies to: 489-532, 629-677
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(app)/configure.tsx around lines 446 - 487, The Weather widget UI contains hardcoded display strings (e.g., "Weather Widget", "Options", "Units", placeholder texts, button labels) — update these to use the react-i18next t() function: replace literal Text contents, ButtonText labels, and InputField placeholder values in the Weather block that references weather, updateWeatherSettings, and WidgetSizeSection with t('your.key') keys (create descriptive keys like 'widget.weather.title', 'widget.weather.options', 'widget.weather.units.standard', 'widget.weather.latitude.placeholder', etc.); ensure you import/use the t function from useTranslation in this configure.tsx component and mirror the same pattern for the other widget blocks called out (the blocks around lines 489-532 and 629-677) so all user-facing strings are wrapped with t().src/stores/dashboard/store.ts (1)
65-65: Widget name is translated once at creation time, not at render time.Calling
translate(...)insideaddWidgetbakes the localized string into the persisted widget object. If a user later changes their language, existing widgets will still display the old language's label.Consider storing only the
WidgetType(or the translation key) and resolving the display name at render time instead.♻️ Suggested approach
const newWidget: Widget = { id, key: id, type, - name: translate(WIDGET_LABEL_KEYS[type] as any), + name: WIDGET_LABEL_KEYS[type], // Store the key, translate at render time x: 0, y: 0, w: size.w, h: size.h, };Then at render time in Dashboard.tsx, use
translate(widget.name)instead of displayingwidget.namedirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/dashboard/store.ts` at line 65, The widget name is being translated once in addWidget (name: translate(WIDGET_LABEL_KEYS[type])) which bakes a localized string into the persisted widget; change addWidget so it stores the WidgetType or the translation key (e.g., set widget.name to WIDGET_LABEL_KEYS[type] or a new widget.labelKey) instead of the translated string, and update the renderer (Dashboard.tsx) to call translate(widget.name or widget.labelKey) at render time so labels update when the user changes language; adjust any references to widget.name accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/__tests__/maintenance.test.tsx`:
- Around line 72-79: The test mutates Env.MAINTENANCE_MODE without cleanup and
uses an un-awaited async assertion; update the test suite to reset
MAINTENANCE_MODE in a beforeEach (or restore it after the test) to avoid state
leakage across tests, and change the assertion to await waitFor(...) so the
expect(mockReplace).toHaveBeenCalledWith('/home') actually waits; locate the
test referencing Env.MAINTENANCE_MODE, the Maintenance component render call,
and the waitFor/assert that checks mockReplace to apply these changes.
In `@src/translations/es.json`:
- Line 356: Update the Spanish translation value for the key
"filter_placeholder" to use the Spanish abbreviation "p. ej." instead of "e.g.";
keep the placeholder token "{{example}}" and surrounding punctuation/spacing
intact so only the abbreviation is changed (e.g., "p. ej. {{example}}").
- Around line 677-683: Translate the scheduledCalls block in es.json by
replacing the English strings for the keys scheduledCalls.activatesIn,
scheduledCalls.noScheduledCalls, scheduledCalls.overdue,
scheduledCalls.scheduledTime, and scheduledCalls.title with Spanish equivalents
(e.g., "Activa en"/"Se activa en" for activatesIn, "No hay llamadas programadas
pendientes" for noScheduledCalls, "Atrasadas" or "Vencidas" for overdue, "Hora
programada" for scheduledTime, and "Llamadas programadas" for title) so the
entire scheduledCalls object is localized to Spanish.
- Around line 824-906: The weatherAlerts namespace contains many English
strings; translate every value under "weatherAlerts" into Spanish (including
nested keys: category.*, certainty.*, detail.*, filter.*, severity.*, sort.*,
status.*, urgency.*, and keys like errorLoading, expired, loadingDetail,
moreAlerts, noActiveAlerts, notEnabled, remaining, time_days_hours,
time_hours_minutes, time_minutes, title, tryChangingFilter) while preserving
interpolation placeholders (e.g., {{count}}, {{days}}, {{hours}}, {{minutes}})
and JSON structure/quoting; update each value to the proper Spanish phrase so
the entire block is consistently localized.
---
Outside diff comments:
In `@src/components/settings/language-item.tsx`:
- Around line 23-35: The langs array is memoized with an empty dependency list
so labels produced by translate(...) won't update when the locale changes;
update the component to use the i18next t function for labels (replace
translate(...) with t(...)) and include t in the React.useMemo dependency array
(or remove useMemo entirely) so that langs is recomputed after calling
setLanguage; target the langs constant and any usages of translate and
setLanguage when making the change.
---
Duplicate comments:
In `@src/app/`(app)/weather-alert-detail.tsx:
- Around line 33-34: The fallback 'N/A' in formatDateTime bypasses i18n; update
formatDateTime to return the translated string using the existing key
call_detail.not_available by either moving formatDateTime inside the React
component so it can call t(...) directly or changing its signature to accept a t
parameter (e.g., formatDateTime(utc: string, t: TFunction)): when utc is falsy
return t('call_detail.not_available') and keep the rest of the formatting logic
unchanged.
---
Nitpick comments:
In `@README.md`:
- Around line 117-128: The fenced code block showing the project tree in
README.md is missing a language specifier; update the opening fence from ``` to
```text (or another appropriate language like ```bash) so syntax highlighting
and linting work correctly for the directory listing; locate the block that
begins with the tree snippet (lines around the src/ ├── app/ entry) and change
its opening fence to include the language identifier.
In `@src/app/`(app)/_layout.tsx:
- Line 199: The resume logic currently calls
useWeatherAlertsStore.getState().fetchActiveAlerts() unconditionally; change it
to only call fetchActiveAlerts when weather alerts are enabled by checking the
same flag used in useWeatherAlertsStore.init() (settings?.WeatherAlertsEnabled).
Modify the Promise.all invocation to conditionally include
useWeatherAlertsStore.getState().fetchActiveAlerts() (or skip it) based on
settings?.WeatherAlertsEnabled so no network request happens when alerts are
disabled.
In `@src/app/`(app)/configure.tsx:
- Around line 446-487: The Weather widget UI contains hardcoded display strings
(e.g., "Weather Widget", "Options", "Units", placeholder texts, button labels) —
update these to use the react-i18next t() function: replace literal Text
contents, ButtonText labels, and InputField placeholder values in the Weather
block that references weather, updateWeatherSettings, and WidgetSizeSection with
t('your.key') keys (create descriptive keys like 'widget.weather.title',
'widget.weather.options', 'widget.weather.units.standard',
'widget.weather.latitude.placeholder', etc.); ensure you import/use the t
function from useTranslation in this configure.tsx component and mirror the same
pattern for the other widget blocks called out (the blocks around lines 489-532
and 629-677) so all user-facing strings are wrapped with t().
In `@src/app/`(app)/weather-alerts.tsx:
- Around line 100-103: SEVERITY_LABELS in weatherAlertEnums.ts is hardcoded
English; update renderAlertCard to use i18n by replacing direct lookups of
SEVERITY_LABELS[item.Severity] with a translation lookup (e.g.,
translate(`weatherAlerts.severity.${key}`)) or add a helper
getSeverityLabel(severity: WeatherAlertSeverity) that maps severity enum ->
translation key and calls translate; update references to SEVERITY_LABELS in
renderAlertCard and ensure fallback still uses translate('common.unknown') so
labels come from the en.json weatherAlerts.severity.* keys.
In `@src/stores/dashboard/store.ts`:
- Line 65: The widget name is being translated once in addWidget (name:
translate(WIDGET_LABEL_KEYS[type])) which bakes a localized string into the
persisted widget; change addWidget so it stores the WidgetType or the
translation key (e.g., set widget.name to WIDGET_LABEL_KEYS[type] or a new
widget.labelKey) instead of the translated string, and update the renderer
(Dashboard.tsx) to call translate(widget.name or widget.labelKey) at render time
so labels update when the user changes language; adjust any references to
widget.name accordingly.
🪄 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: 0a67d689-9423-4d23-a273-fedf78716fd6
📒 Files selected for processing (33)
README.mdsrc/app/(app)/_layout.tsxsrc/app/(app)/configure.tsxsrc/app/(app)/weather-alert-detail.tsxsrc/app/(app)/weather-alerts.tsxsrc/app/[...messing].tsxsrc/app/__tests__/maintenance.test.tsxsrc/app/maintenance.tsxsrc/components/Dashboard.tsxsrc/components/bluetooth/bluetooth-audio-modal.tsxsrc/components/calls/__tests__/call-files-modal.test.tsxsrc/components/calls/call-files-modal.tsxsrc/components/calls/call-images-modal.tsxsrc/components/notifications/NotificationDetail.tsxsrc/components/notifications/NotificationInbox.tsxsrc/components/push-notification/push-notification-status.tsxsrc/components/roles/roles-modal.tsxsrc/components/settings/language-item.tsxsrc/components/ui/list.tsxsrc/lib/i18n/resources.tssrc/stores/dashboard/store.tssrc/stores/signalr/signalr-store.tssrc/stores/widget-settings/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.jsonsrc/types/widget.ts
✅ Files skipped from review due to trivial changes (11)
- src/components/calls/call-images-modal.tsx
- src/components/ui/list.tsx
- src/components/notifications/NotificationDetail.tsx
- src/components/calls/call-files-modal.tsx
- src/translations/pl.json
- src/translations/fr.json
- src/translations/uk.json
- src/translations/it.json
- src/translations/de.json
- src/translations/sv.json
- src/lib/i18n/resources.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/translations/ar.json
- src/components/Dashboard.tsx
- src/stores/widget-settings/store.ts
Comment on lines
+72
to
79
| it('should redirect to home if maintenance mode is disabled', () => { | ||
| (Env as any).MAINTENANCE_MODE = false; | ||
|
|
||
| render(<Maintenance />); | ||
|
|
||
| waitFor(() => { | ||
| expect(mockReplace).toHaveBeenCalledWith('/login'); | ||
| expect(mockReplace).toHaveBeenCalledWith('/home'); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify un-awaited waitFor usage and env mutation in this test file
rg -n -C2 '\bwaitFor\s*\(' src/app/__tests__/maintenance.test.tsx
rg -n -C2 'MAINTENANCE_MODE\s*=\s*(true|false)' src/app/__tests__/maintenance.test.tsxRepository: Resgrid/BigBoard
Length of output: 350
Fix un-awaited async assertion and test state isolation issue.
Line 77 must await the waitFor call to ensure the assertion actually waits for the condition. Additionally, line 73 mutates Env.MAINTENANCE_MODE without cleanup, causing state leakage that will fail the next test (line 82). Reset MAINTENANCE_MODE in beforeEach to maintain test isolation.
Proposed fix
describe('Maintenance', () => {
const mockReplace = jest.fn();
beforeEach(() => {
jest.clearAllMocks();
+ (Env as any).MAINTENANCE_MODE = true;
(useRouter as jest.Mock).mockReturnValue({
replace: mockReplace,
});
});
- it('should redirect to home if maintenance mode is disabled', () => {
+ it('should redirect to home if maintenance mode is disabled', async () => {
(Env as any).MAINTENANCE_MODE = false;
render(<Maintenance />);
- waitFor(() => {
+ await waitFor(() => {
expect(mockReplace).toHaveBeenCalledWith('/home');
});
});
});📝 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.
Suggested change
| it('should redirect to home if maintenance mode is disabled', () => { | |
| (Env as any).MAINTENANCE_MODE = false; | |
| render(<Maintenance />); | |
| waitFor(() => { | |
| expect(mockReplace).toHaveBeenCalledWith('/login'); | |
| expect(mockReplace).toHaveBeenCalledWith('/home'); | |
| }); | |
| it('should redirect to home if maintenance mode is disabled', async () => { | |
| (Env as any).MAINTENANCE_MODE = false; | |
| render(<Maintenance />); | |
| await waitFor(() => { | |
| expect(mockReplace).toHaveBeenCalledWith('/home'); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/__tests__/maintenance.test.tsx` around lines 72 - 79, The test
mutates Env.MAINTENANCE_MODE without cleanup and uses an un-awaited async
assertion; update the test suite to reset MAINTENANCE_MODE in a beforeEach (or
restore it after the test) to avoid state leakage across tests, and change the
assertion to await waitFor(...) so the
expect(mockReplace).toHaveBeenCalledWith('/home') actually waits; locate the
test referencing Env.MAINTENANCE_MODE, the Maintenance component render call,
and the waitFor/assert that checks mockReplace to apply these changes.
| "dispatched": "Despachado", | ||
| "display_options": "Opciones de visualización", | ||
| "filter_help": "Ingrese IDs separados por comas para filtrar. Deje vacío para mostrar todos.", | ||
| "filter_placeholder": "e.g. {{example}}", |
Contributor
There was a problem hiding this comment.
Localize placeholder abbreviation for Spanish locale.
Line 356 uses "e.g."; prefer Spanish "p. ej.".
Proposed fix
- "filter_placeholder": "e.g. {{example}}",
+ "filter_placeholder": "p. ej. {{example}}",📝 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.
Suggested change
| "filter_placeholder": "e.g. {{example}}", | |
| "filter_placeholder": "p. ej. {{example}}", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/translations/es.json` at line 356, Update the Spanish translation value
for the key "filter_placeholder" to use the Spanish abbreviation "p. ej."
instead of "e.g."; keep the placeholder token "{{example}}" and surrounding
punctuation/spacing intact so only the abbreviation is changed (e.g., "p. ej.
{{example}}").
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Updates