feat(validation): better evaluation errors (#226)

* feat: better evaluation errors (#211) * feat: disable activate button if errors (#214) * feat(ScenarioValidationError): aggregate errors (#213) * feat(ScenarioValidation): remove error code from error message * refactor(EvaluationError): use code * feat(ScenarioValidationError): aggregate errors * refactor(evaluation): refactor evaluation errors (#216) * refactor(evaluation): refactor evaluation errors * misc fixes --------- Co-authored-by: Zoé Cadé <[email protected]> --------- Co-authored-by: Zoé Cadé <[email protected]>
checkmarble · Oct 24, 2023 · a242536 · a242536
1 parent 23f526a
commit a242536
Show file tree

Hide file tree

Showing 19 changed files with 379 additions and 82 deletions.
diff --git a/packages/app-builder/public/locales/en/common.json b/packages/app-builder/public/locales/en/common.json
@@ -12,7 +12,7 @@
   "errors.data.duplicate_field_name": "A field with this name already exist",
   "errors.data.duplicate_table_name": "A table with this name already exist",
   "errors.data.duplicate_link_name": "A link with this name already exist",
-  "errors.draft.invalid": "Invalid draft. Please check your draft for error messages, fix them and try again.",
+  "errors.draft.invalid": "This draft can't be published because it contains errors.",
   "cancel": "Cancel",
   "save": "Save",
   "delete": "Delete",

diff --git a/packages/app-builder/public/locales/en/scenarios.json b/packages/app-builder/public/locales/en/scenarios.json
@@ -99,18 +99,37 @@
   "validation.decision.score_review_threshold_required": "Required",
   "validation.decision.score_reject_threshold_required": "Required",
   "validation.decision.score_reject_review_thresholds_missmatch": "Reject threshold must be greater than review threshold",
-  "validation.evaluation_error.unknown_function": "required",
+  "validation.evaluation_error.undefined_function_one": "required",
+  "validation.evaluation_error.undefined_function_other": "{{count}} 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",
-  "validation.evaluation_error.argument_required": "argument is required",
-  "validation.evaluation_error.function_error": "function is incorrect",
+  "validation.evaluation_error.missing_named_argument_one": "missing named argument",
+  "validation.evaluation_error.missing_named_argument_other": "{{count}} missing named arguments",
+  "validation.evaluation_error.arguments_must_be_int_or_float_one": "argument must be an integer or a float",
+  "validation.evaluation_error.arguments_must_be_int_or_float_other": "{{count}} arguments must be integers or floats",
+  "validation.evaluation_error.arguments_must_be_int_float_or_time_one": "argument must be an integer, a float or a time",
+  "validation.evaluation_error.arguments_must_be_int_float_or_time_other": "{{count}} arguments must be integers, floats or times",
+  "validation.evaluation_error.argument_must_be_integer_one": "argument must be an integer",
+  "validation.evaluation_error.argument_must_be_integer_other": "{{count}} arguments must be integers",
+  "validation.evaluation_error.argument_must_be_string_one": "argument must be a string",
+  "validation.evaluation_error.argument_must_be_string_other": "{{count}} arguments must be strings",
+  "validation.evaluation_error.argument_must_be_boolean_one": "argument must be a boolean",
+  "validation.evaluation_error.argument_must_be_boolean_other": "{{count}} arguments must be booleans",
+  "validation.evaluation_error.argument_must_be_list_one": "argument must be a list",
+  "validation.evaluation_error.argument_must_be_list_other": "{{count}} arguments must be lists",
+  "validation.evaluation_error.argument_must_be_convertible_to_duration_one": "argument must be a duration",
+  "validation.evaluation_error.argument_must_be_convertible_to_duration_other": "{{count}} arguments must be durations",
+  "validation.evaluation_error.argument_must_be_time_one": "argument must be a time",
+  "validation.evaluation_error.argument_must_be_time_other": "{{count}} arguments must be times",
+  "validation.evaluation_error.argument_required_one": "argument is required",
+  "validation.evaluation_error.argument_required_other": "{{count}} arguments are required",
+  "validation.evaluation_error.argument_invalid_type_one": "argument has an invalid type",
+  "validation.evaluation_error.argument_invalid_type_other": "{{count}} arguments have invalid types",
+  "validation.evaluation_error.list_not_found_one": "list not found",
+  "validation.evaluation_error.list_not_found_other": "{{count}} lists not found",
+  "validation.evaluation_error.field_not_found_one": "field not found",
+  "validation.evaluation_error.field_not_found_other": "{{count}} fields not found",
+  "validation.evaluation_error.function_error_one": "function contains errors",
+  "validation.evaluation_error.function_error_other": "{{count}} functions contain error",
   "edit_aggregation.title": "Create a variable",
   "edit_aggregation.subtitle": "From Marble database",
   "edit_aggregation.label_title": "Variable name",

diff --git a/...builder/src/components/Scenario/AstBuilder/AstBuilderNode/AggregationEdit/EditFilters.tsx b/...builder/src/components/Scenario/AstBuilder/AstBuilderNode/AggregationEdit/EditFilters.tsx
@@ -1,9 +1,14 @@
 import { scenarioI18n } from '@app-builder/components/Scenario';
+import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidationError';
 import { NewUndefinedAstNode } from '@app-builder/models';
 import {
   adaptEditorNodeViewModel,
   type AstBuilder,
 } from '@app-builder/services/editor/ast-editor';
+import {
+  adaptEvaluationErrorViewModels,
+  useGetNodeEvaluationErrorMessage,
+} from '@app-builder/services/validation';
 import { Button } from '@ui-design-system';
 import { Plus } from '@ui-icons';
 import { useTranslation } from 'react-i18next';
@@ -36,6 +41,7 @@ export const EditFilters = ({
   value: FilterViewModel[];
 }) => {
   const { t } = useTranslation(scenarioI18n);
+  const getNodeEvaluationErrorMessage = useGetNodeEvaluationErrorMessage();
 
   const filteredDataModalFieldOptions = aggregatedField?.tableName
     ? dataModelFieldOptions.filter(
@@ -80,6 +86,9 @@ export const EditFilters = ({
     <div>
       <div className="flex flex-col gap-2">
         {value.map((filter, filterIndex) => {
+          const valueErrorMessages = adaptEvaluationErrorViewModels(
+            filter.value.errors
+          ).map((error) => getNodeEvaluationErrorMessage(error));
           return (
             <div key={filterIndex}>
               <div className="flex flex-row items-center gap-1">
@@ -120,6 +129,13 @@ export const EditFilters = ({
               {filter.errors.filter.length > 0 && (
                 <ErrorMessage errors={filter.errors.filter} />
               )}
+              <div className="mt-2 flex flex-row flex-wrap gap-2">
+                {valueErrorMessages.map((error) => (
+                  <ScenarioValidationError key={error}>
+                    {error}
+                  </ScenarioValidationError>
+                ))}
+              </div>
             </div>
           );
         })}

diff --git a/packages/app-builder/src/components/Scenario/AstBuilder/AstBuilderNode/Operand/Operand.tsx b/packages/app-builder/src/components/Scenario/AstBuilder/AstBuilderNode/Operand/Operand.tsx
@@ -26,9 +26,7 @@ export const computeOperandErrors = (
   viewModel: EditorNodeViewModel
 ): EvaluationError[] => {
   if (viewModel.funcName && functionNodeNames.includes(viewModel.funcName)) {
-    return hasNestedErrors(viewModel)
-      ? [{ error: 'FUNCTION_ERROR', message: 'function has error' }]
-      : [];
+    return viewModel.errors.filter((error) => error.argumentName === undefined);
   } else {
     return [
       ...viewModel.errors,
@@ -38,13 +36,6 @@ export const computeOperandErrors = (
   }
 };
 
-function hasNestedErrors(viewModel: EditorNodeViewModel): boolean {
-  if (viewModel.errors.length > 0) return true;
-  if (viewModel.children.some(hasNestedErrors)) return true;
-  if (Object.values(viewModel.namedChildren).some(hasNestedErrors)) return true;
-  return false;
-}
-
 export function isEditableOperand(node: AstNode): boolean {
   return (
     isConstant(node) ||

diff --git a/...der/src/components/Scenario/AstBuilder/AstBuilderNode/TwoOperandsLine/TwoOperandsLine.tsx b/...der/src/components/Scenario/AstBuilder/AstBuilderNode/TwoOperandsLine/TwoOperandsLine.tsx
@@ -1,11 +1,14 @@
-import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidatioError';
-import { type EvaluationError } from '@app-builder/models';
+import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidationError';
 import {
   type AstBuilder,
   type EditorNodeViewModel,
   findArgumentIndexErrorsFromParent,
 } from '@app-builder/services/editor/ast-editor';
-import { useGetNodeEvaluationErrorMessage } from '@app-builder/services/validation';
+import {
+  adaptEvaluationErrorViewModels,
+  type EvaluationErrorViewModel,
+  useGetNodeEvaluationErrorMessage,
+} from '@app-builder/services/validation';
 
 import {
   computeOperandErrors,
@@ -22,7 +25,7 @@ interface TwoOperandsLineViewModel {
   left: OperandViewModel;
   operator: OperatorViewModel;
   right: OperandViewModel;
-  errors: EvaluationError[];
+  errors: EvaluationErrorViewModel[];
 }
 
 export function TwoOperandsLine({
@@ -36,6 +39,10 @@ export function TwoOperandsLine({
 }) {
   const getNodeEvaluationErrorMessage = useGetNodeEvaluationErrorMessage();
 
+  const errorMessages = twoOperandsViewModel.errors.map((error) =>
+    getNodeEvaluationErrorMessage(error)
+  );
+
   return (
     <div className="flex flex-col gap-2">
       <div className="flex flex-row gap-2">
@@ -67,11 +74,8 @@ export function TwoOperandsLine({
         />
       </div>
       <div className="flex flex-row flex-wrap gap-2">
-        {twoOperandsViewModel.errors.map((error, index) => (
-          // TODO: find a better way to compute error key (flatten errors make it hard)
-          <ScenarioValidationError key={index}>
-            {getNodeEvaluationErrorMessage(error)}
-          </ScenarioValidationError>
+        {errorMessages.map((error) => (
+          <ScenarioValidationError key={error}>{error}</ScenarioValidationError>
         ))}
       </div>
     </div>
@@ -93,11 +97,11 @@ export function adaptTwoOperandsLineViewModel(
     left,
     operator: operatorVm,
     right,
-    errors: [
+    errors: adaptEvaluationErrorViewModels([
       ...computeOperandErrors(left),
       ...vm.errors,
       ...computeOperandErrors(right),
       ...findArgumentIndexErrorsFromParent(vm),
-    ],
+    ]),
   };
 }
diff --git a/packages/app-builder/src/components/Scenario/AstBuilder/ErrorMessage.tsx b/packages/app-builder/src/components/Scenario/AstBuilder/ErrorMessage.tsx
@@ -1,18 +1,28 @@
 import { type EvaluationError } from '@app-builder/models';
-import { useGetNodeEvaluationErrorMessage } from '@app-builder/services/validation';
+import {
+  adaptEvaluationErrorViewModels,
+  useGetNodeEvaluationErrorMessage,
+} from '@app-builder/services/validation';
 
 export interface ErrorMessageProps {
   errors?: EvaluationError[];
 }
 
+/**
+ * @deprecated Use ScenarioValidationError instead
+ */
 export function ErrorMessage({ errors }: ErrorMessageProps) {
   const getNodeEvaluationErrorMessage = useGetNodeEvaluationErrorMessage();
 
   const firstError = errors?.[0];
 
   return (
     <p className="text-s font-medium text-red-100 transition-opacity duration-200 ease-in-out">
-      {firstError && getNodeEvaluationErrorMessage(firstError)}
+      {firstError &&
+        getNodeEvaluationErrorMessage(
+          // glitch for ISO compatibility with former code
+          adaptEvaluationErrorViewModels([firstError])[0]
+        )}
     </p>
   );
 }
diff --git a/packages/app-builder/src/components/Scenario/AstBuilder/RootAstBuilderNode/RootAnd.tsx b/packages/app-builder/src/components/Scenario/AstBuilder/RootAstBuilderNode/RootAnd.tsx
@@ -9,11 +9,14 @@ import {
   type EditorNodeViewModel,
   hasArgumentIndexErrorsFromParent,
 } from '@app-builder/services/editor/ast-editor';
-import { useGetOrAndNodeEvaluationErrorMessage } from '@app-builder/services/validation';
+import {
+  adaptEvaluationErrorViewModels,
+  useGetOrAndNodeEvaluationErrorMessage,
+} from '@app-builder/services/validation';
 import clsx from 'clsx';
 import { Fragment } from 'react';
 
-import { ScenarioValidationError } from '../../ScenarioValidatioError';
+import { ScenarioValidationError } from '../../ScenarioValidationError';
 import { AstBuilderNode } from '../AstBuilderNode/AstBuilderNode';
 import { RemoveButton } from '../RemoveButton';
 import { AddLogicalOperatorButton } from './AddLogicalOperatorButton';
@@ -60,6 +63,10 @@ export function RootAnd({
     rootAndViewModel.errors
   );
 
+  const andErrorMessages = adaptEvaluationErrorViewModels(
+    andNonChildrenErrors
+  ).map(getEvaluationErrorMessage);
+
   function appendAndChild() {
     builder.appendChild(rootAndViewModel.nodeId, NewAndChild());
   }
@@ -143,10 +150,8 @@ export function RootAnd({
         {!viewOnly && (
           <AddLogicalOperatorButton onClick={appendAndChild} operator="and" />
         )}
-        {andNonChildrenErrors.map((error, index) => (
-          <ScenarioValidationError key={index}>
-            {getEvaluationErrorMessage(error)}
-          </ScenarioValidationError>
+        {andErrorMessages.map((error) => (
+          <ScenarioValidationError key={error}>{error}</ScenarioValidationError>
         ))}
       </div>
     </>

diff --git a/packages/app-builder/src/components/Scenario/AstBuilder/RootAstBuilderNode/RootOrWithAnd.tsx b/packages/app-builder/src/components/Scenario/AstBuilder/RootAstBuilderNode/RootOrWithAnd.tsx
@@ -10,11 +10,14 @@ import {
   type EditorNodeViewModel,
   hasArgumentIndexErrorsFromParent,
 } from '@app-builder/services/editor/ast-editor';
-import { useGetOrAndNodeEvaluationErrorMessage } from '@app-builder/services/validation';
+import {
+  adaptEvaluationErrorViewModels,
+  useGetOrAndNodeEvaluationErrorMessage,
+} from '@app-builder/services/validation';
 import clsx from 'clsx';
 import React from 'react';
 
-import { ScenarioValidationError } from '../../ScenarioValidatioError';
+import { ScenarioValidationError } from '../../ScenarioValidationError';
 import { AstBuilderNode } from '../AstBuilderNode/AstBuilderNode';
 import { RemoveButton } from '../RemoveButton';
 import { AddLogicalOperatorButton } from './AddLogicalOperatorButton';
@@ -82,6 +85,10 @@ export function RootOrWithAnd({
     rootOrWithAndViewModel.orErrors
   );
 
+  const rootOrErrorMessages = adaptEvaluationErrorViewModels(
+    rootOrNonChildrenErrors
+  ).map(getEvaluationErrorMessage);
+
   return (
     <div className="flex flex-col gap-4">
       {rootOrWithAndViewModel.ands.map((andChild, childIndex) => {
@@ -90,6 +97,10 @@ export function RootOrWithAnd({
           andChild.errors
         );
 
+        const andErrorMessages = adaptEvaluationErrorViewModels(
+          andNonChildrenErrors
+        ).map(getEvaluationErrorMessage);
+
         function appendAndChild() {
           builder.appendChild(andChild.nodeId, NewAndChild());
         }
@@ -157,9 +168,9 @@ export function RootOrWithAnd({
                 </div>
               )}
 
-              {andNonChildrenErrors.map((error, index) => (
-                <ScenarioValidationError key={index}>
-                  {getEvaluationErrorMessage(error)}
+              {andErrorMessages.map((error) => (
+                <ScenarioValidationError key={error}>
+                  {error}
                 </ScenarioValidationError>
               ))}
             </div>
@@ -172,10 +183,8 @@ export function RootOrWithAnd({
           <AddLogicalOperatorButton onClick={appendOrChild} operator="or" />
         )}
 
-        {rootOrNonChildrenErrors.map((error, index) => (
-          <ScenarioValidationError key={index}>
-            {getEvaluationErrorMessage(error)}
-          </ScenarioValidationError>
+        {rootOrErrorMessages.map((error) => (
+          <ScenarioValidationError key={error}>{error}</ScenarioValidationError>
         ))}
       </div>
     </div>

diff --git a/...nents/Scenario/ScenarioValidatioError.tsx → ...ents/Scenario/ScenarioValidationError.tsx b/...nents/Scenario/ScenarioValidatioError.tsx → ...ents/Scenario/ScenarioValidationError.tsx
diff --git a/...ges/app-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view.tsx b/...ges/app-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view.tsx
@@ -9,7 +9,7 @@ import { VersionSelect } from '@app-builder/components/Scenario/Iteration/Versio
 import { sortScenarioIterations } from '@app-builder/models/scenario-iteration';
 import { useCurrentScenario } from '@app-builder/routes/__builder/scenarios/$scenarioId';
 import { CreateDraftIteration } from '@app-builder/routes/ressources/scenarios/$scenarioId/$iterationId/create_draft';
-import { DeploymentModal } from '@app-builder/routes/ressources/scenarios/deployment';
+import { DeploymentActions } from '@app-builder/routes/ressources/scenarios/deployment';
 import { useEditorMode } from '@app-builder/services/editor';
 import { serverServices } from '@app-builder/services/init.server';
 import {
@@ -76,7 +76,7 @@ export default function ScenarioEditLayout() {
   const withEditTag = editorMode === 'edit';
   const withCreateDraftIteration =
     canManageScenario && currentIteration.type !== 'draft';
-  const withDeploymentModal = canPublishScenario;
+  const withDeploymentActions = canPublishScenario;
 
   return (
     <ScenarioPage.Container>
@@ -104,11 +104,16 @@ export default function ScenarioEditLayout() {
               draftId={draftIteration?.id}
             />
           )}
-          {withDeploymentModal && (
-            <DeploymentModal
+          {withDeploymentActions && (
+            <DeploymentActions
               scenarioId={currentScenario.id}
               liveVersionId={currentScenario.liveVersionId}
               currentIteration={currentIteration}
+              hasScenarioErrors={
+                hasTriggerErrors(scenarioValidation) ||
+                hasRulesErrors(scenarioValidation) ||
+                hasDecisionErrors(scenarioValidation)
+              }
             />
           )}
         </div>

diff --git a/...uilder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view/decision.tsx b/...uilder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view/decision.tsx
@@ -13,7 +13,7 @@ import {
   FormLabel,
 } from '@app-builder/components/Form';
 import { setToastMessage } from '@app-builder/components/MarbleToaster';
-import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidatioError';
+import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidationError';
 import {
   useCurrentScenarioIteration,
   useEditorMode,

diff --git a/...p-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view/rules.tsx b/...p-builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view/rules.tsx
@@ -1,5 +1,5 @@
 import { Ping } from '@app-builder/components/Ping';
-import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidatioError';
+import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidationError';
 import { CreateRule } from '@app-builder/routes/ressources/scenarios/$scenarioId/$iterationId/rules/create';
 import { useEditorMode } from '@app-builder/services/editor';
 import { serverServices } from '@app-builder/services/init.server';

diff --git a/...builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view/trigger.tsx b/...builder/src/routes/__builder/scenarios/$scenarioId/i/$iterationId/__edit-view/trigger.tsx
@@ -6,7 +6,7 @@ import {
 } from '@app-builder/components';
 import { setToastMessage } from '@app-builder/components/MarbleToaster';
 import { AstBuilder } from '@app-builder/components/Scenario/AstBuilder';
-import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidatioError';
+import { ScenarioValidationError } from '@app-builder/components/Scenario/ScenarioValidationError';
 import { ScheduleOption } from '@app-builder/components/Scenario/Trigger';
 import {
   adaptDataModelDto,