From cbf2125f0a24b47b817e20a5e223625895eabd83 Mon Sep 17 00:00:00 2001
From: Usman Baig <mubaig@pm.me>
Date: Wed, 25 Mar 2026 21:59:36 +0100
Subject: [PATCH] fix: batch PageSpeed frequency and notification toggles into
 save flow

---
 .../settings/unified/UnifiedSettingsModal.tsx |  2 +-
 .../settings/unified/tabs/SitePrivacyTab.tsx  | 61 +++++++++++--------
 .../tabs/WorkspaceNotificationsTab.tsx        | 52 ++++++++++------
 3 files changed, 71 insertions(+), 44 deletions(-)

diff --git a/components/settings/unified/UnifiedSettingsModal.tsx b/components/settings/unified/UnifiedSettingsModal.tsx
index f482c70..89029e8 100644
--- a/components/settings/unified/UnifiedSettingsModal.tsx
+++ b/components/settings/unified/UnifiedSettingsModal.tsx
@@ -202,7 +202,7 @@ function TabContent({
       case 'general': return <WorkspaceGeneralTab {...dirtyProps} />
       case 'billing': return <WorkspaceBillingTab />
       case 'members': return <WorkspaceMembersTab />
-      case 'notifications': return <WorkspaceNotificationsTab />
+      case 'notifications': return <WorkspaceNotificationsTab {...dirtyProps} />
       case 'audit': return <WorkspaceAuditTab />
     }
   }
diff --git a/components/settings/unified/tabs/SitePrivacyTab.tsx b/components/settings/unified/tabs/SitePrivacyTab.tsx
index a6532bd..003d79e 100644
--- a/components/settings/unified/tabs/SitePrivacyTab.tsx
+++ b/components/settings/unified/tabs/SitePrivacyTab.tsx
@@ -40,6 +40,7 @@ export default function SitePrivacyTab({ siteId, onDirtyChange, onRegisterSave }
   const [hideUnknownLocations, setHideUnknownLocations] = useState(false)
   const [dataRetention, setDataRetention] = useState(6)
   const [excludedPaths, setExcludedPaths] = useState('')
+  const [psiFrequency, setPsiFrequency] = useState('weekly')
   const [snippetCopied, setSnippetCopied] = useState(false)
   const initialRef = useRef('')
 
@@ -55,25 +56,40 @@ export default function SitePrivacyTab({ siteId, onDirtyChange, onRegisterSave }
     setHideUnknownLocations(site.hide_unknown_locations ?? false)
     setDataRetention(site.data_retention_months ?? 6)
     setExcludedPaths((site.excluded_paths || []).join('\n'))
-    initialRef.current = JSON.stringify({
-      collectPagePaths: site.collect_page_paths ?? true,
-      collectReferrers: site.collect_referrers ?? true,
-      collectDeviceInfo: site.collect_device_info ?? true,
-      collectScreenRes: site.collect_screen_resolution ?? true,
-      collectGeoData: site.collect_geo_data ?? 'full',
-      hideUnknownLocations: site.hide_unknown_locations ?? false,
-      dataRetention: site.data_retention_months ?? 6,
-      excludedPaths: (site.excluded_paths || []).join('\n'),
-    })
     hasInitialized.current = true
   }, [site])
 
