-
Notifications
You must be signed in to change notification settings - Fork 3
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
[OSDEV-1130] SLC. Implement collecting contribution data page (FE) #463
base: main
Are you sure you want to change the base?
[OSDEV-1130] SLC. Implement collecting contribution data page (FE) #463
Conversation
…ies data, mocked sectors
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive update to the React application, focusing on adding a new "Production Location Information" feature. The changes include creating a new route for production location info, developing a detailed React component for capturing production location details, updating utility functions and styles, and making minor improvements to existing components. The update enhances the contribution process by providing a structured way for users to input detailed production location information. Changes
Sequence DiagramsequenceDiagram
participant User
participant ProductionLocationInfo
participant Redux Store
participant Backend
User->>ProductionLocationInfo: Navigate to Production Location Info
ProductionLocationInfo->>Redux Store: Fetch country and facility options
Redux Store-->>ProductionLocationInfo: Return options
User->>ProductionLocationInfo: Fill in location details
ProductionLocationInfo->>ProductionLocationInfo: Validate input
ProductionLocationInfo->>Backend: Submit production location info
Backend-->>ProductionLocationInfo: Confirm submission
ProductionLocationInfo-->>User: Display submission status
Possibly Related PRs
Suggested Reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
React App | Jest test suite - Code coverage reportTotal: 27.7%Your code coverage diff: -0.18% ▾ ✅ All code changes are covered |
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.
Actionable comments posted: 7
🧹 Nitpick comments (8)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (2)
34-166
: Consider relocating large mocked data.
The extensive 'mockedSectors' array adds noise to the component file. Moving it to a separate configuration or constants file could improve readability and maintainability.
225-235
: Remove console.log statements or use a logger.
Production code should avoid console.logs for debugging. Recommended to remove them or use an environment-based logger for better maintainability.src/react/src/components/ArrowDropDownUpIcon.jsx (1)
3-13
: SVG icon looks good.
Implementation is straightforward and succinct. Consider adding a descriptive title or accessible attributes for screen readers if needed.src/react/src/components/Contribute/InputHelperText.jsx (1)
7-14
: Informative helper text component.
The component is well-crafted, straightforward, and uses Material-UI effectively. If used in multiple places, consider making the icon or text more configurable.src/react/src/reducers/ProductionLocationInfoReducer.js (1)
14-35
: Consider adding TypeScript interfaces or PropTypes for better type safetyThe initial state structure is complex with nested objects. Adding type definitions would improve maintainability and catch potential type-related bugs early.
src/react/src/util/styles.js (1)
1050-1054
: Remove commented out code.The commented out styles for
boxShadow
and hover effects should either be implemented or removed to maintain code cleanliness.- // boxShadow: - // '0px 1px 5px 0px rgba(0, 0, 0, 0.2), 0px 2px 2px 0px rgba(0, 0, 0, 0.14), 0px 3px 1px -2px rgba(0, 0, 0, 0.12)', - // '&:hover': { - // backgroundColor: theme.palette.action.dark, - // },src/react/src/util/util.js (2)
738-754
: Consider adding type annotations.The function logic is sound and well-commented. Consider adding TypeScript or JSDoc type annotations to improve code maintainability and type safety.
Example JSDoc annotation:
/** * Maps processing type options based on facility types * @param {Array<{facilityType: string, processingTypes: Array<string>}>} fPTypes - Facility processing types * @param {Array<{value: string, label: string}>} fTypes - Selected facility types * @returns {Array<{value: string, label: string}>} Mapped processing type options */
756-772
: Consider code reuse and type annotations.The function is well-implemented but has similar structure to
mapProcessingTypeOptions
. Consider:
- Adding TypeScript or JSDoc type annotations
- Extracting shared logic into a reusable helper function
Example refactor:
/** * Maps types based on filter criteria * @template T * @param {Array<{facilityType: string, processingTypes: Array<string>}>} fPTypes - Facility processing types * @param {Array<{value: string, label: string}>} filterTypes - Types to filter by * @param {(fPType: any, filterType: any) => boolean} matchFn - Function to match types * @param {(fPType: any) => T} extractFn - Function to extract target type * @returns {Array<{value: T, label: T}>} Mapped type options */ const mapTypeOptions = (fPTypes, filterTypes, matchFn, extractFn) => { let types = []; if (filterTypes.length === 0) { types = fPTypes.map(extractFn); } else { filterTypes.forEach(filterType => { fPTypes.forEach(fPType => { if (matchFn(fPType, filterType)) { types = types.concat(extractFn(fPType)); } }); }); } return mapDjangoChoiceTuplesValueToSelectOptions(uniq(types.sort())); }; export const mapProcessingTypeOptions = (fPTypes, fTypes) => mapTypeOptions( fPTypes, fTypes, (fPType, fType) => fType.value === fPType.facilityType, fPType => fPType.processingTypes ).flat(); export const mapFacilityTypeOptions = (fPTypes, pTypes) => mapTypeOptions( fPTypes, pTypes, (fPType, pType) => fPType.processingTypes.includes(pType.value), fPType => fPType.facilityType );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/react/src/Routes.jsx
(3 hunks)src/react/src/actions/prodactionLocationInfo.js
(1 hunks)src/react/src/components/ArrowDropDownUpIcon.jsx
(1 hunks)src/react/src/components/Contribute/InputHelperText.jsx
(1 hunks)src/react/src/components/Contribute/ProductionLocationInfo.jsx
(1 hunks)src/react/src/components/Contribute/SearchByNameAndAddressTab.jsx
(5 hunks)src/react/src/components/FilterSidebarExtendedSearch.jsx
(1 hunks)src/react/src/components/Filters/CustomReactSelectComponents/CustomDropdownIndicator.jsx
(1 hunks)src/react/src/reducers/ProductionLocationInfoReducer.js
(1 hunks)src/react/src/reducers/index.js
(2 hunks)src/react/src/util/constants.jsx
(1 hunks)src/react/src/util/styles.js
(1 hunks)src/react/src/util/util.js
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/react/src/util/constants.jsx
🔇 Additional comments (8)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (1)
639-643
: Good use of defaultProps and propTypes.
The defaultProps and propTypes are clearly defined and help ensure robust typechecking. Keep them updated if new props are introduced.
Also applies to: 645-655
src/react/src/components/Filters/CustomReactSelectComponents/CustomDropdownIndicator.jsx (1)
4-4
: Nicely handled conditional icon rendering.
The arrowDown prop provides clear logic for toggling between up/down icons. This improves UX clarity.
Also applies to: 7-9
src/react/src/reducers/index.js (1)
39-39
: LGTM! Clean integration of the new reducer
The ProductionLocationInfoReducer is properly imported and integrated into the root reducer while maintaining the existing code structure.
Also applies to: 72-72
src/react/src/components/Contribute/SearchByNameAndAddressTab.jsx (2)
60-61
: Good use of route constant
Replacing the hardcoded route with productionLocationInfoRoute
constant improves maintainability and follows DRY principles.
132-132
: Verify InputHelperText component implementation
The validation feedback has been standardized using the InputHelperText
component. This improves consistency across the application.
Also applies to: 163-163
✅ Verification successful
Let me check the usage patterns to ensure the component is being used consistently across the codebase.
InputHelperText component implementation is consistent and properly used
The InputHelperText
component is well-implemented with:
- Default validation message "This field is required"
- Consistent styling through Material-UI's
withStyles
HOC - Proper usage across form validations in both
SearchByNameAndAddressTab
andProductionLocationInfo
components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check InputHelperText implementation
ast-grep --pattern 'const InputHelperText = $_' src/react/src/components/Contribute/InputHelperText.jsx
Length of output: 903
Script:
#!/bin/bash
# Check usage of InputHelperText to ensure consistent props
rg "InputHelperText" -A 2 -B 2
Length of output: 4129
src/react/src/Routes.jsx (1)
174-179
: Route configuration looks good
The new route for production location info is properly configured with the exact
prop and correct component mapping.
✅ Verification successful
Route configuration is correctly implemented and unique
The route /contribute/production-location/info
is properly defined as a constant, has a single implementation in Routes.jsx, and is correctly used in the SearchByNameAndAddressTab component for URL construction. No conflicts or duplicate routes were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify no duplicate routes exist
rg -A 1 "path=\{productionLocationInfoRoute\}" src/react/src/Routes.jsx
Length of output: 215
Script:
#!/bin/bash
# Check for any other routes that might be using the same component
rg "component=\{ProductionLocationInfo\}" src/react/src/Routes.jsx
# Check the definition and uniqueness of the route constant
rg "productionLocationInfoRoute" src/react/src/
Length of output: 670
Script:
#!/bin/bash
# Check the definition of the route constant to ensure it's unique
rg -A 1 "export const productionLocationInfoRoute =" src/react/src/util/constants.jsx
# Check how the route is being used in SearchByNameAndAddressTab
rg -A 3 "const baseUrl = productionLocationInfoRoute" src/react/src/components/Contribute/SearchByNameAndAddressTab.jsx
Length of output: 458
src/react/src/components/FilterSidebarExtendedSearch.jsx (1)
39-40
: Good refactoring of utility functions
Moving the mapping functions to a separate utility file improves code organization and follows the Single Responsibility Principle.
✅ Verification successful
Utility functions are properly implemented in the util.js file
The verification confirms that both mapProcessingTypeOptions
and mapFacilityTypeOptions
functions are properly implemented in src/react/src/util/util.js
. The functions handle the mapping of facility and processing types with appropriate filtering logic, making them suitable for reuse across components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the utility functions implementation
ast-grep --pattern 'export const mapProcessingTypeOptions = $_' src/react/src/util/util.js
ast-grep --pattern 'export const mapFacilityTypeOptions = $_' src/react/src/util/util.js
Length of output: 2524
src/react/src/util/styles.js (1)
1070-1086
: LGTM!
The inputHelperText
styles are well-structured, follow Material-UI patterns, and maintain immutability using Object.freeze()
.
Dedupe Hub App | Unittest test suite - Code coverage reportTotal: 56.14%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Contricleaner App | Unittest test suite - Code coverage reportTotal: 98.91%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Countries App | Unittest test suite - Code coverage reportTotal: 100%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
Django App | Unittest test suite - Code coverage reportTotal: 80.19%Your code coverage diff: 0.00% ▴ ✅ All code changes are covered |
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.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/react/src/util/styles.js (1)
979-1002
: Consider adding responsive breakpoints for row-based layouts.
While these row-based containers effectively align items on larger screens, they may need additional responsive handling for smaller viewports.src/react/src/components/Contribute/ProductionLocationInfo.jsx (3)
96-122
: Consider adding error handling for fetch actions.In each
useEffect
call where you fetch data (fetchCountries
,fetchFacilityProcessingType
,fetchNumberOfWorkers
), consider adding error handling logic or a fallback UI for when calls fail. This helps avoid silent failures and improves user experience.
125-490
: Break down the component into smaller subcomponents for clarity.This file is quite large and handles many fields. Splitting it into smaller, focused subcomponents (e.g. a dedicated section for “Additional Information”, “Basic Info”, etc.) can improve readability, testability, and maintainability.
485-486
: Restore or finalize the form validation logic.Currently, the
disabled={!isFormValid}
prop is commented out. If form validation is required, reintroduce this logic or provide an alternative to prevent invalid form submissions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/react/src/components/Contribute/InputErrorText.jsx
(1 hunks)src/react/src/components/Contribute/ProductionLocationInfo.jsx
(1 hunks)src/react/src/components/Contribute/SearchByNameAndAddressTab.jsx
(5 hunks)src/react/src/util/COLOURS.js
(1 hunks)src/react/src/util/styles.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/components/Contribute/SearchByNameAndAddressTab.jsx
🔇 Additional comments (7)
src/react/src/components/Contribute/InputErrorText.jsx (3)
1-5
: Imports look consistent with Material-UI usage and project structure.
No issues found; everything is aligned with common React and Material-UI patterns. Good job!
7-14
: Well-structured functional component and default error message.
This concise component design effectively displays error messages by blending icon, text, and styling. Proceed with this straightforward approach for consistent user feedback.
16-16
: Use of withStyles
to inject styles is appropriate.
This pattern ensures separation of concerns by keeping style definitions in the utility file, yet cleanly injecting them here.
src/react/src/util/styles.js (2)
1002-1083
: Styles for large text and container spacing look consistent.
They align with your existing theme usage and follow the standard spacing approach. No immediate issues noted.
1084-1100
: New inputErrorText
export is consistent with the new error component.
It’s good to see synergy between the InputErrorText.jsx
component and these styles, ensuring uniform error formatting.
src/react/src/components/Contribute/ProductionLocationInfo.jsx (1)
474-484
: Implement button functionality or remove placeholder event handlers.
Both the "Go Back" and "Submit" buttons only define empty arrow functions for onClick. This comment matches a previously raised comment, so it's marked as a duplicate.
src/react/src/util/COLOURS.js (1)
29-29
: Addition of a new PURPLE color looks good.
This new color is consistent with standard naming. No further change needed.
Quality Gate passedIssues Measures |
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (3)
296-310
: Consider enabling multiple selections for sectors.
The UI label refers to “Sector(s)” and provides examples of multiple possible sectors, butStyledSelect
is configured for a single selection. If the intent is to support multiple selections, setisMulti={true}
.
329-339
: Enable multiple product types if needed.
Similar to the sector(s) field, if multiple product types can be chosen, consider settingisMulti={true}
onStyledSelect
.
72-77
: Potential improvement for validation logic.
Yourvalidate
function currently checks only for a non-empty string. If more advanced validation is required later (e.g., length limits, special characters, etc.), consider extracting or using a well-tested library (e.g., Formik, Yup) to handle more complex checks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/react/src/components/Contribute/ProductionLocationInfo.jsx
(1 hunks)src/react/src/components/Contribute/SearchByNameAndAddressTab.jsx
(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/react/src/components/Contribute/SearchByNameAndAddressTab.jsx
🔇 Additional comments (2)
src/react/src/components/Contribute/ProductionLocationInfo.jsx (2)
468-469
: [Duplicate from previous reviews] Implement button functionality or remove placeholder event handlers.
Both "Go Back" and "Submit" buttons define empty arrow functions for onClick
. This was mentioned in previous reviews, so be sure to implement navigational or submission logic here.
Would you like help implementing the button event handlers?
Also applies to: 476-477
98-105
: Verify prefilled country logic.
When the component first loads, you filter and set the inputCountry
from countryInQuery
. Ensure all references to countryInQuery
are consistent with any uppercase/lowercase or known country codes, and confirm that no extraneous fetch calls occur if countriesOptions
is an empty array rather than null
.
(classes.inputSectionWrapStyles, | ||
classes.wrapStyles) |
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.
Suspected JS bracket usage error in className.
Using the comma operator (classes.inputSectionWrapStyles, classes.wrapStyles)
will only return classes.wrapStyles
. This is likely a mistake if your intent is to combine both classes.
- className={(classes.inputSectionWrapStyles, classes.wrapStyles)}
+ className={`${classes.inputSectionWrapStyles} ${classes.wrapStyles}`}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
(classes.inputSectionWrapStyles, | |
classes.wrapStyles) | |
`${classes.inputSectionWrapStyles} ${classes.wrapStyles}` |
OSDEV-1130 A new page,
Production Location Information
, has been implemented. It includes the following inputs:* Required and pre-fillable fields:
- Name
- Address
- Country
* Additional information section: Fields for optional contributions from the owner or manager of the production location, including sector(s), product type(s), location type(s), processing type(s), number of workers, and parent company.
The page also features
Go Back
andSubmit
buttons for navigation and form submission.