Skip to content

Commit 3eb06a7

Browse files
committed
front: align margins display and computation to spec in timestops table
Signed-off-by: Alice Khoudli <[email protected]>
1 parent 78c1d2b commit 3eb06a7

16 files changed

+223
-121
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1+
import { keyBy } from 'lodash';
12
import { describe, it, expect } from 'vitest';
23

34
import type { TrainScheduleResult } from 'common/api/osrdEditoastApi';
5+
import type { ScheduleEntry } from 'modules/timesStops/types';
46

5-
import computeMargins from '../computeMargins';
7+
import computeMargins, { getTheoreticalMargins } from '../computeMargins';
68

79
describe('computeMargins', () => {
810
const path = [
@@ -18,33 +20,92 @@ describe('computeMargins', () => {
1820
id: 'c',
1921
uic: 3,
2022
},
23+
{
24+
id: 'd',
25+
uic: 4,
26+
},
27+
{
28+
id: 'e',
29+
uic: 5,
30+
},
2131
];
22-
const margins = { boundaries: ['c'], values: ['10%', '0%'] };
32+
const margins = { boundaries: ['c'], values: ['10%', '5%'] };
2333
const pathItemTimes = {
24-
base: [0, 100 * 1000, 200 * 1000],
25-
provisional: [0, 110 * 1000, 220 * 1000],
26-
final: [0, 115 * 1000, 230 * 1000],
34+
base: [0, 100 * 1000, 200 * 1000, 400 * 1000, 500 * 1000],
35+
provisional: [0, 110 * 1000, 220 * 1000, 430 * 1000, 535 * 1000],
36+
final: [0, 115 * 1000, 230 * 1000, 440 * 1000, 545 * 1000],
2737
};
38+
const schedule = [
39+
{
40+
at: 'a',
41+
},
42+
{
43+
at: 'c',
44+
},
45+
{
46+
at: 'd',
47+
},
48+
{
49+
at: 'e',
50+
},
51+
];
2852

2953
it('should compute simple margin', () => {
30-
const train = { path, margins } as TrainScheduleResult;
31-
expect(computeMargins(train, 0, pathItemTimes)).toEqual({
54+
const train = { path, margins, schedule } as TrainScheduleResult;
55+
const scheduleByAt: Record<string, ScheduleEntry> = keyBy(train.schedule, 'at');
56+
const theoreticalMargins = getTheoreticalMargins(train);
57+
expect(computeMargins(theoreticalMargins, train, scheduleByAt, 0, pathItemTimes)).toEqual({
3258
theoreticalMargin: '10 %',
33-
theoreticalMarginSeconds: '10 s',
34-
calculatedMargin: '15 s',
35-
diffMargins: '5 s',
36-
});
37-
expect(computeMargins(train, 1, pathItemTimes)).toEqual({
38-
theoreticalMargin: '',
59+
isTheoreticalMarginBoundary: true,
3960
theoreticalMarginSeconds: '20 s',
4061
calculatedMargin: '30 s',
4162
diffMargins: '10 s',
4263
});
43-
expect(computeMargins(train, 2, pathItemTimes)).toEqual({
64+
expect(computeMargins(theoreticalMargins, train, scheduleByAt, 1, pathItemTimes)).toEqual({
4465
theoreticalMargin: undefined,
66+
isTheoreticalMarginBoundary: undefined,
4567
theoreticalMarginSeconds: undefined,
4668
calculatedMargin: undefined,
4769
diffMargins: undefined,
4870
});
71+
expect(computeMargins(theoreticalMargins, train, scheduleByAt, 2, pathItemTimes)).toEqual({
72+
theoreticalMargin: '5 %',
73+
isTheoreticalMarginBoundary: true,
74+
theoreticalMarginSeconds: '10 s',
75+
calculatedMargin: '10 s',
76+
diffMargins: '0 s',
77+
});
78+
expect(computeMargins(theoreticalMargins, train, scheduleByAt, 3, pathItemTimes)).toEqual({
79+
theoreticalMargin: '5 %',
80+
isTheoreticalMarginBoundary: false,
81+
theoreticalMarginSeconds: '5 s',
82+
calculatedMargin: '5 s',
83+
diffMargins: '0 s',
84+
});
85+
expect(computeMargins(theoreticalMargins, train, scheduleByAt, 4, pathItemTimes)).toEqual({
86+
theoreticalMargin: undefined,
87+
isTheoreticalMarginBoundary: undefined,
88+
theoreticalMarginSeconds: undefined,
89+
calculatedMargin: undefined,
90+
diffMargins: undefined,
91+
});
92+
});
93+
});
94+
95+
describe('getTheoreticalMargins', () => {
96+
it('should compute theoretical margins with boundaries correctly', () => {
97+
const path = [{ id: 'a' }, { id: 'b' }, { id: 'c' }, { id: 'd' }, { id: 'e' }];
98+
const margins = { boundaries: ['c', 'd'], values: ['10%', '0%', '10 min/100km'] };
99+
const trainSchedule = { path, margins } as TrainScheduleResult;
100+
101+
const theoreticalMargins = getTheoreticalMargins(trainSchedule);
102+
103+
expect(theoreticalMargins).toEqual({
104+
a: { theoreticalMargin: '10%', isBoundary: true },
105+
b: { theoreticalMargin: '10%', isBoundary: false },
106+
c: { theoreticalMargin: '0%', isBoundary: true },
107+
d: { theoreticalMargin: '10 min/100km', isBoundary: true },
108+
e: { theoreticalMargin: '10 min/100km', isBoundary: false },
109+
});
49110
});
50111
});

front/src/modules/timesStops/helpers/computeMargins.ts

+48-35
Original file line numberDiff line numberDiff line change
@@ -3,39 +3,48 @@ import type { TrainScheduleWithDetails } from 'modules/trainschedule/components/
33
import { ms2sec } from 'utils/timeManipulation';
44

55
import { formatDigitsAndUnit } from './utils';
6+
import type { ScheduleEntry, TheoreticalMarginsRecord } from '../types';
67

7-
function getTheoreticalMargin(selectedTrainSchedule: TrainScheduleResult, pathStepId: string) {
8-
if (selectedTrainSchedule.path.length === 0) {
8+
/** Extracts the theoretical margin for each path step in the train schedule,
9+
* and marks whether margins are repeated or correspond to a boundary between margin values */
10+
export function getTheoreticalMargins(selectedTrainSchedule: TrainScheduleResult) {
11+
const { margins } = selectedTrainSchedule;
12+
if (!margins) {
913
return undefined;
1014
}
11-
// pathStep is starting point => we take the first margin
12-
if (selectedTrainSchedule.path[0].id === pathStepId) {
13-
return selectedTrainSchedule.margins?.values[0];
14-
}
15-
const theoreticalMarginBoundaryIndex = selectedTrainSchedule.margins?.boundaries?.findIndex(
16-
(id) => id === pathStepId
17-
);
18-
if (
19-
theoreticalMarginBoundaryIndex === undefined ||
20-
theoreticalMarginBoundaryIndex < 0 ||
21-
theoreticalMarginBoundaryIndex > selectedTrainSchedule.margins!.values.length - 2
22-
) {
23-
return undefined;
24-
}
25-
26-
return selectedTrainSchedule.margins!.values[theoreticalMarginBoundaryIndex + 1];
15+
const theoreticalMargins: TheoreticalMarginsRecord = {};
16+
let marginIndex = 0;
17+
selectedTrainSchedule.path.forEach((step, index) => {
18+
let isBoundary = index === 0;
19+
if (step.id === selectedTrainSchedule.margins?.boundaries[marginIndex]) {
20+
marginIndex += 1;
21+
isBoundary = true;
22+
}
23+
theoreticalMargins[step.id] = {
24+
theoreticalMargin: margins.values[marginIndex],
25+
isBoundary,
26+
};
27+
});
28+
return theoreticalMargins;
2729
}
2830

31+
/** Compute all margins to display for a given train schedule path step */
2932
function computeMargins(
33+
theoreticalMargins: TheoreticalMarginsRecord | undefined,
3034
selectedTrainSchedule: TrainScheduleResult,
35+
scheduleByAt: Record<string, ScheduleEntry>,
3136
pathStepIndex: number,
3237
pathItemTimes: NonNullable<TrainScheduleWithDetails['pathItemTimes']> // in ms
3338
) {
3439
const { path, margins } = selectedTrainSchedule;
40+
const pathStepId = path[pathStepIndex].id;
41+
const schedule = scheduleByAt[pathStepId];
42+
const stepTheoreticalMarginInfo = theoreticalMargins?.[pathStepId];
3543
if (
3644
!margins ||
37-
(margins.values.length === 1 && margins.values[0] === '0%') ||
38-
pathStepIndex === selectedTrainSchedule.path.length - 1
45+
pathStepIndex === selectedTrainSchedule.path.length - 1 ||
46+
!stepTheoreticalMarginInfo ||
47+
!(schedule || stepTheoreticalMarginInfo.isBoundary)
3948
) {
4049
return {
4150
theoreticalMargin: undefined,
@@ -45,14 +54,17 @@ function computeMargins(
4554
};
4655
}
4756

48-
const pathStepId = path[pathStepIndex].id;
49-
const theoreticalMargin = getTheoreticalMargin(selectedTrainSchedule, pathStepId);
57+
const { theoreticalMargin, isBoundary } = stepTheoreticalMarginInfo;
58+
59+
// find the next pathStep where constraints are defined
60+
let nextIndex = path.length - 1;
5061

51-
// find the previous pathStep where margin was defined
52-
let prevIndex = 0;
53-
for (let index = 1; index < pathStepIndex; index += 1) {
54-
if (margins.boundaries.includes(path[index].id)) {
55-
prevIndex = index;
62+
for (let index = pathStepIndex + 1; index < path.length; index += 1) {
63+
const curStepId = path[index].id;
64+
const curStepSchedule = scheduleByAt[curStepId];
65+
if (theoreticalMargins[curStepId]?.isBoundary || curStepSchedule) {
66+
nextIndex = index;
67+
break;
5668
}
5769
}
5870

@@ -61,19 +73,20 @@ function computeMargins(
6173
// provisional = margins
6274
// final = margins + requested arrival times
6375
const { base, provisional, final } = pathItemTimes;
64-
const baseDuration = ms2sec(base[pathStepIndex + 1] - base[prevIndex]);
65-
const provisionalDuration = ms2sec(provisional[pathStepIndex + 1] - provisional[prevIndex]);
66-
const finalDuration = ms2sec(final[pathStepIndex + 1] - final[prevIndex]);
76+
const baseDuration = ms2sec(base[nextIndex] - base[pathStepIndex]);
77+
const provisionalDuration = ms2sec(provisional[nextIndex] - provisional[pathStepIndex]);
78+
const finalDuration = ms2sec(final[nextIndex] - final[pathStepIndex]);
6779

6880
// how much longer it took (s) with the margin than without
69-
const provisionalLostTime = provisionalDuration - baseDuration;
70-
const finalLostTime = finalDuration - baseDuration;
81+
const provisionalLostTime = Math.round(provisionalDuration - baseDuration);
82+
const finalLostTime = Math.round(finalDuration - baseDuration);
7183

7284
return {
7385
theoreticalMargin: formatDigitsAndUnit(theoreticalMargin),
74-
theoreticalMarginSeconds: `${Math.round(provisionalLostTime)} s`,
75-
calculatedMargin: `${Math.round(finalLostTime)} s`,
76-
diffMargins: `${Math.round(finalLostTime - provisionalLostTime)} s`,
86+
isTheoreticalMarginBoundary: isBoundary,
87+
theoreticalMarginSeconds: `${provisionalLostTime} s`,
88+
calculatedMargin: `${finalLostTime} s`,
89+
diffMargins: `${finalLostTime - provisionalLostTime} s`,
7790
};
7891
}
7992

front/src/modules/timesStops/helpers/utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ const getDigits = (unit: string | undefined) =>
114114
unit === MarginUnit.second || unit === MarginUnit.percent ? 0 : 1;
115115

116116
export function formatDigitsAndUnit(fullValue: string | number | undefined, unit?: string) {
117-
if (fullValue === undefined || fullValue === '0%') {
117+
if (fullValue === undefined) {
118118
return '';
119119
}
120120
if (typeof fullValue === 'number') {

front/src/modules/timesStops/hooks/useOutputTableData.ts

+17-3
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { calculateTimeDifferenceInSeconds } from 'utils/timeManipulation';
1515

1616
import { ARRIVAL_TIME_ACCEPTABLE_ERROR_MS } from '../consts';
1717
import { computeInputDatetimes } from '../helpers/arrivalTime';
18-
import computeMargins from '../helpers/computeMargins';
18+
import computeMargins, { getTheoreticalMargins } from '../helpers/computeMargins';
1919
import { formatSchedule } from '../helpers/scheduleData';
2020
import { type ScheduleEntry, type TimeStopsRow } from '../types';
2121

@@ -29,6 +29,8 @@ const useOutputTableData = (
2929
const { t } = useTranslation('timesStops');
3030

3131
const scheduleByAt: Record<string, ScheduleEntry> = keyBy(selectedTrainSchedule?.schedule, 'at');
32+
const theoreticalMargins = selectedTrainSchedule && getTheoreticalMargins(selectedTrainSchedule);
33+
3234
const startDatetime = selectedTrainSchedule
3335
? new Date(selectedTrainSchedule.start_time)
3436
: undefined;
@@ -48,8 +50,19 @@ const useOutputTableData = (
4850
computedArrival,
4951
schedule
5052
);
51-
const { theoreticalMargin, theoreticalMarginSeconds, calculatedMargin, diffMargins } =
52-
computeMargins(selectedTrainSchedule, index, pathItemTimes);
53+
const {
54+
theoreticalMargin,
55+
isTheoreticalMarginBoundary,
56+
theoreticalMarginSeconds,
57+
calculatedMargin,
58+
diffMargins,
59+
} = computeMargins(
60+
theoreticalMargins,
61+
selectedTrainSchedule,
62+
scheduleByAt,
63+
index,
64+
pathItemTimes
65+
);
5366

5467
const { theoreticalArrival, arrival, departure, refDate } = computeInputDatetimes(
5568
startDatetime,
@@ -78,6 +91,7 @@ const useOutputTableData = (
7891
onStopSignal,
7992
shortSlipDistance,
8093
theoreticalMargin,
94+
isTheoreticalMarginBoundary,
8195

8296
theoreticalMarginSeconds,
8397
calculatedMargin,

front/src/modules/timesStops/hooks/useTimeStopsColumns.tsx

+5-2
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ export const useTimeStopsColumns = <T extends TimeStopsRow>(
158158
continuousUpdates: false,
159159
placeholder: !isOutputTable ? t('theoreticalMarginPlaceholder') : '',
160160
formatBlurredInput: (value) => {
161-
if (!value || value === '0%') return '';
161+
if (!value) return '';
162162
if (!isOutputTable && !marginRegExValidation.test(value)) {
163163
return `${value}${t('theoreticalMarginPlaceholder')}`;
164164
}
@@ -168,7 +168,10 @@ export const useTimeStopsColumns = <T extends TimeStopsRow>(
168168
})
169169
),
170170
cellClassName: ({ rowData }) =>
171-
cx({ invalidCell: !isOutputTable && !rowData.isMarginValid }),
171+
cx({
172+
invalidCell: !isOutputTable && !rowData.isMarginValid,
173+
repeatedValue: rowData.isTheoreticalMarginBoundary === false, // the class should be added on false but not undefined
174+
}),
172175
title: t('theoreticalMargin'),
173176
headerClassName: 'padded-header',
174177
minWidth: 100,

front/src/modules/timesStops/styles/_timesStopsDatasheet.scss

+4
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@
2323
color: red !important;
2424
}
2525

26+
.repeatedValue {
27+
color: var(--grey30);
28+
}
29+
2630
.warning-schedule {
2731
background: var(--warning30);
2832
}

front/src/modules/timesStops/types.ts

+6
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ export type TimeStopsRow = {
2121
onStopSignal?: boolean;
2222
shortSlipDistance?: boolean;
2323
theoreticalMargin?: string; // value asked by user
24+
isTheoreticalMarginBoundary?: boolean; // tells whether the theoreticalMargin value was inputted for this line or if it is repeated from a previous line
2425

2526
theoreticalMarginSeconds?: string;
2627
calculatedMargin?: string;
@@ -39,3 +40,8 @@ export enum TableType {
3940
}
4041

4142
export type ScheduleEntry = ArrayElement<TrainScheduleResult['schedule']>;
43+
44+
export type TheoreticalMarginsRecord = Record<
45+
string,
46+
{ theoreticalMargin: string; isBoundary: boolean }
47+
>;

front/tests/assets/operationStudies/simulationSettings/allSettings.json

+8-8
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
"signalReceptionClosed": false,
99
"margin": {
1010
"theoretical": "5 %",
11-
"theoreticalS": "61 s",
12-
"actual": "61 s",
11+
"theoreticalS": "85 s",
12+
"actual": "85 s",
1313
"difference": "0 s"
1414
},
1515
"calculatedArrival": "11:22:40",
@@ -24,9 +24,9 @@
2424
"signalReceptionClosed": false,
2525
"margin": {
2626
"theoretical": "",
27-
"theoreticalS": "85 s",
28-
"actual": "85 s",
29-
"difference": "0 s"
27+
"theoreticalS": "",
28+
"actual": "",
29+
"difference": ""
3030
},
3131
"calculatedArrival": "11:44:13",
3232
"calculatedDeparture": ""
@@ -39,9 +39,9 @@
3939
"stopTime": "124",
4040
"signalReceptionClosed": false,
4141
"margin": {
42-
"theoretical": "",
43-
"theoreticalS": "117 s",
44-
"actual": "117 s",
42+
"theoretical": "5 %",
43+
"theoreticalS": "32 s",
44+
"actual": "32 s",
4545
"difference": "0 s"
4646
},
4747
"calculatedArrival": "11:52:55",

0 commit comments

Comments
 (0)