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

editoast: stdcm: add arrival time parameters #7827

Merged
merged 1 commit into from
Jun 27, 2024
Merged

Conversation

eckter
Copy link
Contributor

@eckter eckter commented Jun 24, 2024

Fix #7692 and #7687

Adds stop time parameters and forwards them from editoast inputs to core. Departure time parameter is now optional.

There's a few other changes on the editoast side to have meaningful values as start/end times despite this change. Eventually we should use them to pre-filter the resource uses (see #7649).

There's one API change on which I'm not 100% sure: I've "deprecated" the departure time parameter, assuming the time of the first waypoint should be used instead. We could instead keep it but not let the user set a time for the first waypoint.

Note: the feature is not entirely supported by core yet. When the departure time is set, the behavior won't change. If it's not set, the departure time will be too early and the scheduled points will be ignored, but it won't crash.

@eckter eckter requested review from a team as code owners June 24, 2024 08:59
@eckter eckter requested a review from Erashin June 24, 2024 08:59
@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2024

Codecov Report

Attention: Patch coverage is 22.22222% with 49 lines in your changes missing coverage. Please review.

Project coverage is 28.54%. Comparing base (1178871) to head (12a5a59).
Report is 1 commits behind head on dev.

Files Patch % Lines
editoast/src/views/v2/timetable/stdcm.rs 2.38% 41 Missing ⚠️
...in/fr/sncf/osrd/api/api_v2/stdcm/STDCMRequestV2.kt 0.00% 5 Missing ⚠️
...kotlin/fr/sncf/osrd/utils/json/UnitJsonAdapters.kt 0.00% 3 Missing ⚠️

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

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #7827      +/-   ##
============================================
- Coverage     28.55%   28.54%   -0.01%     
  Complexity     2059     2059              
============================================
  Files          1249     1249              
  Lines        154130   154183      +53     
  Branches       3040     3041       +1     
============================================
+ Hits          44006    44011       +5     
- Misses       108310   108358      +48     
  Partials       1814     1814              
Flag Coverage Δ
core 74.96% <0.00%> (-0.05%) ⬇️
editoast 71.65% <2.38%> (-0.11%) ⬇️
front 10.01% <100.00%> (+<0.01%) ⬆️
gateway 2.34% <ø> (ø)
railjson_generator 87.49% <ø> (ø)
tests 73.16% <ø> (ø)

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.

@eckter eckter linked an issue Jun 24, 2024 that may be closed by this pull request
@eckter eckter force-pushed the ech/stdcm-add-arrival-times branch from 623f333 to 1ede6d0 Compare June 24, 2024 09:21
@eckter eckter requested a review from a team as a code owner June 24, 2024 09:21
@eckter eckter requested review from woshilapin and SharglutDev June 24, 2024 09:23
@SharglutDev SharglutDev requested review from kmer2016 and SarahBellaha and removed request for SharglutDev June 25, 2024 06:46
Copy link
Contributor

@Erashin Erashin left a comment

Choose a reason for hiding this comment

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

Core part is fine.

@eckter eckter force-pushed the ech/stdcm-add-arrival-times branch 3 times, most recently from 24176d7 to 6d39b82 Compare June 26, 2024 13:30
@eckter eckter requested a review from flomonster June 26, 2024 13:31
Copy link
Contributor

@flomonster flomonster left a comment

Choose a reason for hiding this comment

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

LGTM (reviewed editoast part)

@eckter eckter requested a review from SharglutDev June 27, 2024 08:18
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

Copy link
Contributor

@kmer2016 kmer2016 left a comment

Choose a reason for hiding this comment

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

LGTM

@eckter eckter force-pushed the ech/stdcm-add-arrival-times branch from 6d39b82 to 0677bce Compare June 27, 2024 09:11
@eckter eckter enabled auto-merge June 27, 2024 09:12
@eckter eckter force-pushed the ech/stdcm-add-arrival-times branch from 0677bce to 12a5a59 Compare June 27, 2024 12:22
@eckter eckter added this pull request to the merge queue Jun 27, 2024
Merged via the queue into dev with commit 375aedd Jun 27, 2024
17 checks passed
@eckter eckter deleted the ech/stdcm-add-arrival-times branch June 27, 2024 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants