From 3a76e4733c20042e155242da18c2033ff857b01f Mon Sep 17 00:00:00 2001
From: Philipp Heckel <pheckel@datto.com>
Date: Sun, 6 Mar 2022 21:39:20 -0500
Subject: [PATCH] Cleanup

---
 web/src/app/ConnectionManager.js | 12 ++++-
 web/src/components/App.js        | 75 +++++++++++++++-----------------
 web/src/components/Navigation.js |  6 ++-
 3 files changed, 48 insertions(+), 45 deletions(-)

diff --git a/web/src/app/ConnectionManager.js b/web/src/app/ConnectionManager.js
index 3871c09c..5b56e556 100644
--- a/web/src/app/ConnectionManager.js
+++ b/web/src/app/ConnectionManager.js
@@ -82,13 +82,21 @@ class ConnectionManager {
 
     stateChanged(subscriptionId, state) {
         if (this.stateListener) {
-            this.stateListener(subscriptionId, state);
+            try {
+                this.stateListener(subscriptionId, state);
+            } catch (e) {
+                console.error(`[ConnectionManager] Error updating state of ${subscriptionId} to ${state}`, e);
+            }
         }
     }
 
     notificationReceived(subscriptionId, notification) {
         if (this.notificationListener) {
-            this.notificationListener(subscriptionId, notification);
+            try {
+                this.notificationListener(subscriptionId, notification);
+            } catch (e) {
+                console.error(`[ConnectionManager] Error handling notification for ${subscriptionId}`, e);
+            }
         }
     }
 }
diff --git a/web/src/components/App.js b/web/src/components/App.js
index fa09c826..48654d06 100644
--- a/web/src/components/App.js
+++ b/web/src/components/App.js
@@ -42,49 +42,13 @@ const App = () => {
 const Root = () => {
     const [mobileDrawerOpen, setMobileDrawerOpen] = useState(false);
     const [notificationsGranted, setNotificationsGranted] = useState(notifier.granted());
-    const navigate = useNavigate();
     const location = useLocation();
     const users = useLiveQuery(() => userManager.all());
     const subscriptions = useLiveQuery(() => subscriptionManager.all());
     const selectedSubscription = findSelected(location, subscriptions);
 
-    console.log(window.location);
-    const handleSubscribeSubmit = async (subscription) => {
-        console.log(`[App] New subscription: ${subscription.id}`, subscription);
-        navigate(subscriptionRoute(subscription));
-        handleRequestPermission();
-    };
-
-    const handleRequestPermission = () => {
-        notifier.maybeRequestPermission(granted => setNotificationsGranted(granted));
-    };
-
-    useEffect(() => {
-        poller.startWorker();
-        pruner.startWorker();
-    }, [/* initial render */]);
-
-    useEffect(() => {
-        const handleNotification = async (subscriptionId, notification) => {
-            try {
-                const added = await subscriptionManager.addNotification(subscriptionId, notification);
-                if (added) {
-                    const defaultClickAction = (subscription) => navigate(subscriptionRoute(subscription)); // FIXME
-                    await notifier.notify(subscriptionId, notification, defaultClickAction)
-                }
-            } catch (e) {
-                console.error(`[App] Error handling notification`, e);
-            }
-        };
-        connectionManager.registerStateListener(subscriptionManager.updateState);
-        connectionManager.registerNotificationListener(handleNotification);
-        return () => {
-            connectionManager.resetStateListener();
-            connectionManager.resetNotificationListener();
-        }
-// This is for the use of 'navigate' // FIXME
-//eslint-disable-next-line
-    }, [/* initial render */]);
+    useWorkers();
+    useConnectionListeners();
 
     useEffect(() => {
         connectionManager.refresh(subscriptions, users);
@@ -102,11 +66,10 @@ const Root = () => {
                 <Navigation
                     subscriptions={subscriptions}
                     selectedSubscription={selectedSubscription}
-                    mobileDrawerOpen={mobileDrawerOpen}
                     notificationsGranted={notificationsGranted}
+                    requestNotificationPermission={() => notifier.maybeRequestPermission(granted => setNotificationsGranted(granted))}
+                    mobileDrawerOpen={mobileDrawerOpen}
                     onMobileDrawerToggle={() => setMobileDrawerOpen(!mobileDrawerOpen)}
-                    onSubscribeSubmit={handleSubscribeSubmit}
-                    onRequestPermissionClick={handleRequestPermission}
                 />
             </Box>
             <Main>
@@ -168,4 +131,34 @@ const findSelected = (location, subscriptions) => {
      */
 };
 
+
+const useWorkers = () => {
+    useEffect(() => {
+        poller.startWorker();
+        pruner.startWorker();
+    }, []);
+};
+
+const useConnectionListeners = () => {
+    const navigate = useNavigate();
+    useEffect(() => {
+        const handleNotification = async (subscriptionId, notification) => {
+            const added = await subscriptionManager.addNotification(subscriptionId, notification);
+            if (added) {
+                const defaultClickAction = (subscription) => navigate(subscriptionRoute(subscription));
+                await notifier.notify(subscriptionId, notification, defaultClickAction)
+            }
+        };
+        connectionManager.registerStateListener(subscriptionManager.updateState);
+        connectionManager.registerNotificationListener(handleNotification);
+        return () => {
+            connectionManager.resetStateListener();
+            connectionManager.resetNotificationListener();
+        }
+    },
+    // We have to disable dep checking for "navigate". This is fine, it never changes.
+    // eslint-disable-next-line
+    []);
+};
+
 export default App;
diff --git a/web/src/components/Navigation.js b/web/src/components/Navigation.js
index ee251a9b..f559eed4 100644
--- a/web/src/components/Navigation.js
+++ b/web/src/components/Navigation.js
@@ -66,8 +66,10 @@ const NavList = (props) => {
     }
 
     const handleSubscribeSubmit = (subscription) => {
+        console.log(`[Navigation] New subscription: ${subscription.id}`, subscription);
         handleSubscribeReset();
-        props.onSubscribeSubmit(subscription);
+        navigate(subscriptionRoute(subscription));
+        props.requestNotificationPermission();
     }
 
     const showSubscriptionsList = props.subscriptions?.length > 0;
@@ -77,7 +79,7 @@ const NavList = (props) => {
         <>
             <Toolbar sx={{ display: { xs: 'none', sm: 'block' } }}/>
             <List component="nav" sx={{ paddingTop: (showGrantPermissionsBox) ? '0' : '' }}>
-                {showGrantPermissionsBox && <PermissionAlert onRequestPermissionClick={props.onRequestPermissionClick}/>}
+                {showGrantPermissionsBox && <PermissionAlert onRequestPermissionClick={props.requestNotificationPermission}/>}
                 {showSubscriptionsList &&
                     <>
                         <ListSubheader>Subscribed topics</ListSubheader>