Merge pull request #3302 from bluewave-labs/fix/settings-form

fix: settings form

client/src/Hooks/useSettingsForm.ts

@@ -20,14 +20,14 @@ export const useSettingsForm = ({ data = null }: UseSettingsFormOptions = {}) =>
 			systemEmailAddress: data?.systemEmailAddress || "",
 			systemEmailConnectionHost: data?.systemEmailConnectionHost || "localhost",
 			systemEmailTLSServername: data?.systemEmailTLSServername || "",
-			systemEmailPort: data?.systemEmailPort || "",
+			systemEmailPort: data?.systemEmailPort,
 			globalThresholds: {
-				cpu: data?.globalThresholds?.cpu || "",
-				memory: data?.globalThresholds?.memory || "",
-				disk: data?.globalThresholds?.disk || "",
-				temperature: data?.globalThresholds?.temperature || "",
+				cpu: data?.globalThresholds?.cpu,
+				memory: data?.globalThresholds?.memory,
+				disk: data?.globalThresholds?.disk,
+				temperature: data?.globalThresholds?.temperature,
 			},
-			checkTTL: data?.checkTTL || 30,
+			checkTTL: data?.checkTTL ?? 30,
 			pagespeedApiKey: "",
 			systemEmailPassword: "",
 		};

client/src/Pages/Settings/SettingsStats.jsx

