fix(auth): Make logout into a function (#48)

* fix(auth): make logout into a function

* Fix UserMenu component in the header
* Add aria-label for menu item
* Fix User component in the cabinet
* Added AuthProvider to control restricted access paths
* Update snapshot
* Update e2e-tests
* Fix Formatting

GitHub issue:
#37

Co-authored-by: “Anton <“alantukh@“jwplayer.com>

.gitignore

@@ -10,4 +10,4 @@ yarn-error.log
 # os or editor
 .idea
 .DS_Store
-/.vscode/settings.json
+.vscode/

src/components/MenuButton/MenuButton.tsx

@@ -20,6 +20,7 @@ const MenuButton: React.FC<Props> = ({ label, to, onClick, tabIndex = 0, active
   if (to) {
     return (
       <NavLink
+        aria-label={label}
         className={classNames(styles.menuButton, { [styles.small]: small })}
         onClick={onClick}
         activeClassName={styles.active}
@@ -34,7 +35,7 @@ const MenuButton: React.FC<Props> = ({ label, to, onClick, tabIndex = 0, active
   }
 
   return (
-    <div className={classNames(styles.menuButton, { [styles.active]: active })} onClick={onClick} tabIndex={tabIndex}>
+    <div aria-label={label} className={classNames(styles.menuButton, { [styles.active]: active })} onClick={onClick} tabIndex={tabIndex}>
       {icon}
       <span className={styles.label}>{label}</span>
     </div>

src/components/MenuButton/__snapshots__/MenuButton.test.tsx.snap

@@ -3,6 +3,7 @@
 exports[`<MenuButton> renders and matches snapshot 1`] = `
 <div>
   <div
+    aria-label="Label"
     class="menuButton"
     tabindex="0"
   >

src/components/Popover/__snapshots__/Popover.test.tsx.snap

@@ -10,6 +10,7 @@ exports[`<Popover> renders and matches snapshot 1`] = `
     >
       <a
         aria-current="page"
+        aria-label="Home"
         class="menuButton active"
         href="/"
         tabindex="0"

src/components/UserMenu/UserMenu.tsx

@@ -1,4 +1,5 @@
-import React from 'react';
+import React, { useCallback } from 'react';
+import { useHistory } from 'react-router-dom';
 import { useTranslation } from 'react-i18next';
 import classNames from 'classnames';
 
@@ -7,6 +8,7 @@ import Favorite from '../../icons/Favorite';
 import BalanceWallet from '../../icons/BalanceWallet';
 import Exit from '../../icons/Exit';
 import MenuButton from '../MenuButton/MenuButton';
+import { logout } from '../../stores/AccountStore';
 
 import styles from './UserMenu.module.scss';
 
@@ -18,6 +20,16 @@ type Props = {
 
 const UserMenu = ({ showPaymentsItem, inPopover = false, onClick }: Props) => {
   const { t } = useTranslation('user');
+  const history = useHistory();
+
+  const onLogout = useCallback(() => {
+    if (onClick) {
+      onClick();
+    }
+
+    logout();
+    history.replace('/');
+  }, [onClick, history]);
 
   const menuItems = (
     <ul className={styles.menuItems}>
@@ -34,7 +46,7 @@ const UserMenu = ({ showPaymentsItem, inPopover = false, onClick }: Props) => {
       )}
       <hr className={classNames(styles.divider, { [styles.inPopover]: inPopover })} />
       <li>
-        <MenuButton small={inPopover} onClick={onClick} to="/u/logout" label={t('nav.logout')} startIcon={<Exit />} />
+        <MenuButton small={inPopover} onClick={onLogout} label={t('nav.logout')} startIcon={<Exit />} />
       </li>
     </ul>
   );

src/components/UserMenu/__snapshots__/UserMenu.test.tsx.snap

@@ -7,6 +7,7 @@ exports[`<UserMenu> renders and matches snapshot 1`] = `
   >
     <li>
       <a
+        aria-label="nav.account"
         class="menuButton"
         href="/u/my-account"
         tabindex="0"
@@ -35,6 +36,7 @@ exports[`<UserMenu> renders and matches snapshot 1`] = `
     </li>
     <li>
       <a
+        aria-label="nav.favorites"
         class="menuButton"
         href="/u/favorites"
         tabindex="0"
@@ -63,6 +65,7 @@ exports[`<UserMenu> renders and matches snapshot 1`] = `
     </li>
     <li>
       <a
+        aria-label="nav.payments"
         class="menuButton"
         href="/u/payments"
         tabindex="0"
@@ -93,9 +96,9 @@ exports[`<UserMenu> renders and matches snapshot 1`] = `
       class="divider"
     />
     <li>
-      <a
+      <div
+        aria-label="nav.logout"
         class="menuButton"
-        href="/u/logout"
         tabindex="0"
       >
         <div
@@ -118,7 +121,7 @@ exports[`<UserMenu> renders and matches snapshot 1`] = `
         <span>
           nav.logout
         </span>
-      </a>
+      </div>
     </li>
   </ul>
 </div>

src/containers/Layout/__snapshots__/Layout.test.tsx.snap

@@ -90,6 +90,7 @@ exports[`<Layout /> renders layout 1`] = `
         >
           <a
             aria-current="page"
+            aria-label="home"
             class="menuButton active"
             href="/"
             tabindex="-1"

src/screens/User/User.tsx

@@ -1,5 +1,5 @@
-import React, { useEffect, useState } from 'react';
-import { Redirect, Route, Switch, useHistory, useLocation } from 'react-router-dom';
+import React, { useCallback, useEffect, useState } from 'react';
+import { Redirect, Route, Switch, useHistory } from 'react-router-dom';
 import type { PlaylistItem } from 'types/playlist';
 import { useTranslation } from 'react-i18next';
 
@@ -27,7 +27,6 @@ import styles from './User.module.scss';
 const User = (): JSX.Element => {
   const accessModel = ConfigStore.useState((s) => s.accessModel);
   const history = useHistory();
-  const location = useLocation();
   const { t } = useTranslation('user');
   const breakpoint = useBreakpoint();
   const [clearFavoritesOpen, setClearFavoritesOpen] = useState(false);
@@ -40,6 +39,10 @@ const User = (): JSX.Element => {
 
   const onCardClick = (playlistItem: PlaylistItem) => history.push(cardUrl(playlistItem));
   const onCardHover = (playlistItem: PlaylistItem) => updateBlurImage(playlistItem.image);
+  const onLogout = useCallback(() => {
+    // Empty customer on a user page leads to history.replace (code bellow), so we don't repeat it here
+    logout();
+  }, []);
 
   useEffect(() => updateBlurImage(''), [updateBlurImage]);
 
@@ -49,14 +52,6 @@ const User = (): JSX.Element => {
     }
   }, [history, customer, loading]);
 
-  useEffect(() => {
-    // Todo: Make logout a function, not a route (https://stackoverflow.com/questions/3521290/logout-get-or-post)
-    if (location.pathname === '/u/logout') {
-      logout();
-      history.push('/');
-    }
-  }, [location, history]);
-
   if (!customer) {
     return (
       <div className={styles.user}>
@@ -83,7 +78,7 @@ const User = (): JSX.Element => {
                 </li>
               )}
               <li className={styles.logoutLi}>
-                <Button to="/u/logout" label={t('nav.logout')} variant="text" startIcon={<Exit />} className={styles.button} />
+                <Button onClick={onLogout} label={t('nav.logout')} variant="text" startIcon={<Exit />} className={styles.button} />
               </li>
             </ul>
           </div>
@@ -137,9 +132,6 @@ const User = (): JSX.Element => {
               <Redirect to="/u/my-account" />
             )}
           </Route>
-          <Route path="/u/logout">
-            <LoadingOverlay transparentBackground />
-          </Route>
           <Route path="/u/:other?">
             <Redirect to="/u/my-account" />
           </Route>

test-e2e/utils/steps_file.ts

@@ -52,7 +52,7 @@ module.exports = function () {
         this.openUserMenu();
       }
 
-      this.click('Log out');
+      this.click('div[aria-label="Log out"]');
     },
     // This function will register the user on the first call and return the context
     // then assuming context is passed in the next time, will log that same user back in