fix(bulk-mileage): HOTFIX - vehicle bulk search #16998
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request introduce enhancements to the vehicle mileage registration system. Key updates include the addition of new optional fields in various classes, modifications to GraphQL queries, and the introduction of a new API endpoint. The Changes
Assessment against linked issues
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
|
thorkellmani
added
the
deprecated:automerge
thorkellmani
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (12)
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts (1)
11-12: Consider adding JSDoc documentation for the new field.Adding documentation would help other developers understand the purpose and expected behavior of this filter.
+ /** + * When true, filters the vehicle list to show only vehicles that require mileage registration. + * @default undefined + */ @Field({ nullable: true }) filterOnlyRequiredMileageRegistration?: booleanlibs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts (1)
14-15: Consider adding JSDoc documentation.Adding documentation would help explain the purpose and usage of the
internalIdfield, making it clearer for other developers.+ /** Internal identifier for the mileage registration. */ @Field(() => Int, { nullable: true }) internalId?: numberlibs/service-portal/assets/src/screens/VehicleBulkMileage/types.ts (1)
15-20: LGTM! Consider extracting a shared type.The new structure for
lastMileageRegistrationprovides better type safety and more comprehensive mileage data. However, there's an opportunity to reduce type duplication by extracting the common structure shared withregistrationHistoryitems.Consider creating a shared interface:
+interface MileageRecord { + date: Date + origin: string + mileage: number + internalId?: number +} export interface VehicleType extends VehicleProps { mileageUploadedFromFile?: number isCurrentlyEditing?: boolean - lastMileageRegistration?: { - date: Date - origin: string - mileage: number - internalId?: number - } + lastMileageRegistration?: MileageRecord - registrationHistory?: Array<{ - date: Date - origin: string - mileage: number - }> + registrationHistory?: Array<Omit<MileageRecord, 'internalId'>> }libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql (1)
14-16: Consider paginated loading for lastMileageRegistration data.The addition of lastMileageRegistration field enhances the vehicle list with detailed mileage information. However, fetching this data for all vehicles in the list could impact performance with large datasets.
Consider implementing:
- Lazy loading of mileage details
- Pagination parameters for mileage history
- Caching strategy for frequently accessed vehicles
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.tsx (1)
244-252: Consider extracting the mileage formatting logicWhile the current implementation is correct, consider extracting this display logic into a reusable function to improve maintainability and reduce code duplication.
+ const formatMileageDisplay = (registration?: { mileage: number }) => + registration?.mileage + ? displayWithUnit(registration.mileage, 'km', true) + : '-' { value: formatMileageDisplay(vehicle.lastMileageRegistration), },libs/api/domains/vehicles/src/lib/services/vehicles.service.ts (1)
145-159: Consider simplifying the lastMileageRegistration constructionWhile the logic is correct, the null checks could be simplified using object destructuring and optional chaining.
- let lastMileageRegistration: MileageRegistration | undefined - - if ( - d.latestOriginCode && - d.latestMileage && - d.latestMileageReadDate - ) { - lastMileageRegistration = { - originCode: d.latestOriginCode, - mileage: d.latestMileage, - date: d.latestMileageReadDate, - internalId: d.latestMileageInternalId ?? undefined, - } - } + const lastMileageRegistration = d.latestOriginCode && d.latestMileage && d.latestMileageReadDate + ? { + originCode: d.latestOriginCode, + mileage: d.latestMileage, + date: d.latestMileageReadDate, + internalId: d.latestMileageInternalId ?? undefined, + } + : undefined;Also applies to: 167-167
libs/clients/vehicles/src/clientConfig.json (4)
506-595: LGTM: Well-structured new endpointThe new
/VehicleHistoryV2endpoint is well-defined with:
- Clear parameters with proper types and defaults
- Proper security configuration
- Comprehensive error responses
Consider adding a description field to document the purpose and differences from the original
/VehicleHistoryendpoint.
1197-1380: Consider adding validation rulesThe new
AllVehiclesForPersidnoV2schema is well-structured but could benefit from additional validation rules for critical fields:
persidno: Format pattern for valid ID numbersregno: Format pattern for valid registration numbersvin: Format pattern for valid VIN numbers (17 characters)Example addition:
"vin": { "type": "string", + "pattern": "^[A-HJ-NPR-Z0-9]{17}$", + "description": "17-character Vehicle Identification Number", "nullable": true }
3046-3063: Consider adding validation and documentationThe new mileage tracking fields are well-structured but could benefit from:
- Validation rules for mileage values
- Field descriptions explaining the purpose of each field
Example improvements:
"latestMileage": { "type": "integer", "format": "int32", + "minimum": 0, + "description": "Latest recorded mileage reading in kilometers", "nullable": true }
3667-3670: Add field descriptionThe new
enginefield would benefit from a description explaining its purpose and expected format.Example improvement:
"engine": { "type": "string", + "description": "Engine specifications or identification", "nullable": true }libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx (2)
81-96: Simplify conditional checks using optional chainingYou can simplify the conditional checks for
lastMileageRegistrationby using optional chaining, which enhances readability and reduces complexity.Apply this diff to refactor the code:
- const lastMileageRegistration = - v.mileageDetails?.lastMileageRegistration && - v.mileageDetails.lastMileageRegistration.date && - v.mileageDetails.lastMileageRegistration.mileage && - v.mileageDetails.lastMileageRegistration.originCode - ? { - date: new Date( - v.mileageDetails.lastMileageRegistration.date, - ), - mileage: v.mileageDetails.lastMileageRegistration.mileage, - origin: - v.mileageDetails.lastMileageRegistration.originCode, - internalId: - v.mileageDetails.lastMileageRegistration.internalId ?? - undefined, - } - : undefined + const lastMileageRegistration = v.mileageDetails?.lastMileageRegistration + ? { + date: new Date( + v.mileageDetails?.lastMileageRegistration?.date, + ), + mileage: v.mileageDetails?.lastMileageRegistration?.mileage, + origin: v.mileageDetails?.lastMileageRegistration?.originCode, + internalId: v.mileageDetails?.lastMileageRegistration?.internalId, + } + : undefined🧰 Tools
🪛 Biome (1.9.4)
[error] 81-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
61-110: UseuseCallbackinstead ofuseMemofor debounced functionsUsing
useCallbackis more appropriate for memoizing functions likedebouncedQuery, and it can help manage dependencies effectively.Apply this diff to switch to
useCallback:-import { useEffect, useState, useMemo } from 'react' +import { useEffect, useState, useCallback } from 'react' const debouncedQuery = useMemo(() => { - return debounce(() => { +const debouncedQuery = useCallback( + debounce(() => { vehicleListQuery({ variables: { input: { page, pageSize: 10, query: search ?? undefined, filterOnlyRequiredMileageRegistration: filterValue, }, }, }).then((res) => { // ... }) - }, debounceTime.search) - // eslint-disable-next-line react-hooks/exhaustive-deps -}, [search, filterValue, page]) + }, debounceTime.search), + [search, filterValue, page, vehicleListQuery] +)This change ensures that dependencies are correctly managed and allows you to remove the ESLint disable comment.
🧰 Tools
🪛 Biome (1.9.4)
[error] 81-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts(1 hunks)libs/api/domains/vehicles/src/lib/models/v3/mileageDetails.model.ts(2 hunks)libs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts(1 hunks)libs/api/domains/vehicles/src/lib/models/v3/mileageRegistrationHistory.model.ts(0 hunks)libs/api/domains/vehicles/src/lib/services/vehicles.service.ts(4 hunks)libs/clients/vehicles/src/clientConfig.json(5 hunks)libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql(2 hunks)libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx(3 hunks)libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.tsx(1 hunks)libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx(1 hunks)libs/service-portal/assets/src/screens/VehicleBulkMileage/types.ts(1 hunks)
💤 Files with no reviewable changes (1)
- libs/api/domains/vehicles/src/lib/models/v3/mileageRegistrationHistory.model.ts
🧰 Additional context used
📓 Path-based instructions (10)
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.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/api/domains/vehicles/src/lib/models/v3/mileageDetails.model.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/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.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/api/domains/vehicles/src/lib/services/vehicles.service.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/clients/vehicles/src/clientConfig.json (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/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql (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/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.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/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.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/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.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/service-portal/assets/src/screens/VehicleBulkMileage/types.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 (1.9.4)
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx
[error] 81-82: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (10)
libs/api/domains/vehicles/src/lib/dto/vehiclesListInputV3.ts (1)
11-12: Implementation looks good! Let's verify the integration.
The new field is properly implemented with correct TypeScript and GraphQL decorators. It aligns well with the filtering requirements from the PR objectives.
Let's verify how this field is being used across the codebase:
✅ Verification successful
Implementation is properly integrated and used correctly
The field is well integrated across the codebase:
- Used in the VehicleBulkMileage component for filtering
- Correctly mapped in vehicles.service.ts to
onlyMileageRequiredVehiclesparameter - Properly defined in the DTO with GraphQL decorators
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the usage of filterOnlyRequiredMileageRegistration in GraphQL queries and related components
# Check for GraphQL queries using this field
rg -l "filterOnlyRequiredMileageRegistration" --type graphql
# Check for usage in service implementations
ast-grep --pattern 'filterOnlyRequiredMileageRegistration'
Length of output: 151
Script:
#!/bin/bash
# Let's try a broader search approach
# Search for the field name in all files
rg -l "filterOnlyRequiredMileageRegistration"
# Search for GraphQL query usage with context
rg "filterOnlyRequiredMileageRegistration" -B2 -A2
# Look for potential resolver implementations
ast-grep --pattern 'class $_ {
$$$
async vehicles($_, $_) {
$$$
}
$$$
}'
Length of output: 1868
libs/api/domains/vehicles/src/lib/models/v3/mileageRegistration.model.ts (1)
14-15: LGTM! Well-structured type definition.
The new field is properly defined with correct TypeScript and GraphQL type declarations, maintaining type safety through both layers.
libs/api/domains/vehicles/src/lib/models/v3/mileageDetails.model.ts (2)
3-3: LGTM: Clean import addition
The import statement follows TypeScript best practices and maintains proper module organization.
13-14: LGTM: Well-structured field addition
The new field follows GraphQL and TypeScript best practices with proper typing and nullability configuration. The addition enhances the model's capability to track the most recent mileage registration.
Let's verify the consistency of this field usage across the codebase:
✅ Verification successful
Field is properly integrated and used consistently
The lastMileageRegistration field is correctly implemented and utilized across the codebase:
- Properly mapped in the vehicles service from database fields
- Consistently used in the VehicleBulkMileage component with proper type checking
- Type definitions are aligned in both the model and consuming components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of lastMileageRegistration field in GraphQL queries and related components
# Check GraphQL queries using this field
rg -t graphql "lastMileageRegistration" -A 3
# Check TypeScript/JavaScript files referencing this field
ast-grep --pattern 'lastMileageRegistration'
Length of output: 500
Script:
#!/bin/bash
# Let's check the actual implementation and usage context
# Check the service implementation
rg -A 10 "lastMileageRegistration" "libs/api/domains/vehicles/src/lib/services/vehicles.service.ts"
# Check the bulk mileage component usage
rg -A 10 "lastMileageRegistration" "libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx"
# Check for GraphQL operations in TS files
rg "lastMileageRegistration.*\{" -A 3 -g "*.{ts,tsx}"
Length of output: 3292
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx (1)
Line range hint 1-53: Component follows reusability guidelines.
The component demonstrates good practices for reusability across NextJS apps:
- Uses shared UI components from island-ui/core
- Implements proper TypeScript typing
- Uses shared localization system
- Maintains modularity with clear props interface
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.graphql (1)
74-74: Verify consistent usage of internalId across mutations.
The addition of internalId to the mileageRegistration fragment looks good. Let's verify its consistent usage across the codebase.
✅ Verification successful
The internalId field is consistently used across mileage-related operations.
Based on the codebase analysis, the internalId field is consistently used in:
- GraphQL mutations for vehicle mileage updates
- GraphQL queries for retrieving mileage data
- TypeScript interfaces and models for mileage records
- Frontend components handling mileage registration
The field is properly integrated in both bulk and individual mileage registration flows, maintaining data consistency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of internalId field across GraphQL operations
# Search for GraphQL operations using internalId
rg -t graphql "internalId" --context 3
# Search for TypeScript interfaces/types using internalId
ast-grep --pattern 'interface $_ {
$$$
internalId: $_
$$$
}'
Length of output: 139
Script:
#!/bin/bash
# Search for internalId in all files, focusing on GraphQL and TypeScript files
rg "internalId" -g "*.{ts,tsx,graphql,gql}" --context 3
# Find all GraphQL and TypeScript files that might contain relevant definitions
fd -e ts -e tsx -e graphql -e gql --type f
Length of output: 106678
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageRow.tsx (1)
Line range hint 1-436: LGTM! Component follows library coding guidelines
The component properly implements:
- TypeScript usage with well-defined props and types
- Modular and reusable design
- Proper tree-shaking support through named exports
libs/api/domains/vehicles/src/lib/services/vehicles.service.ts (2)
49-49: LGTM!
The import statement for the MileageRegistration model is correctly placed and follows the project's import structure.
115-115: LGTM!
The addition of onlyMileageRequiredVehicles filter parameter correctly implements the bulk mileage list filtering requirement from the PR objectives.
libs/clients/vehicles/src/clientConfig.json (1)
5-5: LGTM: Version update in description
The description has been updated to reflect the new release version (Release-46 : 20241106.1).
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileageTable.tsx
Show resolved
Hide resolved
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx
Show resolved
Hide resolved
libs/service-portal/assets/src/screens/VehicleBulkMileage/VehicleBulkMileage.tsx
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## release/32.3.0 #16998 +/- ##
==================================================
- Coverage 36.51% 36.48% -0.04%
==================================================
Files 6890 6897 +7
Lines 143819 143940 +121
Branches 40978 41013 +35
==================================================
+ Hits 52520 52521 +1
- Misses 91299 91419 +120
Flags with carried forward coverage won't be shown. Click here to find out more. see 16 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 8 Total Test Services: 0 Failed, 7 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
thorkellmani
dismissed stale reviews from coderabbitai[bot] and thordurhhh
via
26df88d
HOTFIX
#16906
Some manual commits were needed to remove some stuff which hasn't hit prod yet
Checklist:
Summary by CodeRabbit
New Features
/VehicleHistoryV2for vehicle history retrieval.Improvements
Bug Fixes