@@ -75,7 +75,6 @@ const SettingsStats = ({ isAdmin, HEADING_SX, errors }) => {
 SettingsStats.propTypes = {
 	isAdmin: PropTypes.bool,
 	HEADING_SX: PropTypes.object,
-	errors: PropTypes.object,
 };
 
 export default SettingsStats;

client/src/Pages/Settings/index.jsx

@@ -12,7 +12,6 @@ import SettingsGlobalThresholds from "./SettingsGlobalThresholds.jsx";
 import SettingsExport from "./SettingsExport.jsx";
 import Button from "@mui/material/Button";
 // Utils
-import { settingsValidation } from "@/Validation/validation.js";
 import { Controller, useForm } from "react-hook-form";
 import { zodResolver } from "@hookform/resolvers/zod";
 import { useSettingsForm } from "@/Hooks/useSettingsForm.js";
@@ -31,7 +30,6 @@ const BREADCRUMBS = [{ name: `Settings`, path: "/settings" }];
 const Settings = () => {
 	const { data: fetchedSettings, isLoading: isSettingsLoading } = useGet("/settings");
 	// Local state
-	const [errors, setErrors] = useState({});
 	const [isApiKeySet, setIsApiKeySet] = useState(
 		fetchedSettings?.pagespeedKeySet ?? false
 	);
@@ -57,10 +55,9 @@ const Settings = () => {
 		trigger,
 		getValues,
 		setValue,
-		formState: { dirtyFields },
 	} = form;
 	useEffect(() => {
-		reset(defaults);
+		reset(defaults, { keepDirtyValues: false });
 	}, [defaults, reset]);
 
 	useEffect(() => {
@@ -88,13 +85,23 @@ const Settings = () => {
 	};
 
 	const onSubmit = async (data) => {
-		const toSubmit = { ...data };
-		if (!form.formState.dirtyFields.systemEmailPassword) {
-			delete toSubmit.systemEmailPassword;
-		}
-		if (!form.formState.dirtyFields.pagespeedApiKey) {
-			delete toSubmit.pagespeedApiKey;
-		}
+		// Convert undefined to empty strings for proper unsetting on backend
+		const toSubmit = Object.fromEntries(
+			Object.entries(data).map(([key, value]) => {
+				if (value === undefined) return [key, ""];
+				if (typeof value === "object" && value !== null) {
+					// Handle nested objects like globalThresholds
+					return [
+						key,
+						Object.fromEntries(
+							Object.entries(value).map(([k, v]) => [k, v === undefined ? "" : v])
+						),
+					];
+				}
+				return [key, value];
+			})
+		);
+
 		saveSettings(toSubmit);
 	};
 
@@ -135,7 +142,6 @@ const Settings = () => {
 			<SettingsStats
 				isAdmin={isAdmin}
 				HEADING_SX={HEADING_SX}
-				errors={errors}
 			/>
 			<SettingsGlobalThresholds
 				isAdmin={isAdmin}
@@ -195,7 +201,7 @@ const Settings = () => {
 					loading={
 						isSaving || isDeletingMonitorStats || isSettingsLoading || isDeletingMonitors
 					}
-					disabled={Object.keys(errors).length > 0}
+					disabled={!form.formState.isValid}
 					variant="contained"
 					color="accent"
 					sx={{ px: theme.spacing(12), py: theme.spacing(8) }}

client/src/Validation/settings.ts

@@ -1,56 +1,69 @@
 import { z } from "zod";
 
+// Helper to normalize empty strings to undefined
+const optionalString = () =>
+	z.preprocess(
+		(val) =>
+			val === "" || val === null || val === undefined ? undefined : String(val).trim(),
+		z.string().optional()
+	);
+
+// Helper for threshold fields
+const thresholdField = (min: number, max: number, unit: string) =>
+	z.preprocess(
+		(val) => (val === "" || val === null || val === undefined ? undefined : val),
+		z.coerce
+			.number()
+			.int()
+			.min(min, `Min ${min}${unit}`)
+			.max(max, `Max ${max}${unit}`)
+			.optional()
+	);
+
 export const settingsSchema = z.object({
 	systemEmailIgnoreTLS: z.boolean(),
 	systemEmailRequireTLS: z.boolean(),
 	systemEmailRejectUnauthorized: z.boolean(),
-	systemEmailConnectionHost: z.string().optional(),
+	systemEmailConnectionHost: optionalString(),
 	systemEmailSecure: z.boolean().optional(),
 	systemEmailPool: z.boolean().optional(),
 	showURL: z.boolean().optional(),
-	checkTTL: z.coerce.number().int().min(1, "Please enter a value"),
-	pagespeedApiKey: z.string().optional(),
-	systemEmailHost: z.string().optional(),
-	systemEmailPort: z.coerce
+	checkTTL: z.coerce
 		.number()
 		.int()
-		.min(1, "Port must be at least 1")
-		.optional()
-		.or(z.literal("")),
-	systemEmailAddress: z
-		.email("Please enter a valid email address")
-		.transform((val) => val.toLowerCase().trim())
-		.optional()
-		.or(z.literal("")),
-	systemEmailUser: z.string().optional(),
-	systemEmailPassword: z.string().optional(),
-	systemEmailTLSServername: z.string().optional(),
+		.min(1, "Please enter a value")
+		.max(365, "Maximum 365 days"),
+	pagespeedApiKey: optionalString(),
+	systemEmailHost: z.preprocess(
+		(val) =>
+			val === "" || val === null || val === undefined ? undefined : String(val).trim(),
+		z
+			.string()
+			.regex(/^[a-zA-Z0-9.-]+$/, "Invalid hostname or IP address")
+			.optional()
+	),
+	systemEmailPort: z.preprocess(
+		(val) => (val === "" || val === null || val === undefined ? undefined : val),
+		z.coerce
+			.number()
+			.int()
+			.min(1, "Port must be at least 1")
+			.max(65535, "Port must be at most 65535")
+			.optional()
+	),
+	systemEmailAddress: z.preprocess((val) => {
+		if (val === "" || val === null || val === undefined) return undefined;
+		return String(val).toLowerCase().trim();
+	}, z.string().email("Please enter a valid email address").optional()),
+	systemEmailUser: optionalString(),
+	systemEmailPassword: optionalString(),
+	systemEmailTLSServername: optionalString(),
 	globalThresholds: z
 		.object({
-			cpu: z.coerce
-				.number()
-				.min(1, "Min 1%")
-				.max(100, "Max 100%")
-				.optional()
-				.or(z.literal("")),
-			memory: z.coerce
-				.number()
-				.min(1, "Min 1%")
-				.max(100, "Max 100%")
-				.optional()
-				.or(z.literal("")),
-			disk: z.coerce
-				.number()
-				.min(1, "Min 1%")
-				.max(100, "Max 100%")
-				.optional()
-				.or(z.literal("")),
-			temperature: z.coerce
-				.number()
-				.min(1, "Min 1°C")
-				.max(150, "Max 150°C")
-				.optional()
-				.or(z.literal("")),
+			cpu: thresholdField(0, 100, "%"),
+			memory: thresholdField(0, 100, "%"),
+			disk: thresholdField(0, 100, "%"),
+			temperature: thresholdField(0, 150, "°C"),
 		})
 		.optional(),
 });

server/src/repositories/settings/MongoSettingsRepository.ts

@@ -2,7 +2,6 @@ import mongoose from "mongoose";
 import { ISettingsRepository } from "@/repositories/settings/ISettingsRepository.js";
 import type { Settings } from "@/types/index.js";
 import { AppSettingsModel, type AppSettingsDocument } from "@/db/models/index.js";
-import { SERVFAIL } from "dns";
 
 class MongoSettingsRepository implements ISettingsRepository {
 	private toStringId = (value?: mongoose.Types.ObjectId | string | null): string => {
@@ -61,16 +60,25 @@ class MongoSettingsRepository implements ISettingsRepository {
 	};
 
 	update = async (settings: Partial<Settings>) => {
-		const update: Record<string, any> = { $set: { ...settings } };
+		const update: Record<string, any> = { $set: {}, $unset: {} };
 
-		if (settings.pagespeedApiKey === "") {
-			update.$unset = { pagespeedApiKey: "" };
-			delete update.$set.pagespeedApiKey;
+		// Iterate through settings and separate into $set and $unset
+		Object.entries(settings).forEach(([key, value]) => {
+			if (value === undefined || value === "") {
+				// Unset undefined or empty string values
+				update.$unset[key] = "";
+			} else {
+				// Set defined values
+				update.$set[key] = value;
+			}
+		});
+
+		// Clean up empty operators
+		if (Object.keys(update.$set).length === 0) {
+			delete update.$set;
 		}
-
-		if (settings.systemEmailPassword === "") {
-			update.$unset = { systemEmailPassword: "" };
-			delete update.$set.systemEmailPassword;
+		if (Object.keys(update.$unset).length === 0) {
+			delete update.$unset;
 		}
 
 		await AppSettingsModel.findOneAndUpdate({}, update, {

server/src/validation/joi.ts

@@ -420,8 +420,8 @@ const updateAppSettingsBodyValidation = joi.object({
 	systemEmailAddress: joi.string().allow(""),
 	systemEmailPassword: joi.string().allow(""),
 	systemEmailUser: joi.string().allow(""),
-	systemEmailConnectionHost: joi.string().allow("").optional(),
-	systemEmailTLSServername: joi.string().allow("").optional(),
+	systemEmailConnectionHost: joi.string().allow(""),
+	systemEmailTLSServername: joi.string().allow(""),
 	systemEmailSecure: joi.boolean(),
 	systemEmailPool: joi.boolean(),
 	systemEmailIgnoreTLS: joi.boolean(),
@@ -430,10 +430,10 @@ const updateAppSettingsBodyValidation = joi.object({
 
 	globalThresholds: joi
 		.object({
-			cpu: joi.number().min(1).max(100).allow("").optional(),
-			memory: joi.number().min(1).max(100).allow("").optional(),
-			disk: joi.number().min(1).max(100).allow("").optional(),
-			temperature: joi.number().min(1).max(150).allow("").optional(),
+			cpu: joi.number().min(1).max(100).allow(""),
+			memory: joi.number().min(1).max(100).allow(""),
+			disk: joi.number().min(1).max(100).allow(""),
+			temperature: joi.number().min(1).max(150).allow(""),
 		})
 		.optional(),
 });