Merge pull request #36 from Alex-NRCan/feat-better-validations-errorh…

…andling Schema validation improvements on the error messages and better React lifecyle integration (#36)
Canadian-Geospatial-Platform · Oct 19, 2023 · 6e686dd · 6e686dd
2 parents ca6f6d9 + 34b8750
commit 6e686dd
Show file tree

Hide file tree

Showing 3 changed files with 116 additions and 80 deletions.
diff --git a/src/app.tsx b/src/app.tsx
@@ -1,5 +1,5 @@
 import { Chart } from './chart';
-import { ValidatorResult } from './chart-validator';
+import { ChartValidator, ValidatorResult } from './chart-validator';
 
 /**
  * Create a container to visualize a GeoChart in a standalone manner.
@@ -38,37 +38,12 @@ export function App(): JSX.Element {
    */
   const handleError = (dataErrors: ValidatorResult, optionsErrors: ValidatorResult) => {
     // Gather all error messages
-    let msgData = '';
-    dataErrors.errors?.forEach((m: string) => {
-      msgData += `${m}\n`;
-    });
-
-    // Gather all error messages
-    let msgOptions = '';
-    optionsErrors.errors?.forEach((m: string) => {
-      msgOptions += `${m}\n`;
-    });
+    const msgAll = ChartValidator.parseValidatorResultsMessages([dataErrors, optionsErrors]);
 
     // Show the error (actually, can't because the snackbar is linked to a map at the moment and geochart is standalone)
     // TODO: Decide if we want the snackbar outside of a map or not and use showError or not
-    cgpv.api.utilities.showError('', msgData);
-    cgpv.api.utilities.showError('', msgOptions);
-    console.error(dataErrors.errors, optionsErrors.errors);
-    alert('There was an error parsing the Chart inputs. View console for details.');
-  };
-
-  /**
-   * Handles when the Chart X Axis has changed.
-   */
-  const handleChartXAxisChanged = () => {
-    console.log('Handle Chart X Axis');
-  };
-
-  /**
-   * Handles when the Chart Y Axis has changed.
-   */
-  const handleChartYAxisChanged = () => {
-    console.log('Handle Chart Y Axis');
+    cgpv.api.utilities.showError('', msgAll);
+    alert(`There was an error parsing the Chart inputs.\n\n${msgAll}\n\nView console for details.`);
   };
 
   // Effect hook to add and remove event listeners
@@ -80,16 +55,7 @@ export function App(): JSX.Element {
   });
 
   // Render the Chart
-  return (
-    <Chart
-      style={{ width: 800 }}
-      data={data}
-      options={options}
-      handleSliderXChanged={handleChartXAxisChanged}
-      handleSliderYChanged={handleChartYAxisChanged}
-      handleError={handleError}
-    />
-  );
+  return <Chart style={{ width: 800 }} data={data} options={options} handleError={handleError} />;
 }
 
 export default App;
diff --git a/src/chart-validator.ts b/src/chart-validator.ts
@@ -4,6 +4,7 @@ import Ajv from 'ajv';
  * Represents the result of a Chart data or options inputs validations.
  */
 export type ValidatorResult = {
+  param: string;
   valid: boolean;
   errors?: string[];
 };
@@ -145,9 +146,11 @@ export class ChartValidator {
     // Validate
     const valid = validate(data) as boolean;
     return {
+      param: 'data',
       valid,
       errors: validate.errors?.map((e: Ajv.ErrorObject) => {
-        return e.message || 'generic schema error';
+        const m = e.message || 'generic schema error';
+        return `${e.schemaPath} | ${e.keyword} | ${m}`;
       }),
     };
   };
@@ -162,10 +165,31 @@ export class ChartValidator {
     // Validate
     const valid = validate(options) as boolean;
     return {
+      param: 'options',
       valid,
       errors: validate.errors?.map((e: Ajv.ErrorObject) => {
-        return e.message || 'generic schema error';
+        const m = e.message || 'generic schema error';
+        return `${e.schemaPath} | ${e.keyword} | ${m}`;
       }),
     };
   };
+
+  public static parseValidatorResultsMessages(valRes: ValidatorResult[]) {
+    // Gather all error messages for data input
+    let msg = '';
+    valRes.forEach((v) => {
+      // Redirect
+      msg += ChartValidator.parseValidatorResultMessage(v);
+    });
+    return msg.replace(/^\n+|\n+$/gm, '');
+  }
+
+  public static parseValidatorResultMessage(valRes: ValidatorResult) {
+    // Gather all error messages for data input
+    let msg = '';
+    valRes.errors?.forEach((m: string) => {
+      msg += `${m}\n`;
+    });
+    return msg.replace(/^\n+|\n+$/gm, '');
+  }
 }
