fix(samgongustofa): shared vehicle radio form field #16905
Conversation
Stop using mileageRequired from answers in getSelectedVehicle and always look at the value from externalData (it should not change)
…leswithmileageandinsp (not basicVehicleInformation) Cleanup make+vehcom (basicVehicleInformation) vs make (currentvehicles)
Display always E, L and W messages (but E and L first)
Allow all users to retry submit application if all approvals are finished
Display ValidationErrorMessages always in overview, no matter who is reviewing
Add validation per plate if user has more than 5
Add MocablePayment
…github.com/island-is/island.is into fix/samgongustofa-vehicle-radio-form-field
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request involves the removal of the Changes
Possibly related PRs
Suggested labels
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
Documentation and Community
|
johannaagma
changed the base branch from
main
to
fix/transport-authority-error-handling-warnsever
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16905 +/- ##
==========================================
- Coverage 36.45% 36.42% -0.04%
==========================================
Files 6903 6904 +1
Lines 144598 145360 +762
Branches 41288 41545 +257
==========================================
+ Hits 52720 52941 +221
- Misses 91878 92419 +541 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (18)
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleDetails.ts (3)
1-4: Add JSDoc documentation for error message interface.Consider adding JSDoc comments to document the purpose and usage of the ValidationErrorMessage interface and its fields.
+/** + * Represents a validation error message with an optional error number and default message. + */ interface ValidationErrorMessage { + /** Unique identifier for the error */ errorNo?: string | null + /** Human-readable error message */ defaultMessage?: string | null }
6-15: Enhance type safety for vehicle details.Consider the following improvements to make the interface more type-safe:
- Add string literal types for
roleto restrict possible values- Use a number type for
mileageReadingif it represents a numeric value- Consider using non-nullable types where appropriate
export interface VehicleDetails { permno?: string make?: string color?: string - role?: string + role?: 'owner' | 'operator' | 'other' requireMileage?: boolean | null - mileageReading?: string | null + mileageReading?: number | null isDebtLess?: boolean | null validationErrorMessages?: ValidationErrorMessage[] | null }
17-22: Improve date handling in plate ownership interface.Consider using a more specific type for dates to ensure proper date format validation. You could either:
- Use a Date type if working with Date objects
- Add documentation specifying the expected date format if using strings
+/** + * Represents ownership details for a vehicle plate + * @property startDate - ISO 8601 formatted date string (YYYY-MM-DD) + * @property endDate - ISO 8601 formatted date string (YYYY-MM-DD) + */ export interface PlateOwnership { regno: string - startDate: string - endDate: string + startDate: `${number}-${number}-${number}` // Template literal type for YYYY-MM-DD + endDate: `${number}-${number}-${number}` // Template literal type for YYYY-MM-DD validationErrorMessages?: ValidationErrorMessage[] | null }libs/application/templates/transport-authority/order-vehicle-license-plate/src/forms/OrderVehicleLicensePlateForm/InformationSection/plateSizeSubSection.ts (1)
Line range hint
41-54: Consider adding TypeScript type for vehicle informationWhile the ID change from
plateSize.vehicleTypetovehicleInfo.typeimproves consistency, we could enhance type safety by defining an interface for the vehicle information structure.Consider adding:
interface VehicleInfo { plate: string; type: string; }This would make the field structure more maintainable and provide better TypeScript support.
libs/application/templates/transport-authority/license-plate-renewal/src/fields/PlateField/index.tsx (1)
56-74: LGTM! Well-structured field configuration with comprehensive validation.The implementation properly utilizes the shared VehicleRadioFormField component with thorough validation and error handling.
Consider extracting the field configuration to a separate constant for better maintainability:
const PLATE_FIELD_CONFIG = { id: 'pickPlate', title: information.labels.pickPlate.title, // ... rest of the configuration } as const; // Then in JSX: <VehicleRadioFormField {...props} field={PLATE_FIELD_CONFIG} />libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/index.tsx (1)
70-79: Consider adding additional validation props.Based on the AI summary, the
VehicleRadioFormFieldcomponent supports additional validation props that could enhance the user experience:
shouldValidateDebtStatusalertMessageErrorTitlefield={{ id: 'pickVehicle', title: information.labels.pickVehicle.title, type: FieldTypes.VEHICLE_RADIO, component: FieldComponents.VEHICLE_RADIO, children: undefined, itemType: 'VEHICLE', itemList: currentVehicleList?.vehicles, inputErrorMessage: error.requiredValidVehicle, + shouldValidateDebtStatus: true, + alertMessageErrorTitle: error.validationErrorTitle, }}libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/VehicleSelectField.tsx (4)
22-22: Consider adding proper type for the errors propWhile the addition of the errors prop is good, using
anytype in its usage reduces type safety. Consider defining a proper interface for the errors structure.-> = ({ currentVehicleList, application, errors }) => { +interface FormErrors { + pickVehicle?: { + vehicle?: string; + plate?: string; + }; +} +> = ({ currentVehicleList, application, errors }: { errors: FormErrors }) => {
46-48: Consider centralizing the form field pathsThe path 'pickVehicle.plate' is used in multiple places. Consider extracting it to a constant to improve maintainability.
+const FORM_PATHS = { + VEHICLE_PLATE: 'pickVehicle.plate', + VEHICLE: 'pickVehicle.vehicle', +} as const; -const [plate, setPlate] = useState<string>( - getValueViaPath(application.answers, 'pickVehicle.plate', '') as string, -) +const [plate, setPlate] = useState<string>( + getValueViaPath(application.answers, FORM_PATHS.VEHICLE_PLATE, '') as string, +)
Line range hint
51-64: Consider adding explicit error handling for missing permnoThe current implementation silently handles missing permno. Consider adding explicit error handling to improve debugging and user feedback.
const onChange = (option: Option) => { const currentVehicle = currentVehicleList[parseInt(option.value, 10)] setIsLoading(true) - if (currentVehicle.permno) { + try { + if (!currentVehicle?.permno) { + throw new Error('Selected vehicle has no permit number'); + } setSelectedVehicle({ permno: currentVehicle.permno, make: currentVehicle?.make || '', color: currentVehicle?.color || '', role: currentVehicle?.role, }) setValue('pickVehicle.plate', currentVehicle.permno || '') setPlate(currentVehicle.permno || '') - setIsLoading(false) + } catch (error) { + console.error('Error in vehicle selection:', error); + setSelectedVehicle(null); + setValue('pickVehicle.plate', ''); + setPlate(''); + } finally { + setIsLoading(false); } }
98-100: Error display implementation is good but type safety could be improvedThe error display logic with internationalization is well implemented. However, the type casting to
anycould be replaced with the proper error type as suggested earlier.-{!isLoading && plate.length === 0 && (errors as any)?.pickVehicle && ( +{!isLoading && plate.length === 0 && errors?.pickVehicle && (libs/application/templates/transport-authority/order-vehicle-license-plate/src/fields/VehiclesField/index.tsx (2)
69-84: Consider extracting field configurationWhile the implementation is correct, the field configuration object could be extracted to a separate constant or utility function to improve maintainability and reusability.
Consider applying this refactor:
+ const getVehicleRadioFieldConfig = ( + information: any, + error: any, + vehicles: any[] + ) => ({ + id: 'pickVehicle', + title: information.labels.pickVehicle.title, + type: FieldTypes.VEHICLE_RADIO, + component: FieldComponents.VEHICLE_RADIO, + children: undefined, + itemType: 'VEHICLE', + itemList: vehicles, + alertMessageErrorTitle: information.labels.pickVehicle.hasErrorTitle, + validationErrorFallbackMessage: error.validationFallbackErrorMessage, + inputErrorMessage: error.requiredValidVehicle, + }); <VehicleRadioFormField {...props} - field={{ - id: 'pickVehicle', - title: information.labels.pickVehicle.title, - type: FieldTypes.VEHICLE_RADIO, - component: FieldComponents.VEHICLE_RADIO, - children: undefined, - itemType: 'VEHICLE', - itemList: currentVehicleList?.vehicles, - alertMessageErrorTitle: information.labels.pickVehicle.hasErrorTitle, - validationErrorFallbackMessage: error.validationFallbackErrorMessage, - inputErrorMessage: error.requiredValidVehicle, - }} + field={getVehicleRadioFieldConfig( + information, + error, + currentVehicleList?.vehicles + )} />
Line range hint
41-85: Extract magic numbers as named constantsThe vehicle count thresholds (5 and 20) should be extracted as named constants to improve maintainability and make the business logic more explicit.
Consider applying this refactor:
+ const VEHICLE_COUNT_THRESHOLDS = { + SEARCH_FORM: 20, + SELECT_FIELD: 5 + } as const; return ( <Box paddingTop={2}> - {currentVehicleList.totalRecords > 20 ? ( + {currentVehicleList.totalRecords > VEHICLE_COUNT_THRESHOLDS.SEARCH_FORM ? ( <FindVehicleFormField // ... props /> - ) : currentVehicleList.totalRecords > 5 ? ( + ) : currentVehicleList.totalRecords > VEHICLE_COUNT_THRESHOLDS.SELECT_FIELD ? ( <VehicleSelectField // ... props /> ) : ( <VehicleRadioFormField // ... props /> )} </Box> )libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/VehiclesField/index.tsx (1)
106-108: Enhance type safety for itemType and optional itemList.Consider these improvements:
- Use a typed constant for itemType instead of string literal 'VEHICLE'
- Add null check for itemList as currentVehicleList could be undefined
+ const ITEM_TYPE = { + VEHICLE: 'VEHICLE', + } as const; itemType: 'VEHICLE', - itemList: currentVehicleList?.vehicles, + itemType: ITEM_TYPE.VEHICLE, + itemList: currentVehicleList?.vehicles ?? [],libs/application/templates/transport-authority/change-operator-of-vehicle/src/fields/VehiclesField/index.tsx (2)
102-122: LGTM! Enhanced form field implementation.The new
VehicleRadioFormFieldimplementation provides comprehensive validation and error handling. The field configuration is well-structured and type-safe.Consider extracting the magic numbers in the conditions to named constants:
+const MAX_VEHICLES_FOR_RADIO = 5; +const MAX_VEHICLES_FOR_SELECT = 20; -currentVehicleList.totalRecords > 20 ? +currentVehicleList.totalRecords > MAX_VEHICLES_FOR_SELECT ? <FindVehicleFormField ... -: currentVehicleList.totalRecords > 5 ? +: currentVehicleList.totalRecords > MAX_VEHICLES_FOR_RADIO ?
104-122: Consider extracting field configuration.The field configuration object is quite large and could be extracted to improve maintainability and reusability.
Consider this refactor:
const vehicleRadioFieldConfig = { id: 'pickVehicle', title: information.labels.pickVehicle.title, description: information.labels.pickVehicle.description, type: FieldTypes.VEHICLE_RADIO, component: FieldComponents.VEHICLE_RADIO, children: undefined, itemType: 'VEHICLE', itemList: currentVehicleList?.vehicles, shouldValidateDebtStatus: true, alertMessageErrorTitle: information.labels.pickVehicle.hasErrorTitle, validationErrorMessages: applicationCheck.validation, validationErrorFallbackMessage: applicationCheck.validation.fallbackErrorMessage, inputErrorMessage: error.requiredValidVehicle, debtStatusErrorMessage: information.labels.pickVehicle.isNotDebtLessTag, }; // In JSX: <VehicleRadioFormField {...props} field={vehicleRadioFieldConfig} />libs/application/types/src/lib/Fields.ts (1)
677-691: Enhance code organization and documentationThe interface could benefit from better organization and documentation:
- Group validation-related properties
- Add JSDoc comments for complex properties
Consider this refactoring:
interface ValidationConfig { shouldValidateDebtStatus?: boolean; shouldValidateRenewal?: boolean; validationErrorMessages?: Record<string, FormText>; validationErrorFallbackMessage?: FormText; debtStatusErrorMessage?: FormText; validateRenewal?: (item: VehicleItem | PlateItem) => boolean; } /** Represents a radio field for selecting vehicles or plates */ interface VehicleRadioField extends InputField { readonly type: FieldTypes.VEHICLE_RADIO; component: FieldComponents.VEHICLE_RADIO; /** Determines whether the field is for vehicle or plate selection */ itemType: 'VEHICLE' | 'PLATE'; /** List of vehicles or plates to choose from */ itemList: Array<VehicleItem | PlateItem>; /** Configuration for validation rules and error messages */ validation?: ValidationConfig; /** Error message for invalid input */ inputErrorMessage: FormText; /** Optional tag to display renewal expiration */ renewalExpiresAtTag?: StaticText; }libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx (2)
83-168: Specify explicit return type forvehicleOptionsfunctionFor better type safety and clarity, consider specifying the explicit return type for the
vehicleOptionsfunction.Apply the following change:
- const vehicleOptions = (vehicles: VehicleDetails[]) => { + const vehicleOptions = (vehicles: VehicleDetails[]): Option[] => {🧰 Tools
🪛 Biome
[error] 151-151: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
170-255: Specify explicit return type forplateOptionsfunctionFor better type safety and clarity, consider specifying the explicit return type for the
plateOptionsfunction.Apply the following change:
- const plateOptions = (plates: PlateOwnership[]) => { + const plateOptions = (plates: PlateOwnership[]): Option[] => {🧰 Tools
🪛 Biome
[error] 238-238: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (20)
libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/fields/VehiclesField/VehicleRadioField.tsx(0 hunks)libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/fields/VehiclesField/index.tsx(2 hunks)libs/application/templates/transport-authority/change-operator-of-vehicle/src/fields/VehiclesField/VehicleRadioField.tsx(0 hunks)libs/application/templates/transport-authority/change-operator-of-vehicle/src/fields/VehiclesField/index.tsx(2 hunks)libs/application/templates/transport-authority/license-plate-renewal/src/fields/PlateField/PlateRadioField.tsx(0 hunks)libs/application/templates/transport-authority/license-plate-renewal/src/fields/PlateField/index.tsx(2 hunks)libs/application/templates/transport-authority/license-plate-renewal/src/lib/dataSchema.ts(1 hunks)libs/application/templates/transport-authority/order-vehicle-license-plate/src/fields/VehiclesField/VehicleRadioField.tsx(0 hunks)libs/application/templates/transport-authority/order-vehicle-license-plate/src/fields/VehiclesField/index.tsx(2 hunks)libs/application/templates/transport-authority/order-vehicle-license-plate/src/forms/OrderVehicleLicensePlateForm/InformationSection/plateSizeSubSection.ts(2 hunks)libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/VehicleRadioField.tsx(0 hunks)libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/VehicleSelectField.tsx(5 hunks)libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/index.tsx(2 hunks)libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/forms/OrderVehicleRegistrationCertificateForm/InformationSection/vehicleSubSection.ts(2 hunks)libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/VehiclesField/VehicleRadioField.tsx(0 hunks)libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/VehiclesField/index.tsx(2 hunks)libs/application/types/src/lib/Fields.ts(4 hunks)libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleDetails.ts(1 hunks)libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx(1 hunks)libs/application/ui-fields/src/lib/index.ts(1 hunks)
💤 Files with no reviewable changes (6)
- libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/fields/VehiclesField/VehicleRadioField.tsx
- libs/application/templates/transport-authority/change-operator-of-vehicle/src/fields/VehiclesField/VehicleRadioField.tsx
- libs/application/templates/transport-authority/license-plate-renewal/src/fields/PlateField/PlateRadioField.tsx
- libs/application/templates/transport-authority/order-vehicle-license-plate/src/fields/VehiclesField/VehicleRadioField.tsx
- libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/VehicleRadioField.tsx
- libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/VehiclesField/VehicleRadioField.tsx
🧰 Additional context used
📓 Path-based instructions (14)
libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/fields/VehiclesField/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/change-operator-of-vehicle/src/fields/VehiclesField/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/license-plate-renewal/src/fields/PlateField/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/license-plate-renewal/src/lib/dataSchema.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/order-vehicle-license-plate/src/fields/VehiclesField/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/order-vehicle-license-plate/src/forms/OrderVehicleLicensePlateForm/InformationSection/plateSizeSubSection.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/VehicleSelectField.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/forms/OrderVehicleRegistrationCertificateForm/InformationSection/vehicleSubSection.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/VehiclesField/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/types/src/lib/Fields.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleDetails.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/ui-fields/src/lib/index.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
[error] 151-151: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 238-238: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (21)
libs/application/templates/transport-authority/license-plate-renewal/src/lib/dataSchema.ts (2)
Line range hint 1-13: LGTM! Well-structured schema definition
The schema is well-defined using zod with proper TypeScript integration and exports. The modular structure ensures reusability across different NextJS applications.
7-7: Verify the impact of making the 'value' field optional
This change could affect form validation behavior in components using this schema. Please ensure this aligns with the new VehicleRadioFormField requirements.
Let's verify the usage of this schema:
✅ Verification successful
Making 'value' field optional is correct and safe
The schema change aligns perfectly with the component's implementation:
- The
PlateSelectFieldcomponent handles the initial empty state correctly - The field is used as an index selector for the plate list, where an empty initial state is valid
- Form validation is properly managed through the required
regnofield, which is set only after successful plate validation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for imports of LicensePlateRenewalSchema to understand impact
rg -l "LicensePlateRenewalSchema"
# Search for components using pickPlate.value
rg "pickPlate.*value"
# Look for VehicleRadioFormField usage with this schema
rg "VehicleRadioFormField.*LicensePlateRenewal"
Length of output: 907
Script:
#!/bin/bash
# Let's examine the PlateSelectField component and its usage
cat libs/application/templates/transport-authority/license-plate-renewal/src/fields/PlateField/PlateSelectField.tsx
# Check the getSelectedVehicle utility
cat libs/application/templates/transport-authority/license-plate-renewal/src/utils/getSelectedVehicle.ts
# Look at the template to understand the form structure
cat libs/application/templates/transport-authority/license-plate-renewal/src/lib/LicensePlateRenewalTemplate.ts
Length of output: 11552
libs/application/templates/transport-authority/order-vehicle-license-plate/src/forms/OrderVehicleLicensePlateForm/InformationSection/plateSizeSubSection.ts (2)
Line range hint 27-54: Verify form validation after ID changes
The ID changes from plateSize.* to vehicleInfo.* might affect form validation and data persistence. Please ensure:
- Existing form data is migrated or handled appropriately
- Form validation rules are updated to use the new field IDs
- Any dependent components are updated to reference the new structure
Let's check for any validation rules or dependencies:
#!/bin/bash
# Description: Look for validation rules and dependencies that might need updating
# Search for validation rules
echo "Checking for validation rules:"
rg -A 5 "validation.*plateSize\.(vehiclePlate|vehicleType)"
# Search for form data access
echo "Checking for form data access:"
rg -A 5 "answers\[.*plateSize\.(vehiclePlate|vehicleType)"Line range hint 27-40: Verify consistent usage of the new vehicleInfo namespace
The ID change from plateSize.vehiclePlate to vehicleInfo.plate appears to be part of a larger refactoring. The new namespace better represents the data category.
Let's verify this change is consistent across the codebase:
✅ Verification successful
The search results show consistent usage of vehicleInfo.plate across multiple transport authority templates and UI field components. There are no remaining occurrences of the old plateSize.vehiclePlate pattern. Let's verify one more aspect - the schema/type definitions to ensure type safety:
ID change from plateSize.vehiclePlate to vehicleInfo.plate is consistently implemented
The verification shows that:
- The new
vehicleInfo.platepattern is consistently used across multiple transport authority templates - No legacy occurrences of
plateSize.vehiclePlateremain in the codebase - The field ID is properly handled in form components like
VehicleRadioFormFieldandFindVehicleFormField - The change is consistently applied in related vehicle management templates (transfer, registration, operator change)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining occurrences of the old ID pattern and verify consistent usage of the new pattern
# Check for any remaining uses of the old pattern
echo "Checking for remaining uses of old pattern:"
rg "plateSize\.vehiclePlate"
# Check usage of new pattern to ensure consistency
echo "Checking usage of new pattern:"
rg "vehicleInfo\.plate"
Length of output: 2081
Script:
#!/bin/bash
# Check type definitions and interfaces using vehicleInfo
ast-grep --pattern 'interface $_ {
$$$
vehicleInfo?: {
$$$
}
$$$
}'
# Also check for any type imports or references
rg "type.*vehicleInfo" -A 2
Length of output: 482
libs/application/ui-fields/src/lib/index.ts (2)
28-28: LGTM! Export follows established patterns.
The new export follows the consistent pattern used throughout the file and maintains proper organization.
28-28: Verify component implementation meets library standards.
Since this is a shared library component, let's verify the implementation meets our requirements.
libs/application/templates/transport-authority/license-plate-renewal/src/fields/PlateField/index.tsx (1)
1-5: LGTM! Well-structured imports following best practices.
The imports are properly organized, with explicit named exports that enable effective tree-shaking. Good use of shared types and UI components from the application libraries.
Also applies to: 14-16
libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/index.tsx (2)
6-6: LGTM! Import statements follow best practices.
The imports are properly structured for tree-shaking optimization and follow the module organization pattern.
Also applies to: 11-14
68-79: LGTM! Component implementation follows best practices.
The implementation properly uses TypeScript and follows component reusability patterns.
libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/fields/VehiclesField/VehicleSelectField.tsx (1)
4-9: LGTM: Import changes are well-structured
The addition of InputError from the shared UI library aligns with the component's enhanced error handling capabilities while maintaining reusability across different NextJS apps.
libs/application/templates/transport-authority/order-vehicle-license-plate/src/fields/VehiclesField/index.tsx (1)
9-12: LGTM! Imports follow best practices
The imports are properly structured, using shared UI components from the application package which promotes reusability across different NextJS apps.
libs/application/templates/transport-authority/order-vehicle-registration-certificate/src/forms/OrderVehicleRegistrationCertificateForm/InformationSection/vehicleSubSection.ts (1)
Line range hint 1-140: Well-structured and reusable form configuration!
The implementation follows good practices:
- Uses TypeScript effectively with proper type imports
- Implements a modular builder pattern for form construction
- Maintains clear separation of concerns with external data handling
- Provides consistent field configurations
libs/application/templates/transport-authority/transfer-of-vehicle-ownership/src/fields/VehiclesField/index.tsx (2)
14-17: LGTM! Import changes follow best practices.
The imports are properly structured for optimal tree-shaking and reusability across NextJS apps.
97-98: LGTM! Component implementation is well-structured.
The VehicleRadioFormField implementation includes comprehensive validation, error handling, and debt status checking capabilities.
Also applies to: 99-117
libs/application/templates/transport-authority/change-co-owner-of-vehicle/src/fields/VehiclesField/index.tsx (2)
13-16: LGTM! Clean import structure following tree-shaking practices.
The imports are properly organized and follow effective tree-shaking practices by importing specific components from the UI fields package.
97-117: LGTM! Comprehensive field configuration with proper validation.
The new VehicleRadioFormField implementation includes:
- Proper TypeScript prop types
- Comprehensive validation error messages
- Debt status validation
- Reusable field configuration
libs/application/templates/transport-authority/change-operator-of-vehicle/src/fields/VehiclesField/index.tsx (1)
13-16: LGTM! Clean import structure.
The imports follow TypeScript conventions and promote reusability across NextJS apps by importing from the shared UI fields library.
libs/application/types/src/lib/Fields.ts (3)
259-259: LGTM: Enum addition follows conventions
The new VEHICLE_RADIO type follows the established naming pattern and maintains alphabetical ordering.
294-294: LGTM: Component enum addition follows conventions
The new VehicleRadioFormField component follows React naming conventions and maintains consistency with other form field components.
787-787: LGTM: Field union type correctly updated
The VehicleRadioField is properly added to the Field union type, enabling proper type checking throughout the application.
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx (1)
28-284: Component adheres to coding guidelines and is well-implemented
The VehicleRadioFormField component is properly designed with TypeScript types, promoting reusability across different NextJS applications. It effectively manages form state and integrates well with the application.
🧰 Tools
🪛 Biome
[error] 151-151: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 238-238: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
...lication/templates/transport-authority/license-plate-renewal/src/fields/PlateField/index.tsx
Show resolved
Hide resolved
...te/src/forms/OrderVehicleRegistrationCertificateForm/InformationSection/vehicleSubSection.ts
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 23 Total Test Services: 0 Failed, 22 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
libs/application/templates/transport-authority/license-plate-renewal/src/lib/dataSchema.ts
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleDetails.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Outdated
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx (2)
52-78: Optimize selection handler and improve type safetyThe selection handler could be improved in several ways:
- Use type guards instead of type casting
- Implement early returns for better readability
- Batch setValue calls for better performance
Consider this refactor:
const onRadioControllerSelect = (s: string) => { + const index = parseInt(s, 10) + if (isNaN(index)) return + if (field.itemType === 'VEHICLE') { - const currentVehicleList = field.itemList as VehicleDetails[] + if (!Array.isArray(field.itemList)) return + const currentVehicleList = field.itemList as VehicleDetails[] const currentVehicle = currentVehicleList?.[parseInt(s, 10)] + if (!currentVehicle) return const permno = currentVehicle?.permno || '' setSelectedValue(permno) + // Batch update form values + const updates = { + [`${field.id}.plate`]: permno, + [`${field.id}.type`]: currentVehicle.make, + [`${field.id}.color`]: currentVehicle.color || undefined, + 'vehicleMileage.requireMileage': currentVehicle.requireMileage, + 'vehicleMileage.mileageReading': currentVehicle.mileageReading, + } + Object.entries(updates).forEach(([key, value]) => setValue(key, value)) + if (permno) { + setValue('vehicleInfo.plate', permno) + setValue('vehicleInfo.type', currentVehicle.make) + } } // Similar refactor for PLATE type... }
254-281: LGTM! Consider type guard for itemListThe rendering logic is well-implemented. Consider adding a type guard for better type safety:
let options: Option[] = [] if (field.itemType === 'VEHICLE') { + if (!Array.isArray(field.itemList)) return [] options = vehicleOptions(field.itemList as VehicleDetails[]) } else if (field.itemType === 'PLATE') { + if (!Array.isArray(field.itemList)) return [] options = plateOptions(field.itemList as PlateOwnership[]) }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleDetails.ts(1 hunks)libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleDetails.ts
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
[error] 148-148: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 235-235: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (1)
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx (1)
1-28: LGTM! Well-structured type definitions and imports.
The code follows TypeScript best practices with proper type definitions and organized imports.
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Show resolved
Hide resolved
libs/application/ui-fields/src/lib/VehicleRadioFormField/VehicleRadioFormField.tsx
Show resolved
Hide resolved
* Fix typo * Fix mileageReading vs isRequired Stop using mileageRequired from answers in getSelectedVehicle and always look at the value from externalData (it should not change) * Make sure to always use requiresMileageRegistration from currentvehicleswithmileageandinsp (not basicVehicleInformation) Cleanup make+vehcom (basicVehicleInformation) vs make (currentvehicles) * Cleanup * cleanup * cleanup * cleanup * Use shared function to extract messages from error body Display always E, L and W messages (but E and L first) * Use dummy mileage value when validating per vehicle (owner/operator change) * Catch error from mileage api * Fix the way validation errors are displayed in SGS ownerchange Allow all users to retry submit application if all approvals are finished * Apply same change to change co-owner + change operator Display ValidationErrorMessages always in overview, no matter who is reviewing * Cleanup * Cleanup in LicensePlateRenewal + OrderVehicleLicensePlate Add validation per plate if user has more than 5 * Fix the way vehicle subModel is displayed * Fixes after review * Fix the way errors are displayed for RenewLicensePlate Add MocablePayment * Add validation for OrderVehicleLicensePlate * Cleanup * Fix comment * Create RadioFormField * Fix field currentVehicleList * Use RadioFormField in all SGS applications * Use RadioFormField in all SGS applications * Cleanup * cleanup * cleanup field names * Fixes after review --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
VehicleRadioFormFieldcomponent for enhanced vehicle and plate selection with improved validation and error handling.VehicleRadioFieldcomponents across various modules to streamline the codebase.VehicleRadioFieldinterface, expanding available field types for vehicle selection.