OpenRailAssociation / osrd

An open source web application for railway infrastructure design, capacity analysis, timetabling and simulation
https://osrd.fr
458 stars 43 forks source link

table, trains after midnight #8334

Closed anisometropie closed 1 month ago

anisometropie commented 2 months ago

closes #7781

We now use a different format to represent arrival and departure in SuggestedOP and PathStep, an ISO duration string. The previous time string format hh:mm:ss was insufficient for distinguishing times spanning more than 24 hours

These two types are used in different places beyond the scope of just the input/outpt, we should test everything that uses these two types

codecov-commenter commented 2 months ago

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

Codecov Report

Attention: Patch coverage is 48.15618% with 239 lines in your changes missing coverage. Please review.

Project coverage is 37.05%. Comparing base (2d6138c) to head (143da9a). Report is 4 commits behind head on dev.

Files with missing lines Patch % Lines
front/src/modules/timesStops/helpers/utils.ts 75.67% 19 Missing and 26 partials :warning:
front/src/modules/timesStops/TimesStops.tsx 0.00% 39 Missing :warning:
front/src/modules/timesStops/TimeInput.tsx 0.00% 29 Missing :warning:
front/src/modules/timesStops/ReadOnlyTime.tsx 0.00% 22 Missing and 1 partial :warning:
...rc/modules/timesStops/hooks/useTimeStopsColumns.ts 0.00% 18 Missing :warning:
front/src/common/types.ts 0.00% 15 Missing :warning:
...ont/src/modules/timesStops/helpers/scheduleData.ts 28.57% 12 Missing and 3 partials :warning:
front/src/modules/pathfinding/utils.ts 41.17% 5 Missing and 5 partials :warning:
front/src/modules/timesStops/TimesStopsInput.tsx 0.00% 9 Missing :warning:
...src/modules/timesStops/hooks/useOutputTableData.ts 0.00% 9 Missing :warning:
... and 11 more

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #8334 +/- ## ============================================ + Coverage 36.98% 37.05% +0.06% Complexity 2211 2211 ============================================ Files 1259 1260 +1 Lines 114565 114820 +255 Branches 3192 3223 +31 ============================================ + Hits 42374 42546 +172 - Misses 70290 70342 +52 - Partials 1901 1932 +31 ``` | [Flag](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [core](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `74.79% <ø> (ø)` | | | [editoast](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `72.44% <ø> (-0.01%)` | :arrow_down: | | [front](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `15.17% <48.15%> (+0.20%)` | :arrow_up: | | [gateway](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `2.20% <ø> (ø)` | | | [osrdyne](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `2.60% <ø> (ø)` | | | [railjson_generator](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `87.49% <ø> (ø)` | | | [tests](https://app.codecov.io/gh/OpenRailAssociation/osrd/pull/8334/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `86.46% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

theocrsb commented 2 months ago
image

When I try to add 1 hour to 23 hours the requested departure time is not displayed. It is displayed when it is not midnight.

theocrsb commented 2 months ago
image

I don't know if this use case should be possible, but when I try to start the train 2 days later (172800 seconds), the requested departure time only shows J+1.

anisometropie commented 1 month ago

image I don't know if this use case should be possible, but when I try to start the train 2 days later (172800 seconds), the requested departure time only shows J+1.

It’s not a case we currently choose to handle

emersion commented 1 month ago

TBH I am not a huge fan of these new types which don't improve type safety (since they're aliases)…

SharglutDev commented 1 month ago

TBH I am not a huge fan of these new types which don't improve type safety (since they're aliases)…

I agree but it doesn't add to much logic in the code. When the refacto for that will be done it can be adapted ? Or do you prefer to keep stringeverywhere in the meantime ?

anisometropie commented 1 month ago

TBH I am not a huge fan of these new types which don't improve type safety (since they're aliases)…

@emersion

They are meant to define clearly what they are, and give move details when you hover your mouse over them.

I don’t want to lose the info and go back to a string. they can easily be changed with better types if we know where they are