Clean up and enhance simulation report sheet #10001

achrafmohye · 2024-12-10T09:13:29Z

front/src/modules/pathfinding/utils.ts

codecov-commenter · 2024-12-10T09:16:05Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 89.79592% with 20 lines in your changes missing coverage. Please review.

Project coverage is 81.42%. Comparing base (3bc6d02) to head (d040ddf).
Report is 14 commits behind head on dev.

Files with missing lines	Patch %	Lines
...cations/stdcm/utils/formatSimulationReportSheet.ts	0.00%	9 Missing ⚠️
front/src/reducers/osrdconf/stdcmConf/index.ts	18.18%	9 Missing ⚠️
front/src/modules/pathfinding/utils.ts	33.33%	2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev   #10001      +/-   ##
==========================================
+ Coverage   79.94%   81.42%   +1.48%     
==========================================
  Files        1057     1057              
  Lines      106302   104272    -2030     
  Branches      724      720       -4     
==========================================
- Hits        84980    84902      -78     
+ Misses      21280    19328    -1952     
  Partials       42       42

Flag	Coverage Δ
editoast	`73.59% <ø> (-0.34%)`	⬇️
front	`89.16% <89.79%> (-0.04%)`	⬇️
gateway	`2.18% <ø> (ø)`
osrdyne	`3.28% <ø> (ø)`
railjson_generator	`87.50% <ø> (ø)`
tests	`87.00% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Akctarus

Tested, ty for this clean, just left few comments:

Tolerance could be set a little higher

The data seems to be shifted to the right of the column headings, so I think there's an extra margin somewhere. If you fix this problem, you'll also have to watch out for lines with stops, which are displayed in a different way from the others

Akctarus · 2024-12-10T15:56:33Z

front/public/locales/fr/stdcm-simulation-report-sheet.json

-  "conventionalSign": "signe conv.",
  "convoy": "convoi",
-  "crossedATE": "ATE croisé",


I think you forgot to also delete these on the english file

still used here in SimulationReportSheetScenario

You must have the same modifications in the french and english translation files:

either the keys are not used anymore and it should be removed in both files

or they are still used and you should not removed it

Sorry i got it know i misread your comment

Akctarus · 2024-12-10T16:20:01Z

front/src/common/Map/Search/useSearchOperationalPoint.tsx

@@ -112,7 +112,7 @@ export default function useSearchOperationalPoint({

        if (chCodeFilter === undefined) return true;

-        return result.ch.toLocaleLowerCase().includes(chCodeFilter.trim().toLowerCase());
+        return result?.ch?.toLocaleLowerCase().includes(chCodeFilter.trim().toLowerCase());


I don't know if it's necessary, but if it makes the code more robust we should keep it

i encountered some issues not relating to this where i have a blank page :/

Hum are you sure it is necessary ? Can you check again now ? :/

clarani · 2024-12-11T08:51:04Z

front/public/locales/fr/stdcm-simulation-report-sheet.json

-  "conventionalSign": "signe conv.",
  "convoy": "convoi",
-  "crossedATE": "ATE croisé",


You must have the same modifications in the french and english translation files:

either the keys are not used anymore and it should be removed in both files

or they are still used and you should not removed it

clarani · 2024-12-11T08:51:43Z

front/public/locales/en/stdcm-simulation-report-sheet.json

@@ -17,7 +17,7 @@
  "passageStop": "passage",
  "refEngine": "Ref. engine",
  "referenceEngine": "reference engine",
-  "referencePath": "Reference path",
+  "length": "Length",


The keys should be alphabetically ordered :)

clarani · 2024-12-11T09:40:14Z

front/src/applications/stdcm/components/StdcmResults/StdcmResults.tsx

@@ -54,6 +54,7 @@ const StcdmResults = ({
  const hasSimulationResults = hasResults(outputs);

  const { trackConflicts, workConflicts } = useConflictsMessages(t, outputs);
+  const { username } = useAuth();


You can move this hook directly in StdcmReportSheet

i've encountered an issue doing that of redux being out of scope of the Provider

clarani · 2024-12-11T09:41:18Z

front/src/applications/stdcm/hooks/useStdcm.ts

+    stdcmResponse,
+    stdcmTrainResult,
+    setPathProperties,
+    dispatch


You can get the dispatch directly in useStdcmResults with useAppDispatch()

Suggested change

dispatch

clarani · 2024-12-11T09:42:55Z

front/src/applications/stdcm/hooks/useStdcmForm.ts

@@ -38,6 +38,7 @@ const useStdcmForm = (): StdcmSimulationInputs => {
      pathSteps,
      departureDate: originArrival?.arrivalDate,
      departureTime: originArrival?.arrivalTime,
+      tolerances: origin.tolerances,


What if the tolerances are on the destination ?
Moreover, with this data structure, you will loose a piece of information: on which extremity the tolerances are defined :/

clarani · 2024-12-11T09:45:53Z

front/src/applications/stdcm/hooks/useStdcmResults.ts

+            };
+          });
+
+          const operationalPointsWithUniqueIds = operationalPointsWithMetadata.map((op, index) => ({


You don't need to use operationalPointsWithMetadata to define the ops for the manchette, you can keep operational_points

Suggested change

const operationalPointsWithUniqueIds = operationalPointsWithMetadata.map((op, index) => ({

const operationalPointsWithUniqueIds = operational_points.map((op, index) => ({

clarani · 2024-12-11T09:47:56Z

front/src/common/Map/Search/useSearchOperationalPoint.tsx

@@ -112,7 +112,7 @@ export default function useSearchOperationalPoint({

        if (chCodeFilter === undefined) return true;

-        return result.ch.toLocaleLowerCase().includes(chCodeFilter.trim().toLowerCase());
+        return result?.ch?.toLocaleLowerCase().includes(chCodeFilter.trim().toLowerCase());


Hum are you sure it is necessary ? Can you check again now ? :/

clarani · 2024-12-11T10:10:33Z

Found some bugs:

I have these inputs (no linked path, tolerances on the arrival and not on the departure)

and this simulation sheet:
- a linked path is given, but I haven't specified any
- the tolerances are specified on the departure and not the arrival
I can't test it right now, but I think that if I give a posterior train linked, then it may not be correctly displayed

Signed-off-by: Achraf Mohyeddine <[email protected]>

github-actions bot added the area:front Work on Standard OSRD Interface modules label Dec 10, 2024

achrafmohye commented Dec 10, 2024

View reviewed changes

front/src/modules/pathfinding/utils.ts Show resolved Hide resolved

achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch 2 times, most recently from c95d87e to 95d9784 Compare December 10, 2024 09:34

achrafmohye self-assigned this Dec 10, 2024

achrafmohye requested review from SharglutDev and Akctarus December 10, 2024 10:43

achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch from 1bb87f2 to 47d2bbc Compare December 10, 2024 11:08

achrafmohye marked this pull request as ready for review December 10, 2024 11:09

achrafmohye requested a review from a team as a code owner December 10, 2024 11:09

achrafmohye requested review from clarani and removed request for SharglutDev December 10, 2024 11:11

Akctarus reviewed Dec 10, 2024

View reviewed changes

clarani reviewed Dec 11, 2024

View reviewed changes

achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch from 47d2bbc to a5c1dad Compare December 11, 2024 10:02

SharglutDev requested review from clarani and Akctarus December 18, 2024 09:15

achrafmohye added 4 commits December 24, 2024 11:54

front: clean up and enhance simulation report sheet

7d58b46

Signed-off-by: Achraf Mohyeddine <[email protected]>

front: fix typing

bc42763

Signed-off-by: Achraf Mohyeddine <[email protected]>

front: fix comments

b0e4d5c

Signed-off-by: Achraf Mohyeddine <[email protected]>

front: enhance lmr report sheet and fix linked paths

236f1d7

Signed-off-by: Achraf Mohyeddine <[email protected]>

achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch from 205ed74 to 236f1d7 Compare December 24, 2024 15:49

front: fix tolerances after rebase

d040ddf

Signed-off-by: Achraf Mohyeddine <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up and enhance simulation report sheet #10001

Clean up and enhance simulation report sheet #10001

achrafmohye commented Dec 10, 2024

codecov-commenter commented Dec 10, 2024 •

edited

Loading

Akctarus left a comment

Akctarus Dec 10, 2024

achrafmohye Dec 10, 2024

clarani Dec 11, 2024

achrafmohye Dec 11, 2024

achrafmohye Dec 12, 2024

Akctarus Dec 10, 2024

achrafmohye Dec 10, 2024

clarani Dec 11, 2024

achrafmohye Dec 12, 2024

clarani Dec 11, 2024

clarani Dec 11, 2024

achrafmohye Dec 12, 2024

clarani Dec 11, 2024

achrafmohye Dec 12, 2024 •

edited

Loading

clarani Dec 11, 2024

clarani Dec 11, 2024

achrafmohye Dec 12, 2024

clarani Dec 11, 2024

achrafmohye Dec 12, 2024

clarani Dec 11, 2024

clarani commented Dec 11, 2024

	const operationalPointsWithUniqueIds = operationalPointsWithMetadata.map((op, index) => ({
	const operationalPointsWithUniqueIds = operational_points.map((op, index) => ({

Clean up and enhance simulation report sheet #10001

Are you sure you want to change the base?

Clean up and enhance simulation report sheet #10001

Conversation

achrafmohye commented Dec 10, 2024

codecov-commenter commented Dec 10, 2024 • edited Loading

Codecov Report

Akctarus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achrafmohye Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clarani commented Dec 11, 2024

codecov-commenter commented Dec 10, 2024 •

edited

Loading

achrafmohye Dec 12, 2024 •

edited

Loading