-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat(native-app): update vehicleList endpoint to V2 #16775
Conversation
WalkthroughThe pull request introduces substantial updates to the vehicle-related GraphQL fragments and queries within the application. 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
|
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: 3
🧹 Outside diff range and nitpick comments (5)
apps/native/app/src/screens/vehicles/components/vehicle-item.tsx (1)
6-6
: LGTM! Consider type alias for better maintainability.The update to V2 types is correct. However, for better maintainability, consider creating a type alias for the nested NonNullable expression.
+type VehicleListV2 = NonNullable<ListVehiclesV2Query['vehiclesListV2']> -type VehicleListItem = NonNullable< - NonNullable<ListVehiclesV2Query['vehiclesListV2']>['vehicleList'] ->[0] +type VehicleListItem = NonNullable<VehicleListV2['vehicleList']>[0]Also applies to: 14-14
apps/native/app/src/screens/home/vehicles-module.tsx (2)
41-44
: Add null check for safer data accessWhile the V2 migration looks good, consider adding an explicit null check for better type safety.
if ( - data?.vehiclesListV2?.vehicleList?.some( + data?.vehiclesListV2?.vehicleList && + data.vehiclesListV2.vehicleList.some( (vehicle) => vehicle.requiresMileageRegistration, ) ) {
Line range hint
65-82
: Improve type safety in useMemo hookConsider adding explicit type annotations for better maintainability and type safety.
const reorderedVehicles = useMemo( - () => + (): typeof vehicles => vehicles ? [...vehicles]?.sort((a, b) => {apps/native/app/src/screens/home/home.tsx (1)
Line range hint
177-185
: Enhance type safety for V2 query result.While the implementation is correct, consider adding explicit type annotations for better maintainability:
- const vehiclesRes = useListVehiclesV2Query({ + const vehiclesRes: ListVehiclesV2QueryResult = useListVehiclesV2Query({This would make the V2 query result type more explicit and help catch any type mismatches early during development.
Also applies to: 269-273, 290-294
apps/native/app/src/screens/vehicles/vehicles.tsx (1)
136-138
: Consider simplifying the keyExtractor functionSince
permno
is expected to be unique for each vehicle, concatenating it withindex
may be unnecessary. You could simplify the key to justitem.permno
if it is guaranteed to be unique.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
apps/native/app/src/graphql/fragments/vehicle.fragment.graphql
(1 hunks)apps/native/app/src/graphql/queries/vehicles.graphql
(1 hunks)apps/native/app/src/screens/home/home.tsx
(2 hunks)apps/native/app/src/screens/home/vehicles-module.tsx
(5 hunks)apps/native/app/src/screens/vehicles/components/vehicle-item.tsx
(3 hunks)apps/native/app/src/screens/vehicles/vehicles.tsx
(7 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/native/app/src/graphql/fragments/vehicle.fragment.graphql (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/graphql/queries/vehicles.graphql (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/home/home.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/home/vehicles-module.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/vehicles/components/vehicle-item.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/native/app/src/screens/vehicles/vehicles.tsx (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (17)
apps/native/app/src/graphql/fragments/vehicle.fragment.graphql (2)
1-9
: LGTM! Fragment structure aligns with V2 endpoint requirements.
The transition to VehicleListed
type and the streamlined field selection align well with the PR objective of updating to vehicleListV2.
1-9
: Verify the impact of removed fields.
Several fields have been removed from the fragment (vin, type, firstRegDate, vehicleStatus, etc.). Let's verify that these fields are not being referenced elsewhere in the codebase.
✅ Verification successful
No impact from removed fields detected
The removed fields from VehicleFragment appear safe to remove as:
- The only usage of
vin
found in the codebase is as an empty string in query inputs - No references to
firstRegDate
orvehicleStatus
were found - The VehicleFragment is only used in one query (vehicles.graphql)
- The removed fields are not being accessed or used in any TypeScript/React components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any references to the removed fields in the native app
# This helps ensure we haven't missed updating any component that might be expecting these fields
echo "Searching for references to removed vehicle fields..."
rg -t tsx -t ts "vehicleStatus|firstRegDate|vin" apps/native/app/src/
Length of output: 216
Script:
#!/bin/bash
# Search for any references to the removed fields in the native app
# Using ripgrep without file type restrictions since .tsx/.ts extensions might not be recognized
echo "Searching for references to removed vehicle fields..."
rg "vehicleStatus|firstRegDate|vin" apps/native/app/src/
# Also search for these terms in GraphQL files specifically
echo -e "\nSearching in GraphQL files..."
rg "vehicleStatus|firstRegDate|vin" apps/native/app/src/ -g "*.graphql"
# Let's also check if there are any TypeScript/TypeScript React files with a different approach
echo -e "\nSearching in .ts and .tsx files..."
find apps/native/app/src/ -type f \( -name "*.ts" -o -name "*.tsx" \) -exec grep -l "vehicleStatus\|firstRegDate\|vin" {} \;
Length of output: 8380
Script:
#!/bin/bash
# Let's examine the actual usage of 'vin' in the vehicle-related files to understand the context
echo "Checking vehicle-related files for actual usage of removed fields..."
rg -A 5 -B 5 "vin" apps/native/app/src/screens/vehicles/vehicles-detail.tsx apps/native/app/src/screens/vehicles/vehicle-mileage.screen.tsx
# Let's also check for any GraphQL operations that might be using the VehicleFragment
echo -e "\nChecking for GraphQL operations using VehicleFragment..."
rg "VehicleFragment" apps/native/app/src/ -g "*.graphql" -g "*.ts" -g "*.tsx"
Length of output: 2391
apps/native/app/src/graphql/queries/vehicles.graphql (3)
Line range hint 3-11
: Verify fragment field coverage.
The query structure with pagination looks good. However, since the VehicleFragment has been modified to use VehicleListed type, we should verify that all necessary fields are included.
Let's check the fragment definition and its fields:
#!/bin/bash
# Description: Verify the VehicleFragment definition and its usage
# Expected: Should find the fragment definition with all required fields
# Search for the VehicleFragment definition
echo "Searching for VehicleFragment definition:"
ast-grep --pattern 'fragment VehicleFragment on VehicleListed {
$$$
}'
# Search for any other fragments that might be needed
echo -e "\nSearching for related fragments:"
rg "fragment.*on.*Vehicle"
Line range hint 1-11
: Verify alignment with service portal implementation.
To ensure consistency as mentioned in the PR objectives, we should verify that this query matches the service portal implementation.
Let's check for the service portal query implementation:
#!/bin/bash
# Description: Compare with service portal implementation
# Expected: Should find matching query structure in service portal
# Search for similar query in service portal
echo "Searching for vehiclesListV2 query in service portal:"
rg -g "*.{graphql,ts,tsx}" "vehiclesListV2.*GetVehiclesListV2Input"
# Search for any differences in implementation
echo -e "\nSearching for potential discrepancies:"
rg -g "*.{graphql,ts,tsx}" "vehiclesList.*GetVehiclesForUserInput"
1-2
: Verify the input type compatibility with the schema.
The query has been updated to use the new V2 endpoint with GetVehiclesListV2Input
type. This aligns with the PR objective to update the vehicleList endpoint to V2.
Let's verify the input type exists in the schema:
apps/native/app/src/screens/vehicles/components/vehicle-item.tsx (1)
Line range hint 1-85
: Verify consistent field usage across components.
Let's ensure that the new V2 fields (make, colorName, nextMainInspection) are consistently used across other components.
✅ Verification successful
Let me gather more information about the schema and queries to better understand the field changes.
Field usage appears to be localized to vehicle-item component
Based on the verification results:
- The new fields (
make
,colorName
,nextMainInspection
) are only used invehicle-item.tsx
and defined in the GraphQL fragment - Other
item.type
usages found in the codebase are unrelated (from family/inbox/assets components) - No conflicting old field references were found in vehicle-related components
- The changes appear to be properly contained within the vehicle item component and its associated GraphQL schema
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for potential inconsistencies in field usage
echo "Searching for old field references..."
rg "nextInspection\.nextInspectionDate|item\.type|item\.color" "apps/native/app/src"
echo "Verifying new field usage..."
rg "nextMainInspection|item\.make|item\.colorName" "apps/native/app/src"
Length of output: 1656
Script:
#!/bin/bash
# Check GraphQL schema and queries for field definitions
echo "Checking GraphQL schema and queries..."
rg -A 5 "type Vehicle|interface Vehicle|query ListVehiclesV2" --type graphql
# Check for any other vehicle-related components that might need updates
echo "Checking vehicle-related components..."
fd -e tsx -e ts "vehicle" apps/native/app/src
Length of output: 732
Script:
#!/bin/bash
# Check the implementation of other vehicle components for field usage
echo "Checking vehicles-detail.tsx..."
rg -A 3 "make|colorName|nextMainInspection" "apps/native/app/src/screens/vehicles/vehicles-detail.tsx"
echo "Checking vehicles.tsx..."
rg -A 3 "make|colorName|nextMainInspection" "apps/native/app/src/screens/vehicles/vehicles.tsx"
echo "Checking vehicle-card.tsx..."
rg -A 3 "make|colorName|nextMainInspection" "apps/native/app/src/ui/lib/card/vehicle-card.tsx"
Length of output: 492
apps/native/app/src/screens/home/vehicles-module.tsx (3)
20-21
: LGTM: Clean migration to V2 types
The update to V2 types is consistent with the PR objectives and follows proper TypeScript naming conventions.
62-62
: LGTM: Clean V2 endpoint integration
The data access update is consistent with the V2 migration while maintaining existing functionality.
Line range hint 20-173
: Verify complete V1 to V2 migration
Let's ensure no V1 references remain in the codebase.
apps/native/app/src/screens/home/home.tsx (2)
Line range hint 58-62
: LGTM: Import changes align with V2 upgrade.
The import change correctly updates the query hook while maintaining the existing module structure and validation utilities.
Line range hint 177-185
: Verify pagination implementation in V2.
The query implementation looks correct, maintaining the same pagination structure. However, let's verify that the V2 endpoint supports the same pagination parameters.
✅ Verification successful
Pagination implementation in V2 is correctly maintained
The verification confirms that:
- The GraphQL query
ListVehiclesV2
accepts pagination parameters throughGetVehiclesListV2Input
- The input type definition in
libs/api/domains/vehicles/src/lib/dto/getVehiclesForUserInput.ts
includes the requiredpage
andpageSize
fields - The query returns paginated results with metadata (pageNumber, pageSize, totalPages, totalRecords)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify V2 query implementation and pagination support
# Search for the GraphQL query definition and any pagination-related types
# Look for the ListVehiclesV2 query definition
ast-grep --pattern 'query ListVehiclesV2($input: ListVehiclesInput!) {
$$$
}'
# Look for pagination input type definition
ast-grep --pattern 'input ListVehiclesInput {
$$$
page: $_
pageSize: $_
$$$
}'
Length of output: 170
Script:
#!/bin/bash
# Let's try a different approach to find GraphQL schema and query definitions
# Search for GraphQL files containing ListVehiclesV2
fd -e graphql -e gql | xargs rg -l "ListVehiclesV2"
# Search for type definitions in TypeScript files
fd -e ts -e tsx | xargs rg "ListVehiclesV2|ListVehiclesInput" -A 5
# Search for generated types
fd -e ts -e tsx | xargs rg "export (type|interface) ListVehicles(V2)?(Input|Query)" -A 5
Length of output: 5418
Script:
#!/bin/bash
# Let's check the GraphQL query definition file we found
cat apps/native/app/src/graphql/queries/vehicles.graphql
Length of output: 2306
Script:
#!/bin/bash
# Let's find the input type definition for GetVehiclesListV2Input
fd -e ts -e tsx | xargs rg "GetVehiclesListV2Input" -A 5
# Also check for any schema type definitions
fd "schema.ts" | xargs rg "GetVehiclesListV2Input" -A 5
Length of output: 3676
apps/native/app/src/screens/vehicles/vehicles.tsx (6)
17-18
: Correctly updated imports to V2 queries
The imports have been updated to use ListVehiclesV2Query
and useListVehiclesV2Query
, aligning the component with the updated GraphQL API.
35-35
: Updated type definition to match new query schema
The VehicleListItem
type now correctly reflects the structure of ListVehiclesV2Query['vehiclesListV2']['vehicleList'][0]
, ensuring proper type safety with the updated data.
78-78
: Switched to using useListVehiclesV2Query hook
The component now uses useListVehiclesV2Query
, ensuring data is fetched from the updated V2 endpoint.
150-150
: Updated data extraction to use vehiclesListV2
The data memoization now references res.data?.vehiclesListV2?.vehicleList
, aligning with the updated query structure.
187-188
: Adjusted pagination logic to new data structure
Pagination variables now reference vehiclesListV2.paging.pageNumber
and vehiclesListV2.paging.totalPages
, ensuring accurate paging with the updated API.
203-204
: Correctly merged vehicle lists in fetchMore
The updateQuery
function merges previous and new vehicle lists appropriately, ensuring the list appends correctly during pagination.
Also applies to: 206-207
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.
Nice one
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16775 +/- ##
=======================================
Coverage 36.44% 36.44%
=======================================
Files 6853 6853
Lines 143479 143479
Branches 40942 40942
=======================================
Hits 52288 52288
Misses 91191 91191
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
* feat: update vehicleList endpoint to V2 * fix: remove comment --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
Update vehicleList endpoint in app to use vehicleListV2 like the service portal.
Why
So we are sure we have the same data as the service portal.
Checklist:
Summary by CodeRabbit
New Features
ListVehiclesV2
) and updated related hooks for improved data retrieval.make
,colorName
, andnextMainInspection
.Bug Fixes
canRegisterMilage
.Improvements