feat(validation): add more validation errors

checkmarble · Aug 25, 2023 · 4a571c7 · 4a571c7
1 parent 0bf584a
commit 4a571c7
Show file tree

Hide file tree

Showing 10 changed files with 144 additions and 28 deletions.
diff --git a/packages/app-builder/public/locales/en/scenarios.json b/packages/app-builder/public/locales/en/scenarios.json
@@ -77,5 +77,14 @@
   "create_draft.keep_existing_draft": "Keep existing draft",
   "create_draft.override_existing_draft": "Replace existing draft",
   "create_rule.draft_already_exist_possibility": "You can keep the existing draft and edit it or you can replace the existing draft, creating a new one based on this version",
-  "validation.evaluation_error.unknown_function": "Required"
+  "validation.evaluation_error.unknown_function": "required",
+  "validation.evaluation_error.wrong_number_of_arguments": "wrong number of arguments",
+  "validation.evaluation_error.missing_named_argument": "missing named argument",
+  "validation.evaluation_error.arguments_must_be_int_or_float": "arguments must be an integer or a float",
+  "validation.evaluation_error.argument_must_be_integer": "argument must be an integer",
+  "validation.evaluation_error.argument_must_be_string": "argument must be a string",
+  "validation.evaluation_error.argument_must_be_boolean": "argument must be a boolean",
+  "validation.evaluation_error.argument_must_be_list": "argument must be a list",
+  "validation.evaluation_error.argument_must_be_convertible_to_duration": "argument must be a duration",
+  "validation.evaluation_error.argument_must_be_time": "argument must be a time"
 }
