Add maintenance banner to status pages #2888
Conversation
- Create MaintenanceBanner component to display affected services - Show banner when any monitors on status page are in maintenance - Add translation keys for maintenance banner messages - Position banner prominently after header on status pages - Banner lists all affected monitor names and explains no alerts during maintenance
gorkem-bwl
requested review from
ajhollid,
mohicody,
Br0wnHammer,
Owaiseimdad and
karenvicent
as code owners
|
Warning Rate limit exceeded@gorkem-bwl has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 15 seconds before requesting another review. ⌛ 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. 📒 Files selected for processing (8)
WalkthroughAdds a MaintenanceBanner React component (with demo fallback) to the Status page, computes monitorsInMaintenance by filtering monitors with isMaintenance, renders the banner after ControlsHeader, and adds three English i18n strings for maintenance messaging. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant SP as StatusPage
participant MB as MaintenanceBanner
Note over SP: Render PublicStatus
U->>SP: Load page
SP->>SP: Fetch monitors (existing flow)
SP->>SP: Filter isMaintenance === true -> monitorsInMaintenance
alt monitorsInMaintenance is non-empty
SP->>MB: Render with affectedMonitors
MB-->>U: Show warning Alert (maintenance in progress)
else monitorsInMaintenance empty or undefined
SP->>MB: Render in demo mode (hardcoded monitors)
MB-->>U: Show demo warning Alert (includes “(DEMO)” and caption)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes P.S. Canadians apologize for the maintenance; Americans just call it "feature rollout." ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
- Show example banner with 'Web Server, API Gateway' when no real maintenance - Add '(DEMO)' indicator to title - Include explanatory text for demo purposes - Allows testing banner appearance without setting up real maintenance windows
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
client/src/Pages/StatusPage/Status/index.jsx (1)
145-147: Memoize maintenance filter and fix formatting
- Backend confirms
isMaintenanceis present (statusPageModule.js:237), so the filter works.- Wrap the filter in
useMemoto avoid re-running on every render.- Address Prettier warnings by running
npx prettier --write client/src/Pages/StatusPage/Status/index.jsximport React from "react"; +import { useMemo } from "react"; -// Filter monitors that are in maintenance -const monitorsInMaintenance = monitors?.filter(monitor => monitor.isMaintenance) || []; +// Filter monitors that are in maintenance +const monitorsInMaintenance = useMemo( + () => (monitors || []).filter(m => m.isMaintenance === true), + [monitors] +);client/src/locales/en.json (1)
305-305: Put punctuation inside the translatable string (optional)Move the trailing colon into the translation so languages with different punctuation rules can localize it.
Change here:
-"maintenanceAffectedServices": "The following services are currently under maintenance", +"maintenanceAffectedServices": "The following services are currently under maintenance:",And in the banner component, remove the hardcoded colon (see related comment there).
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (4)
5-8: Expose i18n for locale-aware list formatting (prep for next change)You can pull i18n from useTranslation to format lists per locale.
-const { t } = useTranslation(); +const { t, i18n } = useTranslation();
13-14: Locale-aware, robust monitor name list
- Use Intl.ListFormat for proper punctuation per locale (and “A, B, and C” vs “A, B y C”, etc.).
- Guard against missing names.
-const monitorNames = affectedMonitors.map(monitor => monitor.name).join(", "); +const names = affectedMonitors + .map(m => m?.name) + .filter(Boolean); +const monitorNames = new Intl.ListFormat(i18n?.language || "en", { + style: "long", + type: "conjunction", +}).format(names);
25-27: Minor a11y: hide emoji from screen readersScreen readers can read “wrench” redundantly. Mark it decorative.
-<AlertTitle sx={{ fontWeight: 600, mb: theme.spacing(5) }}> - 🔧 {t("maintenanceInProgress")} -</AlertTitle> +<AlertTitle sx={{ fontWeight: 600, mb: theme.spacing(5) }}> + <span aria-hidden="true" style={{ marginRight: 4 }}>🔧</span> + {t("maintenanceInProgress")} +</AlertTitle>
29-29: Translation owns punctuation (only if you applied the i18n change)Remove the hardcoded colon and let the string include it (see en.json suggestion).
-{t("maintenanceAffectedServices")}: <strong>{monitorNames}</strong> +{t("maintenanceAffectedServices")} <strong>{monitorNames}</strong>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx(1 hunks)client/src/Pages/StatusPage/Status/index.jsx(3 hunks)client/src/locales/en.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
client/src/Pages/StatusPage/Status/index.jsx (1)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (1)
MaintenanceBanner(5-36)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (2)
client/src/Pages/StatusPage/Status/index.jsx (2)
theme(25-25)useTranslation(26-26)client/src/Components/Alert/index.jsx (1)
Alert(32-109)
🪛 GitHub Actions: Format Check (Client & Server)
client/src/Pages/StatusPage/Status/index.jsx
[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx
[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
🔇 Additional comments (4)
client/src/Pages/StatusPage/Status/index.jsx (2)
9-9: Import looks goodImport path resolves to the folder index; no issues.
159-159: Render placement is sensibleRendering unconditionally is fine since the component returns null when empty.
client/src/locales/en.json (1)
303-306: New strings LGTMKeys are clear and match component usage.
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (1)
9-11: Early return is perfectAvoids empty chrome when no monitors are affected.
| import { Alert, AlertTitle, Typography } from "@mui/material"; | ||
| import { useTheme } from "@emotion/react"; | ||
| import { useTranslation } from "react-i18next"; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider using the app’s Alert for visual consistency; also fix Prettier
- The repo has a custom Alert component (client/src/Components/Alert). If the design system prefers it, consider switching for consistent theming. If MUI Alert is the new standard, ignore this.
- Prettier is failing on this file; please run the formatter.
Verify formatting:
Americans love shiny new components; Canadians just want them all to look friendly together. Pick your vibe. 😉
🏁 Script executed:
#!/bin/bash
npx prettier --check client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsxLength of output: 284
🏁 Script executed:
#!/bin/bash
# Search for custom Alert component in client/src/Components
echo "Files matching 'Alert' under client/src/Components:"
fd -t f "client/src/Components" | grep -E "Alert(\.jsx?$|/index\.jsx?$)" || echo "No Alert files found"
echo ""
# Inspect export definitions in those files
for file in $(fd -t f "client/src/Components" | grep -E "Alert(\.jsx?$|/index\.jsx?$)"); do
echo "--- $file ---"
rg -n "export .*Alert" "$file" || echo " No export of Alert found in $file"
doneLength of output: 1108
🏁 Script executed:
#!/bin/bash
# Locate any custom Alert component files under client/src/Components
echo "Custom Alert component candidates:"
fd -i 'Alert(\.jsx?$|/index\.jsx?$)' client/src/Components || echo "No matching files"
# Inspect export definitions in each candidate
for file in $(fd -i 'Alert(\.jsx?$|/index\.jsx?$)' client/src/Components); do
echo -e "\n--- $file ---"
rg -nP 'export\s+(default\s+)?Alert' "$file" || echo " No Alert export found"
doneLength of output: 201
🏁 Script executed:
#!/bin/bash
# Locate all directories named components (case-insensitive) under client/src
echo "Components directories found:"
find client/src -type d -iname '*components*'
echo ""
# List contents of each Components directory
for dir in $(find client/src -type d -iname '*components*'); do
echo "Contents of $dir:"
ls -1 "$dir"
echo ""
done
# Find any files or directories with 'Alert' in the name under client/src
echo "Files or directories matching 'Alert' under client/src:"
find client/src -iname '*Alert*'Length of output: 7931
Use internal Alert and fix formatting
- Import
Alertfromclient/src/Components/Alertinstead of MUI for consistent theming. - Run
npx prettier --write client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsxto resolve style warnings.
Americans love shiny new components; Canadians just want them all to look friendly together. 😉
🧰 Tools
🪛 GitHub Actions: Format Check (Client & Server)
[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
🤖 Prompt for AI Agents
In client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx
lines 1-4, the component currently imports Alert from MUI which breaks
consistent theming; replace the Alert import to import Alert from
"client/src/Components/Alert" (keep AlertTitle and Typography imports as
needed), then run npx prettier --write
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx to fix
formatting/style warnings.
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
Auto Pull Request Review from LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Approve with suggestions
This PR adds a useful maintenance banner feature but includes minor suggestions to improve robustness, accessibility, and internationalization.
🌟 Strengths
- Adds a clear and user-friendly banner for maintenance visibility.
- Well-integrated with existing status page components and translations.
| Priority | File | Category | Impact Summary | Anchors |
|---|---|---|---|---|
| P2 | client/src/Pages/StatusPage/Status/index.jsx | Correctness | Assumes isMaintenance property, may fail silently. | |
| P2 | client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx | Robustness | Risk of TypeError if monitor name is missing. | |
| P2 | client/src/locales/en.json | Maintainability | Missing translations in other languages. | |
| P2 | client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx | Accessibility | Emoji may not be accessible for screen readers. |
🔍 Notable Themes
- Data Validation: Multiple suggestions to add defensive checks for object properties to prevent silent failures or runtime errors.
- Internationalization: Ensure all new UI strings are translated across supported languages for a consistent user experience.
⚠️ **Unanchored Suggestions (Manual Review Recommended)**
The following suggestions could not be precisely anchored to a specific line in the diff. This can happen if the code is outside the changed lines, has been significantly refactored, or if the suggestion is a general observation. Please review them carefully in the context of the full file.
📁 File: client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx
If any monitor in affectedMonitors lacks a name property, this will throw a TypeError ("Cannot read properties of undefined"), breaking the entire status page render. This is particularly risky since the component already checks for array existence but not individual element structure.
Suggestion:
const monitorNames = affectedMonitors.map(monitor => monitor?.name).filter(Boolean).join(", ");
Related Code:
const monitorNames = affectedMonitors.map(monitor => monitor.name).join(", ");
💡 LlamaPReview Community
Have feedback on this AI Code review tool? Join our GitHub Discussions to share your thoughts and help shape the future of LlamaPReview.
| "uptimeCreateJsonPath": "This expression will be evaluated against the reponse JSON data and the result will be used to match against the expected value. See", | ||
| "uptimeCreateJsonPathQuery": "for query language documentation.", | ||
| "maintenanceTableActionMenuDialogTitle": "Do you really want to remove this maintenance window?", | ||
| "maintenanceInProgress": "Maintenance in Progress", |
There was a problem hiding this comment.
P2 | Confidence: High
Speculative: The translations are added only to English (en.json). If the application supports multiple languages, these keys must be added to all locale files to avoid missing translations in other languages, which would degrade the user experience for non-English speakers.
| ); | ||
| } | ||
|
|
||
| // Filter monitors that are in maintenance |
There was a problem hiding this comment.
P2 | Confidence: Medium
Speculative: The code assumes monitor objects contain an isMaintenance property, but this property is not visible in the current changes. If the backend doesn't provide this field or uses a different naming convention (e.g., maintenance), the filter will silently fail, and the banner won't display when expected. This could lead to user confusion during actual maintenance periods.
| // Filter monitors that are in maintenance | |
| const monitorsInMaintenance = monitors?.filter(monitor => monitor?.isMaintenance) || []; |
| } | ||
|
|
||
| return ( | ||
| <Alert |
There was a problem hiding this comment.
P2 | Confidence: Medium
The alert uses a text-based emoji (🔧) which may not be properly announced by screen readers. While the visual meaning is clear, assistive technologies might read this as "wrench" or fail to convey the maintenance context, reducing accessibility for visually impaired users.
Code Suggestion:
<AlertTitle sx={{ fontWeight: 600, mb: theme.spacing(5) }} aria-label="Maintenance in progress">
<Icon>build</Icon> {t("maintenanceInProgress")}
</AlertTitle>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (2)
1-1: Use design-system Alert for consistency (or confirm MUI is the new standard).Repo reportedly has a custom Alert. Prefer it for theming consistency; otherwise confirm MUI is now canonical.
Option if design-system is desired (adjust path to your aliasing):
-import { Alert, AlertTitle, Typography } from "@mui/material"; +import Alert from "client/src/Components/Alert"; +import { AlertTitle, Typography } from "@mui/material";Americans love loud banners; Canadians prefer polite notices—pick your vibe. 🇺🇸🇨🇦
1-52: Fix Prettier warnings to unblock the pipeline.Run the formatter on this file.
#!/bin/bash npx prettier --write client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx
🧹 Nitpick comments (3)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (3)
2-2: Prefer MUI’s useTheme import for typings and consistency.If the app standardizes on MUI theme, import from @mui/material/styles.
-import { useTheme } from "@emotion/react"; +import { useTheme } from "@mui/material/styles";Run a quick check across client code to confirm the prevailing pattern before changing.
34-36: Localize the title completely (emoji/text), not just the words.Move the emoji/title into i18n to allow translators full control. If you keep emoji inline, at least make the string a single key.
Example:
- <AlertTitle sx={{ fontWeight: 600, mb: theme.spacing(5) }}> - 🔧 {t("maintenanceInProgress")} - </AlertTitle> + <AlertTitle sx={{ fontWeight: 600, mb: theme.spacing(5) }}> + {t("maintenanceTitle", { defaultValue: "🔧 Maintenance in Progress" })} + </AlertTitle>
25-33: Spacing nit: 80px bottom margin feels heavy for banners.Consider spacing(3–6) and a slightly smaller icon for denser status pages.
- sx={{ - mb: theme.spacing(10), - '& .MuiAlert-icon': { - fontSize: '1.5rem' - } - }} + sx={{ + mb: theme.spacing(4), + '& .MuiAlert-icon': { fontSize: '1.25rem' } + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (1)
client/src/Pages/StatusPage/Status/index.jsx (2)
theme(25-25)useTranslation(26-26)
🪛 GitHub Actions: Format Check (Client & Server)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx
[warning] 1-1: Code style issues found in file. Run Prettier with --write to fix.
🔇 Additional comments (1)
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx (1)
37-42: Translation keys confirmed in locales; keys present in client/src/locales/en.json (lines 303–305). All set—no moose tracks left behind, eh?
client/src/Pages/StatusPage/Status/Components/MaintenanceBanner/index.jsx
Outdated
Show resolved
Hide resolved
- Add MaintenanceBanner component for status pages showing active maintenance windows - Fix status page layout with 800px centered container while keeping breadcrumbs full-width - Update maintenance window detection to support repeating maintenance windows - Remove demo content and focus on real maintenance data display
- Remove database-level isMaintenance calculation from statusPageModule aggregation - Add isInMaintenance property to Monitor model schema - Update SuperSimpleQueueHelper to set Monitor.isInMaintenance when checking maintenance windows - Add maintenance service methods for bulk maintenance status updates - Add periodic maintenance job to update all monitors every 5 minutes - Maintain support for both one-time and repeating maintenance windows - Clean architecture: maintenance status is now properly stored on Monitor model
- Remove periodic maintenance job from scheduler (every 5 minutes) - Remove getMaintenanceJob() method from SuperSimpleQueueHelper - Remove bulk maintenance update methods from maintenanceWindowService - Maintenance status updates only when monitors are actually processed - Simpler, more efficient architecture
Add a prominent banner to status pages that displays when monitors are under maintenance. The banner shows which specific services are affected and explains that no alerts will be generated during maintenance periods.
Summary by CodeRabbit
New Features
Chores