Skip to content

Commit

Permalink
front: fix bug calling pathfinding twice
Browse files Browse the repository at this point in the history
Signed-off-by: Valentin Chanas <[email protected]>
  • Loading branch information
anisometropie committed Nov 21, 2024
1 parent 1cec9ef commit 9459907
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 5 deletions.
33 changes: 30 additions & 3 deletions front/src/modules/pathfinding/hooks/usePathfinding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type { PathStep } from 'reducers/osrdconf/types';
import { useAppDispatch } from 'store';
import { isEmptyArray } from 'utils/array';
import { castErrorToFailure } from 'utils/error';
import { usePrevious } from 'utils/hooks/state';

import useInfraStatus from './useInfraStatus';

Expand Down Expand Up @@ -150,17 +151,28 @@ export const usePathfinding = (
) => {
const { t } = useTranslation(['operationalStudies/manageTrainSchedule']);
const dispatch = useAppDispatch();
const { getInfraID, getOrigin, getDestination, getVias, getPathSteps, getPowerRestriction } =
useOsrdConfSelectors();
const {
getInfraID,
getOrigin,
getDestination,
getVias,
getPathSteps,
getPowerRestriction,
getPathStepsCompletedWithPFResult,
} = useOsrdConfSelectors();
const infraId = useSelector(getInfraID, isEqual);
const origin = useSelector(getOrigin, isEqual);
const destination = useSelector(getDestination, isEqual);
const vias = useSelector(getVias(), isEqual);
const pathSteps = useSelector(getPathSteps);
const pathStepsCompletedWithPF = useSelector(getPathStepsCompletedWithPFResult);
const powerRestrictions = useSelector(getPowerRestriction);
const { infra, reloadCount, setIsInfraError } = useInfraStatus();
const { rollingStock } = useStoreDataForRollingStockSelector();

const previousOrigin = usePrevious(origin);
const previousDestination = usePrevious(destination);

const initializerArgs = {
origin,
destination,
Expand Down Expand Up @@ -196,6 +208,11 @@ export const usePathfinding = (
};

useEffect(() => {
// ignore update made automatically to vias from pathfinding result
// we want only the changes asked manually by user
if (pathStepsCompletedWithPF) {
return;
}
if (isPathfindingInitialized) {
pathfindingDispatch({
type: 'VIAS_CHANGED',
Expand All @@ -209,6 +226,12 @@ export const usePathfinding = (
}, [vias]);

useEffect(() => {
if (
(previousOrigin !== origin || previousDestination !== destination) &&
pathStepsCompletedWithPF
) {
return;
}
if (isPathfindingInitialized) {
pathfindingDispatch({
type: 'PATHFINDING_PARAM_CHANGED',
Expand Down Expand Up @@ -339,7 +362,11 @@ export const usePathfinding = (
);
}
dispatch(
updatePathSteps({ pathSteps: updatedPathSteps, resetPowerRestrictions: true })
updatePathSteps({
pathSteps: updatedPathSteps,
resetPowerRestrictions: true,
completedWithPFResult: true,
})
);

const allWaypoints = upsertPathStepsInOPs(
Expand Down
10 changes: 10 additions & 0 deletions front/src/reducers/osrdconf/osrdConfCommon/__tests__/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
const state = store.getState()[slice.name];
expect(state.pathSteps[1]?.stopFor).toEqual('PT60S');
expect(state.pathSteps[1]?.stopType).toEqual('serviceStop');
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

it('should handle updateFeatureInfoClick', () => {
Expand Down Expand Up @@ -237,6 +238,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
defaultStore.dispatch(slice.actions.updatePathSteps({ pathSteps }));
const state = defaultStore.getState()[slice.name];
expect(state.pathSteps).toEqual(pathSteps);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

it('should handle updateOrigin', () => {
Expand All @@ -247,6 +249,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
defaultStore.dispatch(slice.actions.updateOrigin(newOrigin));
const state = defaultStore.getState()[slice.name];
expect(state.pathSteps[0]).toEqual(newOrigin);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

it('should handle updateDestination', () => {
Expand All @@ -255,6 +258,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
defaultStore.dispatch(slice.actions.updateDestination(newDestination));
const state = defaultStore.getState()[slice.name];
expect(last(state.pathSteps)).toEqual(newDestination);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

it('should handle deleteItinerary', () => {
Expand All @@ -265,6 +269,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
store.dispatch(slice.actions.deleteItinerary());
const state = store.getState()[slice.name];
expect(state.pathSteps).toEqual([null, null]);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

it('should handle clearVias', () => {
Expand All @@ -275,6 +280,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
store.dispatch(slice.actions.clearVias());
const state = store.getState()[slice.name];
expect(state.pathSteps).toEqual([pathSteps[0], last(pathSteps)]);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

it('should handle deleteVia', () => {
Expand All @@ -285,6 +291,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
store.dispatch(slice.actions.deleteVia(0));
const state = store.getState()[slice.name];
expect(state.pathSteps).toEqual(removeElementAtIndex(pathSteps, 1));
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

describe('should handle addVia', () => {
Expand All @@ -305,6 +312,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
store.dispatch(slice.actions.addVia({ newVia: paris, pathProperties }));
const state = store.getState()[slice.name];
expect(state.pathSteps).toStrictEqual([null, parisNoPosition, strasbourg]);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

it('should handle insertion for a route with no existing via and no destination', () => {
Expand Down Expand Up @@ -372,6 +380,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
store.dispatch(slice.actions.moveVia(pathSteps, 0, 2));
const state = store.getState()[slice.name];
expect(state.pathSteps).toStrictEqual([brest, lemans, paris, rennes, strasbourg]);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});

describe('should handle upsertViaFromSuggestedOP', () => {
Expand Down Expand Up @@ -442,6 +451,7 @@ const testCommonConfReducers = (slice: OperationalStudiesConfSlice | StdcmConfSl
store.dispatch(slice.actions.upsertViaFromSuggestedOP(newVia));
const state = store.getState()[slice.name];
expect(state.pathSteps).toEqual([brest, rennes, updatedVia, paris, strasbourg]);
expect(state.pathStepsCompletedWithPFResult).toEqual(false);
});
});

Expand Down
24 changes: 22 additions & 2 deletions front/src/reducers/osrdconf/osrdConfCommon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const defaultCommonConf: OsrdConfState = {
featureInfoClick: { displayPopup: false },
// Corresponds to origin and destination not defined
pathSteps: [null, null],
pathStepsCompletedWithPFResult: false,
rollingStockComfort: 'STANDARD' as const,
startTime: new Date().toISOString(),
};
Expand Down Expand Up @@ -75,7 +76,11 @@ interface CommonConfReducers<S extends OsrdConfState> extends InfraStateReducers
['updateFeatureInfoClick']: CaseReducer<S, PayloadAction<S['featureInfoClick']>>;
['updatePathSteps']: CaseReducer<
S,
PayloadAction<{ pathSteps: S['pathSteps']; resetPowerRestrictions?: boolean }>
PayloadAction<{
pathSteps: S['pathSteps'];
resetPowerRestrictions?: boolean;
completedWithPFResult?: boolean;
}>
>;
['deleteItinerary']: CaseReducer<S>;
['clearVias']: CaseReducer<S>;
Expand Down Expand Up @@ -165,6 +170,7 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
}
return pathStep;
});
state.pathStepsCompletedWithPFResult = false;
},
updateGridMarginBefore(state: Draft<S>, action: PayloadAction<S['gridMarginBefore']>) {
state.gridMarginBefore = action.payload;
Expand All @@ -178,23 +184,31 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
},
updatePathSteps(
state: Draft<S>,
action: PayloadAction<{ pathSteps: S['pathSteps']; resetPowerRestrictions?: boolean }>
action: PayloadAction<{
pathSteps: S['pathSteps'];
resetPowerRestrictions?: boolean;
completedWithPFResult?: boolean;
}>
) {
state.pathSteps = action.payload.pathSteps;
state.pathStepsCompletedWithPFResult = action.payload.completedWithPFResult || false;
if (action.payload.resetPowerRestrictions) {
state.powerRestriction = [];
}
},
deleteItinerary(state: Draft<S>) {
state.pathSteps = [null, null];
state.pathStepsCompletedWithPFResult = false;
},
clearVias(state: Draft<S>) {
state.pathSteps = [state.pathSteps[0], state.pathSteps[state.pathSteps.length - 1]];
state.pathStepsCompletedWithPFResult = false;
},
// Use this action in the via list, not the suggested op list
deleteVia(state: Draft<S>, action: PayloadAction<number>) {
// Index takes count of the origin in the array
state.pathSteps = removeElementAtIndex(state.pathSteps, action.payload + 1);
state.pathStepsCompletedWithPFResult = false;
},
// Use this action only to via added by click on map
addVia(
Expand All @@ -209,10 +223,12 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
action.payload.newVia,
action.payload.pathProperties
);
state.pathStepsCompletedWithPFResult = false;
},
moveVia: {
reducer: (state: Draft<S>, action: PayloadAction<S['pathSteps']>) => {
state.pathSteps = action.payload;
state.pathStepsCompletedWithPFResult = false;
},
prepare: (vias: S['pathSteps'], from: number, to: number) => {
const newVias = Array.from(vias);
Expand All @@ -227,11 +243,13 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
// from the suggested via modal
upsertViaFromSuggestedOP(state: Draft<S>, action: PayloadAction<SuggestedOP>) {
upsertPathStep(state.pathSteps, action.payload);
state.pathStepsCompletedWithPFResult = false;
},
upsertSeveralViasFromSuggestedOP(state: Draft<S>, action: PayloadAction<SuggestedOP[]>) {
action.payload.forEach((suggestedOp) => {
upsertPathStep(state.pathSteps, suggestedOp);
});
state.pathStepsCompletedWithPFResult = false;
},
updateRollingStockComfort(state: Draft<S>, action: PayloadAction<S['rollingStockComfort']>) {
state.rollingStockComfort = action.payload;
Expand All @@ -249,6 +267,7 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
}
: null;
state.pathSteps = updateOriginPathStep(state.pathSteps, newPoint, true);
state.pathStepsCompletedWithPFResult = false;
},
updateDestination(state: Draft<S>, action: PayloadAction<ArrayElement<S['pathSteps']>>) {
const prevDestinationArrivalType = state.pathSteps.at(-1)?.arrivalType;
Expand All @@ -260,6 +279,7 @@ export function buildCommonConfReducers<S extends OsrdConfState>(): CommonConfRe
}
: null;
state.pathSteps = updateDestinationPathStep(state.pathSteps, newPoint, true);
state.pathStepsCompletedWithPFResult = false;
},
};
}
Expand Down
1 change: 1 addition & 0 deletions front/src/reducers/osrdconf/osrdConfCommon/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const buildCommonConfSelectors = <ConfState extends OsrdConfState>(
getPowerRestriction: makeOsrdConfSelector('powerRestriction'),
getFeatureInfoClick: makeOsrdConfSelector('featureInfoClick'),
getPathSteps,
getPathStepsCompletedWithPFResult: makeOsrdConfSelector('pathStepsCompletedWithPFResult'),
getOrigin: (state: RootState) => {
const pathSteps = getPathSteps(state);
return pathSteps[0];
Expand Down
1 change: 1 addition & 0 deletions front/src/reducers/osrdconf/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface OsrdConfState extends InfraState {
gridMarginAfter?: number;
featureInfoClick: { displayPopup: boolean; feature?: Feature; coordinates?: number[] };
pathSteps: (PathStep | null)[];
pathStepsCompletedWithPFResult: boolean;
rollingStockComfort: Comfort;
// Format ISO 8601
startTime: string;
Expand Down

0 comments on commit 9459907

Please sign in to comment.