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: fix power restrictions get reset on editing a train #10696

Merged
merged 4 commits into from
Feb 17, 2025

Conversation

clarani
Copy link
Contributor

@clarani clarani commented Feb 5, 2025

closes #10494

After a hard refresh, the pathfinding was relaunched by the useEffect depending on the rolling stock in usePathfinding. Indeed, the trick with isInitialized did not work because it was set to true before the fetch of the rolling stock. Thus, when the rolling stock was received, the useEffect was triggered and the power restrictions reset.

Now, the hook usePathfinding is considered initialized only if there is no rollingStock selected or if the selected rolling stock has been already received.

See commits

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

codecov-commenter commented Feb 5, 2025

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

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.55%. Comparing base (beff514) to head (ab5f5cc).
Report is 11 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   #10696      +/-   ##
==========================================
+ Coverage   81.93%   82.55%   +0.62%     
==========================================
  Files        1084     1083       -1     
  Lines      107212   107244      +32     
  Branches      737      728       -9     
==========================================
+ Hits        87842    88538     +696     
+ Misses      19329    18665     -664     
  Partials       41       41              
Flag Coverage Δ
editoast 74.37% <ø> (-0.02%) ⬇️
front 90.47% <100.00%> (+0.97%) ⬆️
gateway 2.18% <ø> (ø)
osrdyne 3.28% <ø> (ø)
railjson_generator 87.58% <ø> (ø)
tests 87.91% <ø> (ø)

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.

@clarani clarani force-pushed the cni/10494-power-restrictions-are-reset branch from 8e397d7 to 6ea39d3 Compare February 6, 2025 08:59
@clarani clarani marked this pull request as ready for review February 6, 2025 09:26
@clarani clarani requested a review from a team as a code owner February 6, 2025 09:26
Copy link
Contributor

@RomainValls RomainValls left a comment

Choose a reason for hiding this comment

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

LGTM, tested ✅

Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

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

Hmm, I'm not a fan of adding even more workarounds to the current workaround…

Why does the workaround exist in the first place? What are we trying to achieve with isInitialized?

If we want to launch pathfinding when the hook is mounted and when the infra state is cached and when the rolling stock is loaded, I don't think we need this isInitialized state?

Or do we want a different behavior?

@clarani clarani force-pushed the cni/10494-power-restrictions-are-reset branch from 6ea39d3 to 17bcf9d Compare February 11, 2025 16:41
@clarani clarani requested a review from emersion February 11, 2025 16:42
@clarani clarani force-pushed the cni/10494-power-restrictions-are-reset branch 2 times, most recently from 511fc68 to c667a50 Compare February 12, 2025 10:31
UseSetupItinerary was used to compute the pathfinding of the train the user wants to edit. However, it was quite redundant
with the function launchPathfinding and the cohabitation of the 2 hooks created multiple bugs (power restrictions are reset
when editing a train, the invalid path items are not highligthed when editing a train etc).

From now on, the pathfinding store will only be populated from the launchPathfinding callback, even when opening the edition panel.

Signed-off-by: Clara Ni <[email protected]>
@clarani clarani force-pushed the cni/10494-power-restrictions-are-reset branch from c667a50 to ab5f5cc Compare February 17, 2025 10:09
@clarani clarani requested a review from emersion February 17, 2025 10:10
Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

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

LGTM :)

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.

Power restrictions get reset on first opening of train schedule
4 participants