From cd366a119563eec97c68d66e3b21dc6b67a3dc76 Mon Sep 17 00:00:00 2001 From: Loup Theron Date: Thu, 28 Mar 2024 11:44:56 +0100 Subject: [PATCH 1/3] Prevent usage of index to update mission actions --- .../MissionForm/__tests__/utils.test.ts | 104 ++++++++++++++++-- .../Mission/components/MissionForm/utils.tsx | 22 ++-- .../saveMissionAndMissionActionsByDiff.ts | 6 +- 3 files changed, 104 insertions(+), 28 deletions(-) diff --git a/frontend/src/features/Mission/components/MissionForm/__tests__/utils.test.ts b/frontend/src/features/Mission/components/MissionForm/__tests__/utils.test.ts index ddf66875db..2e9ce0bace 100644 --- a/frontend/src/features/Mission/components/MissionForm/__tests__/utils.test.ts +++ b/frontend/src/features/Mission/components/MissionForm/__tests__/utils.test.ts @@ -1,12 +1,12 @@ import { expect } from '@jest/globals' import { MissionAction } from '../../../missionAction.types' -import { getMissionActionsDataFromMissionActionsFormValues } from '../utils' +import { getMissionActionsToCreateUpdateOrDelete } from '../utils' import MissionActionType = MissionAction.MissionActionType describe('features/Mission/components/MissionForm/utils.getMissionActionsDataFromMissionActionsFormValues()', () => { - it('Should add the id to an updated action', () => { + it('Should delete a previous action delete from the list and create a new action', () => { // Given const missionId = 123 const actionsFormValues = [ @@ -171,22 +171,104 @@ describe('features/Mission/components/MissionForm/utils.getMissionActionsDataFro ] // When - const { deletedMissionActionIds, updatedMissionActionDatas } = getMissionActionsDataFromMissionActionsFormValues( + const { createdOrUpdatedMissionActions, deletedMissionActionIds } = getMissionActionsToCreateUpdateOrDelete( missionId, actionsFormValues, originalMissionActions ) // Then - expect(deletedMissionActionIds).toHaveLength(0) - expect(updatedMissionActionDatas).toHaveLength(1) - expect(updatedMissionActionDatas[0]?.id).toEqual(20) + expect(deletedMissionActionIds).toHaveLength(1) + expect(deletedMissionActionIds[0]).toEqual(20) + expect(createdOrUpdatedMissionActions).toHaveLength(1) + expect(createdOrUpdatedMissionActions[0]?.id).toEqual(undefined) }) it('Should get deleted actions When the action is no more in the action form list', () => { // Given const missionId = 123 - const actionsFormValues = [] + const actionsFormValues = [ + { + actionDatetimeUtc: '2023-12-08T08:27:00Z', + actionType: MissionActionType.SEA_CONTROL, + closedBy: undefined, + controlQualityComments: undefined, + controlUnits: [], + districtCode: 'AY', + emitsAis: undefined, + emitsVms: undefined, + externalReferenceNumber: 'DONTSINK', + facade: 'NAMO', + faoAreas: ['27.8.b', '27.8.c'], + feedbackSheetRequired: false, + flagState: 'FR', + flightGoals: [], + gearInfractions: [], + gearOnboard: [ + { + comments: undefined, + controlledMesh: undefined, + declaredMesh: 70.0, + gearCode: 'OTB', + gearName: 'Chaluts de fond à panneaux', + gearWasControlled: undefined, + hasUncontrolledMesh: false + } + ], + hasSomeGearsSeized: false, + hasSomeSpeciesSeized: false, + id: 20, + internalReferenceNumber: 'FAK000999999', + ircs: 'CALLME', + isAdministrativeControl: undefined, + isComplianceWithWaterRegulationsControl: undefined, + isFromPoseidon: false, + isSafetyEquipmentAndStandardsComplianceControl: undefined, + isSeafarersControl: undefined, + isValid: true, + latitude: 47.648401281163814, + licencesAndLogbookObservations: undefined, + licencesMatchActivity: undefined, + logbookInfractions: [], + logbookMatchesActivity: undefined, + longitude: -4.281934312813745, + missionId: 43, + numberOfVesselsFlownOver: undefined, + otherComments: undefined, + otherInfractions: [], + portLocode: undefined, + portName: undefined, + segments: [{ segment: 'SWW01/02/03', segmentName: 'Bottom trawls' }], + seizureAndDiversion: false, + seizureAndDiversionComments: undefined, + separateStowageOfPreservedSpecies: undefined, + speciesInfractions: [], + speciesObservations: undefined, + speciesOnboard: [ + { + controlledWeight: undefined, + declaredWeight: 471.2, + nbFish: undefined, + speciesCode: 'HKE', + underSized: false + }, + { + controlledWeight: undefined, + declaredWeight: 13.46, + nbFish: undefined, + speciesCode: 'BLI', + underSized: false + } + ], + speciesSizeControlled: undefined, + speciesWeightControlled: undefined, + unitWithoutOmegaGauge: false, + userTrigram: 'LT', + vesselId: 1, + vesselName: 'PHENOMENE', + vesselTargeted: undefined + } + ] const originalMissionActions = [ { actionDatetimeUtc: '2023-12-08T08:27:00Z', @@ -270,15 +352,15 @@ describe('features/Mission/components/MissionForm/utils.getMissionActionsDataFro ] // When - const { deletedMissionActionIds, updatedMissionActionDatas } = getMissionActionsDataFromMissionActionsFormValues( + const { createdOrUpdatedMissionActions, deletedMissionActionIds } = getMissionActionsToCreateUpdateOrDelete( missionId, actionsFormValues, originalMissionActions ) // Then - expect(deletedMissionActionIds).toHaveLength(1) - expect(deletedMissionActionIds).toEqual([20]) - expect(updatedMissionActionDatas).toHaveLength(0) + expect(deletedMissionActionIds).toHaveLength(0) + expect(createdOrUpdatedMissionActions).toHaveLength(1) + expect(createdOrUpdatedMissionActions[0]?.id).toEqual(20) }) }) diff --git a/frontend/src/features/Mission/components/MissionForm/utils.tsx b/frontend/src/features/Mission/components/MissionForm/utils.tsx index bd6e7a8a26..2ffbe8deac 100644 --- a/frontend/src/features/Mission/components/MissionForm/utils.tsx +++ b/frontend/src/features/Mission/components/MissionForm/utils.tsx @@ -18,35 +18,33 @@ import MissionActionType = MissionAction.MissionActionType * @param actionsFormValues * @param originalMissionActions Mission actions as they were previous to the mission edition */ -export function getMissionActionsDataFromMissionActionsFormValues( +export function getMissionActionsToCreateUpdateOrDelete( missionId: MissionAction.MissionAction['missionId'], actionsFormValues: MissionActionFormValues[], originalMissionActions: MissionAction.MissionAction[] = [] ): { + createdOrUpdatedMissionActions: MissionAction.MissionActionData[] deletedMissionActionIds: number[] - updatedMissionActionDatas: MissionAction.MissionActionData[] } { - const updatedMissionActionDatas = actionsFormValues.map((missionActionFormValues, index) => - getMissionActionDataFromFormValues(missionActionFormValues, missionId, originalMissionActions, index) + const createdOrUpdatedMissionActions = actionsFormValues.map(missionActionFormValues => + getMissionActionDataFromFormValues(missionActionFormValues, missionId) ) const originalMissionActionIds = originalMissionActions.map(({ id }) => id) - const updatedMissionActionIds = updatedMissionActionDatas + const updatedMissionActionIds = createdOrUpdatedMissionActions .filter(({ id }) => typeof id === 'number') .map(({ id }) => id as number) const deletedMissionActionIds = difference(originalMissionActionIds, updatedMissionActionIds) return { - deletedMissionActionIds, - updatedMissionActionDatas + createdOrUpdatedMissionActions, + deletedMissionActionIds } } export function getMissionActionDataFromFormValues( missionActionFormValues: MissionActionFormValues, - missionId: MissionAction.MissionAction['missionId'], - originalMissionActions: MissionAction.MissionAction[] = [], - index?: number + missionId: MissionAction.MissionAction['missionId'] ) { const missionActionFormValuesWithAllProps = { ...MISSION_ACTION_FORM_VALUES_SKELETON, @@ -56,12 +54,8 @@ export function getMissionActionDataFromFormValues( const maybeValidMissionActionData = omit(missionActionFormValuesWithAllProps, ['isValid', 'isVesselUnknown']) const validMissionActionData = getValidMissionActionData(maybeValidMissionActionData as MissionActionFormValues) - // We get the action `id` to know if the action is an update - const id = index !== undefined ? originalMissionActions[index]?.id : missionActionFormValues.id - return { ...validMissionActionData, - id, missionId } } diff --git a/frontend/src/features/Mission/useCases/saveMissionAndMissionActionsByDiff.ts b/frontend/src/features/Mission/useCases/saveMissionAndMissionActionsByDiff.ts index e1d17622fd..206ba15084 100644 --- a/frontend/src/features/Mission/useCases/saveMissionAndMissionActionsByDiff.ts +++ b/frontend/src/features/Mission/useCases/saveMissionAndMissionActionsByDiff.ts @@ -1,7 +1,7 @@ import { RTK_FORCE_REFETCH_QUERY_OPTIONS } from '@api/constants' import { missionActionApi } from '@api/missionAction' import { missionFormActions } from '@features/Mission/components/MissionForm/slice' -import { getMissionActionsDataFromMissionActionsFormValues } from '@features/Mission/components/MissionForm/utils' +import { getMissionActionsToCreateUpdateOrDelete } from '@features/Mission/components/MissionForm/utils' import { monitorfishMissionApi } from '@features/Mission/monitorfishMissionApi' import { saveMission } from '@features/Mission/useCases/saveMission' import { logSoftError } from '@mtes-mct/monitor-ui' @@ -25,7 +25,7 @@ export const saveMissionAndMissionActionsByDiff = const currentMissionWithActions = await dispatch( monitorfishMissionApi.endpoints.getMission.initiate(savedMission.id, RTK_FORCE_REFETCH_QUERY_OPTIONS) ).unwrap() - const { deletedMissionActionIds, updatedMissionActionDatas } = getMissionActionsDataFromMissionActionsFormValues( + const { createdOrUpdatedMissionActions, deletedMissionActionIds } = getMissionActionsToCreateUpdateOrDelete( savedMission.id, actionsFormValues, currentMissionWithActions.actions @@ -35,7 +35,7 @@ export const saveMissionAndMissionActionsByDiff = ...deletedMissionActionIds.map(async missionActionId => { await dispatch(missionActionApi.endpoints.deleteMissionAction.initiate(missionActionId)).unwrap() }), - ...updatedMissionActionDatas.map(async missionActionData => { + ...createdOrUpdatedMissionActions.map(async missionActionData => { if (missionActionData.id === undefined) { await dispatch(missionActionApi.endpoints.createMissionAction.initiate(missionActionData)).unwrap() } else { From 70e2c4f0b698f645d78aba3a4fb856aee5ed1ded Mon Sep 17 00:00:00 2001 From: Loup Theron Date: Thu, 28 Mar 2024 11:49:32 +0100 Subject: [PATCH 2/3] Remove duplicate action --- .../mission_form/action_list.spec.ts | 82 ------------------- .../MissionForm/ActionList/FishActionCard.tsx | 12 +-- .../MissionForm/ActionList/index.tsx | 3 - .../Mission/components/MissionForm/index.tsx | 35 -------- 4 files changed, 1 insertion(+), 131 deletions(-) diff --git a/frontend/cypress/e2e/side_window/mission_form/action_list.spec.ts b/frontend/cypress/e2e/side_window/mission_form/action_list.spec.ts index a8b787ee9b..547162af24 100644 --- a/frontend/cypress/e2e/side_window/mission_form/action_list.spec.ts +++ b/frontend/cypress/e2e/side_window/mission_form/action_list.spec.ts @@ -34,88 +34,6 @@ context('Side Window > Mission Form > Action List', () => { cy.get('*[data-cy="action-list-item"]').eq(0).should('contain', 'Une observation.') }) - it('Should send the expected data to the API when duplicating a mission action', () => { - editSideWindowMissionListMissionWithId(4, SeaFrontGroup.MEMN) - - cy.intercept('POST', '/api/v1/missions/4', { - body: { - id: 1 - }, - statusCode: 201 - }).as('updateMission4') - cy.intercept('POST', '/bff/v1/mission_actions').as('createMissionAction') - cy.get('*[data-cy="action-list-item"]').click() - - cy.wait(250) - - cy.clickButton('Dupliquer l’action') - - cy.wait(250) - - cy.waitForLastRequest( - '@createMissionAction', - { - body: { - actionType: 'SEA_CONTROL', - closedBy: null, - controlQualityComments: null, - controlUnits: [], - emitsAis: null, - emitsVms: 'NOT_APPLICABLE', - externalReferenceNumber: null, - facade: 'MEMN', - faoAreas: ['27.8.a'], - feedbackSheetRequired: false, - flagState: 'FR', - flightGoals: [], - gearInfractions: [], - gearOnboard: [], - hasSomeGearsSeized: false, - hasSomeSpeciesSeized: false, - id: null, - internalReferenceNumber: 'U_W0NTFINDME', - ircs: null, - latitude: 53.35, - licencesAndLogbookObservations: null, - licencesMatchActivity: 'NOT_APPLICABLE', - logbookInfractions: [], - logbookMatchesActivity: 'NOT_APPLICABLE', - longitude: -10.85, - missionId: 4, - numberOfVesselsFlownOver: null, - otherComments: 'Commentaires post contrôle', - otherInfractions: [], - portLocode: null, - segments: [], - seizureAndDiversion: false, - seizureAndDiversionComments: null, - separateStowageOfPreservedSpecies: 'NO', - speciesInfractions: [], - speciesObservations: null, - speciesOnboard: [], - speciesSizeControlled: null, - speciesWeightControlled: null, - unitWithoutOmegaGauge: false, - userTrigram: 'JKL', - vesselId: 2, - vesselName: 'MALOTRU', - vesselTargeted: 'YES' - } - }, - 5 - ) - .its('response.statusCode') - .should('eq', 201) - - // And we delete this action - - editSideWindowMissionListMissionWithId(4, SeaFrontGroup.MEMN) - - cy.clickButton('Supprimer l’action', { index: 1 }) - - cy.wait(250) - }) - it('Should send the expected data to the API when deleting a mission action', () => { editSideWindowMissionListMissionWithId(34, SeaFrontGroup.MEMN) diff --git a/frontend/src/features/Mission/components/MissionForm/ActionList/FishActionCard.tsx b/frontend/src/features/Mission/components/MissionForm/ActionList/FishActionCard.tsx index 035fe31340..dd4dd0f4dd 100644 --- a/frontend/src/features/Mission/components/MissionForm/ActionList/FishActionCard.tsx +++ b/frontend/src/features/Mission/components/MissionForm/ActionList/FishActionCard.tsx @@ -15,10 +15,9 @@ import type { Promisable } from 'type-fest' type FishActionCardProps = Readonly<{ missionAction: MissionActionFormValues - onDuplicate: () => Promisable onRemove: () => Promisable }> -export function FishActionCard({ missionAction, onDuplicate, onRemove }: FishActionCardProps) { +export function FishActionCard({ missionAction, onRemove }: FishActionCardProps) { const natinfsAsOptions = useGetNatinfsAsOptions() const isControlAction = @@ -119,15 +118,6 @@ export function FishActionCard({ missionAction, onDuplicate, onRemove }: FishAct

{actionLabel}

- Promisable - onDuplicate: (actionIndex: number) => Promisable onRemove: (actionIndex: number) => Promisable onSelect: (actionIndex: number) => Promisable }> @@ -33,7 +32,6 @@ export function ActionList({ missionId, missionTypes = [], onAdd, - onDuplicate, onRemove, onSelect }: ActionListProps) { @@ -128,7 +126,6 @@ export function ActionList({ // eslint-disable-next-line react/no-array-index-key key={index} missionAction={action as MissionActionFormValues} - onDuplicate={() => onDuplicate(action.index!!)} onRemove={() => onRemove(action.index!!)} /> diff --git a/frontend/src/features/Mission/components/MissionForm/index.tsx b/frontend/src/features/Mission/components/MissionForm/index.tsx index c2bb881dfd..2ac2d88183 100644 --- a/frontend/src/features/Mission/components/MissionForm/index.tsx +++ b/frontend/src/features/Mission/components/MissionForm/index.tsx @@ -26,7 +26,6 @@ import { import { assertNotNullish } from '@utils/assertNotNullish' import { getMissionStatus } from 'domain/entities/mission/utils' import { SideWindowMenuKey } from 'domain/entities/sideWindow/constants' -import { omit } from 'lodash/fp' import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import styled from 'styled-components' import { FrontendErrorBoundary } from 'ui/FrontendErrorBoundary' @@ -334,39 +333,6 @@ export function MissionForm() { ] ) - const duplicateAction = useCallback( - async (actionIndex: number) => { - /** - * If a debounce function is not yet executed, stop there to avoid race condition. - * /!\ This can leads to save the debounced action update to the wrong action index - */ - if (updateEditedActionFormValues.isPending()) { - setTimeout(() => duplicateAction(actionIndex), DEBOUNCE_DELAY) - - return - } - - const actionCopy: MissionActionFormValues = omit(['id'], actionsFormValues[actionIndex]) - setEditedActionIndex(0) - - const createdId = await dispatch( - autoSaveMissionAction(actionCopy, missionIdRef.current, mainFormValues.isClosed, isAutoSaveEnabled) - ) - - const nextActionsWithIdFormValues = [{ ...actionCopy, id: createdId }, ...actionsFormValues] - setActionsFormValues(nextActionsWithIdFormValues) - updateReduxSliceDraft() - }, - [ - dispatch, - updateEditedActionFormValues, - updateReduxSliceDraft, - actionsFormValues, - mainFormValues.isClosed, - isAutoSaveEnabled - ] - ) - const updateEditedActionIndex = useCallback( (nextActionIndex: number | undefined) => { /** @@ -488,7 +454,6 @@ export function MissionForm() { missionId={missionIdRef.current} missionTypes={mainFormValues.missionTypes} onAdd={addAction} - onDuplicate={duplicateAction} onRemove={removeAction} onSelect={updateEditedActionIndex} /> From 5ca7b5c0169ccdd46adb4cfbbe616c5ec0962102 Mon Sep 17 00:00:00 2001 From: Loup Theron Date: Thu, 28 Mar 2024 14:09:20 +0100 Subject: [PATCH 3/3] Add non-regression cypress test --- .../mission_form/sea_control_edition.spec.ts | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/frontend/cypress/e2e/side_window/mission_form/sea_control_edition.spec.ts b/frontend/cypress/e2e/side_window/mission_form/sea_control_edition.spec.ts index ccd81e8be1..89cd151747 100644 --- a/frontend/cypress/e2e/side_window/mission_form/sea_control_edition.spec.ts +++ b/frontend/cypress/e2e/side_window/mission_form/sea_control_edition.spec.ts @@ -266,6 +266,82 @@ context('Side Window > Mission Form > Sea Control Edition', () => { } ) + /** + * Non-regression test to prevent mission actions IDs to be modified instead of update the right ID. + * + * Bug case: + * 1. The first control in this test has the id `4` + * 2. A new control is added + * 3. We save the form + * 3. The bug was sending : + * - 1 POST request with the data of the first control (instead of an update of the id `4`) + * - 1 PUT request with the date of the new control (instead of a creation with a POST request) + */ + it( + 'Should update actions and keep existing actions ids', + { + env: { + FRONTEND_MISSION_FORM_AUTO_SAVE_ENABLED: false + } + }, + () => { + cy.intercept('PUT', '/bff/v1/mission_actions/4', { + body: { + id: 4 + }, + statusCode: 201 + }).as('updateMissionAction') + cy.intercept('POST', '/bff/v1/mission_actions', { + body: { + id: 5 + }, + statusCode: 201 + }).as('createMissionAction') + + // ------------------------------------------------------------------------- + // Form + const endDate = getUtcDateInMultipleFormats(customDayjs().utc().add(7, 'day').toISOString()) + cy.fill('Fin de mission', endDate.utcDateTupleWithTime) + + // Add another control + cy.clickButton('Ajouter') + cy.clickButton('Ajouter un contrôle en mer') + + // Navire + // TODO Handle Automplete in custom `cy.fill()` command once it's used via monitor-ui. + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + cy.get('input[placeholder="Rechercher un navire..."]').type('FR263418260').wait(250) + cy.contains('mark', 'FR263418260').click() + + // Date et heure du contrôle + cy.fill('Date et heure du contrôle', endDate.utcDateTupleWithTime) + + // Saisi par + cy.fill('Saisi par', 'Marlin') + + // We need to wait for some time because there is a throttle on the form + cy.wait(500) + cy.clickButton('Enregistrer') + + // ------------------------------------------------------------------------- + // Request + + cy.waitForLastRequest( + '@updateMissionAction', + { + body: { + internalReferenceNumber: 'U_W0NTFINDME', + vesselName: 'MALOTRU' + } + }, + 5 + ) + .its('response.statusCode') + .should('eq', 201) + cy.wait('@createMissionAction') + } + ) + it('Should not update the mission zone When a CACEM control is newer', () => { editSideWindowMissionListMissionWithId(34, SeaFrontGroup.MEMN)