From 09ac1a7cc04fad18824f4809b74c0780d52aec74 Mon Sep 17 00:00:00 2001 From: Valentin Chanas Date: Fri, 9 Aug 2024 10:15:17 +0200 Subject: [PATCH] front: refactor upsertViaFromSuggestedOP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - using lodash pick to simplify the object construction - no need to lookup twice to find the corresponding pathStep - we don’t need to create copies of the pathSteps array since we are using immer to handle immutability for us - extracting the reducer logic in buildPathStepFromSuggestedOp to be reused --- front/src/modules/pathfinding/utils.ts | 18 +++--- front/src/reducers/osrdconf/helpers.ts | 49 ++++++++++++++++- .../reducers/osrdconf/osrdConfCommon/index.ts | 55 ++----------------- 3 files changed, 61 insertions(+), 61 deletions(-) diff --git a/front/src/modules/pathfinding/utils.ts b/front/src/modules/pathfinding/utils.ts index e33e2b27b49..081ceea1f86 100644 --- a/front/src/modules/pathfinding/utils.ts +++ b/front/src/modules/pathfinding/utils.ts @@ -157,6 +157,14 @@ export const upsertPathStepsInOPs = (ops: SuggestedOP[], pathSteps: PathStep[]): return updatedOPs; }; +export const pathStepMatchesOp = (pathStep: PathStep, op: SuggestedOP, withKP = false) => + ('uic' in pathStep && + 'ch' in pathStep && + pathStep.uic === op.uic && + pathStep.ch === op.ch && + (withKP ? pathStep.kp === op.kp : pathStep.name === op.name)) || + pathStep.id === op.opId; + /** * Check if a suggested operational point is a via. * Some OPs have same uic so we need to check also the ch (can be still not enough @@ -166,12 +174,4 @@ export const upsertPathStepsInOPs = (ops: SuggestedOP[], pathSteps: PathStep[]): * It is used in the times and stops table to check if an operational point is a via. */ export const isVia = (vias: PathStep[], op: SuggestedOP, withKP = false) => - vias.some( - (via) => - ('uic' in via && - 'ch' in via && - via.uic === op.uic && - via.ch === op.ch && - (withKP ? via.kp === op.kp : via.name === op.name)) || - via.id === op.opId - ); + vias.some((via) => pathStepMatchesOp(via, op, withKP)); diff --git a/front/src/reducers/osrdconf/helpers.ts b/front/src/reducers/osrdconf/helpers.ts index 52bd1bcbec3..b815477c43d 100644 --- a/front/src/reducers/osrdconf/helpers.ts +++ b/front/src/reducers/osrdconf/helpers.ts @@ -1,8 +1,11 @@ import { feature, point } from '@turf/helpers'; -import { last } from 'lodash'; +import { compact, last, pick } from 'lodash'; +import nextId from 'react-id-generator'; import { calculateDistanceAlongTrack } from 'applications/editor/tools/utils'; import type { ManageTrainSchedulePathProperties } from 'applications/operationalStudies/types'; +import { pathStepMatchesOp } from 'modules/pathfinding/utils'; +import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types'; import { addElementAtIndex, replaceElementAtIndex } from 'utils/array'; import { formatIsoDate } from 'utils/date'; import { sec2time, time2sec } from 'utils/timeManipulation'; @@ -142,3 +145,47 @@ export const updateDestinationPathStep = ( destination: Partial | null, replaceCompletely: boolean = false ) => updatePathStepAtIndex(pathSteps, pathSteps.length - 1, destination, replaceCompletely); + +/** + * modifies the array statePathSteps in place + */ +export function upsertPathStep(statePathSteps: (PathStep | null)[], op: SuggestedOP) { + // We know that, at this point, origin and destination are defined because pathfinding has been done + const cleanPathSteps = compact(statePathSteps); + + let newVia = { + ...pick(op, [ + 'coordinates', + 'positionOnPath', + 'name', + 'ch', + 'kp', + 'stopFor', + 'arrival', + 'locked', + 'deleted', + 'onStopSignal', + 'theoreticalMargin', + ]), + id: nextId(), + ...(op.uic + ? { uic: op.uic } + : { + track: op.track, + offset: op.offsetOnTrack, + }), + } as PathStep; + + const stepIndex = cleanPathSteps.findIndex((step) => pathStepMatchesOp(step, op)); + if (stepIndex >= 0) { + // Because of import issues, there can be multiple ops with same position on path + // To avoid updating the wrong one, we need to find the one that matches the payload + newVia = { ...newVia, id: cleanPathSteps[stepIndex].id }; // We don't need to change the id of the updated via + statePathSteps[stepIndex] = newVia; + } else { + // Because of import issues, there can be multiple ops at position 0 + // To avoid inserting a new via before the origin we need to check if the index is 0 + const index = cleanPathSteps.findIndex((step) => step.positionOnPath! >= op.positionOnPath); + statePathSteps.splice(index || 1, 0, newVia); + } +} diff --git a/front/src/reducers/osrdconf/osrdConfCommon/index.ts b/front/src/reducers/osrdconf/osrdConfCommon/index.ts index af598b17080..cb4a64762a3 100644 --- a/front/src/reducers/osrdconf/osrdConfCommon/index.ts +++ b/front/src/reducers/osrdconf/osrdConfCommon/index.ts @@ -1,11 +1,9 @@ import type { CaseReducer, PayloadAction, PrepareAction } from '@reduxjs/toolkit'; import type { Draft } from 'immer'; -import { compact, omit } from 'lodash'; -import nextId from 'react-id-generator'; +import { omit } from 'lodash'; import type { ManageTrainSchedulePathProperties } from 'applications/operationalStudies/types'; import { ArrivalTimeTypes } from 'applications/stdcmV2/types'; -import { isVia } from 'modules/pathfinding/utils'; import type { SuggestedOP } from 'modules/trainschedule/components/ManageTrainSchedule/types'; import { type InfraStateReducers, buildInfraStateReducers, infraState } from 'reducers/infra'; import { @@ -13,6 +11,7 @@ import { insertViaFromMap, updateDestinationPathStep, updateOriginPathStep, + upsertPathStep, } from 'reducers/osrdconf/helpers'; import type { OperationalStudiesConfSlice, @@ -22,7 +21,7 @@ import type { OperationalStudiesConfSelectors } from 'reducers/osrdconf/operatio import type { StdcmConfSlice, StdcmConfSliceActions } from 'reducers/osrdconf/stdcmConf'; import type { StdcmConfSelectors } from 'reducers/osrdconf/stdcmConf/selectors'; import type { OsrdConfState, PathStep } from 'reducers/osrdconf/types'; -import { addElementAtIndex, removeElementAtIndex, replaceElementAtIndex } from 'utils/array'; +import { removeElementAtIndex } from 'utils/array'; import { formatIsoDate } from 'utils/date'; import type { ArrayElement } from 'utils/types'; @@ -288,53 +287,7 @@ export function buildCommonConfReducers(): CommonConfRe // Use this action to transform an op to via from times and stop table or // from the suggested via modal upsertViaFromSuggestedOP(state: Draft, action: PayloadAction) { - // We know that, at this point, origin and destination are defined because pathfinding has been done - const pathSteps = compact(state.pathSteps); - - let newVia: PathStep = { - coordinates: action.payload.coordinates, - id: nextId(), - positionOnPath: action.payload.positionOnPath, - name: action.payload.name, - ch: action.payload.ch, - kp: action.payload.kp, - stopFor: action.payload.stopFor, - arrival: action.payload.arrival, - locked: action.payload.locked, - deleted: action.payload.deleted, - onStopSignal: action.payload.onStopSignal, - theoreticalMargin: action.payload.theoreticalMargin, - ...(action.payload.uic - ? { uic: action.payload.uic } - : { - track: action.payload.track, - offset: action.payload.offsetOnTrack, - }), - }; - - const isInVias = isVia(pathSteps, action.payload); - if (isInVias) { - // Because of import issues, there can be multiple ops with same position on path - // To avoid updating the wrong one, we need to find the one that matches the payload - const stepIndex = pathSteps.findIndex( - (step) => - ('uic' in step && - 'ch' in step && - step.uic === action.payload.uic && - step.ch === action.payload.ch && - step.name === action.payload.name) || - step.id === action.payload.opId - ); - newVia = { ...newVia, id: pathSteps[stepIndex].id }; // We don't need to change the id of the updated via - state.pathSteps = replaceElementAtIndex(state.pathSteps, stepIndex, newVia); - } else { - const index = pathSteps.findIndex( - (step) => step.positionOnPath! >= action.payload.positionOnPath - ); - // Because of import issues, there can be multiple ops at position 0 - // To avoid inserting a new via before the origin we need to check if the index is 0 - state.pathSteps = addElementAtIndex(state.pathSteps, index || 1, newVia); - } + upsertPathStep(state.pathSteps, action.payload); }, updateRollingStockComfort(state: Draft, action: PayloadAction) { state.rollingStockComfort = action.payload;