feat: add ntfy notification channel support #2872
Conversation
Add comprehensive ntfy notification support to Checkmate, enabling users to receive push notifications via ntfy (self-hosted or ntfy.sh cloud service). ## Backend Implementation - Add ntfy to notification type enum in Notification model - Add ntfy-specific fields: auth method, credentials, priority level - Implement requestNtfy() method in NetworkService with full auth support: - Basic authentication (username/password) - Bearer token authentication - Public topics (no authentication) - Integrate ntfy handling into NotificationService - Add comprehensive Joi validation with conditional field requirements - Support priority levels 1-5 for different notification urgency ## Frontend Implementation - Add ntfy to notification types dropdown - Create conditional form rendering with ntfy-specific fields: - Topic URL input (required) - Authentication method selector - Username/password inputs (conditional) - Bearer token input (conditional) - Priority level selector (1-5) - Add complete translation support for all ntfy UI elements - Implement proper form validation and error handling ## Features - **Simple Configuration**: Just requires topic URL for basic usage - **Flexible Authentication**: Support for public, basic auth, and bearer token - **Priority Levels**: 5 priority levels (min, low, default, high, urgent) - **Self-hosting Friendly**: Works with ntfy.sh or self-hosted instances - **Developer Focused**: Perfect for monitoring tools and DevOps workflows - **Full Integration**: Works with existing test notification functionality ## Why ntfy? ntfy is a lightweight, HTTP-based pub-sub notification service that's perfect for developers who want simple, self-hostable push notifications without complex API keys or OAuth flows. This enhancement makes Checkmate more appealing to developers and DevOps teams who prefer simple, self-hosted notification solutions while maintaining the same ease of use as existing notification channels.
gorkem-bwl
requested review from
Br0wnHammer,
Owaiseimdad,
ajhollid,
karenvicent and
mohicody
as code owners
WalkthroughAdds ntfy notification support across frontend and backend: new UI fields, constants, translations, client/server validation, schema fields, NetworkService.requestNtfy, and NotificationService routing to send ntfy messages with optional Basic/Bearer auth and priority. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client UI
participant API as Server API
participant VAL as Joi Validation
participant NS as NotificationService
participant NET as NetworkService
participant NTFY as ntfy Server
UI->>API: POST /notifications (type=ntfy, address, ntfyAuthMethod, credentials, priority, ...)
API->>VAL: Validate body (type, address, ntfy fields)
VAL-->>API: Valid or errors
alt Validation OK
API->>NS: sendNotification(notification)
NS->>NET: requestNtfy(url, message, title, notification)
NET->>NTFY: POST topic URL with headers (Title, Priority, Tags, Authorization?)
alt 2xx
NTFY-->>NET: Success
NET-->>NS: { type: "ntfy", status: true, code, payload }
NS-->>API: success status
API-->>UI: 200/201
else Error
NTFY-->>NET: Error
NET-->>NS: { type: "ntfy", status: false, code, message, payload }
NS-->>API: failure status
API-->>UI: 4xx/5xx
end
else Validation Failed
API-->>UI: 400 with field errors
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ 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 (
|
- Fix NTFY_AUTH_METHODS and NTFY_PRIORITIES to use {_id, name, value} format
- Add helper functions to convert dropdown IDs to actual values
- Update form submission to convert ntfy dropdown values correctly
- Fix conditional rendering to use actual values for comparison
- Ensure dropdowns now display properly with selectable options
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
client/src/Pages/Notifications/utils.js (1)
1-8: Format client/src/Pages/Notifications/utils.js with Prettier
Run the project’s local Prettier to satisfy the CI check, e.g.:npx prettier --write client/src/Pages/Notifications/utils.js(and apply to the other blocks around lines 10–18, 19–26, 28–36, 37–44 and 46–60)—otherwise your PR will be as frozen as a Canadian winter, eh?
client/src/Pages/Notifications/create/index.jsx (1)
1-1: Format with Prettier via npx and remove trailing whitespace
Trailing whitespace on lines 243, 256, 280 and 293 is causing CI failures. Prettier isn’t installed globally – runnpx prettier --write client/src/Pages/Notifications/create/index.jsxor add Prettier to devDependencies. Let’s tidy up like a Canadian saying “sorry” and an American chasing efficiency!
server/src/service/infrastructure/notificationService.js (1)
80-85: Bug: missing await when notifying status monitors
notifyAllis async. This branch returns a Promise instead of a boolean, unlike the hardware path (which awaits). Fix:- const success = this.notifyAll({ notificationIDs, subject, html, content }); + const success = await this.notifyAll({ notificationIDs, subject, html, content });
🧹 Nitpick comments (4)
client/src/Pages/Notifications/utils.js (1)
7-8: Type list: display name casingConsider “ntfy” vs “NTFY” for consistency with other entries. If you’re following brand styling (lowercase), keep it; otherwise match “Slack/Discord” style.
client/src/Pages/Notifications/create/index.jsx (2)
120-134: Validation UX: clear stale errors when switching auth methodIf the user switches auth method, previous field errors can linger. Clear errors for ntfyUsername/ntfyPassword/ntfyBearerToken when ntfyAuthMethod changes.
- if (name === "type") { + if (name === "type") { validationError["type"] = extractError(error, "type"); validationError["address"] = extractError(error, "address"); - } else { + } else { validationError[name] = extractError(error, name); } + if (name === "ntfyAuthMethod") { + delete validationError.ntfyUsername; + delete validationError.ntfyPassword; + delete validationError.ntfyBearerToken; + }
243-304: Select prop shape + helperTextAfter fixing utils.js constants, these Selects will work. Note: your Select component doesn’t accept helperText; those props are ignored. If you want helper text under Select, extend Select to pass helperText to FieldWrapper.
server/src/validation/joi.js (1)
570-672: Message consistency and UXError messages are precise. Consider reusing constants for “Priority must be between 1 and 5” to avoid duplication.
📜 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 (7)
client/src/Pages/Notifications/create/index.jsx(3 hunks)client/src/Pages/Notifications/utils.js(5 hunks)client/src/locales/en.json(1 hunks)server/src/db/models/Notification.js(2 hunks)server/src/service/infrastructure/networkService.js(1 hunks)server/src/service/infrastructure/notificationService.js(1 hunks)server/src/validation/joi.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
server/src/service/infrastructure/networkService.js (1)
client/src/Pages/Notifications/create/index.jsx (1)
notification(58-68)
server/src/service/infrastructure/notificationService.js (1)
client/src/Pages/Notifications/create/index.jsx (2)
type(165-165)notification(58-68)
client/src/Pages/Notifications/create/index.jsx (2)
client/src/Components/Inputs/Select/index.jsx (1)
Select(43-169)client/src/Pages/Notifications/utils.js (4)
NTFY_AUTH_METHODS(47-51)NTFY_AUTH_METHODS(47-51)NTFY_PRIORITIES(53-59)NTFY_PRIORITIES(53-59)
🪛 GitHub Actions: Format Check (Client & Server)
server/src/service/infrastructure/networkService.js
[error] 1-1: Prettier formatting check failed for src/service/infrastructure/networkService.js. Run 'prettier --write' to fix. Step: 'npm run format-check' (prettier --check .) exited with code 1.
client/src/Pages/Notifications/create/index.jsx
[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
client/src/Pages/Notifications/utils.js
[warning] 1-1: Prettier formatting issue detected in this file. Run 'prettier --write' to fix.
🪛 Biome (2.1.2)
server/src/validation/joi.js
[error] 609-609: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 621-621: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 627-627: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 629-629: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 640-640: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 642-642: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 653-653: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 655-655: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 666-666: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (12)
client/src/Pages/Notifications/utils.js (4)
16-17: LGTM: title map includes ntfy
25-26: LGTM: description map includes ntfy
34-35: LGTM: label map for address
43-44: LGTM: placeholder mapclient/src/Pages/Notifications/create/index.jsx (2)
32-34: LGTM: importing ntfy constants
309-339: Nice conditional renderingOnly shows ntfy fields when needed—polite, like Canadians at a four-way stop. Americans would probably add two more options and a pickup truck. Carry on.
server/src/db/models/Notification.js (2)
19-20: LGTM: added "ntfy" to enum
31-51: Protect secrets at rest and in queries
- Add
trim: truetontfyUsername.- Mark
ntfyPasswordandntfyBearerTokenwithselect: false.- Confirm these fields are encrypted on save as with other secrets (no encryption plugin or pre-save hooks detected).
Let’s keep these tokens more locked down than a Canadian ice rink in January, eh?
server/src/validation/joi.js (2)
576-580: LGTM: type includes ntfy
607-615: Address rules for ntfyValidating as URI is fine; consider allowing self-hosted topic paths without scheme if your UX expects it. Otherwise, document the requirement.
server/src/service/infrastructure/notificationService.js (1)
47-50: ntfy branch wiring looks correctSubject → Title, content → body, and returning
response.statusaligns with the webhook pattern. Nice.server/src/service/infrastructure/networkService.js (1)
446-491: Prettier formatting applied; CI checks are now passing (even Canadians won’t apologize for this prettiness)
| "ntfySettings": { | ||
| "title": "ntfy", | ||
| "description": "Send push notifications via ntfy (self-hosted or ntfy.sh)", | ||
| "urlLabel": "ntfy Topic URL", | ||
| "urlPlaceholder": "https://ntfy.sh/my-topic", | ||
| "authMethodLabel": "Authentication Method", | ||
| "usernameLabel": "Username", | ||
| "usernamePlaceholder": "Enter username", | ||
| "passwordLabel": "Password", | ||
| "passwordPlaceholder": "Enter password", | ||
| "bearerTokenLabel": "Bearer Token", | ||
| "bearerTokenPlaceholder": "Enter bearer token", | ||
| "priorityLabel": "Priority" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add i18n for ntfy option labels to avoid hard-coded UI text
NTFY auth-method and priority options are hard-coded in utils.js. Provide localized strings here so the UI can use i18n consistently.
"ntfySettings": {
"title": "ntfy",
"description": "Send push notifications via ntfy (self-hosted or ntfy.sh)",
"urlLabel": "ntfy Topic URL",
"urlPlaceholder": "https://ntfy.sh/my-topic",
"authMethodLabel": "Authentication Method",
+ "authMethodOptions": {
+ "none": "None",
+ "username_password": "Username/Password",
+ "bearer_token": "Bearer Token"
+ },
"usernameLabel": "Username",
"usernamePlaceholder": "Enter username",
"passwordLabel": "Password",
"passwordPlaceholder": "Enter password",
"bearerTokenLabel": "Bearer Token",
"bearerTokenPlaceholder": "Enter bearer token",
- "priorityLabel": "Priority"
+ "priorityLabel": "Priority",
+ "priorityOptions": {
+ "1": "Min",
+ "2": "Low",
+ "3": "Default",
+ "4": "High",
+ "5": "Urgent"
+ }
},📝 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.
| "ntfySettings": { | |
| "title": "ntfy", | |
| "description": "Send push notifications via ntfy (self-hosted or ntfy.sh)", | |
| "urlLabel": "ntfy Topic URL", | |
| "urlPlaceholder": "https://ntfy.sh/my-topic", | |
| "authMethodLabel": "Authentication Method", | |
| "usernameLabel": "Username", | |
| "usernamePlaceholder": "Enter username", | |
| "passwordLabel": "Password", | |
| "passwordPlaceholder": "Enter password", | |
| "bearerTokenLabel": "Bearer Token", | |
| "bearerTokenPlaceholder": "Enter bearer token", | |
| "priorityLabel": "Priority" | |
| }, | |
| "ntfySettings": { | |
| "title": "ntfy", | |
| "description": "Send push notifications via ntfy (self-hosted or ntfy.sh)", | |
| "urlLabel": "ntfy Topic URL", | |
| "urlPlaceholder": "https://ntfy.sh/my-topic", | |
| "authMethodLabel": "Authentication Method", | |
| "authMethodOptions": { | |
| "none": "None", | |
| "username_password": "Username/Password", | |
| "bearer_token": "Bearer Token" | |
| }, | |
| "usernameLabel": "Username", | |
| "usernamePlaceholder": "Enter username", | |
| "passwordLabel": "Password", | |
| "passwordPlaceholder": "Enter password", | |
| "bearerTokenLabel": "Bearer Token", | |
| "bearerTokenPlaceholder": "Enter bearer token", | |
| "priorityLabel": "Priority", | |
| "priorityOptions": { | |
| "1": "Min", | |
| "2": "Low", | |
| "3": "Default", | |
| "4": "High", | |
| "5": "Urgent" | |
| } | |
| }, |
🤖 Prompt for AI Agents
client/src/locales/en.json around lines 693 to 706, the ntfy UI currently uses
hard-coded option labels from utils.js; add localized string entries for the
auth-method options and priority options so the UI can use i18n consistently.
Add keys under "ntfySettings" like "authMethodOptions": { "none": "None",
"basic": "Basic", "bearer": "Bearer" } and "priorityOptions": { "min": "Min",
"low": "Low", "default": "Default", "high": "High", "urgent": "Urgent" } (or
match the exact option identifiers used in utils.js) so components can reference
these i18n keys instead of hard-coded text.
| type: NOTIFICATION_TYPES[0]._id, | ||
| // ntfy-specific fields | ||
| ntfyAuthMethod: "none", | ||
| ntfyUsername: "", | ||
| ntfyPassword: "", | ||
| ntfyBearerToken: "", | ||
| ntfyPriority: 3, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
State defaults: align with Select values and reset behavior
Defaults match your intended choices, but ensure they align with {_id} values in constants (after refactor). Recommend resetting dependent fields when auth method changes.
- const onChange = (e) => {
+ const onChange = (e) => {
const { name, value } = e.target;
- let rawNotification = { ...notification, [name]: value };
+ let rawNotification = { ...notification, [name]: value };
+ // Reset dependent ntfy fields when auth method changes
+ if (name === "ntfyAuthMethod") {
+ if (value !== "username_password") {
+ rawNotification.ntfyUsername = "";
+ rawNotification.ntfyPassword = "";
+ }
+ if (value !== "bearer_token") {
+ rawNotification.ntfyBearerToken = "";
+ }
+ }
let newNotification = {
...rawNotification,
type: getNotificationTypeValue(rawNotification.type),
};📝 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.
| type: NOTIFICATION_TYPES[0]._id, | |
| // ntfy-specific fields | |
| ntfyAuthMethod: "none", | |
| ntfyUsername: "", | |
| ntfyPassword: "", | |
| ntfyBearerToken: "", | |
| ntfyPriority: 3, | |
| }); | |
| const onChange = (e) => { | |
| const { name, value } = e.target; | |
| let rawNotification = { ...notification, [name]: value }; | |
| // Reset dependent ntfy fields when auth method changes | |
| if (name === "ntfyAuthMethod") { | |
| if (value !== "username_password") { | |
| rawNotification.ntfyUsername = ""; | |
| rawNotification.ntfyPassword = ""; | |
| } | |
| if (value !== "bearer_token") { | |
| rawNotification.ntfyBearerToken = ""; | |
| } | |
| } | |
| let newNotification = { | |
| ...rawNotification, | |
| type: getNotificationTypeValue(rawNotification.type), | |
| }; | |
| // …the rest of your update logic (e.g. setNotification(newNotification)) | |
| }; |
🤖 Prompt for AI Agents
In client/src/Pages/Notifications/create/index.jsx around lines 61-68, the
ntfy-related state defaults should be derived from the constants' _id values (to
match refactored constants) and you must reset dependent fields when the auth
method changes; update the initial state so ntfyAuthMethod uses the correct
constant _id (not a hardcoded string) and ensure the onChange handler or
useEffect for auth method clears or resets ntfyUsername, ntfyPassword,
ntfyBearerToken and restores ntfyPriority to its default when switching methods
so form values stay consistent with the selected auth option.
|
|
||
| async requestNtfy(url, message, title, notification) { | ||
| try { | ||
| // Build headers | ||
| const headers = { | ||
| "Title": title, | ||
| "Priority": notification.ntfyPriority?.toString() || "3", | ||
| "Tags": "checkmate,monitoring", | ||
| "Content-Type": "text/plain" | ||
| }; | ||
|
|
||
| // Add authentication headers based on method | ||
| if (notification.ntfyAuthMethod === "username_password" && notification.ntfyUsername && notification.ntfyPassword) { | ||
| const auth = Buffer.from(`${notification.ntfyUsername}:${notification.ntfyPassword}`).toString('base64'); | ||
| headers["Authorization"] = `Basic ${auth}`; | ||
| } else if (notification.ntfyAuthMethod === "bearer_token" && notification.ntfyBearerToken) { | ||
| headers["Authorization"] = `Bearer ${notification.ntfyBearerToken}`; | ||
| } | ||
|
|
||
| // Send the notification | ||
| const response = await this.axios.post(url, message, { headers }); | ||
|
|
||
| return { | ||
| type: "ntfy", | ||
| status: true, | ||
| code: response.status, | ||
| message: "Successfully sent ntfy notification", | ||
| payload: response.data, | ||
| }; | ||
| } catch (error) { | ||
| this.logger.warn({ | ||
| message: error.message, | ||
| service: this.SERVICE_NAME, | ||
| method: "requestNtfy", | ||
| url: url | ||
| }); | ||
|
|
||
| return { | ||
| type: "ntfy", | ||
| status: false, | ||
| code: error.response?.status || this.NETWORK_ERROR, | ||
| message: `Failed to send ntfy notification: ${error.message}`, | ||
| payload: error.response?.data, | ||
| }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden ntfy request: add timeout and clamp priority [1..5]
Avoid hung requests to third-party/self-hosted ntfy and ensure priority is always valid.
async requestNtfy(url, message, title, notification) {
try {
- // Build headers
- const headers = {
- "Title": title,
- "Priority": notification.ntfyPriority?.toString() || "3",
- "Tags": "checkmate,monitoring",
- "Content-Type": "text/plain"
- };
+ // Build headers
+ const priority = String(Math.min(5, Math.max(1, Number(notification?.ntfyPriority ?? 3))));
+ const headers = {
+ Title: title,
+ Priority: priority,
+ Tags: "checkmate,monitoring",
+ "Content-Type": "text/plain",
+ };
@@
- // Send the notification
- const response = await this.axios.post(url, message, { headers });
+ // Send the notification (timeout to avoid hanging jobs)
+ const response = await this.axios.post(url, message ?? "", { headers, timeout: 10000 });📝 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.
| async requestNtfy(url, message, title, notification) { | |
| try { | |
| // Build headers | |
| const headers = { | |
| "Title": title, | |
| "Priority": notification.ntfyPriority?.toString() || "3", | |
| "Tags": "checkmate,monitoring", | |
| "Content-Type": "text/plain" | |
| }; | |
| // Add authentication headers based on method | |
| if (notification.ntfyAuthMethod === "username_password" && notification.ntfyUsername && notification.ntfyPassword) { | |
| const auth = Buffer.from(`${notification.ntfyUsername}:${notification.ntfyPassword}`).toString('base64'); | |
| headers["Authorization"] = `Basic ${auth}`; | |
| } else if (notification.ntfyAuthMethod === "bearer_token" && notification.ntfyBearerToken) { | |
| headers["Authorization"] = `Bearer ${notification.ntfyBearerToken}`; | |
| } | |
| // Send the notification | |
| const response = await this.axios.post(url, message, { headers }); | |
| return { | |
| type: "ntfy", | |
| status: true, | |
| code: response.status, | |
| message: "Successfully sent ntfy notification", | |
| payload: response.data, | |
| }; | |
| } catch (error) { | |
| this.logger.warn({ | |
| message: error.message, | |
| service: this.SERVICE_NAME, | |
| method: "requestNtfy", | |
| url: url | |
| }); | |
| return { | |
| type: "ntfy", | |
| status: false, | |
| code: error.response?.status || this.NETWORK_ERROR, | |
| message: `Failed to send ntfy notification: ${error.message}`, | |
| payload: error.response?.data, | |
| }; | |
| } | |
| } | |
| async requestNtfy(url, message, title, notification) { | |
| try { | |
| // Build headers | |
| const priority = String(Math.min(5, Math.max(1, Number(notification?.ntfyPriority ?? 3)))); | |
| const headers = { | |
| Title: title, | |
| Priority: priority, | |
| Tags: "checkmate,monitoring", | |
| "Content-Type": "text/plain", | |
| }; | |
| // Add authentication headers based on method | |
| if (notification.ntfyAuthMethod === "username_password" && notification.ntfyUsername && notification.ntfyPassword) { | |
| const auth = Buffer.from(`${notification.ntfyUsername}:${notification.ntfyPassword}`).toString('base64'); | |
| headers["Authorization"] = `Basic ${auth}`; | |
| } else if (notification.ntfyAuthMethod === "bearer_token" && notification.ntfyBearerToken) { | |
| headers["Authorization"] = `Bearer ${notification.ntfyBearerToken}`; | |
| } | |
| // Send the notification (timeout to avoid hanging jobs) | |
| const response = await this.axios.post(url, message ?? "", { headers, timeout: 10000 }); | |
| return { | |
| type: "ntfy", | |
| status: true, | |
| code: response.status, | |
| message: "Successfully sent ntfy notification", | |
| payload: response.data, | |
| }; | |
| } catch (error) { | |
| this.logger.warn({ | |
| message: error.message, | |
| service: this.SERVICE_NAME, | |
| method: "requestNtfy", | |
| url: url | |
| }); | |
| return { | |
| type: "ntfy", | |
| status: false, | |
| code: error.response?.status || this.NETWORK_ERROR, | |
| message: `Failed to send ntfy notification: ${error.message}`, | |
| payload: error.response?.data, | |
| }; | |
| } | |
| } |
| // ntfy-specific fields | ||
| ntfyAuthMethod: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.string().valid("none", "username_password", "bearer_token").default("none"), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyUsername: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "username_password", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": "Username cannot be empty when using username/password authentication", | ||
| "any.required": "Username is required for username/password authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyPassword: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "username_password", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": "Password cannot be empty when using username/password authentication", | ||
| "any.required": "Password is required for username/password authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyBearerToken: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "bearer_token", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": "Bearer token cannot be empty when using bearer token authentication", | ||
| "any.required": "Bearer token is required for bearer token authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyPriority: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.number().min(1).max(5).default(3).messages({ | ||
| "number.min": "Priority must be between 1 and 5", | ||
| "number.max": "Priority must be between 1 and 5", | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Biome “noThenProperty” false positives on Joi .when()
Biome flags objects with a “then” key. This is standard Joi usage. Suppress the rule locally.
- ntfyAuthMethod: joi.when("type", {
+ /* biome-ignore lint/suspicious/noThenProperty: Joi .when requires 'then' */
+ ntfyAuthMethod: joi.when("type", {
is: "ntfy",
then: joi.string().valid("none", "username_password", "bearer_token").default("none"),
otherwise: joi.forbidden(),
}),Repeat the same suppression comment above each .when block using then/otherwise in this ntfy section (address, ntfyUsername, ntfyPassword, ntfyBearerToken, ntfyPriority).
📝 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.
| // ntfy-specific fields | |
| ntfyAuthMethod: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.string().valid("none", "username_password", "bearer_token").default("none"), | |
| otherwise: joi.forbidden(), | |
| }), | |
| ntfyUsername: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.when("ntfyAuthMethod", { | |
| is: "username_password", | |
| then: joi.string().required().messages({ | |
| "string.empty": "Username cannot be empty when using username/password authentication", | |
| "any.required": "Username is required for username/password authentication", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), | |
| ntfyPassword: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.when("ntfyAuthMethod", { | |
| is: "username_password", | |
| then: joi.string().required().messages({ | |
| "string.empty": "Password cannot be empty when using username/password authentication", | |
| "any.required": "Password is required for username/password authentication", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), | |
| ntfyBearerToken: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.when("ntfyAuthMethod", { | |
| is: "bearer_token", | |
| then: joi.string().required().messages({ | |
| "string.empty": "Bearer token cannot be empty when using bearer token authentication", | |
| "any.required": "Bearer token is required for bearer token authentication", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), | |
| ntfyPriority: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.number().min(1).max(5).default(3).messages({ | |
| "number.min": "Priority must be between 1 and 5", | |
| "number.max": "Priority must be between 1 and 5", | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), | |
| /* biome-ignore lint/suspicious/noThenProperty: Joi .when requires 'then' */ | |
| ntfyAuthMethod: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.string().valid("none", "username_password", "bearer_token").default("none"), | |
| otherwise: joi.forbidden(), | |
| }), |
🧰 Tools
🪛 Biome (2.1.2)
[error] 621-621: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 627-627: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 629-629: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 640-640: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 642-642: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 653-653: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 655-655: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 666-666: Do not add then to an object.
(lint/suspicious/noThenProperty)
🤖 Prompt for AI Agents
In server/src/validation/joi.js around lines 618 to 671, Biome is producing
false positives for the “noThenProperty” rule on legitimate Joi .when(...) uses;
add a local suppression comment immediately above each .when block in this ntfy
section (ntfyAuthMethod, ntfyUsername, ntfyPassword, ntfyBearerToken,
ntfyPriority) — e.g. place a single-line biome-ignore comment (// biome-ignore
noThenProperty) directly above the line that calls .when(...) for each of those
fields so the rule is suppressed only for these Joi patterns.
![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: Request Changes
This PR adds comprehensive ntfy notification support but introduces critical security vulnerabilities with plaintext credential storage and sensitive data logging, alongside minor architectural and maintainability improvements.
🌟 Strengths
- Well-integrated frontend and backend changes with conditional UI rendering.
- Comprehensive validation schemas and internationalization support.
⚡ Key Risks & Improvements (P1)
- server/src/db/models/Notification.js: Stores passwords and tokens in plaintext, risking exposure of sensitive credentials in database breaches.
- server/src/service/infrastructure/networkService.js: Logs full URLs in errors, potentially exposing authentication details and topic names in application logs.
💡 Suggestions (P2)
- client/src/Pages/Notifications/create/index.jsx: Uses a monolithic state object, which could lead to accidental data persistence and submission across notification type switches.
- server/src/validation/joi.js: Allows empty strings for optional fields, potentially causing unnecessary storage of empty values instead of undefined in the database.
- server/src/service/infrastructure/notificationService.js: Returns only boolean status for ntfy, reducing debugging visibility and error distinction compared to other notification types.
💡 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.
| }, | ||
| ntfyUsername: { | ||
| type: String, | ||
| }, |
There was a problem hiding this comment.
P1 | Confidence: High
The code comments indicate passwords and bearer tokens 'Will be encrypted', but no encryption mechanism is implemented. These sensitive credentials are stored in plaintext, creating a significant security vulnerability. The related context shows no existing encryption pattern in the Notification model, and the pre-save hook only handles alert threshold logic. This violates security best practices and exposes authentication credentials to anyone with database access.
Code Suggestion:
Implement encryption using Node.js crypto or a library like bcrypt:
const cipher = crypto.createCipheriv(algorithm, key, iv);
let encrypted = cipher.update(plaintext, 'utf8', 'hex');
encrypted += cipher.final('hex');| message: "Successfully sent ntfy notification", | ||
| payload: response.data, | ||
| }; | ||
| } catch (error) { |
There was a problem hiding this comment.
P1 | Confidence: High
The error logging includes the full ntfy URL, which may contain sensitive topic names and authentication credentials if using HTTP Basic Auth in the URL. This could expose sensitive configuration details in logs. The related context shows other network service methods (e.g., requestPagerDuty) don't log sensitive data like routing keys.
Code Suggestion:
Sanitize URLs before logging:
const safeUrl = new URL(url);
safeUrl.password = ''; // Remove credentials
safeUrl.search = ''; // Remove query params
logger.warn({ url: safeUrl.toString(), ... });| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyUsername: joi.when("type", { |
There was a problem hiding this comment.
P2 | Confidence: High
The validation schema allows empty strings for optional fields, which doesn't align with Mongoose's default behavior of omitting undefined fields. This could lead to empty strings being stored in the database instead of undefined values. The related context shows the Notification model defines these fields without default values, so empty strings would persist unnecessarily.
Code Suggestion:
Use .default(undefined) and .empty("").default(undefined) to ensure clean data:
otherwise: joi.string().empty("").default(undefined)- Add ntfy to valid notification types in frontend validation - Add ntfy-specific field validation (ntfyAuthMethod, ntfyUsername, ntfyPassword, ntfyBearerToken, ntfyPriority) - Add conditional validation based on authentication method - Fix 'field is not allowed' errors when entering ntfy credentials - Ensure frontend validation matches backend validation schema
- Convert ntfy dropdown IDs to actual values in onChange handler for proper validation - Add special handling for ntfyAuthMethod changes to update all related field errors - Fix red border validation errors when selecting dropdown options - Ensure validation runs on actual values, not dropdown IDs
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/Pages/Notifications/create/index.jsx (1)
1-369: Run Prettier on client/src/Pages/Notifications/create/index.jsx to fix formatting
CI still reports this file as unformatted; runnpx prettier --write client/src/Pages/Notifications/create/index.jsx(or
npx prettier --write .for all staged files) and commit the changes.
Be as polite as a Canadian and as bold as an American—get that code picture-perfect!
♻️ Duplicate comments (1)
client/src/Pages/Notifications/create/index.jsx (1)
123-145: Reset dependent fields on auth/type change and normalize before validation.Mirror prior suggestion: clear creds when auth method changes; clear all ntfy fields when type ≠ ntfy; validate on normalized values so conditional rules trigger correctly.
Apply this diff:
const onChange = (e) => { const { name, value } = e.target; - let rawNotification = { ...notification, [name]: value }; - let newNotification = { - ...rawNotification, - type: getNotificationTypeValue(rawNotification.type), - }; + let rawNotification = { ...notification, [name]: value }; + + // Reset dependent ntfy fields when auth method changes + if (name === "ntfyAuthMethod") { + const method = getNtfyAuthMethodValue(rawNotification.ntfyAuthMethod); + if (method !== "username_password") { + rawNotification.ntfyUsername = ""; + rawNotification.ntfyPassword = ""; + } + if (method !== "bearer_token") { + rawNotification.ntfyBearerToken = ""; + } + } + + // If switching away from ntfy, clear ntfy-specific state + if (name === "type") { + const nextType = getNotificationTypeValue(rawNotification.type); + if (nextType !== "ntfy") { + rawNotification = { + ...rawNotification, + ntfyAuthMethod: NTFY_AUTH_METHODS[0]._id, + ntfyUsername: "", + ntfyPassword: "", + ntfyBearerToken: "", + ntfyPriority: NTFY_PRIORITIES[2]._id, + }; + } + } + + // Normalize IDs -> values prior to validation + const newNotification = { + ...rawNotification, + type: getNotificationTypeValue(rawNotification.type), + ntfyAuthMethod: getNtfyAuthMethodValue(rawNotification.ntfyAuthMethod), + ntfyPriority: getNtfyPriorityValue(rawNotification.ntfyPriority), + }; const { error } = notificationValidation.validate(newNotification, { abortEarly: false, }); let validationError = { ...errors }; if (name === "type") { validationError["type"] = extractError(error, "type"); validationError["address"] = extractError(error, "address"); + } else if (name === "ntfyAuthMethod") { + validationError["ntfyAuthMethod"] = extractError(error, "ntfyAuthMethod"); + validationError["ntfyUsername"] = extractError(error, "ntfyUsername"); + validationError["ntfyPassword"] = extractError(error, "ntfyPassword"); + validationError["ntfyBearerToken"] = extractError(error, "ntfyBearerToken"); } else { validationError[name] = extractError(error, name); } setNotification(rawNotification); setErrors(validationError); };
🧹 Nitpick comments (2)
client/src/Pages/Notifications/create/index.jsx (2)
97-100: DRY: factor submit normalization into a helper.Use one normalizer for both submit and test to avoid divergence.
Apply this diff:
+const normalizeForSubmit = (n) => ({ + ...n, + type: getNotificationTypeValue(n.type), + ntfyAuthMethod: getNtfyAuthMethodValue(n.ntfyAuthMethod), + ntfyPriority: getNtfyPriorityValue(n.ntfyPriority), +}); ... - const form = { - ...notification, - type: getNotificationTypeValue(notification.type), - // Convert ntfy dropdown IDs to actual values - ntfyAuthMethod: getNtfyAuthMethodValue(notification.ntfyAuthMethod), - ntfyPriority: getNtfyPriorityValue(notification.ntfyPriority), - }; + const form = normalizeForSubmit(notification);
151-154: Use the same normalizer here.Keep submit/test payloads identical.
Apply this diff:
- const form = { - ...notification, - type: getNotificationTypeValue(notification.type), - // Convert ntfy dropdown IDs to actual values - ntfyAuthMethod: getNtfyAuthMethodValue(notification.ntfyAuthMethod), - ntfyPriority: getNtfyPriorityValue(notification.ntfyPriority), - }; + const form = normalizeForSubmit(notification);
📜 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 (2)
client/src/Pages/Notifications/create/index.jsx(6 hunks)client/src/Pages/Notifications/utils.js(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/Pages/Notifications/utils.js
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/Pages/Notifications/create/index.jsx (2)
client/src/Pages/Notifications/utils.js (4)
NTFY_AUTH_METHODS(47-51)NTFY_AUTH_METHODS(47-51)NTFY_PRIORITIES(53-59)NTFY_PRIORITIES(53-59)client/src/Components/Inputs/Select/index.jsx (1)
Select(43-169)
🪛 GitHub Actions: Format Check (Client & Server)
client/src/Pages/Notifications/create/index.jsx
[warning] 1-1: Prettier formatting issues detected in this file. Run 'prettier --write' to fix.
🔇 Additional comments (2)
client/src/Pages/Notifications/create/index.jsx (2)
32-34: NTFY constants import looks right.Good reuse of centralized constants; avoids UI/validation drift.
257-318: Conditional ntfy UI looks correct.Auth-mode gates and password masking are handled properly; priority Select wiring matches constants.
| const getNtfyAuthMethodValue = (authMethodId) => { | ||
| return NTFY_AUTH_METHODS.find((method) => method._id === authMethodId)?.value || "none"; | ||
| }; | ||
|
|
||
| const getNtfyPriorityValue = (priorityId) => { | ||
| return NTFY_PRIORITIES.find((priority) => priority._id === priorityId)?.value || 3; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mapping helpers are fine; also use them during live validation.
Currently only submit/test normalize these; use them in onChange before validation so Joi conditions work.
🤖 Prompt for AI Agents
In client/src/Pages/Notifications/create/index.jsx around lines 80 to 86, the
helper functions getNtfyAuthMethodValue and getNtfyPriorityValue are defined but
only used on submit/test — update the onChange handler(s) so inputs are
normalized immediately using these helpers before running live Joi validation:
when the authMethodId or priorityId fields change, replace the raw id with
getNtfyAuthMethodValue(authMethodId) and getNtfyPriorityValue(priorityId) (using
their existing fallback defaults) and then call the validation routine with the
normalized values so Joi conditions evaluate correctly.
| const extractError = (error, field) => | ||
| error?.details.find((d) => d.path.includes(field))?.message; | ||
|
|
There was a problem hiding this comment.
Fix optional chaining to avoid runtime crash when no errors.
error?.details.find will throw because details can be undefined. Chain details too.
Apply this diff:
-const extractError = (error, field) =>
- error?.details.find((d) => d.path.includes(field))?.message;
+const extractError = (error, field) =>
+ error?.details?.find((d) => d.path?.includes(field))?.message;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const extractError = (error, field) => | |
| error?.details.find((d) => d.path.includes(field))?.message; | |
| const extractError = (error, field) => | |
| error?.details?.find((d) => d.path?.includes(field))?.message; |
🤖 Prompt for AI Agents
In client/src/Pages/Notifications/create/index.jsx around lines 88 to 90, the
helper uses error?.details.find(...) which will throw if details is undefined;
change the optional chaining to access details as well (e.g.,
error?.details?.find(...)) so the call short-circuits when details is missing
and preserve the existing optional chaining on the message.
|
|
||
| {/* ntfy-specific fields */} | ||
| {type === "ntfy" && ( | ||
| <> | ||
| <Select | ||
| items={NTFY_AUTH_METHODS} | ||
| label={t("createNotifications.ntfySettings.authMethodLabel")} | ||
| name="ntfyAuthMethod" | ||
| value={notification.ntfyAuthMethod} | ||
| onChange={onChange} | ||
| error={Boolean(errors.ntfyAuthMethod)} | ||
| helperText={errors["ntfyAuthMethod"]} | ||
| /> | ||
|
|
||
| {getNtfyAuthMethodValue(notification.ntfyAuthMethod) === "username_password" && ( | ||
| <> | ||
| <TextInput | ||
| label={t("createNotifications.ntfySettings.usernameLabel")} | ||
| name="ntfyUsername" | ||
| placeholder={t("createNotifications.ntfySettings.usernamePlaceholder")} | ||
| value={notification.ntfyUsername} | ||
| onChange={onChange} | ||
| error={Boolean(errors.ntfyUsername)} | ||
| helperText={errors["ntfyUsername"]} | ||
| /> | ||
| <TextInput | ||
| type="password" | ||
| label={t("createNotifications.ntfySettings.passwordLabel")} | ||
| name="ntfyPassword" | ||
| placeholder={t("createNotifications.ntfySettings.passwordPlaceholder")} | ||
| value={notification.ntfyPassword} | ||
| onChange={onChange} | ||
| error={Boolean(errors.ntfyPassword)} | ||
| helperText={errors["ntfyPassword"]} | ||
| /> | ||
| </> | ||
| )} | ||
|
|
||
| {getNtfyAuthMethodValue(notification.ntfyAuthMethod) === "bearer_token" && ( | ||
| <TextInput | ||
| type="password" | ||
| label={t("createNotifications.ntfySettings.bearerTokenLabel")} | ||
| name="ntfyBearerToken" | ||
| placeholder={t("createNotifications.ntfySettings.bearerTokenPlaceholder")} | ||
| value={notification.ntfyBearerToken} | ||
| onChange={onChange} | ||
| error={Boolean(errors.ntfyBearerToken)} | ||
| helperText={errors["ntfyBearerToken"]} | ||
| /> | ||
| )} | ||
|
|
||
| <Select | ||
| items={NTFY_PRIORITIES} | ||
| label={t("createNotifications.ntfySettings.priorityLabel")} | ||
| name="ntfyPriority" | ||
| value={notification.ntfyPriority} | ||
| onChange={onChange} | ||
| error={Boolean(errors.ntfyPriority)} | ||
| helperText={errors["ntfyPriority"]} | ||
| /> | ||
| </> | ||
| )} |
There was a problem hiding this comment.
Select’s helperText prop is ignored; ntfy Select errors won’t display.
Your Select component doesn’t accept/forward helperText. Either add it to Select and pass through to FieldWrapper, or drop it here.
Apply this change in client/src/Components/Inputs/Select/index.jsx:
-const Select = ({
+const Select = ({
id,
label,
placeholder,
isHidden,
value,
items,
onChange,
onBlur,
sx,
error = false,
name = "",
+ helperText = "",
labelControlSpacing = 6,
maxWidth,
//FieldWrapper's props
labelMb,
labelFontWeight,
labelVariant,
labelSx = {},
fieldWrapperSx = {},
}) => {
...
- return (
- <FieldWrapper ...>
+ return (
+ <FieldWrapper
+ ...
+ error={error}
+ helperText={helperText}
+ >
<MuiSelect
value={value}
onChange={onChange}
onBlur={onBlur}
- displayEmpty
- error={error}
+ displayEmpty
+ error={error}
name={name}
inputProps={{ id: id }}
...
>If you’d rather not touch Select, remove helperText props from the two ntfy Select usages to avoid confusion.
📝 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.
| {/* ntfy-specific fields */} | |
| {type === "ntfy" && ( | |
| <> | |
| <Select | |
| items={NTFY_AUTH_METHODS} | |
| label={t("createNotifications.ntfySettings.authMethodLabel")} | |
| name="ntfyAuthMethod" | |
| value={notification.ntfyAuthMethod} | |
| onChange={onChange} | |
| error={Boolean(errors.ntfyAuthMethod)} | |
| helperText={errors["ntfyAuthMethod"]} | |
| /> | |
| {getNtfyAuthMethodValue(notification.ntfyAuthMethod) === "username_password" && ( | |
| <> | |
| <TextInput | |
| label={t("createNotifications.ntfySettings.usernameLabel")} | |
| name="ntfyUsername" | |
| placeholder={t("createNotifications.ntfySettings.usernamePlaceholder")} | |
| value={notification.ntfyUsername} | |
| onChange={onChange} | |
| error={Boolean(errors.ntfyUsername)} | |
| helperText={errors["ntfyUsername"]} | |
| /> | |
| <TextInput | |
| type="password" | |
| label={t("createNotifications.ntfySettings.passwordLabel")} | |
| name="ntfyPassword" | |
| placeholder={t("createNotifications.ntfySettings.passwordPlaceholder")} | |
| value={notification.ntfyPassword} | |
| onChange={onChange} | |
| error={Boolean(errors.ntfyPassword)} | |
| helperText={errors["ntfyPassword"]} | |
| /> | |
| </> | |
| )} | |
| {getNtfyAuthMethodValue(notification.ntfyAuthMethod) === "bearer_token" && ( | |
| <TextInput | |
| type="password" | |
| label={t("createNotifications.ntfySettings.bearerTokenLabel")} | |
| name="ntfyBearerToken" | |
| placeholder={t("createNotifications.ntfySettings.bearerTokenPlaceholder")} | |
| value={notification.ntfyBearerToken} | |
| onChange={onChange} | |
| error={Boolean(errors.ntfyBearerToken)} | |
| helperText={errors["ntfyBearerToken"]} | |
| /> | |
| )} | |
| <Select | |
| items={NTFY_PRIORITIES} | |
| label={t("createNotifications.ntfySettings.priorityLabel")} | |
| name="ntfyPriority" | |
| value={notification.ntfyPriority} | |
| onChange={onChange} | |
| error={Boolean(errors.ntfyPriority)} | |
| helperText={errors["ntfyPriority"]} | |
| /> | |
| </> | |
| )} | |
| // client/src/Components/Inputs/Select/index.jsx | |
| import React from "react"; | |
| import { Select as MuiSelect, MenuItem } from "@mui/material"; | |
| import FieldWrapper from "../FieldWrapper"; | |
| const Select = ({ | |
| id, | |
| label, | |
| placeholder, | |
| isHidden, | |
| value, | |
| items, | |
| onChange, | |
| onBlur, | |
| sx, | |
| error = false, | |
| name = "", | |
| helperText = "", | |
| labelControlSpacing = 6, | |
| maxWidth, | |
| // FieldWrapper's props | |
| labelMb, | |
| labelFontWeight, | |
| labelVariant, | |
| labelSx = {}, | |
| fieldWrapperSx = {}, | |
| }) => { | |
| return ( | |
| <FieldWrapper | |
| id={id} | |
| label={label} | |
| placeholder={placeholder} | |
| isHidden={isHidden} | |
| error={error} | |
| helperText={helperText} | |
| labelControlSpacing={labelControlSpacing} | |
| maxWidth={maxWidth} | |
| mb={labelMb} | |
| fontWeight={labelFontWeight} | |
| variant={labelVariant} | |
| labelSx={labelSx} | |
| sx={fieldWrapperSx} | |
| > | |
| <MuiSelect | |
| value={value} | |
| onChange={onChange} | |
| onBlur={onBlur} | |
| displayEmpty | |
| error={error} | |
| name={name} | |
| inputProps={{ id }} | |
| sx={sx} | |
| > | |
| {items.map((item) => ( | |
| <MenuItem key={item.value} value={item.value}> | |
| {item.label} | |
| </MenuItem> | |
| ))} | |
| </MuiSelect> | |
| </FieldWrapper> | |
| ); | |
| }; | |
| export default Select; |
🤖 Prompt for AI Agents
In client/src/Pages/Notifications/create/index.jsx lines 257-318 the Select
components pass helperText but the Select implementation doesn’t forward it, so
ntfy Select errors won’t display; fix this by editing
client/src/Components/Inputs/Select/index.jsx to accept a helperText prop
(alongside error) and forward it into the underlying FieldWrapper (or equivalent
wrapper that renders the label/validation UI), update the Select
propTypes/defaultProps to include helperText, and ensure existing callers
continue to work (alternatively remove helperText from these two ntfy Select
usages if you prefer not to change Select).
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
client/src/Pages/Notifications/create/index.jsx (3)
123-133: Normalize for validation AND reset dependent creds when auth/type changes.
- For validation: drop ntfy* when type ≠ "ntfy".
- When ntfyAuthMethod changes: clear irrelevant creds.
- When switching type away from ntfy: reset all ntfy fields to safe defaults.
const onChange = (e) => { const { name, value } = e.target; - let rawNotification = { ...notification, [name]: value }; - let newNotification = { - ...rawNotification, - type: getNotificationTypeValue(rawNotification.type), - // Convert ntfy dropdown IDs to actual values for validation - ntfyAuthMethod: getNtfyAuthMethodValue(rawNotification.ntfyAuthMethod), - ntfyPriority: getNtfyPriorityValue(rawNotification.ntfyPriority), - }; + let rawNotification = { ...notification, [name]: value }; + // Reset dependent ntfy fields when auth method changes + if (name === "ntfyAuthMethod") { + const nextAuth = getNtfyAuthMethodValue(value); + if (nextAuth !== "username_password") { + rawNotification.ntfyUsername = ""; + rawNotification.ntfyPassword = ""; + } + if (nextAuth !== "bearer_token") { + rawNotification.ntfyBearerToken = ""; + } + } + // Clear ntfy fields if switching away from ntfy + if (name === "type") { + const nextType = getNotificationTypeValue(value); + if (nextType !== "ntfy") { + rawNotification.ntfyAuthMethod = NTFY_AUTH_METHODS[0]._id; + rawNotification.ntfyUsername = ""; + rawNotification.ntfyPassword = ""; + rawNotification.ntfyBearerToken = ""; + rawNotification.ntfyPriority = NTFY_PRIORITIES[2]._id; + } + } + // Build normalized object for Joi + const typeValue = getNotificationTypeValue(rawNotification.type); + let newNotification; + if (typeValue === "ntfy") { + newNotification = { + ...rawNotification, + type: typeValue, + ntfyAuthMethod: getNtfyAuthMethodValue(rawNotification.ntfyAuthMethod), + ntfyPriority: getNtfyPriorityValue(rawNotification.ntfyPriority), + }; + } else { + const { + ntfyAuthMethod, + ntfyUsername, + ntfyPassword, + ntfyBearerToken, + ntfyPriority, + ...rest + } = rawNotification; + newNotification = { ...rest, type: typeValue }; + }Also applies to: 139-148
88-90: Fix crash when no validation details are present.Optional-chain details and path.
-const extractError = (error, field) => - error?.details.find((d) => d.path.includes(field))?.message; +const extractError = (error, field) => + error?.details?.find((d) => d?.path?.includes?.(field))?.message;
270-278: Select doesn’t surface helperText → ntfy Select errors won’t show.Either add helperText passthrough to Select or drop it here. Prefer passthrough fix below.
Diff for client/src/Components/Inputs/Select/index.jsx:
const Select = ({ id, label, placeholder, isHidden, value, items, onChange, onBlur, sx, error = false, name = "", + helperText = "", ... }) => { ... - return ( - <FieldWrapper + return ( + <FieldWrapper ... error={error} + helperText={helperText} > <MuiSelect value={value} onChange={onChange} onBlur={onBlur} error={error} name={name} ...Also applies to: 317-325
🧹 Nitpick comments (4)
client/src/Pages/Notifications/create/index.jsx (1)
80-86: Avoid drift: centralize these mapping helpers.Consider exporting these from ../utils to keep a single source of truth (same shape as NOTIFICATION_TYPES).
client/src/Validation/validation.js (3)
517-529: Trim password input.Same rationale as username.
- then: joi.string().required().messages({ + then: joi.string().trim().required().messages({ "string.empty": "Password cannot be empty when using username/password authentication", "any.required": "Password is required for username/password authentication", }),
457-496: Biome lint: additional “then” branches here will also trigger.Apply the same inline ignore comment before this when/switch block (and other ntfy when blocks) or adjust Biome config for Joi files. Otherwise CI may be grumpy like an American without coffee.
441-551: End-to-end checks: add tests for ntfy validation branches.
- ntfy + none: URL required; creds optional/forbidden.
- ntfy + username/password: username/password required; token forbidden.
- ntfy + bearer: token required; username/password forbidden.
Do you want me to draft these unit tests?
📜 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 (2)
client/src/Pages/Notifications/create/index.jsx(8 hunks)client/src/Validation/validation.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/Pages/Notifications/create/index.jsx (2)
client/src/Pages/Notifications/utils.js (4)
NTFY_AUTH_METHODS(47-51)NTFY_AUTH_METHODS(47-51)NTFY_PRIORITIES(53-59)NTFY_PRIORITIES(53-59)client/src/Components/Inputs/Select/index.jsx (1)
Select(43-169)
🪛 Biome (2.1.2)
client/src/Validation/validation.js
[error] 488-488: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 500-500: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 506-506: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 508-508: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 519-519: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 521-521: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 532-532: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 534-534: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 545-545: Do not add then to an object.
(lint/suspicious/noThenProperty)
🪛 GitHub Actions: Format Check (Client & Server)
client/src/Validation/validation.js
[warning] 1-1: Prettier formatting issue detected by 'prettier --check .'. Run 'prettier --write .' to fix.
client/src/Pages/Notifications/create/index.jsx
[warning] 1-1: Prettier formatting issue detected by 'prettier --check .'. Run 'prettier --write .' to fix.
🔇 Additional comments (6)
client/src/Pages/Notifications/create/index.jsx (3)
32-34: LGTM: ntfy constants imported correctly.
1-1: Pipeline: run Prettier.CI reports a Prettier check failure; please run: prettier --write .
160-163: Verify create flow test coverage for payload sanitization
I couldn’t find any test file underclient/src/Pages/Notifications/create; please confirm where the “submit” flow tests live and ensure they mirror this fix by sanitizing the payload before assertions—eh?client/src/Validation/validation.js (3)
449-455: LGTM: ntfy added to allowed types and message updated.
498-503: Suppress Biome’s noThenProperty for Joi branches (example).Biome flags object keys named “then”. Add an inline ignore on Joi branch objects (repeat for similar blocks in this schema).
- ntfyAuthMethod: joi.when("type", { + /* biome-ignore lint/suspicious/noThenProperty: Joi schema legitimately uses "then" */ + ntfyAuthMethod: joi.when("type", { is: "ntfy", then: joi.string().valid("none", "username_password", "bearer_token").default("none"), otherwise: joi.forbidden(), }),
1-1: Pipeline: run Prettier.CI flagged formatting; run: prettier --write .
| ntfyUsername: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "username_password", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": "Username cannot be empty when using username/password authentication", | ||
| "any.required": "Username is required for username/password authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Trim username; keep empty allowed only when not required.
Prevents leading/trailing spaces sneaking in.
- then: joi.string().required().messages({
+ then: joi.string().trim().required().messages({
"string.empty": "Username cannot be empty when using username/password authentication",
"any.required": "Username is required for username/password authentication",
}),
otherwise: joi.string().optional().allow(""),📝 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.
| ntfyUsername: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.when("ntfyAuthMethod", { | |
| is: "username_password", | |
| then: joi.string().required().messages({ | |
| "string.empty": "Username cannot be empty when using username/password authentication", | |
| "any.required": "Username is required for username/password authentication", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), | |
| ntfyUsername: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.when("ntfyAuthMethod", { | |
| is: "username_password", | |
| then: joi.string().trim().required().messages({ | |
| "string.empty": "Username cannot be empty when using username/password authentication", | |
| "any.required": "Username is required for username/password authentication", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), |
🧰 Tools
🪛 Biome (2.1.2)
[error] 506-506: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 508-508: Do not add then to an object.
(lint/suspicious/noThenProperty)
🤖 Prompt for AI Agents
In client/src/Validation/validation.js around lines 504 to 516, the ntfyUsername
schema should trim whitespace and only allow empty strings when not required;
change the branches to use joi.string().trim() so the required branch becomes
joi.string().trim().required().messages({...}) and the otherwise branch becomes
joi.string().trim().optional().allow(""), ensuring trimmed whitespace is removed
before validation and empty strings are only permitted when not required.
| ntfyBearerToken: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "bearer_token", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": "Bearer token cannot be empty when using bearer token authentication", | ||
| "any.required": "Bearer token is required for bearer token authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Harden bearer token: trim and forbid whitespace.
ntfy tokens shouldn’t contain spaces.
- then: joi.string().required().messages({
+ then: joi.string().trim().pattern(/^\S+$/).required().messages({
"string.empty": "Bearer token cannot be empty when using bearer token authentication",
"any.required": "Bearer token is required for bearer token authentication",
+ "string.pattern.base": "Bearer token must not contain spaces",
}),📝 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.
| ntfyBearerToken: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.when("ntfyAuthMethod", { | |
| is: "bearer_token", | |
| then: joi.string().required().messages({ | |
| "string.empty": "Bearer token cannot be empty when using bearer token authentication", | |
| "any.required": "Bearer token is required for bearer token authentication", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), | |
| ntfyBearerToken: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.when("ntfyAuthMethod", { | |
| is: "bearer_token", | |
| then: joi.string().trim().pattern(/^\S+$/).required().messages({ | |
| "string.empty": "Bearer token cannot be empty when using bearer token authentication", | |
| "any.required": "Bearer token is required for bearer token authentication", | |
| "string.pattern.base": "Bearer token must not contain spaces", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), |
🧰 Tools
🪛 Biome (2.1.2)
[error] 532-532: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 534-534: Do not add then to an object.
(lint/suspicious/noThenProperty)
🤖 Prompt for AI Agents
In client/src/Validation/validation.js around lines 530 to 541, the
ntfyBearerToken schema accepts tokens with surrounding or internal whitespace;
update the Joi rule to trim input and reject any whitespace characters (e.g.,
use .trim() and a pattern like /^\S+$/) while preserving the conditional
required/optional behavior and custom messages; add a clear validation message
for the whitespace/pattern failure so tokens with spaces are forbidden and
empty/required messages remain intact.
| ntfyPriority: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.number().min(1).max(5).default(3).messages({ | ||
| "number.min": "Priority must be between 1 and 5", | ||
| "number.max": "Priority must be between 1 and 5", | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce integer priority 1–5.
Avoids floats like 3.14.
- then: joi.number().min(1).max(5).default(3).messages({
+ then: joi.number().integer().min(1).max(5).default(3).messages({
+ "number.integer": "Priority must be an integer between 1 and 5",
"number.min": "Priority must be between 1 and 5",
"number.max": "Priority must be between 1 and 5",
}),📝 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.
| ntfyPriority: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.number().min(1).max(5).default(3).messages({ | |
| "number.min": "Priority must be between 1 and 5", | |
| "number.max": "Priority must be between 1 and 5", | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), | |
| ntfyPriority: joi.when("type", { | |
| is: "ntfy", | |
| then: joi.number().integer().min(1).max(5).default(3).messages({ | |
| "number.integer": "Priority must be an integer between 1 and 5", | |
| "number.min": "Priority must be between 1 and 5", | |
| "number.max": "Priority must be between 1 and 5", | |
| }), | |
| otherwise: joi.forbidden(), | |
| }), |
🧰 Tools
🪛 Biome (2.1.2)
[error] 545-545: Do not add then to an object.
(lint/suspicious/noThenProperty)
🤖 Prompt for AI Agents
In client/src/Validation/validation.js around lines 543 to 550, the ntfyPriority
schema allows non-integer numbers (e.g., 3.14); update the schema to require
integers by adding .integer() to the joi.number() chain and adjust validation
messages to include an integer-specific message (e.g., "number.integer":
"Priority must be an integer between 1 and 5") while keeping the min(1), max(5)
and default(3); ensure otherwise remains joi.forbidden().
- Add null safety checks for greeting state access - Fix greeting array index bounds checking - Clean up notification service conditional formatting
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
client/src/Features/UI/uiSlice.js (1)
57-61: Defensive init is good; also harden payload handling to avoid undefined writesIf callers ever dispatch without a full payload, these direct writes can set
undefined. Destructure with defaults and assign once.- setGreeting(state, action) { - if (!state.greeting) { - state.greeting = { index: 0, lastUpdate: null }; - } - state.greeting.index = action.payload.index; - state.greeting.lastUpdate = action.payload.lastUpdate; - }, + setGreeting(state, action) { + if (!state.greeting) { + state.greeting = { index: 0, lastUpdate: null }; + } + const { index = state.greeting.index, lastUpdate = state.greeting.lastUpdate } = + action.payload ?? {}; + state.greeting = { index, lastUpdate }; + },Tiny joke while we’re here: this avoids the greeting going “undefined” like Canadians at a four-way stop—everyone’s polite, nobody moves.
client/src/Utils/greeting.jsx (1)
156-157: Nice bounds-check; also remove magic number for random selectionYou already guard with
safeIndex. To future-proof, derive the random upper bound from the active list length rather than hard-coding 5.Minimal change (outside changed lines 149):
const current = hour < 6 ? early : hour < 12 ? morning : hour < 18 ? afternoon : evening; const random = Math.floor(Math.random() * current.length); dispatch(setGreeting({ index: random, lastUpdate: hour }));
📜 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/Features/UI/uiSlice.js(1 hunks)client/src/Utils/greeting.jsx(2 hunks)server/src/service/infrastructure/notificationService.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/src/service/infrastructure/notificationService.js
- Fix code formatting in server files - Fix code formatting in client files - Ensure compliance with project style guidelines
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/Pages/Notifications/create/index.jsx (1)
125-156: Live validation should normalize and conditionally omit ntfy fields; also reset creds/tokens on auth change.Avoid Joi forbidden errors during typing and clear stale secrets when auth changes.
const onChange = (e) => { const { name, value } = e.target; - let rawNotification = { ...notification, [name]: value }; - let newNotification = { - ...rawNotification, - type: getNotificationTypeValue(rawNotification.type), - // Convert ntfy dropdown IDs to actual values for validation - ntfyAuthMethod: getNtfyAuthMethodValue(rawNotification.ntfyAuthMethod), - ntfyPriority: getNtfyPriorityValue(rawNotification.ntfyPriority), - }; + let rawNotification = { ...notification, [name]: value }; + // Reset dependent fields when auth method changes + if (name === "ntfyAuthMethod") { + const method = getNtfyAuthMethodValue(value); + if (method !== "username_password") { + rawNotification.ntfyUsername = ""; + rawNotification.ntfyPassword = ""; + } + if (method !== "bearer_token") { + rawNotification.ntfyBearerToken = ""; + } + } + const typeValue = getNotificationTypeValue(rawNotification.type); + let newNotificationBase = { ...rawNotification, type: typeValue }; + // For validation: include ntfy fields only when needed and normalize values + let newNotification; + if (typeValue === "ntfy") { + newNotification = { + ...newNotificationBase, + ntfyAuthMethod: getNtfyAuthMethodValue(rawNotification.ntfyAuthMethod), + ntfyPriority: getNtfyPriorityValue(rawNotification.ntfyPriority), + }; + } else { + const { + ntfyAuthMethod, + ntfyUsername, + ntfyPassword, + ntfyBearerToken, + ntfyPriority, + ...sanitized + } = newNotificationBase; + newNotification = sanitized; + } const { error } = notificationValidation.validate(newNotification, { abortEarly: false, }); let validationError = { ...errors }; if (name === "type") { validationError["type"] = extractError(error, "type"); validationError["address"] = extractError(error, "address"); - } else if (name === "ntfyAuthMethod") { + // Also refresh ntfy field errors when switching into/out of ntfy + if (typeValue === "ntfy") { + validationError["ntfyAuthMethod"] = extractError(error, "ntfyAuthMethod"); + validationError["ntfyUsername"] = extractError(error, "ntfyUsername"); + validationError["ntfyPassword"] = extractError(error, "ntfyPassword"); + validationError["ntfyBearerToken"] = extractError(error, "ntfyBearerToken"); + validationError["ntfyPriority"] = extractError(error, "ntfyPriority"); + } else { + delete validationError["ntfyAuthMethod"]; + delete validationError["ntfyUsername"]; + delete validationError["ntfyPassword"]; + delete validationError["ntfyBearerToken"]; + delete validationError["ntfyPriority"]; + } + } else if (name === "ntfyAuthMethod") { // When auth method changes, update all ntfy field errors validationError["ntfyAuthMethod"] = extractError(error, "ntfyAuthMethod"); validationError["ntfyUsername"] = extractError(error, "ntfyUsername"); validationError["ntfyPassword"] = extractError(error, "ntfyPassword"); validationError["ntfyBearerToken"] = extractError(error, "ntfyBearerToken"); } else { validationError[name] = extractError(error, name); } setNotification(rawNotification); setErrors(validationError); };Canadian aside: let’s not put ketchup (ntfy fields) on every payload—save it for fries (type === "ntfy"). 🇨🇦
♻️ Duplicate comments (9)
client/src/Pages/Notifications/create/index.jsx (5)
62-68: Defaults OK; add dependent resets when auth method changes.Good defaults. But stale creds/tokens persist when switching auth methods; reset them on change to avoid leaking unintended secrets in submissions.
Apply resets in the ntfyAuthMethod branch of onChange (see patch on Lines 125-156).
90-91: Fix optional chaining to avoid crash when no details exist.
error?.details.find(...)can throw when details is undefined.-const extractError = (error, field) => - error?.details.find((d) => d.path.includes(field))?.message; +const extractError = (error, field) => + error?.details?.find((d) => d.path?.includes(field))?.message;
96-102: Blocker: strip ntfy fields when type ≠ "ntfy" before validating/submitting.Joi forbids these outside ntfy; current code will fail non-ntfy submits.
-const form = { - ...notification, - type: getNotificationTypeValue(notification.type), - // Convert ntfy dropdown IDs to actual values - ntfyAuthMethod: getNtfyAuthMethodValue(notification.ntfyAuthMethod), - ntfyPriority: getNtfyPriorityValue(notification.ntfyPriority), -}; +const typeValue = getNotificationTypeValue(notification.type); +const base = { ...notification, type: typeValue }; +let form; +if (typeValue === "ntfy") { + form = { + ...base, + ntfyAuthMethod: getNtfyAuthMethodValue(notification.ntfyAuthMethod), + ntfyPriority: getNtfyPriorityValue(notification.ntfyPriority), + }; +} else { + const { + ntfyAuthMethod, + ntfyUsername, + ntfyPassword, + ntfyBearerToken, + ntfyPriority, + ...sanitized + } = base; + form = sanitized; +}Optionally also pass
{ stripUnknown: true }to Joi validate to harden against stray fields.
162-165: Mirror submit fix in the Test flow.Same forbidden-field issue applies to test payloads.
-const form = { - ...notification, - type: getNotificationTypeValue(notification.type), - // Convert ntfy dropdown IDs to actual values - ntfyAuthMethod: getNtfyAuthMethodValue(notification.ntfyAuthMethod), - ntfyPriority: getNtfyPriorityValue(notification.ntfyPriority), -}; +const typeValue = getNotificationTypeValue(notification.type); +const base = { ...notification, type: typeValue }; +const form = + typeValue === "ntfy" + ? { + ...base, + ntfyAuthMethod: getNtfyAuthMethodValue(notification.ntfyAuthMethod), + ntfyPriority: getNtfyPriorityValue(notification.ntfyPriority), + } + : (({ ntfyAuthMethod, ntfyUsername, ntfyPassword, ntfyBearerToken, ntfyPriority, ...rest }) => rest)(base);
269-337: Select helperText isn’t rendered; ntfy Select errors won’t show.Your Select doesn’t forward helperText; either update the component or remove these props here to avoid confusion.
<Select items={NTFY_AUTH_METHODS} label={t("createNotifications.ntfySettings.authMethodLabel")} name="ntfyAuthMethod" value={notification.ntfyAuthMethod} onChange={onChange} error={Boolean(errors.ntfyAuthMethod)} - helperText={errors["ntfyAuthMethod"]} /> ... <Select items={NTFY_PRIORITIES} label={t("createNotifications.ntfySettings.priorityLabel")} name="ntfyPriority" value={notification.ntfyPriority} onChange={onChange} error={Boolean(errors.ntfyPriority)} - helperText={errors["ntfyPriority"]} />Preferably, enhance client/src/Components/Inputs/Select to accept/forward helperText to FieldWrapper so errors display inline.
client/src/Validation/validation.js (4)
509-516: Trim username.Avoid leading/trailing spaces sneaking in.
- then: joi.string().required().messages({ + /* biome-ignore lint/suspicious/noThenProperty: Joi uses 'then'. */ + then: joi.string().trim().required().messages({ "string.empty": "Username cannot be empty when using username/password authentication", "any.required": "Username is required for username/password authentication", }), - otherwise: joi.string().optional().allow(""), + otherwise: joi.string().trim().optional().allow(""),
537-543: Harden bearer token: trim and forbid whitespace.ntfy tokens shouldn’t contain spaces.
- then: joi.string().required().messages({ + /* biome-ignore lint/suspicious/noThenProperty: Joi uses 'then'. */ + then: joi.string().trim().pattern(/^\S+$/).required().messages({ "string.empty": "Bearer token cannot be empty when using bearer token authentication", "any.required": "Bearer token is required for bearer token authentication", + "string.pattern.base": "Bearer token must not contain spaces", }), - otherwise: joi.string().optional().allow(""), + otherwise: joi.string().trim().optional().allow(""),
549-554: Enforce integer priority 1–5.Prevents 3.14-like values.
- then: joi.number().min(1).max(5).default(3).messages({ + /* biome-ignore lint/suspicious/noThenProperty: Joi uses 'then'. */ + then: joi.number().integer().min(1).max(5).default(3).messages({ + "number.integer": "Priority must be an integer between 1 and 5", "number.min": "Priority must be between 1 and 5", "number.max": "Priority must be between 1 and 5", }),
487-494: Tighten ntfy URL validation: trim + restrict scheme to http/https.Prevents malformed schemes and stray spaces.
- { - is: "ntfy", - then: joi.string().uri().required().messages({ - "string.empty": "ntfy URL cannot be empty", - "any.required": "ntfy URL is required", - "string.uri": "Please enter a valid ntfy URL", - }), - }, + { + is: "ntfy", + /* biome-ignore lint/suspicious/noThenProperty: Joi conditional schema legitimately uses 'then'. */ + then: joi + .string() + .trim() + .uri({ scheme: ["http", "https"] }) + .required() + .messages({ + "string.empty": "ntfy URL cannot be empty", + "any.required": "ntfy URL is required", + "string.uri": "Please enter a valid ntfy URL", + }), + },
📜 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/Notifications/create/index.jsx(8 hunks)client/src/Pages/Notifications/utils.js(5 hunks)client/src/Validation/validation.js(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- client/src/Pages/Notifications/utils.js
🧰 Additional context used
🧬 Code graph analysis (1)
client/src/Pages/Notifications/create/index.jsx (2)
client/src/Pages/Notifications/utils.js (4)
NTFY_AUTH_METHODS(47-51)NTFY_AUTH_METHODS(47-51)NTFY_PRIORITIES(53-59)NTFY_PRIORITIES(53-59)client/src/Components/Inputs/Select/index.jsx (1)
Select(43-169)
🪛 Biome (2.1.2)
client/src/Validation/validation.js
[error] 489-489: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 501-501: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 507-507: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 509-509: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 521-521: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 523-523: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 535-535: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 537-537: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 549-549: Do not add then to an object.
(lint/suspicious/noThenProperty)
🔇 Additional comments (3)
client/src/Pages/Notifications/create/index.jsx (1)
80-89: Helper mappers LGTM.Straightforward and defensive fallbacks. Keep.
client/src/Validation/validation.js (2)
449-456: ntfy type addition LGTM.Enum updated and message aligned.
498-505: Auth method schema OK; consider marking required instead of default if server expects explicit value.If API defaults it, keep as-is.
Do server-side defaults match "none"? If not, switch to
.required()here.
| { | ||
| is: "ntfy", | ||
| then: joi.string().uri().required().messages({ | ||
| "string.empty": "ntfy URL cannot be empty", | ||
| "any.required": "ntfy URL is required", | ||
| "string.uri": "Please enter a valid ntfy URL", | ||
| }), | ||
| }, | ||
| ], | ||
| }), | ||
|
|
||
| // ntfy-specific fields | ||
| ntfyAuthMethod: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.string().valid("none", "username_password", "bearer_token").default("none"), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyUsername: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "username_password", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": | ||
| "Username cannot be empty when using username/password authentication", | ||
| "any.required": "Username is required for username/password authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyPassword: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "username_password", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": | ||
| "Password cannot be empty when using username/password authentication", | ||
| "any.required": "Password is required for username/password authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyBearerToken: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.when("ntfyAuthMethod", { | ||
| is: "bearer_token", | ||
| then: joi.string().required().messages({ | ||
| "string.empty": | ||
| "Bearer token cannot be empty when using bearer token authentication", | ||
| "any.required": "Bearer token is required for bearer token authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), | ||
|
|
||
| ntfyPriority: joi.when("type", { | ||
| is: "ntfy", | ||
| then: joi.number().min(1).max(5).default(3).messages({ | ||
| "number.min": "Priority must be between 1 and 5", | ||
| "number.max": "Priority must be between 1 and 5", | ||
| }), | ||
| otherwise: joi.forbidden(), | ||
| }), |
There was a problem hiding this comment.
Silence Biome’s false-positive on Joi’s then keys.
Biome’s noThenProperty trips on Joi condition objects; add inline ignores (as shown) or disable this rule for this file in config. Otherwise CI will fail despite valid Joi.
Happy to open a quick PR to add targeted /* biome-ignore lint/suspicious/noThenProperty */ comments or tweak .biomerc config.
🧰 Tools
🪛 Biome (2.1.2)
[error] 489-489: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 501-501: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 507-507: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 509-509: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 521-521: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 523-523: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 535-535: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 537-537: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 549-549: Do not add then to an object.
(lint/suspicious/noThenProperty)
🤖 Prompt for AI Agents
In client/src/Validation/validation.js around lines 487 to 554, Biome's
false-positive rule lint/suspicious/noThenProperty flags Joi conditional
objects' "then" keys; fix by adding targeted inline Biome ignore comments before
the affected Joi.when expressions (e.g. place /* biome-ignore
lint/suspicious/noThenProperty */ immediately above each Joi.when that uses a
then key) or, if preferred, add a single file-scoped disable (/* biome-disable
lint/suspicious/noThenProperty */ at the top of this file) so CI stops failing
while preserving the valid Joi schema code.
| then: joi.string().required().messages({ | ||
| "string.empty": | ||
| "Password cannot be empty when using username/password authentication", | ||
| "any.required": "Password is required for username/password authentication", | ||
| }), | ||
| otherwise: joi.string().optional().allow(""), | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Trim password.
Same rationale as username.
- then: joi.string().required().messages({
+ /* biome-ignore lint/suspicious/noThenProperty: Joi uses 'then'. */
+ then: joi.string().trim().required().messages({
"string.empty":
"Password cannot be empty when using username/password authentication",
"any.required": "Password is required for username/password authentication",
}),
- otherwise: joi.string().optional().allow(""),
+ otherwise: joi.string().trim().optional().allow(""),📝 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.
| then: joi.string().required().messages({ | |
| "string.empty": | |
| "Password cannot be empty when using username/password authentication", | |
| "any.required": "Password is required for username/password authentication", | |
| }), | |
| otherwise: joi.string().optional().allow(""), | |
| }), | |
| /* biome-ignore lint/suspicious/noThenProperty: Joi uses 'then'. */ | |
| then: joi.string().trim().required().messages({ | |
| "string.empty": | |
| "Password cannot be empty when using username/password authentication", | |
| "any.required": "Password is required for username/password authentication", | |
| }), | |
| otherwise: joi.string().trim().optional().allow(""), | |
| }), |
🧰 Tools
🪛 Biome (2.1.2)
[error] 523-523: Do not add then to an object.
(lint/suspicious/noThenProperty)
🤖 Prompt for AI Agents
In client/src/Validation/validation.js around lines 523 to 529, the password
schema doesn't trim input like the username does; update the Joi schema to call
.trim() on both the required and optional branches so leading/trailing
whitespace is removed (i.e., use joi.string().trim().required() in the then
branch and joi.string().trim().optional().allow("") in the otherwise branch).
- Add missing ntfy-specific fields to useGetNotificationById hook - Map ntfyAuthMethod, ntfyUsername, ntfyPassword, ntfyBearerToken, ntfyPriority from API response - Convert stored values back to dropdown IDs for form population - Include fallback values for missing or invalid ntfy field data
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
server/src/service/infrastructure/networkService.js (2)
451-455: Harden ntfy request: clamp priority [1..5], default empty body, and add timeout.Prevents bad headers, avoids hanging jobs, and ensures body isn’t
undefined. Even Canadians won’t apologize if our requests don’t hang, eh.- // Build headers - const headers = { - Title: title, - Priority: notification.ntfyPriority?.toString() || "3", - Tags: "checkmate,monitoring", - "Content-Type": "text/plain", - }; + // Build headers + const priority = String(Math.min(5, Math.max(1, Number(notification?.ntfyPriority ?? 3)))); + const headers = { + Priority: priority, + Tags: "checkmate,monitoring", + "Content-Type": "text/plain", + }; + if (title) headers.Title = title; @@ - const response = await this.axios.post(url, message, { headers }); + const response = await this.axios.post(url, message ?? "", { headers, timeout: 10000 });Also applies to: 466-466
476-481: Sanitize URL in logs to avoid leaking secrets/topic names.Logging raw
urlmay expose credentials or sensitive topics. Redact before logging.- this.logger.warn({ - message: error.message, - service: this.SERVICE_NAME, - method: "requestNtfy", - url: url, - }); + const safeUrl = (() => { + try { + const u = new URL(url); + u.username = ""; + u.password = ""; + u.search = ""; + return u.toString(); + } catch { + return undefined; + } + })(); + this.logger.warn({ + message: error.message, + service: this.SERVICE_NAME, + method: "requestNtfy", + url: safeUrl, + });
🧹 Nitpick comments (3)
client/src/Hooks/useNotifications.js (1)
110-121: Use nullish coalescing for safe fallbacks (avoid 0/"" pitfalls) and keep string defaults consistent.Current
||can misfire if an_idever becomes0. Prefer??and keep field defaults via?? "".- ntfyAuthMethod: - NTFY_AUTH_METHODS.find( - (method) => method.value === notification?.ntfyAuthMethod - )?._id || NTFY_AUTH_METHODS[0]._id, - ntfyUsername: notification?.ntfyUsername || "", - ntfyPassword: notification?.ntfyPassword || "", - ntfyBearerToken: notification?.ntfyBearerToken || "", - ntfyPriority: - NTFY_PRIORITIES.find( - (priority) => priority.value === notification?.ntfyPriority - )?._id || NTFY_PRIORITIES[2]._id, + ntfyAuthMethod: + (NTFY_AUTH_METHODS.find( + (method) => method.value === notification?.ntfyAuthMethod + )?._id) ?? NTFY_AUTH_METHODS[0]._id, + ntfyUsername: notification?.ntfyUsername ?? "", + ntfyPassword: notification?.ntfyPassword ?? "", + ntfyBearerToken: notification?.ntfyBearerToken ?? "", + ntfyPriority: + (NTFY_PRIORITIES.find( + (priority) => priority.value === notification?.ntfyPriority + )?._id) ?? NTFY_PRIORITIES[2]._id,server/src/service/infrastructure/networkService.js (2)
458-463: Warn on credentials sent over non-HTTPS.If auth is Basic/Bearer and
urlishttp:, emit a warning. Americans might YOLO it; we shouldn’t.// Add authentication headers based on method if (notification.ntfyAuthMethod === "username_password" && notification.ntfyUsername && notification.ntfyPassword) { const auth = Buffer.from(`${notification.ntfyUsername}:${notification.ntfyPassword}`).toString("base64"); headers["Authorization"] = `Basic ${auth}`; } else if (notification.ntfyAuthMethod === "bearer_token" && notification.ntfyBearerToken) { headers["Authorization"] = `Bearer ${notification.ntfyBearerToken}`; } + + // Warn if credentials will travel over non-HTTPS + try { + const { protocol } = new URL(url); + if ( + protocol !== "https:" && + (notification.ntfyAuthMethod === "username_password" || notification.ntfyAuthMethod === "bearer_token") + ) { + this.logger.warn({ + message: "Sending ntfy credentials over non-HTTPS", + service: this.SERVICE_NAME, + method: "requestNtfy", + }); + } + } catch {}
447-491: Add unit tests for header building and error paths.Cover: public, Basic, Bearer; priority clamp; timeout behavior (mock axios); sanitized logging on failures. I can scaffold Jest tests with axios mocks.
📜 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/Hooks/useNotifications.js(2 hunks)server/src/service/infrastructure/networkService.js(1 hunks)server/src/service/infrastructure/notificationService.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- server/src/service/infrastructure/notificationService.js
🧰 Additional context used
🧬 Code graph analysis (2)
client/src/Hooks/useNotifications.js (2)
client/src/Pages/Notifications/utils.js (4)
NTFY_AUTH_METHODS(47-51)NTFY_AUTH_METHODS(47-51)NTFY_PRIORITIES(53-59)NTFY_PRIORITIES(53-59)client/src/Pages/Notifications/create/index.jsx (1)
notification(58-68)
server/src/service/infrastructure/networkService.js (1)
client/src/Pages/Notifications/create/index.jsx (1)
notification(58-68)
🔇 Additional comments (1)
client/src/Hooks/useNotifications.js (1)
6-10: Imports look correct and scoped to actual usage.All three symbols are used below; no dead imports.
|
Nice feature, can't wait to see it in action 👍🏻 |
Summary
Adds comprehensive ntfy notification support to Checkmate, enabling users to receive push notifications via ntfy service (self-hosted or ntfy.sh cloud).
Features Added
Backend Implementation
Database Changes
ntfyto notification type enum in Notification modelntfyAuthMethod,ntfyUsername,ntfyPassword,ntfyBearerToken,ntfyPriorityService Layer
requestNtfy()method with comprehensive auth supportFrontend Implementation
UI Components
Translation Support
Configuration Options
Required Fields
https://ntfy.sh/my-topicorhttps://my-server.com/topicOptional Fields