add error handling to TemplateSurvey

awx/ui_next/src/components/ErrorDetail/ErrorDetail.jsx

@@ -54,7 +54,7 @@ class ErrorDetail extends Component {
     const { response } = error;
 
     let message = '';
-    if (response.data) {
+    if (response?.data) {
       message =
         typeof response.data === 'string'
           ? response.data
@@ -64,8 +64,8 @@ class ErrorDetail extends Component {
     return (
       <Fragment>
         <CardBody>
-          {response.config.method.toUpperCase()} {response.config.url}{' '}
+          {response?.config?.method.toUpperCase()} {response?.config?.url}{' '}
-          <strong>{response.status}</strong>
+          <strong>{response?.status}</strong>
         </CardBody>
         <CardBody>{message}</CardBody>
       </Fragment>

awx/ui_next/src/screens/Template/TemplateSurvey.jsx

@@ -1,56 +1,104 @@
-import React, { useState, useEffect } from 'react';
+import React, { useState, useEffect, useCallback } from 'react';
 import { Switch, Route } from 'react-router-dom';
+import { withI18n } from '@lingui/react';
+import { t } from '@lingui/macro';
 import { JobTemplatesAPI } from '@api';
+import ContentError from '@components/ContentError';
+import AlertModal from '@components/AlertModal';
+import ErrorDetail from '@components/ErrorDetail';
+import useRequest, { useDismissableError } from '@util/useRequest';
 import SurveyList from './shared/SurveyList';
 
-export default function TemplateSurvey({ template }) {
+function TemplateSurvey({ template, i18n }) {
-  const [survey, setSurvey] = useState(null);
   const [surveyEnabled, setSurveyEnabled] = useState(template.survey_enabled);
 
-  useEffect(() => {
+  const {
-    (async () => {
+    result: survey,
+    request: fetchSurvey,
+    isLoading,
+    error: loadingError,
+    setValue: setSurvey,
+  } = useRequest(
+    useCallback(async () => {
       const { data } = await JobTemplatesAPI.readSurvey(template.id);
-      setSurvey(data);
+      return data;
-    })();
+    }, [template.id])
-  }, [template.id]);
+  );
+  useEffect(() => {
+    fetchSurvey();
+  }, [fetchSurvey]);
 
-  const updateSurvey = async newQuestions => {
+  const {
-    await JobTemplatesAPI.updateSurvey(template.id, {
+    request: updateSurvey,
-      ...survey,
+    isLoading: isUpdateLoading,
-      spec: newQuestions,
+    error: updateError,
-    });
+  } = useRequest(
-    setSurvey({
+    useCallback(
-      ...survey,
+      async updatedSurvey => {
-      spec: newQuestions,
+        await JobTemplatesAPI.updateSurvey(template.id, updatedSurvey);
-    });
+        setSurvey(updatedSurvey);
-  };
+      },
+      [template.id, setSurvey]
+    )
+  );
 
-  const deleteSurvey = async () => {
+  const {
-    await JobTemplatesAPI.destroySurvey(template.id);
+    request: deleteSurvey,
-    setSurvey(null);
+    isLoading: isDeleteLoading,
-  };
+    error: deleteError,
+  } = useRequest(
+    useCallback(async () => {
+      await JobTemplatesAPI.destroySurvey(template.id);
+      setSurvey(null);
+    }, [template.id, setSurvey])
+  );
 
-  const toggleSurvey = async () => {
+  const {
-    await JobTemplatesAPI.update(template.id, {
+    request: toggleSurvey,
-      survey_enabled: !surveyEnabled,
+    isLoading: isToggleLoading,
-    });
+    error: toggleError,
-    setSurveyEnabled(!surveyEnabled);
+  } = useRequest(
-  };
+    useCallback(async () => {
+      await JobTemplatesAPI.update(template.id, {
+        survey_enabled: !surveyEnabled,
+      });
+      setSurveyEnabled(!surveyEnabled);
+    }, [template.id, surveyEnabled])
+  );
 
-  // TODO
+  const { error, dismissError } = useDismissableError(
-  // if (contentError) {
+    updateError || deleteError || toggleError
-  // return <ContentError error={contentError} />;
+  );
+
+  if (loadingError) {
+    return <ContentError error={loadingError} />;
+  }
   return (
-    <Switch>
+    <>
-      <Route path="/templates/:templateType/:id/survey">
+      <Switch>
-        <SurveyList
+        <Route path="/templates/:templateType/:id/survey">
-          survey={survey}
+          <SurveyList
-          surveyEnabled={surveyEnabled}
+            survey={survey}
-          toggleSurvey={toggleSurvey}
+            surveyEnabled={surveyEnabled}
-          updateSurvey={updateSurvey}
+            toggleSurvey={toggleSurvey}
-          deleteSurvey={deleteSurvey}
+            updateSurvey={spec => updateSurvey({ ...survey, spec })}
-        />
+            deleteSurvey={deleteSurvey}
-      </Route>
+          />
-    </Switch>
+        </Route>
+      </Switch>
+      {error && (
+        <AlertModal
+          isOpen={error}
+          variant="error"
+          title={i18n._(t`Error!`)}
+          onClose={dismissError}
+        >
+          {i18n._(t`Failed to update survey.`)}
+          <ErrorDetail error={error} />
+        </AlertModal>
+      )}
+    </>
   );
 }
+
+export default withI18n()(TemplateSurvey);

awx/ui_next/src/screens/Template/shared/SurveyList.jsx

@@ -1,22 +1,17 @@
-import React, { useEffect, useCallback, useState } from 'react';
+import React, { useState } from 'react';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
 
-import useRequest from '@util/useRequest';
 import { Button } from '@patternfly/react-core';
 
 import ContentError from '@components/ContentError';
 import ContentLoading from '@components/ContentLoading';
 import ErrorDetail from '@components/ErrorDetail';
-import { JobTemplatesAPI } from '@api';
 import ContentEmpty from '@components/ContentEmpty';
 import AlertModal from '@components/AlertModal';
 import SurveyListItem from './SurveyListItem';
 import SurveyToolbar from './SurveyToolbar';
 
-// survey.name
-// survey.description
-// survey.spec
 function SurveyList({
   survey,
   surveyEnabled,
@@ -27,7 +22,6 @@ function SurveyList({
 }) {
   const questions = survey?.spec || [];
   const [selected, setSelected] = useState([]);
-  // const [showError, setShowError] = useState(false);
   const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false);
 
   const isAllSelected =

awx/ui_next/src/util/useRequest.js

@@ -8,11 +8,13 @@ import {
 
 /*
  * The useRequest hook accepts a request function and returns an object with
- * four values:
+ * six values:
  *   request: a function to call to invoke the request
  *   result: the value returned from the request function (once invoked)
  *   isLoading: boolean state indicating whether the request is in active/in flight
  *   error: any caught error resulting from the request
+ *   setValue: setter to explicitly set the result value
+ *   isInitialized: set to true once the result is initially fetched
  *
  * The hook also accepts an optional second parameter which is a default
  * value to set as result before the first time the request is made.
@@ -34,23 +36,42 @@ export default function useRequest(makeRequest, initialValue) {
     result,
     error,
     isLoading,
-    request: useCallback(async () => {
+    request: useCallback(
-      setIsLoading(true);
+      async (...args) => {
-      try {
+        setIsLoading(true);
-        const response = await makeRequest();
+        try {
-        if (isMounted.current) {
+          const response = await makeRequest(...args);
-          setResult(response);
+          if (isMounted.current) {
+            setResult(response);
+          }
+        } catch (err) {
+          if (isMounted.current) {
+            setError(err);
+          }
+        } finally {
+          if (isMounted.current) {
+            setIsLoading(false);
+          }
         }
-      } catch (err) {
+      },
-        if (isMounted.current) {
+      [makeRequest]
-          setError(err);
+    ),
-        }
+    setValue: setResult,
-      } finally {
+  };
-        if (isMounted.current) {
+}
-          setIsLoading(false);
+
-        }
+export function useDismissableError(error) {
-      }
+  const [showError, setShowError] = useState(false);
-    }, [makeRequest]),
+
+  useEffect(() => {
+    if (error) {
+      setShowError(true);
+    }
+  }, [error]);
+
+  return {
+    error: showError && error,
+    dismissError: () => setShowError(false),
   };
 }
 
@@ -60,14 +81,8 @@ export function useDeleteItems(
 ) {
   const location = useLocation();
   const history = useHistory();
-  const [showError, setShowError] = useState(false);
+  const { requestError, isLoading, request } = useRequest(makeRequest, null);
-  const { error, isLoading, request } = useRequest(makeRequest, null);
+  const { error, dismissError } = useDismissableError(requestError);
-
-  useEffect(() => {
-    if (error) {
-      setShowError(true);
-    }
-  }, [error]);
 
   const deleteItems = async () => {
     await request();
@@ -91,7 +106,7 @@ export function useDeleteItems(
   return {
     isLoading,
     deleteItems,
-    deletionError: showError && error,
+    deletionError: error,
-    clearDeletionError: () => setShowError(false),
+    clearDeletionError: dismissError,
   };
 }