[Signalement] Lors de l'écriture d'un signalement, le raffraichissement de l'app efface les données saisies dans le formulaire #3002
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe updates focus on enhancing the reporting functionality within the frontend, specifically around creating and editing reports related to vessel activities. Changes include the introduction of new types for editable and edited reporting, improvements in form validation and state management, adjustments to UI components for better usability, and the implementation of local storage to preserve form data during app refreshes. These adjustments aim to streamline the reporting process, making it more efficient and user-friendly. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (24)
- frontend/src/api/reporting.ts (2 hunks)
- frontend/src/domain/types/reporting.ts (1 hunks)
- frontend/src/features/MainWindow.tsx (1 hunks)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx (1 hunks)
- frontend/src/features/Reporting/components/ReportingForm/schemas.ts (1 hunks)
- frontend/src/features/Reporting/components/ReportingForm/utils.ts (1 hunks)
- frontend/src/features/Reporting/components/ReportingList/constants.ts (4 hunks)
- frontend/src/features/Reporting/components/ReportingList/index.tsx (8 hunks)
- frontend/src/features/Reporting/components/VesselReportings/Current/CreateOrEditReporting.tsx (2 hunks)
- frontend/src/features/Reporting/components/VesselReportings/Current/utils.ts (1 hunks)
- frontend/src/features/Reporting/components/VesselReportings/ReportingCard.tsx (2 hunks)
- frontend/src/features/Reporting/components/VesselReportings/index.tsx (4 hunks)
- frontend/src/features/Reporting/components/VesselReportings/utils.ts (1 hunks)
- frontend/src/features/Reporting/slice.ts (2 hunks)
- frontend/src/features/Reporting/useCases/updateReporting.ts (2 hunks)
- frontend/src/features/SideWindow/Alert/AlertListAndReportingList/EditReporting.tsx (2 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/BeaconMalfunctionDetails.tsx (1 hunks)
- frontend/src/features/SideWindow/BeaconMalfunctionBoard/VesselStatusSelect.tsx (1 hunks)
- frontend/src/features/VesselSidebar/beacon_malfunctions/resume/CurrentBeaconMalfunctionBody.tsx (1 hunks)
- frontend/src/features/commonStyles/Buttons.style.jsx (1 hunks)
- frontend/src/features/map/overlays/VesselCardOverlay/VesselCard.tsx (6 hunks)
- frontend/src/features/map/overlays/VesselCardOverlay/constants.ts (1 hunks)
- frontend/src/features/map/overlays/VesselCardOverlay/index.tsx (2 hunks)
- frontend/src/hooks/useSaveInLocalStorage.tsx (1 hunks)
Files skipped from review due to trivial changes (1)
- frontend/src/features/commonStyles/Buttons.style.jsx
Additional comments: 40
frontend/src/hooks/useSaveInLocalStorage.tsx (1)
- 1-3: The implementation of
useGetInLocalStoragecustom hook looks good and aligns with the PR's objectives. Ensure that thegetLocalStorageStateutility function robustly handles exceptions related to local storage access, such as quota limits or user privacy settings that might block local storage.frontend/src/features/Reporting/components/VesselReportings/utils.ts (1)
- 7-7: The change from
nulltoundefinedfor theunitparameter ingetReportingActoraligns with standardizing optional value representation. Ensure that all related logic and function calls are updated to handle theundefinedvalue correctly.frontend/src/features/map/overlays/VesselCardOverlay/constants.ts (1)
- 27-34: The addition of
marginsWithThreeWarningconstant is a good enhancement for accommodating different UI states. Ensure that these new margin values are tested across various screen sizes and resolutions to maintain a consistent and responsive UI.frontend/src/features/Reporting/components/VesselReportings/Current/utils.ts (1)
- 4-4: The updates to imports and types, along with the addition of the
mapControlUnitsToUniqueSortedIdsAsOptionsfunction, enhance the application's architecture and improve code maintainability. Ensure the correctness of the sorting and mapping logic in the new utility function.frontend/src/features/Reporting/components/ReportingList/constants.ts (1)
- 4-4: The update to the import path for
TableOptionsand adjustments to column widths inREPORTING_LIST_TABLE_OPTIONSare positive changes for code readability and UI enhancement. Verify the visual impact of these adjustments on the table layout across different screen sizes to ensure a balanced and readable presentation.frontend/src/features/Reporting/components/ReportingForm/schemas.ts (1)
- 8-37: The
CreateOrEditReportingSchemavalidation schema is well-defined, usingyupfor clear validation rules and conditional validation. Review all validation messages for user-friendliness and ensure comprehensive coverage of validation cases to enhance form usability.frontend/src/features/Reporting/components/VesselReportings/Current/CreateOrEditReporting.tsx (1)
- 1-68: The refactoring and updates in the
CreateOrEditReportingcomponent improve maintainability and user experience. Ensure that styling adjustments are consistent with the application's overall design and that event handling logic is robust to enhance usability.frontend/src/features/SideWindow/BeaconMalfunctionBoard/VesselStatusSelect.tsx (1)
- 32-41: The use of
setPropertywithimportantfor style specificity inVesselStatusSelectis practical for ensuring correct style application. Review the necessity of usingimportantto ensure it does not negatively impact CSS maintainability or introduce style inconsistencies.frontend/src/features/Reporting/components/ReportingForm/utils.ts (1)
- 7-68: The
getFormFieldsandupdateReportingActorutility functions enhance form state management and user interaction handling. Ensure thorough testing of these functions, particularly the conditional logic inupdateReportingActor, to maintain form usability and relevance.frontend/src/domain/types/reporting.ts (1)
- 62-100: The introduction of
EditableReportingandEditedReportingtypes, along with updates to existing types, enhances type safety and code maintainability. Verify the consistent application of these new types and the use ofundefinedfor optional fields across the codebase.frontend/src/features/Reporting/useCases/updateReporting.ts (3)
- 1-1: The use of module aliases (
@api/reporting) improves code readability and maintainability by making import paths clearer and more concise.- 14-14: Changing the type reference from
ReportingUpdatetoEditedReportingaligns with the PR's objective to enhance reporting functionality. This change ensures that the correct type is used throughout the application, improving type safety and code clarity.- 15-15: Utilizing a module alias for
MainAppThunkis consistent with the approach of improving code readability and maintainability. This change makes it easier to understand where the type is coming from and reduces the complexity of import paths.frontend/src/features/Reporting/components/VesselReportings/index.tsx (4)
- 1-2: Updating imports to use custom hooks (
useMainAppDispatchanduseMainAppSelector) enhances modularity and encapsulation. This change promotes the use of custom hooks that abstract away the Redux store logic, making the component more maintainable and easier to understand.- 31-33: Refactoring the
useEffecthook to optimize dispatch calls based on vessel changes is a good practice. This optimization prevents unnecessary dispatch calls when the selected vessel identity has not changed, improving the component's performance.- 65-65: Replacing
COLORS.charcoalwithTHEME.color.charcoalfor the spinner color aligns with the approach of centralizing theme-related values. This change ensures consistency in color usage across the application and facilitates easier theme adjustments in the future.- 92-92: The slight decrease in
max-heightfor theBodystyled component may impact the layout and visibility of theVesselReportingscomponent. Ensure that this change does not adversely affect the user interface, especially on devices with different screen sizes or resolutions.frontend/src/api/reporting.ts (1)
- 98-98: Changing the type of the
nextReportingparameter in theupdateReportingFromAPIfunction toEditedReportingis consistent with the PR's objective to refine the reporting functionality. This change ensures that the function expects the correct type, enhancing type safety and clarity in the reporting update process.frontend/src/features/MainWindow.tsx (1)
- 71-71: Moving the rendering of
VesselSidebaroutside ofLegacyRsuiteComponentsWrapperand making it conditional on theisVesselSidebarOpenflag is a significant change. This adjustment likely aims to improve the layout and visibility of the sidebar component. Ensure that this change does not introduce any layout shifts or affect the overall user experience negatively.frontend/src/features/map/overlays/VesselCardOverlay/index.tsx (1)
- 95-97: Adding the
marginsWithThreeWarningconstant and utilizing it based on the number of warnings for a vessel is a thoughtful approach to dynamically adjust the overlay's margins. This change ensures that the overlay's layout adapts to the content, improving the user interface for cases with multiple warnings.frontend/src/features/VesselSidebar/beacon_malfunctions/resume/CurrentBeaconMalfunctionBody.tsx (1)
- 32-39: Adjusting the selectors for setting background color and text color to target specific child elements within
.rs-picker-selectand.rs-picker-toggle-valuerespectively, ensures that the styling is applied correctly. This change improves the visual representation of the vessel status, making it clearer and more consistent with the intended design.frontend/src/features/SideWindow/Alert/AlertListAndReportingList/EditReporting.tsx (3)
- 1-10: Refactoring imports to use custom hooks and module aliases enhances the modularity and readability of the code. This change aligns with best practices for organizing imports and using custom hooks for Redux store logic.
- 23-55: The restructuring of the
EditReportingcomponent, including the use of styled-components for styling and adjustments to component props handling, improves the component's readability and maintainability. The use ofdata-cyattributes also facilitates easier testing. Ensure that these changes have been tested thoroughly to confirm that the component's functionality remains intact and the user experience is not adversely affected.- 64-142: The extensive use of styled-components for defining the component's styling is a good practice, as it enhances the separation of concerns and allows for more maintainable and scalable styling. Ensure that the styles are consistent with the application's design system and that there are no unintended side effects on the layout or appearance.
frontend/src/features/Reporting/slice.ts (2)
- 16-17: The update of
editedReportingandeditedReportingInSideWindowtypes toEditableReportingis a positive change for consistency and flexibility. It allows for a more generic handling of reporting entities, potentially accommodating future extensions or variations in reporting types.- 129-129: The action
setEditedReportingInSideWindowcorrectly acceptsEditableReporting | undefinedas its payload. This change aligns with the updated state types and ensures that the application can handle both setting and clearing the edited reporting in the side window context.frontend/src/features/Reporting/components/VesselReportings/ReportingCard.tsx (2)
- 96-96: The change from
isAlertto$isAlertfor theArchiveButtonstyled component is a good practice in styled-components to indicate thatisAlertis a prop used exclusively for styling purposes and not meant to be passed to the DOM element as an attribute. This prevents unnecessary warnings and keeps the DOM cleaner.- 201-201: The dynamic styling based on the
$isAlertprop ensures that theArchiveButton's margin-top is adjusted according to whether there are any alerts. This is a neat way to conditionally style components based on props.frontend/src/features/Reporting/components/ReportingForm/index.tsx (3)
- 1-28: The imports have been updated to include necessary hooks, utilities, and components for the reporting form. It's good to see that the code is organized and imports are grouped logically. Ensure that all imported entities are used within the component to avoid unnecessary imports.
- 68-108: The
createOrEditReportingfunction is well-structured and handles both the creation and updating of reportings. It makes good use of theuseCallbackhook for memoization. However, ensure thatpreviousReportingType.currentis correctly managed and updated as needed, as its value seems to be used but not set within this component. This could potentially lead to incorrect behavior when updating reportings.- 110-234: The form structure and field definitions are clear and well-organized. Using
Formikfor form management is a good choice for handling form state and validation. The dynamic rendering of form fields based on the reporting type and actor is a nice touch that enhances the user experience. Ensure that all form fields are correctly validated according to theCreateOrEditReportingSchema.frontend/src/features/Reporting/components/ReportingList/index.tsx (4)
- 32-36: The addition of
ObservationReportingto the imported types suggests an expansion in the types of reportings the component can handle. Ensure that the component's logic and UI correctly accommodate this new type, reflecting the application's domain logic.- 107-107: The change in the parameter type of the
editfunction to includeObservationReportingis a logical extension to accommodate new reporting types. Ensure that the editing functionality is fully compatible withObservationReporting, including any necessary UI adjustments or validations.- 175-175: Adjusting the width of the table to
1195pxmay improve the layout and user experience by ensuring better alignment and spacing of content. Verify this change in the context of the entire application to maintain a consistent design language.- 363-363: Modifying the padding of the
Contentstyled component to20px 0px 40px 10pxcould enhance the visual appeal and readability of the content. Ensure this adjustment aligns with the overall design and spacing conventions of the application.frontend/src/features/map/overlays/VesselCardOverlay/VesselCard.tsx (5)
- 6-6: The import
marginsWithThreeWarninghas been added toVesselCard.tsx. Ensure that this constant is used appropriately within the file and that its values are correctly calculated based on the design requirements.- 337-351: The dynamic styling for
BottomTriangleShadowcorrectly adjusts themargin-leftbased on thenumberOfWarnings. This approach ensures that the visual representation of warnings is consistent with the design requirements. However, ensure that the constants used (marginsWithOneWarning,marginsWithTwoWarning,marginsWithThreeWarning,marginsWithoutAlert) are defined with accurate values to maintain visual consistency.- 366-395: The
TopTriangleShadowcomponent's dynamic styling formargin-leftandmargin-topis well-implemented, allowing for adjustments based on thenumberOfWarnings. This method of handling styling ensures that the component's appearance is dynamically adjusted to reflect the current state accurately. As with theBottomTriangleShadow, verify that the constants used for margins are correctly defined.- 409-438: The
RightTriangleShadowcomponent's styling logic, which adjusts bothmargin-leftandmargin-topbased on thenumberOfWarnings, is correctly implemented. This dynamic adjustment is crucial for maintaining the visual integrity of the component based on the state. Continue to ensure that the constants defining the margins are accurate and reflect the intended design.- 454-468: The
LeftTriangleShadowcomponent's dynamic margin-top adjustment based on thenumberOfWarningsis implemented correctly. This approach ensures that the component's visual representation is consistent with the application's design requirements. As with other triangle shadow components, it's important to verify the accuracy of the constants used for margin calculations.
Comment on lines
+51
to
+53
| const selectElement = vesselStatusRef.current.querySelector('.rs-picker-select > div') as HTMLElement | null | ||
| if (selectElement?.style) { | ||
| selectElement.style.background = vesselStatus.color | ||
| selectElement.style.setProperty('background', vesselStatus.color, 'important') |
There was a problem hiding this comment.
Direct manipulation of DOM element styles, especially using setProperty with the 'important' flag, is generally discouraged in React applications. It's recommended to use styled-components or CSS classes for styling. This approach enhances maintainability and avoids potential specificity conflicts that can arise from the use of 'important'. Consider refactoring this to use a styled-component or a CSS class.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- frontend/cypress/e2e/side_window/reporting_list.spec.ts (1 hunks)
- frontend/cypress/e2e/vessel_sidebar/reporting.spec.ts (2 hunks)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx
Additional comments: 5
frontend/cypress/e2e/vessel_sidebar/reporting.spec.ts (4)
- 66-73: The use of custom Cypress commands like
cy.clickButtonandcy.fillsignificantly improves the readability and maintainability of the test script by abstracting repetitive actions into reusable functions. This change aligns well with best practices for writing clean and maintainable test code.- 75-75: It's good practice to use
cy.interceptto mock network requests and responses. This ensures that the tests are not dependent on the actual network environment, which can lead to flakiness. However, ensure that the mocked responses closely mimic the real API responses to maintain test accuracy.- 81-82: The modification of a reporting to an infraction suspicion is well tested here, demonstrating the flexibility of the test suite to cover different user scenarios. This is crucial for ensuring that the application behaves as expected under various conditions.
- 158-169: The
createReportingfunction encapsulates the steps to create a new reporting, making the test cases more concise and focused on the test logic rather than the setup. This is a good example of DRY (Don't Repeat Yourself) principle in test automation.frontend/cypress/e2e/side_window/reporting_list.spec.ts (1)
- 185-187: The use of custom Cypress commands such as
cy.fillandcy.clickButtonin themodifyReportingToObservationfunction is a great way to improve the readability and maintainability of the test script. These commands abstract away the details of interacting with the UI, allowing the test logic to be more easily understood.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- frontend/cypress/e2e/side_window/reporting_list.spec.ts (2 hunks)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- frontend/cypress/e2e/side_window/reporting_list.spec.ts
- frontend/src/features/Reporting/components/ReportingForm/index.tsx
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- frontend/src/domain/shared_slices/Vessel.ts (1 hunks)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx (1 hunks)
- frontend/src/features/Reporting/useCases/updateReporting.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx
- frontend/src/features/Reporting/useCases/updateReporting.ts
Additional comments: 1
frontend/src/domain/shared_slices/Vessel.ts (1)
- 137-140: The addition of a condition to check
state.selectedVesselIdentitybefore comparing it withaction.payload.vesselFeatureIdis a logical improvement. This ensures that operations are only performed if a vessel is selected, which can prevent potential errors or unintended behavior when no vessel is selected. However, it's important to ensure that all related components and state management logic that depend onselectedVesselIdentityare aware of and properly handle cases whereselectedVesselIdentitymight be null or undefined.
ivangabriele
approved these changes
Mar 5, 2024
There was a problem hiding this comment.
Je ne suis pas sûr d'avoir compris si le besoin est une sauvegarde en live du formulaire. Là j'ai l'impression de ne voir qu'une sauvegarde standard via le backend seulement une fois qu'on clique sur le submit button. Il n'y a pas de redux-persist ou de test de rechargement de page à moins que j'ai raté quelques chose ?
| }: ReportingFormProps) { | ||
| const dispatch = useMainAppDispatch() | ||
| const infractions = useMainAppSelector(state => state.infraction.infractions) | ||
| const controlUnitsQuery = useGetLegacyControlUnitsQuery(undefined) |
There was a problem hiding this comment.
Pourquoi utiliser les unités legacy ?
There was a problem hiding this comment.
+1, utilisation des nouvelles unités
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx (1 hunks)
- frontend/src/features/Reporting/components/VesselReportings/Current/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- frontend/src/features/Reporting/components/ReportingForm/index.tsx
Additional comments: 2
frontend/src/features/Reporting/components/VesselReportings/Current/utils.ts (2)
- 1-3: Ensure that the updated imports and types (
ReportingandControlUnit) are correctly used throughout this file. Given the removal ofgetReportingValueErrors, verify that the updated types still meet all necessary validation and functionality requirements within the reporting feature.- 1-13: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [12-20]
The
sortByValidationOrCreationDateDescfunction provides a clear and logical way to sort reports. However, ensure that allReportingobjects have either avalidationDateor acreationDateto avoid unexpected behavior when either date is missing or invalid.
Comment on lines
+6
to
+10
| export const mapControlUnitsToUniqueSortedIdsAsOptions = (controlUnits: ControlUnit.ControlUnit[]): Option<number>[] => | ||
| Array.from(controlUnits) | ||
| .sort((a, b) => Number(b.name) - Number(a.name)) | ||
| .map(controlUnit => ({ | ||
| label: `${controlUnit.name} (${controlUnit.administration})`, | ||
| label: `${controlUnit.name} (${controlUnit.administration.name})`, |
There was a problem hiding this comment.
The mapControlUnitsToUniqueSortedIdsAsOptions function correctly maps and sorts control units. However, the sorting logic seems to sort by name in descending order, which might not be intuitive for user selection. Consider if ascending order (Number(a.name) - Number(b.name)) would be more user-friendly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Linked issues
Summary by CodeRabbit
New Features
ReportingFormcomponent for creating or editing vessel reporting data.VesselSidebar.EditableReportingandEditedReportingtypes.useGetInLocalStoragehook for retrieving data from local storage.Bug Fixes
BeaconMalfunctionDetailsandVesselStatusSelectfor improved specificity.Refactor
EditReportingcomponent for better prop handling and styling.useEffecthooks inVesselReportingsfor dispatch calls.Style
VesselCardOverlaystyling based on the number of warnings.BackofficeSecondaryButtonstyled component.Documentation
Tests
Chores
Revert