front, editoast: invalid effort curve if speed has duplicates or if effort/speed have less than 2 coordinates.

front: prevent user to send duplicates coordinates from effortCurve spreadsheet

editoast: ensure speed values are unique and strictly increasing

front: speed and effort values must be strictly positive, and speed values must be unique

editoast: use json as input in all tests of rollin_stock

editoast: refacto. + throw an error if curves has less than 2 values

front: refacto. test and correct invalidity curve condition

front: filter curve before check validity

front: display invalid curve

front: improve curve identifier

front: fix translate

front: improve error message style

Author: kmer2016

editoast/src/schema/rolling_stock/mod.rs

@@ -180,6 +180,30 @@ impl<'de> Deserialize<'de> for EffortCurve {
             ));
         }
 
+        if inner.max_efforts.len() < 2 {
+            return Err(serde::de::Error::custom(
+                "effort curve should have at least 2 points.",
+            ));
+        }
+
+        if inner.max_efforts.iter().any(|&x| x < 0.0) {
+            return Err(serde::de::Error::custom(
+                "max_efforts values must be equal or greater than 0.",
+            ));
+        };
+
+        if inner.speeds.iter().any(|&x| x < 0.0) {
+            return Err(serde::de::Error::custom(
+                "speeds values must be equal or greater than 0.",
+            ));
+        };
+
+        if inner.speeds.windows(2).any(|window| window[0] >= window[1]) {
+            return Err(serde::de::Error::custom(
+                "speeds values must be strictly increasing.",
+            ));
+        }
+
         Ok(EffortCurve {
             speeds: inner.speeds,
             max_efforts: inner.max_efforts,
@@ -279,20 +303,41 @@ pub enum SignalingSystem {
 #[cfg(test)]
 mod tests {
     use crate::schema::rolling_stock::EffortCurve;
+    use serde_json::{from_value, json};
 
     #[test]
     fn test_de_effort_curve_valid() {
-        assert!(serde_json::from_str::<EffortCurve>(
-            r#"{ "speeds": [0, 1], "max_efforts": [0, 2] }"#
-        )
-        .is_ok());
+        let curve = json!({ "speeds": [0, 1], "max_efforts": [0, 2] });
+        assert!(from_value::<EffortCurve>(curve).is_ok());
+    }
+
+    #[test]
+    fn test_effort_curve_invalid_due_to_single_point() {
+        let curve = json!({ "speeds": [0], "max_efforts": [0] });
+        assert!(from_value::<EffortCurve>(curve).is_err());
+    }
+
+    #[test]
+    fn test_de_effort_curve_invalid_due_to_mismatched_lengths() {
+        let curve = json!({ "speeds": [0, 1], "max_efforts": [] });
+        assert!(from_value::<EffortCurve>(curve).is_err());
+    }
+
+    #[test]
+    fn test_de_effort_curve_invalid_due_to_negative_max_efforts() {
+        let curve = json!({ "speeds": [0, 1, 2], "max_efforts": [5, 4, -3] });
+        assert!(from_value::<EffortCurve>(curve).is_err());
+    }
+
+    #[test]
+    fn test_de_effort_curve_invalid_due_to_negative_speeds() {
+        let curve = json!({ "speeds": [-1, 0, 1], "max_efforts": [5, 4, 3] });
+        assert!(from_value::<EffortCurve>(curve).is_err());
     }
 
     #[test]
-    fn test_de_effort_curve_unvalid() {
-        assert!(
-            serde_json::from_str::<EffortCurve>(r#"{ "speeds": [0, 1], "max_efforts": [] }"#)
-                .is_err()
-        );
+    fn test_de_effort_curve_invalid_due_to_unordered_speeds() {
+        let curve = json!({ "speeds": [0, 2, 1], "max_efforts": [5, 4, 3] });
+        assert!(from_value::<EffortCurve>(curve).is_err());
     }
 }

front/public/locales/en/rollingstock.json

@@ -69,7 +69,8 @@
     "rollingStockDuplicateName": "Rolling stock of the same name already exists.",
     "rollingStockNotAdded": "Rolling stock not added",
     "rollingStockNotUpdated": "Rolling stock not updated",
-    "rollingStockNotDeleted": "Rolling stock not deleted"
+    "rollingStockNotDeleted": "Rolling stock not deleted",
+    "invalidEffortCurves": "Invalid curves: {{invalidEffortCurves}}. Each curve must have at least 2 combinations of speed / effort values (with no duplication of speed values). Combinations with at least one empty field are ignored."
   },
   "name": "Name",
   "no": "No",

front/public/locales/fr/rollingstock.json

@@ -71,7 +71,8 @@
     "rollingStockDuplicateName": "Un matériel roulant du même nom existe déjà.",
     "rollingStockNotAdded": "Le matériel roulant n'a pas été ajouté.",
     "rollingStockNotUpdated": "Le matériel roulant n'a pas été mis à jour.",
-    "rollingStockNotDeleted": "Le matériel roulant n'a pas été supprimé."
+    "rollingStockNotDeleted": "Le matériel roulant n'a pas été supprimé.",
+    "invalidEffortCurves": "Courbes invalides : {{invalidEffortCurves}}. Chaque courbe doit posséder au moins 2 combinaisons de valeurs vitesse / effort (sans doublon sur les valeurs de vitesse). Les combinaisons ayant au moins un champs vide sont ignorées."
   },
   "name": "Nom",
   "no": "Non",

front/src/modules/rollingStock/components/RollingStockEditor/RollingStockEditorForm.tsx

@@ -184,10 +184,8 @@ const RollingStockEditorForm = ({
       return;
     }
 
-    const { invalidFields, validRollingStockForm } = checkRollingStockFormValidity(
-      data,
-      effortCurves
-    );
+    const { invalidFields, validRollingStockForm, invalidEffortCurves } =
+      checkRollingStockFormValidity(data, effortCurves, t);
     if (invalidFields.length) {
       setRollingStockValues(validRollingStockForm);
       setErrorMessage(
@@ -196,18 +194,27 @@ const RollingStockEditorForm = ({
           count: invalidFields.length,
         })
       );
-    } else {
-      setErrorMessage('');
-      const payload = rollingStockEditorQueryArg(validRollingStockForm, effortCurves!);
-      openModal(
-        <RollingStockEditorFormModal
-          setAddOrEditState={setAddOrEditState}
-          request={isAdding ? addNewRollingstock(payload) : updateRollingStock(payload)}
-          mainText={t('confirmUpdateRollingStock')}
-          buttonText={t('translation:common.yes')}
-        />
+
+      return;
+    }
+
+    if (invalidEffortCurves.length > 0) {
+      setErrorMessage(
+        t('messages.invalidEffortCurves', { invalidEffortCurves: invalidEffortCurves.join(', ') })
       );
+      return;
     }
+
+    setErrorMessage('');
+    const payload = rollingStockEditorQueryArg(validRollingStockForm, effortCurves!);
+    openModal(
+      <RollingStockEditorFormModal
+        setAddOrEditState={setAddOrEditState}
+        request={isAdding ? addNewRollingstock(payload) : updateRollingStock(payload)}
+        mainText={t('confirmUpdateRollingStock')}
+        buttonText={t('translation:common.yes')}
+      />
+    );
   };
 
   const cancel = () => {
@@ -297,9 +304,7 @@ const RollingStockEditorForm = ({
       <Tabs pills fullWidth tabs={[tabRollingStockDetails, tabRollingStockCurves]} />
       <div className="d-flex justify-content-end">
         <div className="d-flex flex-column justify-content-end">
-          {errorMessage && (
-            <p className="text-danger mb-1 ml-auto error-message text-wrap">{errorMessage}</p>
-          )}
+          {errorMessage && <p className="text-danger mb-1 p-3">{errorMessage}</p>}
           <div className="d-flex justify-content-end">
             <button
               type="button"

front/src/modules/rollingStock/helpers/__tests__/utils.spec.ts

@@ -2,6 +2,15 @@ import { checkRollingStockFormValidity, makeEffortCurve } from 'modules/rollingS
 
 import type { EffortCurves } from 'common/api/osrdEditoastApi';
 import type { EffortCurveForms, RollingStockParametersValues } from 'modules/rollingStock/types';
+import { TFunction } from 'i18next';
+
+function setupEffortCurve(tractionMode: string, max_efforts: number[], speeds: number[]) {
+  const curves = makeEffortCurve(tractionMode);
+  curves.curves[0].curve.max_efforts = max_efforts;
+  curves.curves[0].curve.speeds = speeds;
+  return curves;
+}
+const tMock: TFunction = (key: string) => key;
 
 describe('checkRollingStockFormValidity', () => {
   describe('Non electric stock', () => {
@@ -39,8 +48,9 @@ describe('checkRollingStockFormValidity', () => {
           rollingResistanceB: 0,
           rollingResistanceC: 0,
         },
+        invalidEffortCurves: [],
       };
-      const result = checkRollingStockFormValidity(rsForm, effortCurves);
+      const result = checkRollingStockFormValidity(rsForm, effortCurves, tMock);
       expect(result).toEqual(expected);
     });
   });
@@ -89,8 +99,90 @@ describe('checkRollingStockFormValidity', () => {
           raisePantographTime: 15,
           basePowerClass: null,
         },
+        invalidEffortCurves: ['comfortTypes.STANDARD > 15000 > unspecified > unspecified'],
+      };
+      const result = checkRollingStockFormValidity(rsForm, effortCurves, tMock);
+      expect(result).toEqual(expected);
+    });
+  });
+  describe('Invalid curve', () => {
+    const rsForm = {
+      name: 'auietsrn',
+      mass: 155,
+      maxSpeed: 10000,
+      loadingGauge: 'G1',
+      basePowerClass: null,
+    } as RollingStockParametersValues;
+    const expected = {
+      invalidFields: [
+        'length',
+        'startupAcceleration',
+        'comfortAcceleration',
+        'startupTime',
+        'gammaValue',
+        'inertiaCoefficient',
+        'rollingResistanceA',
+        'rollingResistanceB',
+        'rollingResistanceC',
+        'electricalPowerStartupTime',
+        'raisePantographTime',
+      ],
+      validRollingStockForm: {
+        name: 'auietsrn',
+        length: 0,
+        mass: 155,
+        maxSpeed: 10000,
+        startupAcceleration: 0,
+        comfortAcceleration: 0.01,
+        startupTime: 0,
+        gammaValue: 0.01,
+        inertiaCoefficient: 1,
+        rollingResistanceA: 0,
+        rollingResistanceB: 0,
+        rollingResistanceC: 0,
+        loadingGauge: 'G1',
+        electricalPowerStartupTime: 5,
+        raisePantographTime: 15,
+        basePowerClass: null,
+      },
+      invalidEffortCurves: [],
+    };
+    it('should return invalidEffortCurves as true when any curve contains fewer than two values', () => {
+      const effortCurves: EffortCurveForms = {
+        '15000': setupEffortCurve('1500', [1], [10]),
+        '1000': setupEffortCurve('1000', [3, 4, 3], [30, 40, 50]),
       };
-      const result = checkRollingStockFormValidity(rsForm, effortCurves);
+
+      const result = checkRollingStockFormValidity(rsForm, effortCurves, tMock);
+      expect(result).toEqual({
+        ...expected,
+        invalidEffortCurves: ['comfortTypes.STANDARD > 15000 > unspecified > unspecified'],
+      });
+    });
+
+    it('should return invalidEffortCurves as true when any curve includes duplicate speed values', () => {
+      const effortCurves: EffortCurveForms = {
+        '15000': setupEffortCurve('1500', [1, 2, 1], [10, 20, 10]),
+        '1000': setupEffortCurve('1000', [3, 4, 3], [30, 40, 30]),
+      };
+
+      const result = checkRollingStockFormValidity(rsForm, effortCurves, tMock);
+      expect(result).toEqual({
+        ...expected,
+        invalidEffortCurves: [
+          'comfortTypes.STANDARD > 1000 > unspecified > unspecified',
+          'comfortTypes.STANDARD > 15000 > unspecified > unspecified',
+        ],
+      });
+    });
+
+    it('should return invalidEffortCurves as false when all curves contain unique speed values and at least two values each', () => {
+      const effortCurves: EffortCurveForms = {
+        '15000': setupEffortCurve('1500', [1, 2, 1], [10, 20, 30]),
+        '1000': setupEffortCurve('1000', [3, 4, 3], [30, 40, 50]),
+      };
+
+      const result = checkRollingStockFormValidity(rsForm, effortCurves, tMock);
       expect(result).toEqual(expected);
     });
   });

front/src/modules/rollingStock/helpers/utils.ts

@@ -176,8 +176,13 @@ type Conditions = Record<string, (effortCurves: EffortCurveForms | null) => bool
 
 export const checkRollingStockFormValidity = (
   rollingStockForm: RollingStockParametersValues,
-  effortCurves: EffortCurveForms | null
-): { invalidFields: string[]; validRollingStockForm: RollingStockParametersValidValues } => {
+  effortCurves: EffortCurveForms | null,
+  t: TFunction
+): {
+  invalidFields: string[];
+  validRollingStockForm: RollingStockParametersValidValues;
+  invalidEffortCurves: string[];
+} => {
   const conditions = RS_SCHEMA_PROPERTIES.reduce<Conditions>((acc, val) => {
     if (val.condition) {
       return { ...acc, [val.title]: val.condition };
@@ -196,15 +201,50 @@ export const checkRollingStockFormValidity = (
     return false;
   });
 
+  let invalidEffortCurves: string[] = [];
+  Object.entries(effortCurves || {}).forEach(([mode, { curves }]) => {
+    curves.forEach(
+      ({ curve, cond: { comfort, electrical_profile_level, power_restriction_code } }) => {
+        const filteredCurve = filterUndefinedValueInCurve(curve);
+
+        if (
+          filteredCurve.max_efforts.length < 2 ||
+          filteredCurve.speeds.length < 2 ||
+          new Set(filteredCurve.speeds).size !== filteredCurve.speeds.length
+        ) {
+          const formattedComfort = formatCurveCondition(comfort, t, 'comfortTypes');
+          const formattedElecProfile = formatCurveCondition(electrical_profile_level, t);
+          const formattedResCode = formatCurveCondition(power_restriction_code, t);
+
+          invalidEffortCurves = [
+            ...invalidEffortCurves,
+            `${formattedComfort} > ${t(mode)} > ${formattedElecProfile} > ${formattedResCode}`,
+          ];
+        }
+      }
+    );
+  });
+
   return {
     invalidFields,
     validRollingStockForm: {
       ...pick(RS_REQUIRED_FIELDS, invalidFields),
       ...omit(rollingStockForm, invalidFields),
     } as RollingStockParametersValidValues,
+    invalidEffortCurves,
   };
 };
 
+const formatCurveCondition = (
+  conditionValue: string | null,
+  t: TFunction,
+  translationCategory?: string
+): string => {
+  if (conditionValue === null) return t('unspecified');
+  if (translationCategory) return t(getTranslationKey(translationCategory, conditionValue));
+  return t(conditionValue);
+};
+
 export const createEmptyCurve = (
   comfort: RollingStockComfortType,
   electricalProfile: string | null = null,

front/src/styles/scss/applications/rollingStockEditor/_rollingStockForm.scss

@@ -62,11 +62,6 @@
       }
     }
 
-    .error-message {
-      padding: 20px 0;
-      max-width: 80%;
-    }
-
     .rollingstock-card-body {
       height: 18rem;
     }