From 0036550ae8c4e5ab808ba6f82ec674a475e09a67 Mon Sep 17 00:00:00 2001
From: picklesrus <picklesrus@users.noreply.github.com>
Date: Thu, 30 Jul 2020 10:55:18 -0400
Subject: [PATCH] Move timeout id out of state to a member variable and add
 some unittests.

---
 src/components/navigation/www/navigation.jsx | 27 +++++--------
 test/unit/components/navigation.test.jsx     | 40 +++++++++++++++++++-
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/src/components/navigation/www/navigation.jsx b/src/components/navigation/www/navigation.jsx
index 4844fe8ca..81c83681e 100644
--- a/src/components/navigation/www/navigation.jsx
+++ b/src/components/navigation/www/navigation.jsx
@@ -30,15 +30,14 @@ class Navigation extends React.Component {
             'handleSearchSubmit',
             'pollForMessages'
         ]);
-        this.state = {
-            messageCountIntervalId: -1 // javascript method interval id for getting messsage count.
-        };
+        // Keep the timeout id so we can cancel it (e.g. when we unmount)
+        this.messageCountTimeoutId = -1;
     }
     componentDidMount () {
         if (this.props.user) {
             // Setup polling for messages to start in 2 minutes.
             const twoMinInMs = 2 * 60 * 1000;
-            setTimeout(this.pollForMessages.bind(this, twoMinInMs), twoMinInMs);
+            this.messageCountTimeoutId = setTimeout(this.pollForMessages.bind(this, twoMinInMs), twoMinInMs);
         }
     }
     componentDidUpdate (prevProps) {
@@ -46,25 +45,21 @@ class Navigation extends React.Component {
             this.props.handleCloseAccountNav();
             if (this.props.user) {
                 const twoMinInMs = 2 * 60 * 1000;
-                setTimeout(this.pollForMessages.bind(this, twoMinInMs), twoMinInMs);
+                this.messageCountTimeoutId = setTimeout(this.pollForMessages.bind(this, twoMinInMs), twoMinInMs);
             } else {
                 // clear message count check, and set to default id.
-                clearTimeout(this.state.messageCountIntervalId);
+                clearTimeout(this.messageCountTimeoutId);
                 this.props.setMessageCount(0);
-                this.setState({ // eslint-disable-line react/no-did-update-set-state
-                    messageCountIntervalId: -1
-                });
+                this.messageCountTimeoutId = -1;
             }
         }
     }
     componentWillUnmount () {
         // clear message interval if it exists
-        if (this.state.messageCountIntervalId !== -1) {
-            clearTimeout(this.state.messageCountIntervalId);
+        if (this.messageCountTimeoutId !== -1) {
+            clearTimeout(this.messageCountTimeoutId);
             this.props.setMessageCount(0);
-            this.setState({
-                messageCountIntervalId: -1
-            });
+            this.messageCountTimeoutId = -1;
         }
     }
     getProfileUrl () {
@@ -82,9 +77,7 @@ class Navigation extends React.Component {
             const timeoutId = setTimeout(() => {
                 this.pollForMessages(nextFetch);
             }, nextFetch);
-            this.setState({ // eslint-disable-line react/no-did-mount-set-state
-                messageCountIntervalId: timeoutId
-            });
+            this.messageCountTimeoutId = timeoutId;
         }
     }
 
diff --git a/test/unit/components/navigation.test.jsx b/test/unit/components/navigation.test.jsx
index 07c8a7139..fa180f63a 100644
--- a/test/unit/components/navigation.test.jsx
+++ b/test/unit/components/navigation.test.jsx
@@ -128,14 +128,49 @@ describe('Navigation', () => {
                 childContextTypes: {store}
             });
 
-        intlWrapper.children().find('Navigation')
+        const navInstance = intlWrapper.children().find('Navigation')
             .instance();
         const twoMin = 2 * 60 * 1000;
         expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), twoMin);
+        expect(navInstance.messageCountTimeoutId).not.toEqual(-1);
         // Advance timers passed the intial two minutes.
         jest.advanceTimersByTime(twoMin + 1);
         expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), twoMin * 2);
         expect(props.getMessageCount).toHaveBeenCalled();
+        expect(navInstance.messageCountTimeoutId).not.toEqual(-1);
+    });
+    test('Component cancels timers when it unmounts', () => {
+        store = mockStore({
+            navigation: {
+                registrationOpen: false
+            },
+            messageCount: {
+                messageCount: 5
+            }
+        });
+        const props = {
+            user: {
+                thumbnailUrl: 'scratch.mit.edu',
+                username: 'auser'
+            },
+            getMessageCount: jest.fn()
+        };
+        const intlWrapper = mountWithIntl(
+            <Navigation
+                {...props}
+            />, {context: {store},
+                childContextTypes: {store}
+            });
+
+        const navInstance = intlWrapper.children().find('Navigation')
+            .instance();
+        const twoMin = 2 * 60 * 1000;
+        expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), twoMin);
+        expect(navInstance.messageCountTimeoutId).not.toEqual(-1);
+        debugger;
+        navInstance.componentWillUnmount();
+        expect(clearTimeout).toHaveBeenCalledWith(expect.any(Number));
+        expect(navInstance.messageCountTimeoutId).toEqual(-1);
     });
 
     test('pollForMessages polls for messages 5 times', () => {
@@ -170,8 +205,8 @@ describe('Navigation', () => {
         let twoMinInMs = 2 * 60 * 1000; // 2 minutes in ms.
         navInstance.pollForMessages(twoMinInMs);
 
+        expect(navInstance.messageCountTimeoutId).not.toEqual(-1);
         // Check that we set the timeout to backoff exponentially
-
         for (let count = 1; count < 5; ++count) {
             jest.advanceTimersByTime(twoMinInMs + 1);
             expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), twoMinInMs * 2);
@@ -187,5 +222,6 @@ describe('Navigation', () => {
         // setTimeout happens 1 fewer since the original call to
         // pollForMessages isn't counted here.
         expect(setTimeout).toHaveBeenCalledTimes(4);
+        expect(navInstance.messageCountTimeoutId).not.toEqual(-1);
     });
 });