Skip to content

[Thématiques et tag] Refacto des plans de controles et ajout des themes #2234

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Apr 24, 2025
Merged

[Thématiques et tag] Refacto des plans de controles et ajout des themes #2234

merged 3 commits into from
Apr 24, 2025

Conversation

maximeperraultdev
Copy link
Collaborator

Related Pull Requests & Issues

  • Tests E2E (Cypress)

@maximeperraultdev maximeperraultdev changed the base branch from main to maxime/feat/2170/add_tags_to_vigilance_area April 11, 2025 07:54
@maximeperraultdev maximeperraultdev force-pushed the maxime/feat/2170/add_tags_to_vigilance_area branch from 56babee to 5646e0c Compare April 11, 2025 08:18
@maximeperraultdev maximeperraultdev force-pushed the maxime/feat/2170/refacto_control_plan_themes branch 3 times, most recently from 872aa09 to 40c504d Compare April 15, 2025 15:29
claire2212
claire2212 reviewed Apr 16, 2025
View reviewed changes
...rc/main/kotlin/fr/gouv/cacem/monitorenv/domain/entities/mission/envAction/EnvActionEntity.kt
@@ -35,7 +36,6 @@ abstract class EnvActionEntity(
open val isComplianceWithWaterRegulationsControl: Boolean? = null,
open val isSafetyEquipmentAndStandardsComplianceControl: Boolean? = null,
open val isSeafarersControl: Boolean? = null,
open val missionId: Int? = null,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pourquoi le supprimer?

...end/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/database/model/EnvActionModel.kt
)
when (action) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

un if .... else n'est pas suffisant?
si c'est une note alors sinon ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est un peu kiff-kiff. Le when sert à couvrir le cas par défaut. Je l'ai viré par compréhension.

...v/cacem/monitorenv/infrastructure/database/repositories/interfaces/IDBEnvActionRepository.kt
""",
nativeQuery = true,
)
fun deleteAllByIdNotInAndMissionId(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ca n'a pas l'air d'être utilisé et j'ai du mal à voir le cas d'usage

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est du code mort et oublié.

...em/monitorenv/infrastructure/database/repositories/interfaces/IDBRegulatoryAreaRepository.kt
// @Query(
// value = """
// SELECT DISTINCT r FROM RegulatoryAreaModel r
// JOIN r.tags tagReg
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a supprimer ?

frontend/src/features/Mission/hooks/useGetFilteredMissionsQuery.ts
@@ -94,6 +95,7 @@ export const useGetFilteredMissionsQuery = () => {
isMissionPartOfSelectedAdministrationNames(mission, selectedAdministrationNames) &&
isMissionPartOfSelectedControlUnitIds(mission, selectedControlUnitIds) &&
isMissionPartOfSelectedControlPlans(mission, selectedThemes) &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tu gardes ce filtre isMissionPartOfSelectedControlPlans ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui je le supprimerais toutes les notions de controlplans dans un pr a part

frontend/src/features/Mission/useCases/saveMission.ts
const sortedActions = [...values.envActions].sort((a, b) => a.id - b.id)
const sortedActions = [...values.envActions]
// .map(envAction => {
// if (envAction.id.includes('new-')) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supprimer le code commenté

...tend/src/features/Reportings/components/ReportingForm/FormComponents/ThemeSelector/index.tsx
@@ -31,6 +32,7 @@ export function ThemeSelector({ label, name }) {
}, [year])

const isVesselInformationRequested = useMemo(() => {
// TODO(02/04/2025): Be careful here
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mais c'est toi qui les mets les TODO qui te font peur ! ;-)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non je l'ai juste laissé !

frontend/src/features/Themes/useCases/getThemesAsOptions.ts
}))
.sort((a, b) => a.label.localeCompare(b.label))

export const getThemesAsOptionsCheckPicker = (themes: ThemeAPI[]): Option<number>[] =>
themes.map(theme => ({ label: theme.name, value: theme.id })).sort((a, b) => a.label.localeCompare(b.label))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

même remarque quand dans la pr précédente il me semble, mais pour moi ce n'est pas une feature et pas un useCase juste des fonctions "utiles"

backend/src/main/kotlin/fr/gouv/cacem/monitorenv/infrastructure/database/model/MissionModel.kt
@@ -406,44 +400,6 @@ class MissionModel(
updatedAtUtc = mission.updatedAtUtc?.toInstant(),
)

mission.envActions?.map {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

par curiosité, pourquoi déplacer ce code dans le JpaMissionRepository?

@claire2212
Copy link
Collaborator

Je n'ai pas pu tester le composant CheckTreePicker par contre

@maximeperraultdev maximeperraultdev marked this pull request as ready for review April 23, 2025 16:20
@maximeperraultdev maximeperraultdev force-pushed the maxime/feat/2170/add_tags_to_vigilance_area branch from 5646e0c to 624364e Compare April 24, 2025 06:43
Base automatically changed from maxime/feat/2170/add_tags_to_vigilance_area to maxime/epic/themes_tags April 24, 2025 06:43
@maximeperraultdev maximeperraultdev force-pushed the maxime/feat/2170/refacto_control_plan_themes branch from 40c504d to 47403a3 Compare April 24, 2025 06:46
@maximeperraultdev maximeperraultdev merged commit e3dce8f into maxime/epic/themes_tags Apr 24, 2025
3 of 4 checks passed
@maximeperraultdev maximeperraultdev deleted the maxime/feat/2170/refacto_control_plan_themes branch April 24, 2025 06:47
@tristanrobert
Copy link
Contributor

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
9 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@claire2212 claire2212 claire2212 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refonte des thématiques actuelles
4 participants
@maximeperraultdev @claire2212 @tristanrobert @maximeperrault