Config error path (#33)

* expand the node configuration to include more than just the the array of nodes * add error handling for instance when nodes config is not specified * add integration test showing we dispaly an error if there is an error parsing the config * error message component accepts an optional message to render instead of the default verbage
USEPA · Feb 28, 2024 · 1c529e8 · 1c529e8
1 parent f06e034
commit 1c529e8
Show file tree

Hide file tree

Showing 8 changed files with 383 additions and 340 deletions.
diff --git a/public/default.json b/public/default.json
diff --git a/src/App.spec.tsx b/src/App.spec.tsx
@@ -7,16 +7,18 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from 'vitest
 
 const handlers = [
   http.get('/default.json', async () => {
-    return HttpResponse.json([
-      {
-        id: '1',
-        type: 'default',
-        data: {
-          label: 'I like turtles',
-          children: ['2'],
+    return HttpResponse.json({
+      nodes: [
+        {
+          id: '1',
+          type: 'default',
+          data: {
+            label: 'I like turtles',
+            children: ['2'],
+          },
         },
-      },
-    ]);
+      ],
+    });
   }),
 ];
 
@@ -27,23 +29,28 @@ afterEach(() => {
   vi.unstubAllEnvs();
 });
 beforeAll(() => server.listen());
-afterAll(() => server.close());
+afterAll(() => {
+  server.close();
+  server.resetHandlers();
+});
 
 describe('App', () => {
   it('shows a spinner while waiting for config', () => {
     server.use(
       http.get('/default.json', async () => {
         await delay(100);
-        return HttpResponse.json([
-          {
-            id: '1',
-            type: 'default',
-            data: {
-              label: 'I like turtles',
-              children: ['2'],
+        return HttpResponse.json({
+          nodes: [
+            {
+              id: '1',
+              type: 'default',
+              data: {
+                label: 'I like turtles',
+                children: ['2'],
+              },
             },
-          },
-        ]);
+          ],
+        });
       })
     );
     render(<App />);
@@ -61,13 +68,15 @@ describe('App', () => {
     await waitFor(() => expect(screen.queryByTestId('spinner')).not.toBeInTheDocument());
     expect(screen.getByText('The Manifest Game')).toBeInTheDocument();
   });
-  it('Throws an error if there is an error fetching the config', async () => {
-    // ToDo - implement this test
-    expect(true).toBe(true);
-  });
   it('minimap is visible by default', async () => {
     render(<App />);
     await waitFor(() => expect(screen.queryByTestId('spinner')).not.toBeInTheDocument());
     expect(screen.getByTestId(/minimap/i)).toBeInTheDocument();
   });
+  it('Throws an error if there is an error fetching the config', async () => {
+    server.use(http.get('/default.json', async () => HttpResponse.error()));
+    render(<App />);
+    await waitFor(() => expect(screen.queryByTestId('spinner')).not.toBeInTheDocument());
+    expect(screen.getByText(/Error/i)).toBeInTheDocument();
+  });
 });
diff --git a/src/App.tsx b/src/App.tsx
@@ -1,3 +1,4 @@
+import { ErrorMsg } from 'components/Error';
 import { Header } from 'components/Header/Header';
 import { Spinner } from 'components/Spinner/Spinner';
 import { Tree } from 'components/Tree/Tree';
@@ -12,15 +13,15 @@ import { useState } from 'react';
  */
 export default function App() {
   const title = import.meta.env.VITE_APP_TITLE ?? 'The Manifest Game';
-  const { config, isLoading, error } = useFetchConfig('/default.json');
+  const {
+    config,
+    isLoading: configIsLoading,
+    error: configError,
+  } = useFetchConfig('/default.json');
   const [direction, setDirection] = useTreeDirection();
   const { nodes, edges, onClick } = useDecisionTree(config);
   const [mapVisible, setMapVisible] = useState(true);
 
-  if (isLoading || !config) return <Spinner />;
-
-  if (error || (!config && !isLoading)) throw new Error('Failed to fetch config');
-
   return (
     <>
       <Header
@@ -30,7 +31,13 @@ export default function App() {
         mapVisible={mapVisible}
         setMapVisible={setMapVisible}
       />
-      <Tree nodes={nodes} edges={edges} onClick={onClick} mapVisible={mapVisible} />
+      {configIsLoading ? (
+        <Spinner />
+      ) : configError ? (
+        <ErrorMsg message={'Error parsing the Decision Tree'} />
+      ) : (
+        <Tree nodes={nodes} edges={edges} onClick={onClick} mapVisible={mapVisible} />
+      )}
     </>
   );
 }
diff --git a/src/components/Error/ErrorMsg.spec.tsx b/src/components/Error/ErrorMsg.spec.tsx
@@ -9,12 +9,14 @@ beforeAll(() => {
 beforeEach(() => {
   vi.unstubAllEnvs();
 });
-afterEach(() => cleanup());
+afterEach(() => {
+  cleanup();
+});
 
 describe('ErrorMsg', () => {
   it('renders', () => {
     render(<ErrorMsg />);
-    expect(screen.getByText(/Something went wrong/i)).toBeInTheDocument();
+    expect(screen.getByText(/An Error occurred/i)).toBeInTheDocument();
   });
   it('provides a configurable link to file a ticket', () => {
     const issueUrl = 'https://example.com/issues/new';
@@ -26,9 +28,13 @@ describe('ErrorMsg', () => {
     expect(issueLink).toHaveAttribute('href', issueUrl);
   });
   it('Does not render a link if environment variable is not defined', () => {
-    vi.unstubAllEnvs();
     render(<ErrorMsg />);
     const issueLink = screen.queryByRole('link', { name: 'file a ticket' });
     expect(issueLink).not.toBeInTheDocument();
   });
+  it('renders an error message if one if passed', () => {
+    const message = 'alright meow, license and registration';
+    render(<ErrorMsg message={message} />);
+    expect(screen.getByText(message)).toBeInTheDocument();
+  });
 });
diff --git a/src/components/Error/ErrorMsg.tsx b/src/components/Error/ErrorMsg.tsx
@@ -1,14 +1,18 @@
 import styles from './errorMsg.module.css';
 
+interface ErrorMsgProps {
+  message?: string;
+}
+
 /**
  * ErrorMsg displays a simple error message and a link to file a ticket.
  */
-export const ErrorMsg = () => {
+export const ErrorMsg = ({ message }: ErrorMsgProps) => {
   const issueURL = import.meta.env.VITE_ISSUE_URL;
   return (
     <div className={styles.center}>
       <div className={styles.errorBox}>
-        <h1>Something went wrong</h1>
+        <h2>{message ?? 'An error occurred'}</h2>
         {issueURL && (
           <p>
             <a href={issueURL} className={styles.errorLink}>

diff --git a/src/components/Error/errorMsg.module.css b/src/components/Error/errorMsg.module.css
@@ -30,9 +30,10 @@
 .center {
     display: flex;
     align-content: center;
+    text-align: center;
     justify-content: center;
-    margin-top: 20px;
-    margin-bottom: 20px;
-    font-size: 20px;
+    margin-top: 30%;
+    margin-bottom: 30%;
+    font-size: 1.2rem;
     font-weight: bold;
 }
diff --git a/src/hooks/useFetchConfig/useFetchConfig.spec.tsx b/src/hooks/useFetchConfig/useFetchConfig.spec.tsx
@@ -7,7 +7,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, test } from 'vitest';
 
 const handlers = [
   http.get('/default.json', () => {
-    return HttpResponse.json([{ id: '1', type: 'default', label: 'foo', children: [] }]);
+    return HttpResponse.json({ nodes: [{ id: '1', type: 'default', label: 'foo', children: [] }] });
   }),
 ];
 
@@ -51,13 +51,15 @@ describe('useFetchConfig', async () => {
   });
   test('sets the first node in the array to visible and the rest to hidden', async () => {
     server.use(
-      http.get('/default.json', () => {
-        return HttpResponse.json([
-          { id: '1', type: 'default', label: 'foo', children: [] },
-          { id: '2', type: 'default', label: 'bar', children: [] },
-          { id: '3', type: 'BoolNode', label: 'bool', children: [] },
-        ]);
-      })
+      http.get('/default.json', () =>
+        HttpResponse.json({
+          nodes: [
+            { id: '1', type: 'default', label: 'foo', children: [] },
+            { id: '2', type: 'default', label: 'bar', children: [] },
+            { id: '3', type: 'BoolNode', label: 'bool', children: [] },
+          ],
+        })
+      )
     );
     render(<TestComponent />);
     expect(screen.queryByText(/data/i)).not.toBeInTheDocument();
@@ -66,4 +68,15 @@ describe('useFetchConfig', async () => {
     expect(screen.queryByText('data id: 2 - hidden')).toBeInTheDocument();
     expect(screen.queryByText('data id: 3 - hidden')).toBeInTheDocument();
   });
+  test('sets an error if the config fails to parse', async () => {
+    server.use(
+      http.get('/default.json', () => {
+        return HttpResponse.json({ foo: 'bar' });
+      })
+    );
+    render(<TestComponent />);
+    expect(screen.queryByText(/data/i)).not.toBeInTheDocument();
+    await waitFor(() => screen.queryByText(/data/i));
+    expect(screen.queryByText('error')).toBeInTheDocument();
+  });
 });
diff --git a/src/hooks/useFetchConfig/useFetchConfig.tsx b/src/hooks/useFetchConfig/useFetchConfig.tsx
@@ -4,54 +4,48 @@ import { TreeNode } from 'store';
 import { PositionUnawareDecisionTree } from 'store/DagSlice/dagSlice';
 
 /**
- * data needed by all TreeNodes that contains the nodes expanded state, the node's children, and the node's text
+ * Data needed by all TreeNodes that contains the nodes expanded state,
+ * the node's children, and the node's text
  */
 export interface NodeData {
   label: string;
   children: string[];
   expanded?: boolean;
 }
 
-/**
- * data needed by the BooleanTreeNode to render decisions
- */
+/** data needed by the BooleanTreeNode to render decisions*/
 export interface BooleanNodeData extends NodeData {
   yesId: string;
   noId: string;
 }
 
-/**
- * An individual object to configure a node, part of the tree config
- */
-export interface TreeNodeConfig {
+/** Configuration for an individual node, part of the larger config*/
+export interface NodeConfig {
   id: string;
   data: NodeData | BooleanNodeData;
   type?: string;
 }
 
 /**
- * A tree config is a JSON serializable array of object that contains all the position unaware
+ * A JSON serializable array of object that contains all the position unaware
  * nodes in the decision tree, before it is loaded into the store
  */
-export type TreeConfig = Array<TreeNodeConfig>;
+export type ConfigFile = {
+  name: string;
+  nodes?: Array<NodeConfig>;
+};
 
 interface UseFetchConfigError {
   message: string;
 }
 
-export interface BaseConfig {
-  id: string;
-  type?: 'default' | 'BoolNode';
-  data: {
-    label: string;
-    children?: string[];
-  };
-}
-
 /** Parses the config file (an array of BoolNodeConfig and DefaultNodeConfig types) and returns a DecisionTree */
-const parseConfig = (config: TreeConfig): PositionUnawareDecisionTree => {
+const parseConfig = (config: ConfigFile): PositionUnawareDecisionTree => {
   const tree: PositionUnawareDecisionTree = {};
-  config.forEach((node, index) => {
+  if (config.nodes === undefined || config.nodes.length === 0) {
+    throw new Error('Error Parsing Config');
+  }
+  config.nodes.forEach((node, index) => {
     if (node.type === 'BoolNode') {
       const { id, data } = node;
       tree[id] = {
@@ -90,9 +84,16 @@ export const useFetchConfig = (configPath: string) => {
     fetch(configPath)
       .then((response) => response.json())
       .then((data) => {
-        setConfig(parseConfig(data));
+        try {
+          const config = parseConfig(data);
+          setConfig(config);
+        } catch {
+          setError({ message: 'Error Parsing Config' });
+        }
+      })
+      .catch((error) => {
+        setError({ message: `Network error: ${error}` });
       })
-      .catch((error) => setError(error))
       .finally(() => setIsLoading(false));
   }, [configPath]);