From a4e819d22061268c153fb64c77a5a3a242a20748 Mon Sep 17 00:00:00 2001
From: Tola Leng <tolaleng.oss@gmail.com>
Date: Wed, 13 Aug 2025 17:20:24 +0700
Subject: [PATCH] Fix: Preserve notification IDs when disabling notifications

- Allowing notifications to be re-enabled correctly without losing previously configured
---
 .../add-service/ServiceNotificationFields.tsx |  1 -
 .../services/add-service/serviceFormUtils.ts  | 23 +++++-----
 application/src/services/serviceService.ts    | 44 +++++++------------
 3 files changed, 27 insertions(+), 41 deletions(-)

diff --git a/application/src/components/services/add-service/ServiceNotificationFields.tsx b/application/src/components/services/add-service/ServiceNotificationFields.tsx
index d5234fe..075b253 100644
--- a/application/src/components/services/add-service/ServiceNotificationFields.tsx
+++ b/application/src/components/services/add-service/ServiceNotificationFields.tsx
@@ -173,7 +173,6 @@ export function ServiceNotificationFields({ form }: ServiceNotificationFieldsPro
               <FormControl>
                 <Select 
                   onValueChange={(value) => {
-                    console.log("Alert template changed to:", value);
                     field.onChange(value);
                   }} 
                   value={field.value || ""}
diff --git a/application/src/components/services/add-service/serviceFormUtils.ts b/application/src/components/services/add-service/serviceFormUtils.ts
index 3429c41..b606d84 100644
--- a/application/src/components/services/add-service/serviceFormUtils.ts
+++ b/application/src/components/services/add-service/serviceFormUtils.ts
@@ -74,7 +74,6 @@ export const mapServiceToFormData = (service: Service): ServiceFormData => {
         notificationChannels.push(...parsedChannels);
       }
     } catch (error) {
-     // console.warn("Failed to parse notification_channel JSON:", error);
       // If parsing fails, treat as single channel ID
       notificationChannels.push(service.notification_channel);
     }
@@ -91,16 +90,13 @@ export const mapServiceToFormData = (service: Service): ServiceFormData => {
     }
   }
 
