Ahoyapps 331 join room bug (#93)

* Disable join room button when fetching token

* Set isFetching to false on error

* Add tests for isFetching variable

* Add menubar test

* Update comments

src/components/MenuBar/MenuBar.test.tsx

@@ -58,13 +58,21 @@ describe('the MenuBar component', () => {
     expect(container.querySelectorAll('input').length).toEqual(2);
   });
 
-  it('should display a loading spinner when connecting to a room', () => {
+  it('should display a loading spinner while connecting to a room', () => {
     mockedUseRoomState.mockImplementation(() => 'disconnected');
     mockedUseVideoContext.mockImplementation(() => ({ isConnecting: true, room: {} } as any));
     const { container } = render(renderComponent());
     expect(container.querySelector('svg')).not.toBeNull();
   });
 
+  it('should display a loading spinner while fetching a token', () => {
+    mockedUseRoomState.mockImplementation(() => 'disconnected');
+    mockedUseVideoContext.mockImplementation(() => ({ isConnecting: false, room: {} } as any));
+    mockUseAppState.mockImplementationOnce(() => ({ isFetching: true }));
+    const { container } = render(renderComponent());
+    expect(container.querySelector('svg')).not.toBeNull();
+  });
+
   it('should disable the Join Room button when the Name input or Room input are empty', () => {
     mockedUseRoomState.mockImplementation(() => 'disconnected');
     mockedUseVideoContext.mockImplementation(() => ({ isConnecting: false, room: {} } as any));
@@ -96,7 +104,6 @@ describe('the MenuBar component', () => {
   });
 
   it('should update the URL to include the room name on submit', () => {
-
     mockedUseRoomState.mockImplementation(() => 'disconnected');
     mockedUseVideoContext.mockImplementation(() => ({ isConnecting: false, connect: mockConnect, room: {} } as any));
     const { getByLabelText, getByText } = render(renderComponent());

src/components/MenuBar/MenuBar.tsx

@@ -43,7 +43,7 @@ const useStyles = makeStyles((theme: Theme) =>
 export default function MenuBar() {
   const classes = useStyles();
   const { URLRoomName } = useParams();
-  const { user, getToken } = useAppState();
+  const { user, getToken, isFetching } = useAppState();
   const { isConnecting, connect } = useVideoContext();
   const roomState = useRoomState();
 
@@ -100,10 +100,15 @@ export default function MenuBar() {
               onChange={handleRoomNameChange}
               margin="dense"
             />
-            <Button type="submit" color="primary" variant="contained" disabled={isConnecting || !name || !roomName}>
+            <Button
+              type="submit"
+              color="primary"
+              variant="contained"
+              disabled={isConnecting || !name || !roomName || isFetching}
+            >
               Join Room
             </Button>
-            {isConnecting && <CircularProgress className={classes.loadingSpinner} />}
+            {(isConnecting || isFetching) && <CircularProgress className={classes.loadingSpinner} />}
           </form>
         ) : (
           <h3>{roomName}</h3>

src/state/index.test.tsx

@@ -9,6 +9,8 @@ import usePasscodeAuth from './usePasscodeAuth/usePasscodeAuth';
 jest.mock('./useFirebaseAuth/useFirebaseAuth', () => jest.fn(() => ({ user: 'firebaseUser' })));
 jest.mock('./usePasscodeAuth/usePasscodeAuth', () => jest.fn(() => ({ user: 'passcodeUser' })));
 
+const mockUsePasscodeAuth = usePasscodeAuth as jest.Mock<any>;
+
 // @ts-ignore
 window.fetch = jest.fn(() => Promise.resolve({ text: () => 'mockVideoToken' }));
 
@@ -34,7 +36,11 @@ describe('the useAppState hook', () => {
 
     const { result } = renderHook(useAppState, { wrapper });
 
-    const token = await result.current.getToken('testname', 'testroom');
+    let token;
+    await act(async () => {
+      token = await result.current.getToken('testname', 'testroom');
+    });
+
     expect(token).toBe('mockVideoToken');
 
     expect(window.fetch).toHaveBeenCalledWith('http://test.com/api/token?identity=testname&roomName=testroom', {
@@ -44,7 +50,7 @@ describe('the useAppState hook', () => {
 
   describe('with auth disabled', () => {
     it('should not use any auth hooks', async () => {
-      delete process.env.REACT_APP_USE_FIREBASE_AUTH;
+      delete process.env.REACT_APP_SET_AUTH;
       renderHook(useAppState, { wrapper });
       expect(useFirebaseAuth).not.toHaveBeenCalled();
       expect(usePasscodeAuth).not.toHaveBeenCalled();
@@ -56,7 +62,7 @@ describe('the useAppState hook', () => {
       process.env.REACT_APP_SET_AUTH = 'firebase';
       const { result } = renderHook(useAppState, { wrapper });
       expect(useFirebaseAuth).toHaveBeenCalled();
-      expect(result.current.user).toBe('firebaseUser')
+      expect(result.current.user).toBe('firebaseUser');
     });
   });
 
@@ -65,7 +71,65 @@ describe('the useAppState hook', () => {
       process.env.REACT_APP_SET_AUTH = 'passcode';
       const { result } = renderHook(useAppState, { wrapper });
       expect(usePasscodeAuth).toHaveBeenCalled();
-      expect(result.current.user).toBe('passcodeUser')
+      expect(result.current.user).toBe('passcodeUser');
+    });
+  });
+
+  describe('the getToken function', () => {
+    it('should set isFetching to true after getToken is called, and false after getToken succeeds', async () => {
+      // Using passcode auth because it's easier to mock the getToken function
+      process.env.REACT_APP_SET_AUTH = 'passcode';
+      mockUsePasscodeAuth.mockImplementation(() => {
+        return {
+          getToken: () =>
+            new Promise(resolve => {
+              // Using fake timers so we can control when the promise resolves
+              setTimeout(() => resolve({ text: () => 'mockVideoToken' }), 10);
+            }),
+        };
+      });
+
+      jest.useFakeTimers();
+
+      const { result, waitForNextUpdate } = renderHook(useAppState, { wrapper });
+
+      expect(result.current.isFetching).toEqual(false);
+
+      await act(async () => {
+        result.current.getToken('test', 'test');
+        await waitForNextUpdate();
+        expect(result.current.isFetching).toEqual(true);
+        jest.runOnlyPendingTimers();
+        await waitForNextUpdate();
+        expect(result.current.isFetching).toEqual(false);
+      });
+    });
+
+    it('should set isFetching to true after getToken is called, and false after getToken succeeds', async () => {
+      process.env.REACT_APP_SET_AUTH = 'passcode';
+      mockUsePasscodeAuth.mockImplementation(() => {
+        return {
+          getToken: () =>
+            new Promise((_, reject) => {
+              setTimeout(() => reject({ text: () => 'mockVideoToken' }), 10);
+            }),
+        };
+      });
+
+      jest.useFakeTimers();
+
+      const { result, waitForNextUpdate } = renderHook(useAppState, { wrapper });
+
+      expect(result.current.isFetching).toEqual(false);
+
+      await act(async () => {
+        result.current.getToken('test', 'test').catch(() => {});
+        await waitForNextUpdate();
+        expect(result.current.isFetching).toEqual(true);
+        jest.runOnlyPendingTimers();
+        await waitForNextUpdate();
+        expect(result.current.isFetching).toEqual(false);
+      });
     });
   });
 });

src/state/index.tsx

@@ -12,6 +12,7 @@ export interface StateContextType {
   signIn?(passcode?: string): Promise<void>;
   signOut?(): Promise<void>;
   isAuthReady?: boolean;
+  isFetching: boolean;
 }
 
 export const StateContext = createContext<StateContextType>(null!);
@@ -27,9 +28,12 @@ export const StateContext = createContext<StateContextType>(null!);
 */
 export default function AppStateProvider(props: React.PropsWithChildren<{}>) {
   const [error, setError] = useState<TwilioError | null>(null);
+  const [isFetching, setIsFetching] = useState(false);
+
   let contextValue = {
     error,
     setError,
+    isFetching,
   } as StateContextType;
 
   if (process.env.REACT_APP_SET_AUTH === 'firebase') {
@@ -55,11 +59,20 @@ export default function AppStateProvider(props: React.PropsWithChildren<{}>) {
     };
   }
 
-  const getToken: StateContextType['getToken'] = (name, room) =>
-    contextValue.getToken(name, room).catch(err => {
-      setError(err);
-      return Promise.reject(err);
-    });
+  const getToken: StateContextType['getToken'] = (name, room) => {
+    setIsFetching(true);
+    return contextValue
+      .getToken(name, room)
+      .then(res => {
+        setIsFetching(false);
+        return res;
+      })
+      .catch(err => {
+        setError(err);
+        setIsFetching(false);
+        return Promise.reject(err);
+      });
+  };
 
   return <StateContext.Provider value={{ ...contextValue, getToken }}>{props.children}</StateContext.Provider>;
 }