fix(auth): only show retry button for auth failures (#545)

cryostatio · Oct 11, 2022 · c8dbc3d · c8dbc3d
1 parent 311cdd9
commit c8dbc3d
Show file tree

Hide file tree

Showing 13 changed files with 715 additions and 106 deletions.
diff --git a/src/app/ErrorView/ErrorView.tsx b/src/app/ErrorView/ErrorView.tsx
@@ -49,6 +49,7 @@ import {
 import { ExclamationCircleIcon } from '@patternfly/react-icons';
 
 export const authFailMessage = 'Auth failure';
+export const isAuthFail = (message: string) => message === authFailMessage;
 export interface ErrorViewProps {
   title: string | React.ReactNode;
   message: string | React.ReactNode;

diff --git a/src/app/Events/EventTemplates.tsx b/src/app/Events/EventTemplates.tsx
@@ -54,7 +54,6 @@ import {
   ToolbarGroup,
   ToolbarItem,
   TextInput,
-  Text,
 } from '@patternfly/react-core';
 import { UploadIcon } from '@patternfly/react-icons';
 import {
@@ -72,7 +71,7 @@ import {
 import { useHistory } from 'react-router-dom';
 import { concatMap, filter, first } from 'rxjs/operators';
 import { LoadingView } from '@app/LoadingView/LoadingView';
-import { authFailMessage, ErrorView } from '@app/ErrorView/ErrorView';
+import { authFailMessage, ErrorView, isAuthFail } from '@app/ErrorView/ErrorView';
 import { DeleteWarningType } from '@app/Modal/DeleteWarningUtils';
 import { DeleteWarningModal } from '@app/Modal/DeleteWarningModal';
 
@@ -95,12 +94,15 @@ export const EventTemplates = () => {
   const [rowDeleteData, setRowDeleteData] = React.useState({} as IRowData);
   const addSubscription = useSubscriptions();
 
-  const tableColumns = [
-    { title: 'Name', transforms: [sortable] },
-    'Description',
-    { title: 'Provider', transforms: [sortable] },
-    { title: 'Type', transforms: [sortable] },
-  ];
+  const tableColumns = React.useMemo(
+    () => [
+      { title: 'Name', transforms: [sortable] },
+      'Description',
+      { title: 'Provider', transforms: [sortable] },
+      { title: 'Type', transforms: [sortable] },
+    ],
+    [sortable]
+  );
 
   React.useEffect(() => {
     let filtered;
@@ -219,14 +221,17 @@ export const EventTemplates = () => {
     [filteredTemplates]
   );
 
-  const handleDelete = (rowData) => {
-    addSubscription(
-      context.api
-        .deleteCustomEventTemplate(rowData[0])
-        .pipe(first())
-        .subscribe(() => {} /* do nothing - notification will handle updating state */)
-    );
-  };
+  const handleDelete = React.useCallback(
+    (rowData) => {
+      addSubscription(
+        context.api
+          .deleteCustomEventTemplate(rowData[0])
+          .pipe(first())
+          .subscribe(() => {} /* do nothing - notification will handle updating state */)
+      );
+    },
+    [addSubscription, context.api]
+  );
 
   const actionResolver = (rowData: IRowData, extraData: IExtraData) => {
     if (typeof extraData.rowIndex == 'undefined') {
@@ -268,7 +273,7 @@ export const EventTemplates = () => {
     return actions;
   };
 
-  const handleModalToggle = () => {
+  const handleModalToggle = React.useCallback(() => {
     setModalOpen((v) => {
       if (v) {
         setUploadFile(undefined);
@@ -277,15 +282,18 @@ export const EventTemplates = () => {
       }
       return !v;
     });
-  };
+  }, [setModalOpen, setUploadFile, setUploadFilename, setUploading]);
 
-  const handleFileChange = (value, filename) => {
-    setFileRejected(false);
-    setUploadFile(value);
-    setUploadFilename(filename);
-  };
+  const handleFileChange = React.useCallback(
+    (value, filename) => {
+      setFileRejected(false);
+      setUploadFile(value);
+      setUploadFilename(filename);
+    },
+    [setFileRejected, setUploadFile, setUploadFilename]
+  );
 
-  const handleUploadSubmit = () => {
+  const handleUploadSubmit = React.useCallback(() => {
     if (!uploadFile) {
       window.console.error('Attempted to submit template upload without a file selected');
       return;
@@ -304,21 +312,33 @@ export const EventTemplates = () => {
           }
         })
     );
-  };
+  }, [
+    uploadFile,
+    window.console,
+    setUploading,
+    addSubscription,
+    context.api,
+    setUploadFile,
+    setUploadFilename,
+    setModalOpen,
+  ]);
 
-  const handleUploadCancel = () => {
+  const handleUploadCancel = React.useCallback(() => {
     setUploadFile(undefined);
     setUploadFilename('');
     setModalOpen(false);
-  };
+  }, [setUploadFile, setUploadFilename, setModalOpen]);
 
-  const handleFileRejected = () => {
+  const handleFileRejected = React.useCallback(() => {
     setFileRejected(true);
-  };
+  }, [setFileRejected]);
 
-  const handleSort = (event, index, direction) => {
-    setSortBy({ index, direction });
-  };
+  const handleSort = React.useCallback(
+    (event, index, direction) => {
+      setSortBy({ index, direction });
+    },
+    [setSortBy]
+  );
 
   const handleDeleteButton = React.useCallback(
     (rowData) => {
@@ -379,7 +399,13 @@ export const EventTemplates = () => {
   }, [context.target, context.target.setAuthRetry]);
 
   if (errorMessage != '') {
-    return <ErrorView title={'Error retrieving event templates'} message={errorMessage} retry={authRetry} />;
+    return (
+      <ErrorView
+        title={'Error retrieving event templates'}
+        message={errorMessage}
+        retry={isAuthFail(errorMessage) ? authRetry : undefined}
+      />
+    );
   } else if (isLoading) {
     return (
       <>

diff --git a/src/app/Events/EventTypes.tsx b/src/app/Events/EventTypes.tsx
@@ -46,13 +46,11 @@ import {
   ToolbarItemVariant,
   Pagination,
   TextInput,
-  Text,
-  Button,
 } from '@patternfly/react-core';
 import { expandable, Table, TableBody, TableHeader, TableVariant } from '@patternfly/react-table';
 import { concatMap, filter, first } from 'rxjs/operators';
 import { LoadingView } from '@app/LoadingView/LoadingView';
-import { authFailMessage, ErrorView } from '@app/ErrorView/ErrorView';
+import { authFailMessage, ErrorView, isAuthFail } from '@app/ErrorView/ErrorView';
 
 export interface EventType {
   name: string;
@@ -75,6 +73,10 @@ type Row = {
   fullWidth?: boolean;
 };
 
+const getCategoryString = (eventType: EventType): string => {
+  return eventType.category.join(', ').trim();
+};
+
 export const EventTypes = () => {
   const context = React.useContext(ServiceContext);
   const addSubscription = useSubscriptions();
@@ -89,15 +91,18 @@ export const EventTypes = () => {
   const [isLoading, setIsLoading] = React.useState(false);
   const [errorMessage, setErrorMessage] = React.useState('');
 
-  const tableColumns = [
-    {
-      title: 'Name',
-      cellFormatters: [expandable],
-    },
-    'Type ID',
-    'Description',
-    'Categories',
-  ];
+  const tableColumns = React.useMemo(
+    () => [
+      {
+        title: 'Name',
+        cellFormatters: [expandable],
+      },
+      'Type ID',
+      'Description',
+      'Categories',
+    ],
+    [expandable]
+  );
 
   const handleTypes = React.useCallback(
     (types) => {
@@ -128,10 +133,10 @@ export const EventTypes = () => {
             context.api.doGet<EventType[]>(`targets/${encodeURIComponent(target.connectUrl)}/events`)
           )
         )
-        .subscribe(
-          (value) => handleTypes(value),
-          (err) => handleError(err)
-        )
+        .subscribe({
+          next: handleTypes,
+          error: handleError,
+        })
     );
   }, [addSubscription, context.target, context.api]);
 
@@ -145,16 +150,9 @@ export const EventTypes = () => {
   }, [addSubscription, context, context.target, refreshEvents]);
 
   React.useEffect(() => {
-    const sub = context.target.authFailure().subscribe(() => {
-      setErrorMessage(authFailMessage);
-    });
-    return () => sub.unsubscribe();
+    addSubscription(context.target.authFailure().subscribe(() => setErrorMessage(authFailMessage)));
   }, [context.target]);
 
-  const getCategoryString = (eventType: EventType): string => {
-    return eventType.category.join(', ').trim();
-  };
-
   const filterTypesByText = React.useCallback(() => {
     if (!filterText) {
       return types;
@@ -196,38 +194,53 @@ export const EventTypes = () => {
     setDisplayedTypes(rows);
   }, [currentPage, perPage, filterTypesByText, openRow]);
 
-  const onCurrentPage = (evt, currentPage) => {
-    setOpenRow(-1);
-    setCurrentPage(currentPage);
-  };
+  const onCurrentPage = React.useCallback(
+    (evt, currentPage) => {
+      setOpenRow(-1);
+      setCurrentPage(currentPage);
+    },
+    [setOpenRow, setCurrentPage]
+  );
 
-  const onPerPage = (evt, perPage) => {
-    const offset = (currentPage - 1) * prevPerPage.current;
-    prevPerPage.current = perPage;
-    setOpenRow(-1);
-    setPerPage(perPage);
-    setCurrentPage(1 + Math.floor(offset / perPage));
-  };
+  const onPerPage = React.useCallback(
+    (evt, perPage) => {
+      const offset = (currentPage - 1) * prevPerPage.current;
+      prevPerPage.current = perPage;
+      setOpenRow(-1);
+      setPerPage(perPage);
+      setCurrentPage(1 + Math.floor(offset / perPage));
+    },
+    [currentPage, prevPerPage, setOpenRow, setPerPage, setCurrentPage]
+  );
 
-  const onCollapse = (event, rowKey, isOpen) => {
-    if (isOpen) {
-      if (openRow === -1) {
-        setOpenRow(rowKey);
+  const onCollapse = React.useCallback(
+    (event, rowKey, isOpen) => {
+      if (isOpen) {
+        if (openRow === -1) {
+          setOpenRow(rowKey);
+        } else {
+          setOpenRow(rowKey > openRow ? rowKey - 1 : rowKey);
+        }
       } else {
-        setOpenRow(rowKey > openRow ? rowKey - 1 : rowKey);
+        setOpenRow(-1);
       }
-    } else {
-      setOpenRow(-1);
-    }
-  };
+    },
+    [setOpenRow, openRow]
+  );
 
   const authRetry = React.useCallback(() => {
     context.target.setAuthRetry();
   }, [context.target, context.target.setAuthRetry]);
 
   // TODO replace table with data list so collapsed event options can be custom formatted
   if (errorMessage != '') {
-    return <ErrorView title={'Error retrieving event types'} message={errorMessage} retry={authRetry} />;
+    return (
+      <ErrorView
+        title={'Error retrieving event types'}
+        message={errorMessage}
+        retry={isAuthFail(errorMessage) ? authRetry : undefined}
+      />
+    );
   } else if (isLoading) {
     return <LoadingView />;
   } else {

diff --git a/src/app/Recordings/ActiveRecordingsTable.tsx b/src/app/Recordings/ActiveRecordingsTable.tsx
@@ -258,11 +258,7 @@ export const ActiveRecordingsTable: React.FunctionComponent<ActiveRecordingsTabl
   }, [addSubscription, context, context.notificationChannel, setRecordings]);
 
   React.useEffect(() => {
-    addSubscription(
-      context.target.authFailure().subscribe(() => {
-        setErrorMessage(authFailMessage);
-      })
-    );
+    addSubscription(context.target.authFailure().subscribe(() => setErrorMessage(authFailMessage)));
   }, [context, context.target, setErrorMessage, addSubscription]);
 
   React.useEffect(() => {

diff --git a/src/app/Recordings/RecordingActions.tsx b/src/app/Recordings/RecordingActions.tsx
@@ -68,12 +68,13 @@ export const RecordingActions: React.FunctionComponent<RecordingActionsProps> =
   const addSubscription = useSubscriptions();
 
   React.useEffect(() => {
-    const sub = context.api
-      .grafanaDatasourceUrl()
-      .pipe(first())
-      .subscribe(() => setGrafanaEnabled(true));
-    return () => sub.unsubscribe();
-  }, [context.api, setGrafanaEnabled]);
+    addSubscription(
+      context.api
+        .grafanaDatasourceUrl()
+        .pipe(first())
+        .subscribe(() => setGrafanaEnabled(true))
+    );
+  }, [context.api, setGrafanaEnabled, addSubscription]);
 
   const grafanaUpload = React.useCallback(() => {
     notifications.info('Upload Started', `Recording "${props.recording.name}" uploading...`);

diff --git a/src/app/Recordings/RecordingsTable.tsx b/src/app/Recordings/RecordingsTable.tsx
@@ -48,7 +48,7 @@ import {
 import SearchIcon from '@patternfly/react-icons/dist/esm/icons/search-icon';
 import { TableComposable, Thead, Tr, Th, OuterScrollContainer, InnerScrollContainer } from '@patternfly/react-table';
 import { LoadingView } from '@app/LoadingView/LoadingView';
-import { ErrorView } from '@app/ErrorView/ErrorView';
+import { ErrorView, isAuthFail } from '@app/ErrorView/ErrorView';
 import { ServiceContext } from '@app/Shared/Services/Services';
 
 export interface RecordingsTableProps {
@@ -77,7 +77,11 @@ export const RecordingsTable: React.FunctionComponent<RecordingsTableProps> = (p
   if (props.errorMessage != '') {
     view = (
       <>
-        <ErrorView title={'Error retrieving recordings'} message={props.errorMessage} retry={authRetry}></ErrorView>
+        <ErrorView
+          title={'Error retrieving recordings'}
+          message={props.errorMessage}
+          retry={isAuthFail(props.errorMessage) ? authRetry : undefined}
+        />
       </>
     );
   } else if (props.isLoading) {