+  // Sync PSI frequency separately (comes from a different SWR hook)
+  const psiInitialized = useRef(false)
+  useEffect(() => {
+    if (!psiConfig || psiInitialized.current) return
+    setPsiFrequency(psiConfig.frequency || 'weekly')
+    psiInitialized.current = true
+  }, [psiConfig])
+
+  // Build initial snapshot once both site + psi are loaded
+  useEffect(() => {
+    if (!hasInitialized.current || !initialRef.current && site) {
+      initialRef.current = JSON.stringify({
+        collectPagePaths: site?.collect_page_paths ?? true,
+        collectReferrers: site?.collect_referrers ?? true,
+        collectDeviceInfo: site?.collect_device_info ?? true,
+        collectScreenRes: site?.collect_screen_resolution ?? true,
+        collectGeoData: site?.collect_geo_data ?? 'full',
+        hideUnknownLocations: site?.hide_unknown_locations ?? false,
+        dataRetention: site?.data_retention_months ?? 6,
+        excludedPaths: (site?.excluded_paths || []).join('\n'),
+        psiFrequency: psiConfig?.frequency || 'weekly',
+      })
+    }
+  }, [site, psiConfig])
+
   // Track dirty state
   useEffect(() => {
     if (!initialRef.current) return
-    const current = JSON.stringify({ collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths })
+    const current = JSON.stringify({ collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths, psiFrequency })
     onDirtyChange?.(current !== initialRef.current)
-  }, [collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths, onDirtyChange])
+  }, [collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths, psiFrequency, onDirtyChange])
 
   const handleSave = useCallback(async () => {
     try {
@@ -88,14 +104,19 @@ export default function SitePrivacyTab({ siteId, onDirtyChange, onRegisterSave }
         data_retention_months: dataRetention,
         excluded_paths: excludedPaths.split('\n').map(p => p.trim()).filter(Boolean),
       })
+      // Save PSI frequency separately if it changed
+      if (psiConfig?.enabled && psiFrequency !== (psiConfig.frequency || 'weekly')) {
+        await updatePageSpeedConfig(siteId, { enabled: psiConfig.enabled, frequency: psiFrequency })
+        mutatePSIConfig()
+      }
       await mutate()
-      initialRef.current = JSON.stringify({ collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths })
+      initialRef.current = JSON.stringify({ collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths, psiFrequency })
       onDirtyChange?.(false)
       toast.success('Privacy settings updated')
     } catch {
       toast.error('Failed to save')
     }
-  }, [siteId, site?.name, collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths, mutate, onDirtyChange])
+  }, [siteId, site?.name, collectPagePaths, collectReferrers, collectDeviceInfo, collectScreenRes, collectGeoData, hideUnknownLocations, dataRetention, excludedPaths, psiFrequency, psiConfig, mutatePSIConfig, mutate, onDirtyChange])
 
   // Register save handler with modal
   useEffect(() => {
@@ -195,16 +216,8 @@ export default function SitePrivacyTab({ siteId, onDirtyChange, onRegisterSave }
             </div>
             {psiConfig?.enabled ? (
               <Select
-                value={psiConfig.frequency || 'weekly'}
-                onChange={async (v) => {
-                  try {
-                    await updatePageSpeedConfig(siteId, { enabled: psiConfig.enabled, frequency: v })
-                    mutatePSIConfig()
-                    toast.success('Check frequency updated')
-                  } catch {
-                    toast.error('Failed to update')
-                  }
-                }}
+                value={psiFrequency}
+                onChange={(v) => setPsiFrequency(v)}
                 options={[
                   { value: 'daily', label: 'Daily' },
                   { value: 'weekly', label: 'Weekly' },
diff --git a/components/settings/unified/tabs/WorkspaceNotificationsTab.tsx b/components/settings/unified/tabs/WorkspaceNotificationsTab.tsx
index 154820e..7032999 100644
--- a/components/settings/unified/tabs/WorkspaceNotificationsTab.tsx
+++ b/components/settings/unified/tabs/WorkspaceNotificationsTab.tsx
@@ -1,15 +1,17 @@
 'use client'
 
-import { useState, useEffect } from 'react'
-import { toast, Spinner } from '@ciphera-net/ui'
+import { useState, useEffect, useRef, useCallback } from 'react'
+import { Toggle, toast, Spinner } from '@ciphera-net/ui'
 import { useAuth } from '@/lib/auth/context'
 import { getNotificationSettings, updateNotificationSettings, type NotificationSettingsResponse } from '@/lib/api/notification-settings'
 
-export default function WorkspaceNotificationsTab() {
+export default function WorkspaceNotificationsTab({ onDirtyChange, onRegisterSave }: { onDirtyChange?: (dirty: boolean) => void; onRegisterSave?: (fn: () => Promise<void>) => void }) {
   const { user } = useAuth()
   const [data, setData] = useState<NotificationSettingsResponse | null>(null)
   const [settings, setSettings] = useState<Record<string, boolean>>({})
   const [loading, setLoading] = useState(true)
+  const initialRef = useRef('')
+  const hasInitialized = useRef(false)
 
   useEffect(() => {
     if (!user?.org_id) return
@@ -17,23 +19,40 @@ export default function WorkspaceNotificationsTab() {
       .then(resp => {
         setData(resp)
         setSettings(resp.settings || {})
+        if (!hasInitialized.current) {
+          initialRef.current = JSON.stringify(resp.settings || {})
+          hasInitialized.current = true
+        }
       })
       .catch(() => {})
       .finally(() => setLoading(false))
   }, [user?.org_id])
 
-  const handleToggle = async (key: string) => {
-    const prev = { ...settings }
-    const updated = { ...settings, [key]: !settings[key] }
-    setSettings(updated)
-    try {
-      await updateNotificationSettings(updated)
-    } catch {
-      setSettings(prev)
-      toast.error('Failed to update notification preference')
-    }
+  // Track dirty state
+  useEffect(() => {
+    if (!initialRef.current) return
+    onDirtyChange?.(JSON.stringify(settings) !== initialRef.current)
+  }, [settings, onDirtyChange])
+
+  const handleToggle = (key: string) => {
+    setSettings(prev => ({ ...prev, [key]: !prev[key] }))
   }
 
+  const handleSave = useCallback(async () => {
+    try {
+      await updateNotificationSettings(settings)
+      initialRef.current = JSON.stringify(settings)
+      onDirtyChange?.(false)
+      toast.success('Notification preferences updated')
+    } catch {
+      toast.error('Failed to update notification preferences')
+    }
+  }, [settings, onDirtyChange])
+
+  useEffect(() => {
+    onRegisterSave?.(handleSave)
+  }, [handleSave, onRegisterSave])
+
   if (loading) return <div className="flex items-center justify-center py-12"><Spinner className="w-6 h-6 text-neutral-500" /></div>
 
   return (
@@ -50,12 +69,7 @@ export default function WorkspaceNotificationsTab() {
               <p className="text-sm font-medium text-white">{cat.label}</p>
               <p className="text-xs text-neutral-400">{cat.description}</p>
             </div>
-            <button
-              onClick={() => handleToggle(cat.id)}
-              className={`relative w-10 h-6 rounded-full transition-colors duration-200 ${settings[cat.id] ? 'bg-brand-orange' : 'bg-neutral-700'}`}
-            >
-              <span className={`absolute top-0.5 left-0.5 w-5 h-5 bg-white rounded-full shadow transition-transform duration-200 ${settings[cat.id] ? 'translate-x-4' : ''}`} />
-            </button>
+            <Toggle checked={settings[cat.id] ?? false} onChange={() => handleToggle(cat.id)} />
           </div>
         ))}