-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Mission] Pré-remplir une infraction avec les données du signalement #1198
[Mission] Pré-remplir une infraction avec les données du signalement #1198
Conversation
5714739
to
2c80e60
Compare
d16f95c
to
817b020
Compare
WalkthroughThis set of changes primarily focuses on the transition from using an enumerated type ( Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (17)
- backend/src/main/kotlin/fr/gouv/cacem/monitorenv/domain/entities/mission/envAction/envActionControl/infraction/InfractionEntity.kt (1 hunks)
- backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/api/adapters/bff/inputs/missions/MissionEnvActionControlInfractionDataInput.kt (2 hunks)
- backend/src/main/resources/db/migration/internal/V0.116__update_infraction_vessel_size_type.sql (1 hunks)
- backend/src/main/resources/db/testdata/V666.10__insert_dummy_env_actions.sql (1 hunks)
- backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/api/endpoints/bff/LegacyMissionsITests.kt (4 hunks)
- backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/database/repositories/JpaMissionRepositoryITests.kt (2 hunks)
- frontend/cypress/e2e/side_window/mission_form/attach_actions_to_reportings_in_mission.spec.ts (2 hunks)
- frontend/cypress/e2e/side_window/mission_form/delete_mission.spec.ts (2 hunks)
- frontend/cypress/e2e/side_window/mission_form/mission_actions.spec.ts (1 hunks)
- frontend/src/domain/entities/missions.ts (3 hunks)
- frontend/src/features/missions/MissionForm/ActionCards/ReportingCard.tsx (3 hunks)
- frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionForm/InfractionForm.tsx (1 hunks)
- frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionForm/InfractionFormHeaderVehicle.tsx (2 hunks)
- frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionForm/VesselTypeSelector.tsx (1 hunks)
- frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionsForm.tsx (2 hunks)
- frontend/src/features/missions/MissionForm/Schemas/Infraction.ts (2 hunks)
- frontend/src/features/missions/Missions.helpers.ts (2 hunks)
Files skipped from review due to trivial changes (1)
- frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionForm/VesselTypeSelector.tsx
Additional comments: 18
backend/src/main/kotlin/fr/gouv/cacem/monitorenv/domain/entities/mission/envAction/envActionControl/infraction/InfractionEntity.kt (1)
- 15-15: The change from
VesselSizeEnum?
toNumber?
forvesselSize
aligns with the PR's objectives to implement a new vessel size categorization system. Ensure that the handling ofvesselSize
throughout the application is compatible with this change to maintain type safety and functionality.backend/src/main/resources/db/migration/internal/V0.116__update_infraction_vessel_size_type.sql (1)
- 2-26: The SQL migration script correctly updates
vesselSize
values withinenv_actions
to align with the new vessel size categories. Ensure thorough testing on a backup of the production database to confirm the script's effectiveness and to prevent unintended side effects.frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionForm/InfractionFormHeaderVehicle.tsx (1)
- 12-16: The replacement of
VesselSizeSelector
withFormikNumberInput
for vessel size input and the addition of vessel type handling logic align with the PR's objectives and enhance the form's functionality. Ensure thorough testing to verify the user experience, focusing on the usability of theFormikNumberInput
for vessel size input.Also applies to: 30-36
backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/api/adapters/bff/inputs/missions/MissionEnvActionControlInfractionDataInput.kt (1)
- 20-20: The update of
vesselSize
toNumber?
inMissionEnvActionControlInfractionDataInput
is consistent with the PR's objectives for the new vessel size categorization system. Ensure compatibility with this change across the application to maintain data handling integrity.frontend/src/features/missions/MissionForm/Schemas/Infraction.ts (1)
- 40-40: The update of the
vesselSize
validation toYup.number().nullable()
aligns with the transition to numeric vessel sizes and is crucial for form validation. Ensure thorough testing to confirm that the validation behaves as expected and does not introduce usability issues.frontend/cypress/e2e/side_window/mission_form/attach_actions_to_reportings_in_mission.spec.ts (1)
- 128-137: The new test case for creating a control with infraction from a reporting is well-structured and follows Cypress best practices. Ensure that the
createMissionWithAttachedReportingAndAttachedAction
utility function correctly sets up the necessary preconditions for this test.frontend/src/domain/entities/missions.ts (1)
- 298-298: The change from
VesselSizeEnum
to aNumber
type forvesselSize
inNewInfraction
aligns with the PR objectives and TypeScript best practices. Ensure this change is consistently applied across the codebase where vessel sizes are handled.backend/src/main/resources/db/testdata/V666.10__insert_dummy_env_actions.sql (3)
- 8-8: The update to
vesselSize
to use numeric values in theCONTROL
action type aligns with the PR objectives. Ensure this script is thoroughly tested in a development or staging environment to confirm the correct application of changes.- 11-11: The update to
vesselSize
to numeric values and adjustments tovesselType
in anotherCONTROL
action type entry are consistent with the PR's goals. Verify the script's execution in a test environment to ensure data integrity.- 13-13: The changes to
vesselSize
andvesselType
in thisCONTROL
action type entry are in line with the PR's objectives of standardizing vessel sizes. Confirm that these updates are accurately reflected in the database after running the script.frontend/src/features/missions/Missions.helpers.ts (2)
- 14-15: The addition of
NewInfraction
andInfraction
types enhances the type safety and clarity of the code by explicitly defining the structure of infraction-related data. This is a positive change that aligns with best practices for TypeScript development.- 29-29: The modification in the
infractionFactory
function to accept an optionalPartial<Infraction>
parameter and return aNewInfraction
is a good practice. It allows for the creation of infraction objects with default values while also enabling customization through partial overrides. This approach improves code reusability and maintainability.frontend/cypress/e2e/side_window/mission_form/mission_actions.spec.ts (1)
- 43-43: Updating the assertion for
vesselSize
from comparing with a string to comparing with the number45
aligns with the changes made in the application logic, where vessel sizes are now handled as numeric values instead of strings. This change ensures that the test accurately reflects the application's behavior and is a good practice for maintaining test reliability.backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/api/endpoints/bff/LegacyMissionsITests.kt (4)
- 181-181: The change from
VesselSizeEnum.FROM_12_TO_24m
to a direct integer value23
forvesselSize
in test assertions correctly aligns with the PR's objective to transition to numeric representation for vessel sizes. This update is consistent and correctly implemented in the context of the test setup.- 447-447: The update of
vesselSize
to23
in this test assertion is consistent with the PR's objective and the new vessel size categorization system. It's important to ensure that all related test cases are similarly updated to reflect this change across the codebase.- 495-495: Changing the
vesselSize
to a direct integer value23
in this test assertion is in line with the PR's enhancements. This modification ensures that the test reflects the new way of handling vessel sizes, contributing to the overall consistency and accuracy of the test suite.- 745-745: The modification of
vesselSize
to23
here is correctly implemented, aligning with the PR's goal of transitioning to a numeric representation for vessel sizes. This change is crucial for maintaining the integrity and relevance of the test suite in light of the updated domain model.backend/src/test/kotlin/fr/gouv/cacem/monitorenv/infrastructure/database/repositories/JpaMissionRepositoryITests.kt (1)
- 862-862: The change from using
VesselSizeEnum
to directly settingvesselSize
as an integer (23
) aligns with the PR objectives to handle vessel sizes as integers for standardized categorization. This update is crucial for consistency with the new vessel size handling approach across the application. However, it's essential to ensure that all related tests and application logic are updated to reflect this change, avoiding potential mismatches or errors in data handling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionForm/InfractionFormHeaderVehicle.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/features/missions/MissionForm/ActionForm/ControlForm/InfractionForm/InfractionFormHeaderVehicle.tsx
backend/src/main/resources/db/migration/internal/V0.116__update_infraction_vessel_size_type.sql
Outdated
Show resolved
Hide resolved
WHERE | ||
action_type = 'CONTROL' | ||
AND jsonb_array_length(t.value->'infractions') > 0 | ||
AND mission_id > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
il y a un souci avec la mission 0
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on a des missions avec des id négatifs (qui viennent de Poséidon)
backend/src/main/resources/db/testdata/V666.10__insert_dummy_env_actions.sql
Outdated
Show resolved
Hide resolved
backend/src/main/resources/db/migration/internal/V0.116__update_infraction_vessel_size_type.sql
Outdated
Show resolved
Hide resolved
...s/missions/MissionForm/ActionForm/ControlForm/InfractionForm/InfractionFormHeaderVehicle.tsx
Show resolved
Hide resolved
@@ -31,8 +32,8 @@ export function InfractionsForm({ canAddInfraction, envActionIndex, form, push, | |||
} | |||
|
|||
const handleDuplicateInfraction = index => () => { | |||
const numberOfInfractions = form?.values.envActions[envActionIndex]?.infractions.length || 0 | |||
const selectedInfraction = form?.values.envActions[envActionIndex]?.infractions[index] as Infraction | |||
const numberOfInfractions = infractions.length || 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
j'ai l'impression que infractions
n'est pas typé. En l'occurence, je pense qu'il faut écrire infractions?.length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
je type la variable plutôt et met un tableau vide par défaut:
const infractions: Array<Infraction> = form?.values.envActions[envActionIndex]?.infractions ?? []
c6f4f53
to
e19fe32
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK pour la partie migration, testé sur les données de prod sur les cas de contrôles avec :
- une infraction avec
vesselSize=null
- une infraction avec
vesselSize=FROM_24_TO_46m
- deux infractions avec
vesselSize
identiques - deux infractions avec
vesselSize
différents
Le résultat est OK dans tous les cas.
Lorsqu'un signalement est rattaché à une mission on peut créer un contrôle depuis ce signalement. Certaines données étaient déjà recopiées du signalement au contrôle, j'ai ajouté le nom, l'immatriculation et la taille du navire.
Si plusieurs cibles ont été renseignées dans le signalement on créé plusieurs infractions pré-remplies avec les infos des cibles.
Migration pour mettre à jour les tailles de navires dans une infraction (validé avec le CACEM):
Related Pull Requests & Issues
Summary by CodeRabbit
New Features
Refactor
vesselSize
property from an enum to a numeric value across various components and tests to improve flexibility in specifying vessel sizes.Bug Fixes
Style
data-cy
attributes to specific form components for improved testability.