diff --git a/packages/app-builder/src/components/Edit/EditAstNode.tsx b/packages/app-builder/src/components/Edit/EditAstNode.tsx
@@ -1,7 +1,7 @@
 import {
   adaptAstNodeToViewModelFromIdentifier,
   type AstNode,
-  NewUnknownAstNode,
+  NewUndefinedAstNode,
 } from '@app-builder/models';
 import {
   useEditorIdentifiers,
@@ -10,6 +10,7 @@ import {
   useGetOperatorName,
   useIsEditedOnce,
 } from '@app-builder/services/editor';
+import { getInvalidStates } from '@app-builder/services/validation/scenario-validation';
 import { Combobox, Select } from '@ui-design-system';
 import { forwardRef, useState } from 'react';
 
@@ -23,7 +24,7 @@ export function EditAstNode({ name }: { name: string }) {
     <FormField
       name={name}
       render={({ fieldState: { error } }) => {
-        const isParentError = !!error?.type;
+        const invalidStates = getInvalidStates(error);
 
         return (
           <div className="relative">
@@ -35,7 +36,11 @@ export function EditAstNode({ name }: { name: string }) {
                     <FormControl>
                       <EditOperand
                         {...field}
-                        invalid={fieldState.invalid || isParentError}
+                        invalid={
+                          fieldState.invalid ||
+                          invalidStates.root ||
+                          invalidStates.children[field.name]
+                        }
                       />
                     </FormControl>
                     <FormMessage />
@@ -49,7 +54,11 @@ export function EditAstNode({ name }: { name: string }) {
                     <FormControl>
                       <EditOperator
                         {...field}
-                        invalid={fieldState.invalid || isParentError}
+                        invalid={
+                          fieldState.invalid ||
+                          invalidStates.root ||
+                          invalidStates.name
+                        }
                       />
                     </FormControl>
                   </FormItem>
@@ -62,15 +71,19 @@ export function EditAstNode({ name }: { name: string }) {
                     <FormControl>
                       <EditOperand
                         {...field}
-                        invalid={fieldState.invalid || isParentError}
+                        invalid={
+                          fieldState.invalid ||
+                          invalidStates.root ||
+                          invalidStates.children[field.name]
+                        }
                       />
                     </FormControl>
                     <FormMessage />
                   </FormItem>
                 )}
               />
             </div>
-            <FormMessage />
+            {invalidStates.root && isFirstChildEditedOnce && <FormMessage />}
           </div>
         );
       }}
@@ -105,7 +118,7 @@ const EditOperand = forwardRef<
       value={selectedItem}
       onChange={(value) => {
         setInputValue(value?.label ?? '');
-        onChange(value?.astNode ?? NewUnknownAstNode());
+        onChange(value?.astNode ?? NewUndefinedAstNode());
       }}
       nullable
     >

diff --git a/packages/app-builder/src/components/Edit/RootOrWithAnd.tsx b/packages/app-builder/src/components/Edit/RootOrWithAnd.tsx
@@ -1,4 +1,4 @@
-import { type AstNode, NewUnknownAstNode } from '@app-builder/models';
+import { type AstNode, NewUndefinedAstNode } from '@app-builder/models';
 import { Button, type ButtonProps } from '@ui-design-system';
 import { Plus } from '@ui-icons';
 import clsx from 'clsx';
@@ -31,8 +31,8 @@ const AddLogicalOperatorButton = React.forwardRef<
 AddLogicalOperatorButton.displayName = 'AddLogicalOperatorButton';
 
 function NewBinaryAstNode() {
-  return NewUnknownAstNode({
-    children: [NewUnknownAstNode(), NewUnknownAstNode()],
+  return NewUndefinedAstNode({
+    children: [NewUndefinedAstNode(), NewUndefinedAstNode()],
   });
 }
 

diff --git a/packages/app-builder/src/models/ast-node.ts b/packages/app-builder/src/models/ast-node.ts
@@ -8,7 +8,7 @@ export interface AstNode {
   namedChildren: Record<string, AstNode>;
 }
 
-const unknownAstNodeName = 'Unknown';
+const undefinedAstNodeName = 'Undefined';
 
 export type ConstantType =
   | number
@@ -33,13 +33,13 @@ export function NewAstNode({
   };
 }
 
-export function NewUnknownAstNode({
+export function NewUndefinedAstNode({
   constant,
   children,
   namedChildren,
 }: Partial<Omit<AstNode, 'name'>> = {}): AstNode {
   return NewAstNode({
-    name: unknownAstNodeName,
+    name: undefinedAstNodeName,
     constant,
     children,
     namedChildren,
@@ -65,7 +65,7 @@ export function adaptAstNode(astNode: AstNode): NodeDto {
 }
 
 export function isAstNodeUnknown(node: AstNode): boolean {
-  return node.name === unknownAstNodeName;
+  return node.name === undefinedAstNodeName;
 }
 
 export interface ConstantAstNode<T extends ConstantType = ConstantType> {

diff --git a/packages/app-builder/src/models/scenario-validation.ts b/packages/app-builder/src/models/scenario-validation.ts
@@ -9,14 +9,29 @@ import type { ConstantType } from './ast-node';
 
 export type EvaluationErrorCode =
   | 'UNEXPECTED_ERROR'
-  | 'UNKNOWN_FUNCTION'
-  | 'WRONG_NUMBER_OF_ARGUMENTS';
+  | 'UNDEFINED_FUNCTION'
+  | 'WRONG_NUMBER_OF_ARGUMENTS'
+  | 'MISSING_NAMED_ARGUMENT'
+  | 'ARGUMENTS_MUST_BE_INT_OR_FLOAT'
+  | 'ARGUMENT_MUST_BE_INTEGER'
+  | 'ARGUMENT_MUST_BE_STRING'
+  | 'ARGUMENT_MUST_BE_BOOLEAN'
+  | 'ARGUMENT_MUST_BE_LIST'
+  | 'ARGUMENT_MUST_BE_CONVERTIBLE_TO_DURATION'
+  | 'ARGUMENT_MUST_BE_TIME';
 
 export interface EvaluationError {
   error: EvaluationErrorCode;
   message: string;
 }
 
+export function isUndefinedFunctionError(evaluationError: {
+  error: string;
+  message: string;
+}): evaluationError is { error: 'UNDEFINED_FUNCTION'; message: string } {
+  return evaluationError.error === 'UNDEFINED_FUNCTION';
+}
+
 interface CommonNodeEvaluation {
   returnValue?: ConstantType;
   children: NodeEvaluation[];

diff --git a/...-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/edit.rules.$ruleId.tsx b/...-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/edit.rules.$ruleId.tsx
@@ -160,12 +160,11 @@ export default function RuleEdit() {
     );
     allEvaluationErrors.forEach((flattenNodeEvaluationErrors) => {
       if (flattenNodeEvaluationErrors.state === 'invalid') {
+        const firstError = flattenNodeEvaluationErrors.errors[0];
         //@ts-expect-error path is a string
         setError(flattenNodeEvaluationErrors.path, {
-          type: 'custom',
-          message: getNodeEvaluationErrorMessage(
-            flattenNodeEvaluationErrors.errors[0]
-          ),
+          type: firstError.error,
+          message: getNodeEvaluationErrorMessage(firstError),
         });
       }
     });

diff --git a/packages/app-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/view.tsx b/packages/app-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/view.tsx
@@ -39,9 +39,12 @@ const LINKS: ScenariosLinkProps[] = [
 
 export async function loader({ request, params }: LoaderArgs) {
   const { authService } = serverServices;
-  const { editor, scenario } = await authService.isAuthenticated(request, {
-    failureRedirect: '/login',
-  });
+  const { editor, scenario, user } = await authService.isAuthenticated(
+    request,
+    {
+      failureRedirect: '/login',
+    }
+  );
 
   const scenarioId = fromParams(params, 'scenarioId');
   const iterationId = fromParams(params, 'iterationId');

diff --git a/packages/app-builder/src/services/validation/scenario-validation.ts b/packages/app-builder/src/services/validation/scenario-validation.ts
@@ -1,9 +1,11 @@
 import {
   type EvaluationError,
+  isUndefinedFunctionError,
   type NodeEvaluation,
   type ScenarioValidation,
 } from '@app-builder/models';
 import { useCallback } from 'react';
+import { type FieldError } from 'react-hook-form';
 import { useTranslation } from 'react-i18next';
 import invariant from 'tiny-invariant';
 
@@ -56,12 +58,79 @@ export function useGetNodeEvaluationErrorMessage() {
   return useCallback(
     (evaluationError: EvaluationError) => {
       switch (evaluationError.error) {
-        case 'UNKNOWN_FUNCTION':
+        case 'UNDEFINED_FUNCTION':
           return t('scenarios:validation.evaluation_error.unknown_function');
+        case 'WRONG_NUMBER_OF_ARGUMENTS':
+          return t(
+            'scenarios:validation.evaluation_error.wrong_number_of_arguments'
+          );
+        case 'MISSING_NAMED_ARGUMENT':
+          return t(
+            'scenarios:validation.evaluation_error.missing_named_argument'
+          );
+        case 'ARGUMENTS_MUST_BE_INT_OR_FLOAT':
+          return t(
+            'scenarios:validation.evaluation_error.arguments_must_be_int_or_float'
+          );
+        case 'ARGUMENT_MUST_BE_INTEGER':
+          return t(
+            'scenarios:validation.evaluation_error.argument_must_be_integer'
+          );
+        case 'ARGUMENT_MUST_BE_STRING':
+          return t(
+            'scenarios:validation.evaluation_error.argument_must_be_string'
+          );
+        case 'ARGUMENT_MUST_BE_BOOLEAN':
+          return t(
+            'scenarios:validation.evaluation_error.argument_must_be_boolean'
+          );
+        case 'ARGUMENT_MUST_BE_LIST':
+          return t(
+            'scenarios:validation.evaluation_error.argument_must_be_list'
+          );
+        case 'ARGUMENT_MUST_BE_CONVERTIBLE_TO_DURATION':
+          return t(
+            'scenarios:validation.evaluation_error.argument_must_be_convertible_to_duration'
+          );
+        case 'ARGUMENT_MUST_BE_TIME':
+          return t(
+            'scenarios:validation.evaluation_error.argument_must_be_time'
+          );
+
         default:
-          return evaluationError.message;
+          return `${evaluationError.error}:${evaluationError.message}`;
       }
     },
     [t]
   );
 }
+
+interface InvalidStates {
+  root: boolean; // propagate invalid state to all subcomponents
+  children: Record<string, boolean>; // propagate invalid state to a specific children
+  name: boolean; // propagate invalid state to the name field
+}
+
+//TODO(builder): remove this function when we will have our own state management
+export function getInvalidStates(error?: FieldError): InvalidStates {
+  if (!error) return { root: false, children: {}, name: false };
+
+  // Rebuild evaluation error from react-hook-form error
+  // TODO(builder): we should directly handle EvaluationError here in the future
+  const evaluationError = {
+    error: error.type,
+    message: error.message ?? '',
+  };
+  if (isUndefinedFunctionError(evaluationError)) {
+    return {
+      root: false,
+      children: {},
+      name: true,
+    };
+  }
+  return {
+    root: true,
+    children: {},
+    name: true,
+  };
+}
diff --git a/packages/marble-api/scripts/openapi.yaml b/packages/marble-api/scripts/openapi.yaml
@@ -2110,8 +2110,16 @@ components:
       type: string
       enum:
         - UNEXPECTED_ERROR
-        - UNKNOWN_FUNCTION
+        - UNDEFINED_FUNCTION
         - WRONG_NUMBER_OF_ARGUMENTS
+        - MISSING_NAMED_ARGUMENT
+        - ARGUMENTS_MUST_BE_INT_OR_FLOAT
+        - ARGUMENT_MUST_BE_INTEGER
+        - ARGUMENT_MUST_BE_STRING
+        - ARGUMENT_MUST_BE_BOOLEAN
+        - ARGUMENT_MUST_BE_LIST
+        - ARGUMENT_MUST_BE_CONVERTIBLE_TO_DURATION
+        - ARGUMENT_MUST_BE_TIME
     Identifier:
       type: object
       required:

diff --git a/packages/marble-api/src/generated/marble-api.ts b/packages/marble-api/src/generated/marble-api.ts
@@ -167,7 +167,7 @@ export type UpdateScenarioIterationBody = {
         scoreRejectThreshold?: number;
     };
 };
-export type EvaluationErrorCodeDto = "UNEXPECTED_ERROR" | "UNKNOWN_FUNCTION" | "WRONG_NUMBER_OF_ARGUMENTS";
+export type EvaluationErrorCodeDto = "UNEXPECTED_ERROR" | "UNDEFINED_FUNCTION" | "WRONG_NUMBER_OF_ARGUMENTS" | "MISSING_NAMED_ARGUMENT" | "ARGUMENTS_MUST_BE_INT_OR_FLOAT" | "ARGUMENT_MUST_BE_INTEGER" | "ARGUMENT_MUST_BE_STRING" | "ARGUMENT_MUST_BE_BOOLEAN" | "ARGUMENT_MUST_BE_LIST" | "ARGUMENT_MUST_BE_CONVERTIBLE_TO_DURATION" | "ARGUMENT_MUST_BE_TIME";
 export type EvaluationErrorDto = {
     error: EvaluationErrorCodeDto;
     message: string;