front: fix opstudies itinary reversal dysfunctioning when times and margin where set

[Synar]

Close #10036 (🔆🔆🔆nicer one premium shiny pack🔆🔆🔆 version)

[codecov-commenter]

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

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.82%. Comparing base (9314f35) to head (e1d30a1).
Report is 85 commits behind head on dev.

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

Additional details and impacted files

@@           Coverage Diff           @@
##              dev   #10438   +/-   ##
=======================================
  Coverage   81.82%   81.82%           
=======================================
  Files        1073     1074    +1     
  Lines      106829   106856   +27     
  Branches      730      735    +5     
=======================================
+ Hits        87413    87436   +23     
- Misses      19377    19381    +4     
  Partials       39       39           

Flag	Coverage Δ
editoast	74.28% <ø> (-0.02%)	⬇️
front	89.35% <100.00%> (+<0.01%)	⬆️
gateway	2.18% <ø> (ø)
osrdyne	3.28% <ø> (ø)
railjson_generator	87.50% <ø> (ø)
tests	88.14% <ø> (ø)

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.

[SharglutDev]

Lgtm and tested, thank you for the shiny pack version 🔆🔆🔆

[achrafmohye]

LGTM and tested 👍

[emersion]

I think we want to move the margin of each path step to the next path step before reversing. I think this version would be easier to follow:

const newPathSteps = pathSteps.map((step, index) => ({
  ...step,
  arrival: null,
  theoreticalMargin: pathSteps[index - 1]?.theoreticalMargin,
})).reverse();

Or am I missing something?

Would be nice to have tests for this.

[emersion]

Dear GitHub, I've looked at this PR, now you can hide it from my review queue.

[Synar]

I think we want to move the margin of each path step to the next path step before reversing. I think this version would be easier to follow:

const newPathSteps = pathSteps.map((step, index) => ({
  ...step,
  arrival: null,
  theoreticalMargin: pathSteps[index - 1]?.theoreticalMargin,
})).reverse();

Or am I missing something?

Would be nice to have tests for this.

The issue is that a lot of intermediary path steps don't have a theoretical margins, and we should skip them when exchanging margins. So you should only set margins that were already set (plus start/end), the value to replace it with should be the last margin that was actually set, not simply the previous margin.

To give an example, [0%, undef, 3%, undef, undef, undef, 1%, undef] means we have 0% margin for 2 steps traveled, then 3% for 4, then 1% for 1.

So it should give first [undef, undef, 0%, undef, undef, undef, 3%, 1%] before reversing, then after [1%, 3%, undef, undef, undef, 0%, undef, undef] which is 1% for 1 step, 3% for 4, and 0% for 2. Same as the initial distrib, but reversed.

Your version would give [undef, 0%, undef, 3%, undef, undef, undef, 1%] then [1%, undef, undef, undef, 3%, undef, 0%, undef] so 1% for 4 steps, 3% for 2, and 0% for 1. Different from the initial distrib.

I will write a test ^^

[Synar]

@emersion I've added a couple tests ^^