diff --git a/src/chart.tsx b/src/chart.tsx
@@ -48,72 +48,54 @@ export function Chart(props: TypeChartChartProps<GeoChartType>): JSX.Element {
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
   const w = window as any;
   const { cgpv } = w;
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
   const { useEffect, useState, useRef, CSSProperties } = cgpv.react;
   const { Grid, Checkbox, Slider, Typography } = cgpv.ui.elements;
-  const { style: elStyle, data, options: elOptions, action: elAction } = props;
+  const {
+    style: elStyle,
+    data,
+    options: elOptions,
+    action: elAction,
+    defaultColors,
+    handleSliderXChanged,
+    handleSliderYChanged,
+    handleError,
+  } = props;
 
   // Cast the style
   const style = elStyle as typeof CSSProperties;
 
   // Attribute the ChartJS default colors
-  if (props.defaultColors?.backgroundColor) ChartJS.defaults.backgroundColor = props.defaultColors?.backgroundColor;
-  if (props.defaultColors?.borderColor) ChartJS.defaults.borderColor = props.defaultColors?.borderColor;
-  if (props.defaultColors?.color) ChartJS.defaults.color = props.defaultColors?.color;
+  if (defaultColors?.backgroundColor) ChartJS.defaults.backgroundColor = defaultColors?.backgroundColor;
+  if (defaultColors?.borderColor) ChartJS.defaults.borderColor = defaultColors?.borderColor;
+  if (defaultColors?.color) ChartJS.defaults.color = defaultColors?.color;
 
   // Merge default options
   const options: GeoChartOptions = { ...Chart.defaultProps.options, ...elOptions } as GeoChartOptions;
-
-  // If options and data are specified
-  if (options && data) {
-    // Validate the data and options as received
-    const validator = new ChartValidator();
-    const resOptions: ValidatorResult = validator.validateOptions(options);
-    const resData: ValidatorResult = validator.validateData(data);
-
-    // If any errors
-    if (!resOptions.valid || !resData.valid) {
-      // If a callback is defined
-      if (props.handleError) props.handleError(resData, resOptions);
-      else console.error(resData, resOptions);
-    }
+  if (!options?.geochart?.chart) {
+    // Deep assign, in case geochart was specified in elOptions, but geochart wasn't (losing the default for 'chart' by accident)
+    options.geochart.chart = Chart.defaultProps.options.geochart.chart as GeoChartType;
   }
 
   // STATE / REF SECTION *******
   const [redraw, setRedraw] = useState(elAction?.shouldRedraw);
   const chartRef = useRef(null);
-  // const [selectedDatasets, setSelectedDatasets] = useState();
-
-  // If redraw is true, reset the property, set the redraw property to true for the chart, then prep a timer to reset it to false after the redraw has happened.
-  // A bit funky, but as documented online.
-  if (elAction?.shouldRedraw) {
-    elAction!.shouldRedraw = false;
-    setRedraw(true);
-    setTimeout(() => {
-      setRedraw(false);
-    }, 200);
-  }
 
   /**
    * Handles when the X Slider changes
    * @param value number | number[] Indicates the slider value
    */
   const handleSliderXChange = (value: number | number[]) => {
-    // If callback set
-    if (props.handleSliderXChanged) {
-      props.handleSliderXChanged!(value);
-    }
+    // Callback
+    handleSliderXChanged?.(value);
   };
 
   /**
    * Handles when the Y Slider changes
    * @param value number | number[] Indicates the slider value
    */
   const handleSliderYChange = (value: number | number[]) => {
-    // If callback set
-    if (props.handleSliderYChanged) {
-      props.handleSliderYChanged!(value);
-    }
+    // Callback
+    handleSliderYChanged?.(value);
   };
 
   /**
@@ -251,13 +233,73 @@ export function Chart(props: TypeChartChartProps<GeoChartType>): JSX.Element {
     return <div />;
   };
 
-  return renderChartContainer();
+  /**
+   * Renders the whole Chart container JSX.Element or an empty div
+   * @returns The whole Chart container JSX.Element or an empty div
+   */
+  const renderChartContainerFailed = (): JSX.Element => {
+    return <div style={{ color: 'red' }}>Error rendering the Chart. Check console for details.</div>;
+  };
+
+  //
+  // PROCEED WITH LOGIC HERE!
+  //
+
+  // If options and data are specified
+  let resOptions: ValidatorResult | undefined;
+  let resData: ValidatorResult | undefined;
+  if (options && data) {
+    // Validate the data and options as received
+    const validator = new ChartValidator();
+    resOptions = validator.validateOptions(options) || undefined;
+    resData = validator.validateData(data);
+  }
+
+  // Effect hook to raise the error on the correct React state.
+  // This is because it's quite probable the handling of the error will want to modify the state of another
+  // component (e.g. Snackbar) and React will throw a warning if this is not done in the useEffect().
+  useEffect(() => {
+    // If the options or data schemas were checked and had errors
+    if (resData && resOptions && (!resData.valid || !resOptions.valid)) {
+      // If a callback is defined
+      handleError?.(resData, resOptions);
+      console.error(resData, resOptions);
+    }
+  }, [handleError, resData, resOptions]);
+
+  // If options and data are specified
+  if (options && data) {
+    // If no errors
+    if (resOptions?.valid && resData?.valid) {
+      // If redraw is true, reset the property, set the redraw property to true for the chart, then prep a timer to reset it to false after the redraw has happened.
+      // A bit funky, but as documented online.
+      if (elAction?.shouldRedraw) {
+        elAction!.shouldRedraw = false;
+        setRedraw(true);
+        setTimeout(() => {
+          setRedraw(false);
+        }, 200);
+      }
+
+      // Render the chart
+      return renderChartContainer();
+    }
+
+    // Failed to render
+    return renderChartContainerFailed();
+  }
+
+  // Nothing to render, no errors either
+  return <div />;
 }
 
 /**
  * React's default properties for the GeoChart
  */
 Chart.defaultProps = {
+  style: null,
+  defaultColors: null,
+  data: null,
   options: {
     responsive: true,
     plugins: {
@@ -269,4 +311,8 @@ Chart.defaultProps = {
       chart: 'line',
     },
   },
+  action: null,
+  handleSliderXChanged: null,
+  handleSliderYChanged: null,
+  handleError: null,
 };