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

front: take into account new flag for using speed limits #10859

Merged
merged 1 commit into from
Feb 24, 2025

Conversation

axrolld
Copy link
Contributor

@axrolld axrolld commented Feb 19, 2025

The idea is to make front end keep this flag when it is present (from a json import for example), and keep its default value otherwise. Not adding anything in the interface.

Fixes #10858

You can test with this trainschedule:

  • import the trainSchedule, change the rollingStock for a TGV and see that the train can hit 300km
  • change in the json the option usingSpeedLimits, import again the trainSchedule, change the rollingStock for the same TGV. The GEV should not be the same
  • check that you can't create a train which will not use the speed limits:
    • click on edit on the train which does not use the speed limits
    • close the manageTrainSchedule page immediately and create a new train
    • the new train should use the speed limits

@github-actions github-actions bot added the area:front Work on Standard OSRD Interface modules label Feb 19, 2025
@codecov-commenter
Copy link

codecov-commenter commented Feb 19, 2025

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

Codecov Report

Attention: Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 82.48%. Comparing base (85702d1) to head (f310a47).
Report is 46 commits behind head on dev.

Files with missing lines Patch % Lines
.../reducers/osrdconf/operationalStudiesConf/index.ts 66.66% 2 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10859      +/-   ##
==========================================
- Coverage   82.59%   82.48%   -0.12%     
==========================================
  Files        1084     1084              
  Lines      107320   107637     +317     
  Branches      729      739      +10     
==========================================
+ Hits        88644    88786     +142     
- Misses      18634    18809     +175     
  Partials       42       42              
Flag Coverage Δ
editoast 74.37% <ø> (-0.03%) ⬇️
front 90.32% <92.00%> (-0.20%) ⬇️
gateway 2.18% <ø> (ø)
osrdyne 3.28% <ø> (ø)
railjson_generator 87.58% <ø> (ø)
tests 87.90% <ø> (-0.02%) ⬇️

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.

@axrolld axrolld force-pushed the ard/fix-train-import-speed-limits-flag branch from 6ffa561 to b890424 Compare February 19, 2025 14:26
@clarani clarani force-pushed the ard/fix-train-import-speed-limits-flag branch from b890424 to 50ad060 Compare February 20, 2025 15:33
@clarani clarani marked this pull request as ready for review February 20, 2025 15:34
@clarani clarani requested a review from a team as a code owner February 20, 2025 15:34
Copy link
Contributor Author

@axrolld axrolld left a comment

Choose a reason for hiding this comment

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

lgtm, thank you 🙏

Copy link
Contributor

@SarahBellaha SarahBellaha left a comment

Choose a reason for hiding this comment

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

LGTM :)

@clarani clarani requested a review from Math-R February 21, 2025 08:37
Signed-off-by: Alex Rolland <[email protected]>
Signed-off-by: Clara Ni <[email protected]>
@clarani clarani force-pushed the ard/fix-train-import-speed-limits-flag branch from 50ad060 to f310a47 Compare February 24, 2025 09:43
@clarani
Copy link
Contributor

clarani commented Feb 24, 2025

Nice catch ! I remove the field from the persist config, so it is not persisted anymore and if a user opens a new tab, the config will have usingSpeedLimit = true

@Math-R Math-R requested a review from SharglutDev February 24, 2025 10:13
@clarani clarani enabled auto-merge February 24, 2025 10:18
Copy link
Contributor

@Math-R Math-R left a comment

Choose a reason for hiding this comment

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

looks good to me, just wait @SharglutDev to resolve :)

@SharglutDev
Copy link
Contributor

looks good to me, just wait @SharglutDev to resolve :)

Was waiting for @clarani to rebase but I ended up rebasing in local to test :)

Copy link
Contributor

@SharglutDev SharglutDev left a comment

Choose a reason for hiding this comment

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

Lgtm and tested ✅

@clarani clarani added this pull request to the merge queue Feb 24, 2025
Merged via the queue into dev with commit 33f63cf Feb 24, 2025
27 checks passed
@clarani clarani deleted the ard/fix-train-import-speed-limits-flag branch February 24, 2025 13:59
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.

front: editing train schedule doesn't take into account the new option use_speed_limits_for_simulation
6 participants