Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up and enhance simulation report sheet #10001

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

achrafmohye
Copy link
Contributor

Closes #10000

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

codecov-commenter commented Dec 10, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 93.84615% with 12 lines in your changes missing coverage. Please review.

Project coverage is 81.53%. Comparing base (0d30501) to head (0edbc00).
Report is 23 commits behind head on dev.

Files with missing lines Patch % Lines
...cations/stdcm/utils/formatSimulationReportSheet.ts 0.00% 10 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   81.52%   81.53%            
========================================
  Files        1059     1059            
  Lines      104457   104594   +137     
  Branches      722      722            
========================================
+ Hits        85162    85280   +118     
- Misses      19254    19273    +19     
  Partials       41       41            
Flag Coverage Δ
editoast 73.71% <ø> (-0.03%) ⬇️
front 89.25% <93.84%> (+<0.01%) ⬆️
gateway 2.18% <ø> (ø)
osrdyne 3.28% <ø> (ø)
railjson_generator 87.50% <ø> (ø)
tests 87.05% <ø> (ø)

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.

@achrafmohye 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 achrafmohye self-assigned this Dec 10, 2024
@achrafmohye achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch from 1bb87f2 to 47d2bbc Compare December 10, 2024 11:08
@achrafmohye achrafmohye marked this pull request as ready for review December 10, 2024 11:09
@achrafmohye achrafmohye requested a review from a team as a code owner December 10, 2024 11:09
@achrafmohye achrafmohye requested review from clarani and removed request for SharglutDev December 10, 2024 11:11
Copy link
Contributor

@Akctarus Akctarus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Tolerance could be set a little higher
Capture d’écran 2024-12-10 à 17 30 23

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

Capture d’écran 2024-12-10 à 17 32 22

front/public/locales/fr/stdcm-simulation-report-sheet.json Outdated Show resolved Hide resolved
front/src/common/Map/Search/useSearchOperationalPoint.tsx Outdated Show resolved Hide resolved
@achrafmohye achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch from 47d2bbc to a5c1dad Compare December 11, 2024 10:02
@clarani
Copy link
Contributor

clarani commented Dec 11, 2024

Found some bugs:

  • I have these inputs (no linked path, tolerances on the arrival and not on the departure)
    image
    and this simulation sheet:
    image

    • 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

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

@clarani clarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still found some bugs:

  • the banner for the anterior train should not be displayed if I selected no train
  • "au plus tôt" should be in the departure column and "13h00" should be in the arrival colum (see the mock-up)
  • the stop time is not displayed in the top table
    image

mockup:

Image

@achrafmohye achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch 3 times, most recently from f2a58b9 to 105da8c Compare January 8, 2025 15:00
@achrafmohye achrafmohye requested a review from a team as a code owner January 8, 2025 15:00
@achrafmohye achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch from 105da8c to 8b801a7 Compare January 8, 2025 15:01
@Maymanaf Maymanaf removed the request for review from a team January 8, 2025 15:15
@achrafmohye achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch 3 times, most recently from 25a91e3 to ed241ee Compare January 9, 2025 08:37
Signed-off-by: Achraf Mohyeddine <a.mohyeddine@gmail.com>
@achrafmohye achrafmohye force-pushed the ame/clean-stdcm-simualtion-report-sheet branch from ed241ee to 0edbc00 Compare January 9, 2025 09:49
Copy link
Contributor

@clarani clarani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅

@achrafmohye achrafmohye added this pull request to the merge queue Jan 9, 2025
Merged via the queue into dev with commit 3e7e74e Jan 9, 2025
27 checks passed
@achrafmohye achrafmohye deleted the ame/clean-stdcm-simualtion-report-sheet branch January 9, 2025 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:front Work on Standard OSRD Interface modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clean up and enhance simulation report sheet
4 participants