Conversation
Synar
force-pushed
the
ali/improve-nge-events
branch
from
caefb63 to
fbfd9c5
Compare
| const { id: _id, train_name: _name, ...duplicatedForwardBase } = duplicatedForwardTimetableItem; | ||
| const { id: __id, train_name: __name, ...duplicatedReturnBase } = duplicatedReturnTimetableItem; |
| export type NGEEvent = { | ||
| type: 'create' | 'delete' | 'update'; | ||
| } & ( | ||
| type TrainrunUpdateTag = |
There was a problem hiding this comment.
export type TrainrunUpdateTag
| // thus the trip that needs to be deleted is always the return trip | ||
| await storeRoundTrip(dispatch, newForwardTimetableItem.id); | ||
| await deleteTimetableItemById(timetableItemIds[1], dispatch, addDeletedTimetableItemIds); | ||
| const oldReturnId = |
louisgreiner
left a comment
louisgreiner
left a comment
There was a problem hiding this comment.
Testing by injecting your PR on the fork locally, and had this bug
Enregistrement.de.l.ecran.2026-03-12.182627.mp4
Signed-off-by: Alice Khoudli <alice.khoudli@polytechnique.org>
Signed-off-by: Alice Khoudli <alice.khoudli@polytechnique.org>
Signed-off-by: Alice Khoudli <alice.khoudli@polytechnique.org>
Signed-off-by: Alice Khoudli <alice.khoudli@polytechnique.org>
Synar
force-pushed
the
ali/improve-nge-events
branch
from
fbfd9c5 to
aca239d
Compare
|
@louisgreiner could you try with the current version that uses the published package? ^^ |
There was a problem hiding this comment.
Tested again this works perfectly! Could not find a bug
Oops, spoken too quickly: I created this timetable on your branch timetable.json and the path steps seem broken
Enregistrement.de.l.ecran.2026-03-24.094123.mp4
I don't know if this is relative to this PR.
Testing on dev:
Enregistrement.de.l.ecran.2026-03-24.094401.mp4
Except that, this looks like working well!
|
Ah, I think you've found a large issue. What happens is that when importing a train, the ids of path items contained in the file seem to be discarded entirely. Then when we receive an update event, we completely reinvent the path items ids, even if the path stayed the same. So the ids used in the schedule no longer corresponds to the stored path. I see multiple potential solution ideas, but I'm not 100% sure what the best one is. Perhaps we could relabel the ids on initialization, making sure to correctly shift margins? Or perhaps do it on update when the path has not changed? Or perhaps when the path is unchanged we should make sure to use the old path items ids when creating the schedule? This is more brittle than I thought it would be. |
3 participants
Close #13034
Close #10428
Close #14862
Close #13033
Depend on OpenRailAssociation/netzgrafik-editor-frontend#853
Depend on osrd-project/netzgrafik-editor-frontend#22
Still to do: change the version number once the above prs are merged in the wip commit,
and delete the temp commit at the end (which enables local testing by using local nge assuming the osrd and nge folders are siblings in the file structure, and that nge is using the fork git branch ali/improve-events-with-fork).(Done.)Also the first 2 commits are an unrelated fix to prevent stale cache and come from #15388, they should be dropped once that pr is merged or the issue is otherwise fixed.(Merged.)Please review commit by commit,
ignoring the first 2 and the last one.(Merged.)