-//  console.log("Mapping service to form data:", {
- //   serviceName: service.name,
- //   notification_status: service.notification_status,
- //   notification_channel: service.notification_channel,
- //   notificationChannel: service.notificationChannel,
- //   mappedChannels: notificationChannels,
-  //  regionalAgents: regionalAgents,
-  //  region_name: service.region_name,
-  ///  agent_id: service.agent_id
- // });
+  // Handle notification_status - it can be boolean or string
+  let notificationStatus: "enabled" | "disabled" = "disabled";
+  if (typeof service.notification_status === "boolean") {
+    notificationStatus = service.notification_status ? "enabled" : "disabled";
+  } else if (typeof service.notification_status === "string") {
+    notificationStatus = service.notification_status === "enabled" ? "enabled" : "disabled";
+  }
 
   return {
     name: service.name || "",
@@ -109,7 +105,7 @@ export const mapServiceToFormData = (service: Service): ServiceFormData => {
     port: portValue,
     interval: String(service.interval || 60),
     retries: String(service.retries || 3),
-    notificationStatus: service.notification_status || "disabled",
+    notificationStatus: notificationStatus,
     notificationChannels: notificationChannels,
     alertTemplate: service.alertTemplate === "default" ? "" : service.alertTemplate || "",
     regionalMonitoringEnabled: isRegionalEnabled,
@@ -152,7 +148,8 @@ export const mapFormDataToServiceData = (data: ServiceFormData) => {
     type: data.type,
     interval: parseInt(data.interval),
     retries: parseInt(data.retries),
-    notificationStatus: data.notificationStatus || "disabled",
+    // Convert string status to boolean for notification_status field
+    notificationStatus: data.notificationStatus === "enabled",
     notificationChannels: data.notificationChannels || [],
     alertTemplate: data.alertTemplate === "default" ? "" : data.alertTemplate,
     // Use regional_status field and store multiple agents as comma-separated values
diff --git a/application/src/services/serviceService.ts b/application/src/services/serviceService.ts
index af22318..88ad19b 100644
--- a/application/src/services/serviceService.ts
+++ b/application/src/services/serviceService.ts
@@ -9,7 +9,6 @@ export type { Service, UptimeData, CreateServiceParams };
 export const serviceService = {
   async getServices(): Promise<Service[]> {
     try {
-      
       // First get the total count of records
       const countResponse = await pb.collection('services').getList(1, 1, {
         sort: 'name',
@@ -37,7 +36,7 @@ export const serviceService = {
         retries: item.max_retries || item.retries || 3,
         notificationChannel: item.notification_id,
         notification_channel: item.notification_channel, // Add this field for multiple channels support
-        notification_status: item.notification_status || "disabled",
+        notification_status: item.notification_status || false,
         alertTemplate: item.template_id,
         muteAlerts: item.alerts === "muted",  // Convert string to boolean for compatibility
         alerts: item.alerts || "unmuted",     // Store actual database field
@@ -49,7 +48,6 @@ export const serviceService = {
         regional_monitoring_enabled: item.regional_status === "enabled", // Backward compatibility
       }));
     } catch (error) {
-    //  console.error("Error fetching services:", error);
       throw new Error('Failed to load services data.');
     }
   },
@@ -60,7 +58,6 @@ export const serviceService = {
       const serviceType = params.type.toLowerCase();
       
       // Debug log to check what we're sending
-     // console.log("Creating service with params:", params);
       
       const data = {
         name: params.name,
@@ -71,16 +68,16 @@ export const serviceService = {
         last_checked: new Date().toLocaleString(),
         heartbeat_interval: params.interval,
         max_retries: params.retries,
-        notification_status: params.notificationStatus || "disabled",
-        // Store multiple notification channels as JSON string
+        // Store notification_status as boolean
+        notification_status: params.notificationStatus === true,
+        // Always store channels and template if provided (don't clear based on status)
         notification_channel: params.notificationChannels && params.notificationChannels.length > 0 
           ? JSON.stringify(params.notificationChannels)
           : null,
-        // Store multiple notification IDs as comma-separated string in notification_id field
         notification_id: params.notificationChannels && params.notificationChannels.length > 0 
           ? params.notificationChannels.join(',')
           : null,
-        template_id: params.alertTemplate,
+        template_id: params.alertTemplate || null,
         // Regional monitoring fields - use regional_status
         regional_status: params.regionalStatus || "disabled",
         region_name: params.regionName || "",
@@ -96,9 +93,7 @@ export const serviceService = {
         )
       };
 
-     // console.log("Creating service with data:", data);
       const record = await pb.collection('services').create(data);
-     // console.log("Service created, returned record:", record);
       
       // Return the newly created service
       const newService = {
@@ -117,7 +112,7 @@ export const serviceService = {
         retries: record.max_retries || 3,
         notificationChannel: record.notification_id,
         notification_channel: record.notification_channel,
-        notification_status: record.notification_status || "disabled",
+        notification_status: record.notification_status || false,
         alertTemplate: record.template_id,
         regional_status: record.regional_status || "disabled",
         regional_monitoring_enabled: record.regional_status === "enabled",
@@ -130,7 +125,6 @@ export const serviceService = {
       
       return newService;
     } catch (error) {
-    //  console.error("Error creating service:", error);
       throw new Error('Failed to create service.');
     }
   },
@@ -141,23 +135,22 @@ export const serviceService = {
       const serviceType = params.type.toLowerCase();
       
       // Debug log to check what we're updating
-    //  console.log("Updating service with params:", params);
       
       const data = {
         name: params.name,
         service_type: serviceType,
         heartbeat_interval: params.interval,
         max_retries: params.retries,
-        notification_status: params.notificationStatus || "disabled",
-        // Store multiple notification channels as JSON string
-        notification_channel: params.notificationChannels && params.notificationChannels.length > 0 
-          ? JSON.stringify(params.notificationChannels)
-          : null,
-        // Store multiple notification IDs as comma-separated string in notification_id field
-        notification_id: params.notificationChannels && params.notificationChannels.length > 0 
-          ? params.notificationChannels.join(',')
-          : null,
-        template_id: params.alertTemplate || null,
+        // Store notification_status as boolean - preserve existing channels/template
+        notification_status: params.notificationStatus === true,
+        // Only update channels and template if they are provided (don't clear them)
+        ...(params.notificationChannels && params.notificationChannels.length > 0 && {
+          notification_channel: JSON.stringify(params.notificationChannels),
+          notification_id: params.notificationChannels.join(',')
+        }),
+        ...(params.alertTemplate && {
+          template_id: params.alertTemplate
+        }),
         // Regional monitoring fields - use regional_status
         regional_status: params.regionalStatus || "disabled",
         region_name: params.regionName || "",
@@ -173,7 +166,6 @@ export const serviceService = {
         )
       };
 
-     // console.log("Updating service with data:", data);
       
       // Use timeout to ensure the request doesn't hang
       const timeoutPromise = new Promise<never>((_, reject) => {
@@ -182,7 +174,6 @@ export const serviceService = {
       
       const updatePromise = pb.collection('services').update(id, data);
       const record = await Promise.race([updatePromise, timeoutPromise]) as any;
-     // console.log("Service updated, returned record:", record);
       
       // Return the updated service
       const updatedService = {
@@ -201,7 +192,7 @@ export const serviceService = {
         retries: record.max_retries || 3,
         notificationChannel: record.notification_id,
         notification_channel: record.notification_channel,
-        notification_status: record.notification_status || "disabled",
+        notification_status: record.notification_status || false,
         alertTemplate: record.template_id,
         regional_status: record.regional_status || "disabled",
         regional_monitoring_enabled: record.regional_status === "enabled",
@@ -211,7 +202,6 @@ export const serviceService = {
       
       return updatedService;
     } catch (error) {
-      //console.error("Error updating service:", error);
       throw new Error(`Failed to update service: ${error instanceof Error ? error.message : 'Unknown error'}`);
     }
   },