feat(driving-license): add pickup option to submit data for 65+ #16453
Conversation
WalkthroughThis pull request introduces a new method for renewing driving licenses specifically for individuals aged 65 and over, along with a corresponding input type. It also adds a new enumeration for pickup options and updates various services and schemas to accommodate these changes. The API endpoints are expanded to include new functionalities related to driving license applications and statistics. 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
|
stjanilofts
force-pushed
the
feat/dl-pickup
branch
from
28005d6 to
57eb2e5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (14)
libs/application/templates/driving-license/src/lib/types.ts (1)
9-12: LGTM! Consider using 'as const' for improved type safety.The
Pickupenum is well-defined and aligns with the PR objectives. It follows TypeScript best practices and uses clear, descriptive naming.For improved type safety, consider using 'as const' assertion:
export const Pickup = { POST: 'post', DISTRICT: 'district', } as const; export type Pickup = typeof Pickup[keyof typeof Pickup];This approach creates a union type of string literals, which can provide better type inference in some scenarios.
libs/application/templates/driving-license/src/forms/draft/subSectionDelivery.ts (2)
15-16: Approve import changes with a minor suggestion.The reorganization of imports improves type safety and separation of concerns. Moving
Pickupto the types file is a good practice for TypeScript projects.Consider grouping related imports together for better readability:
import { B_FULL_RENEWAL_65 } from '../../lib/constants' import { Pickup } from '../../lib/types' import { Jurisdiction } from '@island.is/clients/driving-license'This change would group all imports from external libraries separately from local imports.
Line range hint
52-63: Approve usage of B_FULL_RENEWAL_65 constant with a suggestion for improved reusability.The use of the
B_FULL_RENEWAL_65constant in condition checks is good practice, improving code readability and maintainability.To enhance reusability across different NextJS apps, consider extracting the condition check into a separate function:
const isFullRenewal65 = (answers: any) => answers.applicationFor === B_FULL_RENEWAL_65; // Usage condition: isFullRenewal65,This approach would allow easier reuse of this condition in other parts of the application if needed, and simplify adding new conditions in the future.
libs/application/templates/driving-license/src/lib/dataSchema.ts (1)
15-15: LGTM: New 'pickup' field is correctly implemented.The new 'pickup' field is well-integrated into the schema, using the Pickup enum for type safety and marked as optional for backward compatibility. This aligns with the PR objectives.
Consider adding a brief comment above this field to explain its purpose and when it's used, especially since it's related to a specific age group (65+). This would enhance code readability and maintainability.
libs/api/domains/driving-license/src/lib/drivingLicense.type.ts (2)
12-16: LGTM! Consider adding JSDoc comments for clarity.The
PostRenewal65AndOverInputinterface is well-defined and uses appropriate TypeScript types. It adheres to the coding guidelines for reusability and TypeScript usage.Consider adding JSDoc comments to explain the purpose of each property, especially the meaning of
nullvalues for boolean properties. This would enhance code readability and maintainability. For example:export interface PostRenewal65AndOverInput { /** The ID of the district for pickup, if applicable */ districtId?: number; /** Whether to pick up the plastic license at the district. null if not specified */ pickupPlasticAtDistrict?: boolean | null; /** Whether to send the plastic license to the person. null if not specified */ sendPlasticToPerson?: boolean | null; }
18-21: LGTM! Consider using uppercase for enum values.The
Pickupenum is well-defined and uses appropriate TypeScript syntax. It adheres to the coding guidelines for reusability and TypeScript usage.For consistency with common TypeScript/JavaScript conventions, consider using uppercase for the enum values:
export enum Pickup { POST = 'POST', DISTRICT = 'DISTRICT', }This change would align the enum with the typical style used for constants and improve code readability.
libs/application/templates/driving-license/src/forms/draft/subSectionSummary.ts (1)
31-31: Consider grouping similar imports together.The new import for
Pickupis correctly added, adhering to TypeScript usage. However, for better code organization, consider grouping it with other similar imports from the same module or related modules.libs/application/templates/driving-license/src/lib/drivingLicenseTemplate.ts (1)
Line range hint
50-70: Approved with suggestion: Enhance error handlingThe changes to the
getCodesfunction successfully implement the new pickup option feature, adhering to TypeScript usage and promoting reusability. This is consistent with the PR objectives for adding a pickup option for 65+ driving license submissions.However, I suggest improving the error handling for the
targetCode:Consider moving the error check for
targetCodeafter all modifications to thecodesarray:const codes: string[] = [] const DEFAULT_ITEM_CODE = CHARGE_ITEM_CODES[B_FULL] const targetCode = typeof applicationFor === 'string' ? CHARGE_ITEM_CODES[applicationFor] ? CHARGE_ITEM_CODES[applicationFor] : DEFAULT_ITEM_CODE : DEFAULT_ITEM_CODE codes.push(targetCode) if (pickup === Pickup.POST) { codes.push(CHARGE_ITEM_CODES[DELIVERY_FEE]) } -if (!targetCode) { - throw new Error('No selected charge item code') -} return codes + +if (codes.length === 0) { + throw new Error('No charge item codes selected') +}This change ensures that the error is thrown only if no charge item codes are selected, accounting for both the
targetCodeand the potential delivery fee.libs/application/template-api-modules/src/lib/modules/templates/driving-license-submission/driving-license-submission.service.ts (1)
178-186: LGTM with a minor suggestion: Pickup option handlingThe implementation of the pickup option handling is correct and aligns well with the new functionality for license renewal for individuals aged 65 and over. It effectively uses conditional logic and the spread operator to add properties based on the pickup value.
For improved readability, consider extracting the pickup-related logic into a separate function:
const getPickupOptions = (pickup?: Pickup) => pickup ? { pickupPlasticAtDistrict: pickup === Pickup.DISTRICT, sendPlasticToPerson: pickup === Pickup.POST, } : {}; // Then in the method call: return this.drivingLicenseService.renewDrivingLicense65AndOver( auth.authorization.replace('Bearer ', ''), { ...(jurisdictionId && { districtId: jurisdictionId as number }), ...getPickupOptions(pickup), }, );This refactoring would enhance modularity and make the main method call cleaner.
libs/api/domains/driving-license/src/lib/drivingLicense.service.ts (1)
Line range hint
517-528: LGTM: Well-implemented method for 65+ driving license renewal.The
renewDrivingLicense65AndOvermethod is well-structured and consistent with the existing codebase. It properly handles the API response and returns a typed result.Consider adding error logging for better debugging:
async renewDrivingLicense65AndOver( auth: User['authorization'], input: PostRenewal65AndOverInput, ): Promise<NewDrivingLicenseResult> { - const response = await this.drivingLicenseApi.postRenewLicenseOver65({ - input, - auth: auth, - }) + try { + const response = await this.drivingLicenseApi.postRenewLicenseOver65({ + input, + auth: auth, + }) + return { + success: response.isOk ?? false, + errorMessage: response.errorCode ?? null, + } + } catch (error) { + this.logger.error(`${LOGTAG} Error renewing license for 65+`, error) + return { + success: false, + errorMessage: 'An unexpected error occurred', + } + } - return { - success: response.isOk ?? false, - errorMessage: response.errorCode ?? null, - } }This change will help with debugging and provide a more robust error handling mechanism.
libs/clients/driving-license/src/lib/drivingLicenseApi.service.ts (1)
512-524: LGTM! Consider adding JSDoc comments for better documentation.The changes to the
postRenewLicenseOver65method improve type safety and clarity by using a structured input. The spread operator ensures all properties from the input are included in the API call, which is a good practice.Consider adding JSDoc comments to describe the purpose of the method and its parameters. This would enhance the documentation and make the code more maintainable. For example:
/** * Renews a driving license for individuals over 65 years old. * @param params - The parameters for the renewal * @param params.input - The input data for the renewal * @param params.auth - The authentication token * @returns A promise that resolves with the renewal result */ async postRenewLicenseOver65(params: { input: v5.PostRenewal65AndOver auth: string }) { // ... (rest of the method implementation) }libs/clients/driving-license/src/v5/clientConfig.json (3)
Line range hint
2649-2760: New endpoints for 65+ license renewals look good!The addition of the following endpoints aligns well with the PR objectives:
- GET /api/drivinglicense/v5/canapplyfor/renewal65
- POST /api/drivinglicense/v5/applications/renewal65
These endpoints provide the necessary functionality to check eligibility and submit applications for 65+ license renewals.
Consider adding a brief description for each endpoint to improve API documentation. For example:
"summary": "Check eligibility for 65+ license renewal", "description": "Determines if a person over 65 years old is eligible to apply for a driving license renewal."
3652-3661: Schema updates for 65+ license renewals are appropriate.The PostRenewal65AndOver schema has been updated with new properties that address the PR objective:
- pickupPlasticAtDistrict
- sendPlasticToPerson
These additions allow specifying whether the license should be picked up at the district office or sent to the person, which is in line with the new pickup option feature.
Consider adding a validation rule to ensure that only one of these options can be true at a time, as it wouldn't make sense for both to be true simultaneously. You could add a note in the schema description or implement this validation in the backend logic.
Line range hint
2761-3651: New statistics endpoints and schemas add valuable functionality.Several new endpoints and schemas have been added for retrieving statistics on temporary licenses, full licenses, and driving assessments. These include:
- GET /api/drivinglicense/v5/statistics/temporarylicenses/created/detailed/{yearfrom}/{monthfrom}
- GET /api/drivinglicense/v5/statistics/temporarylicenses/created/grouped/{yearfrom}/{monthfrom}
- GET /api/drivinglicense/v5/statistics/licenses/created/detailed/{yearfrom}/{monthfrom}
- GET /api/drivinglicense/v5/statistics/licenses/created/grouped/{yearfrom}/{monthfrom}
- GET /api/drivinglicense/v5/statistics/drivingassessments/created/detailed/{yearfrom}/{monthfrom}
- GET /api/drivinglicense/v5/statistics/drivingassessments/created/grouped/{yearfrom}/{monthfrom}
These additions provide valuable data for analyzing system usage and performance.
Consider implementing a more generic statistics endpoint that could handle different types of data (temporary licenses, full licenses, driving assessments) through query parameters. This could reduce code duplication and make the API more flexible for future additions. For example:
GET /api/drivinglicense/v5/statistics/{dataType}/created/{format}/{yearfrom}/{monthfrom}Where
dataTypecould be "temporarylicenses", "licenses", or "drivingassessments", andformatcould be "detailed" or "grouped".
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
- libs/api/domains/driving-license/src/lib/drivingLicense.service.ts (2 hunks)
- libs/api/domains/driving-license/src/lib/drivingLicense.type.ts (1 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/driving-license-submission/driving-license-submission.service.ts (3 hunks)
- libs/application/templates/driving-license/src/forms/draft/subSectionDelivery.ts (1 hunks)
- libs/application/templates/driving-license/src/forms/draft/subSectionSummary.ts (1 hunks)
- libs/application/templates/driving-license/src/lib/constants.ts (0 hunks)
- libs/application/templates/driving-license/src/lib/dataSchema.ts (2 hunks)
- libs/application/templates/driving-license/src/lib/drivingLicenseTemplate.ts (1 hunks)
- libs/application/templates/driving-license/src/lib/types.ts (1 hunks)
- libs/clients/driving-license/src/lib/drivingLicenseApi.service.ts (1 hunks)
- libs/clients/driving-license/src/v5/clientConfig.json (1 hunks)
💤 Files with no reviewable changes (1)
- libs/application/templates/driving-license/src/lib/constants.ts
🧰 Additional context used
📓 Path-based instructions (10)
libs/api/domains/driving-license/src/lib/drivingLicense.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/api/domains/driving-license/src/lib/drivingLicense.type.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/template-api-modules/src/lib/modules/templates/driving-license-submission/driving-license-submission.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/application/templates/driving-license/src/forms/draft/subSectionDelivery.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/driving-license/src/forms/draft/subSectionSummary.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/driving-license/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/driving-license/src/lib/drivingLicenseTemplate.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/driving-license/src/lib/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."
libs/clients/driving-license/src/lib/drivingLicenseApi.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/driving-license/src/v5/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."
🔇 Additional comments (13)
libs/application/templates/driving-license/src/lib/types.ts (2)
Line range hint
1-33: Adherence to coding guidelines confirmed.The code in this file follows the specified guidelines for
libs/**/*:
- The
Pickupenum and other types are potentially reusable across different NextJS apps.- TypeScript is used effectively for defining and exporting types.
- The code structure supports effective tree-shaking and bundling practices.
9-12: Verify usage of the new Pickup enum in related components.While the addition of the
Pickupenum doesn't directly impact existing types in this file, it's important to ensure it's properly integrated into the application.Please run the following script to check for potential usage of the new
Pickupenum:This will help identify areas where the new enum should be used or where existing code might need updates.
✅ Verification successful
Pickup enum is properly integrated and utilized in the relevant files.
No further action is required.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential usage of the new Pickup enum # Search for files that might use the Pickup enum echo "Files potentially using the Pickup enum:" rg --type typescript --type javascript -l "Pickup\.(POST|DISTRICT)" # Search for files that might need to be updated to use the new Pickup enum echo "\nFiles that might need to be updated to use the new Pickup enum:" rg --type typescript --type javascript -l "pickup.*['\"](?:post|district)['\"]"Length of output: 457
Script:
#!/bin/bash # Description: Check for potential usage of the new Pickup enum # Search for files that might use the Pickup enum echo "Files potentially using the Pickup enum:" rg --type ts --type js -l "Pickup\.(POST|DISTRICT)" # Search for files that might need to be updated to use the new Pickup enum echo "\nFiles that might need to be updated to use the new Pickup enum:" rg --type ts --type js -l "pickup.*['\"](?:post|district)['\"]"Length of output: 787
libs/application/templates/driving-license/src/forms/draft/subSectionDelivery.ts (1)
Line range hint
1-63: Approve adherence to coding guidelines for libs/ directory.The code successfully meets the guidelines for files in the
libs/directory:
- It uses TypeScript for defining props and exporting types, enhancing type safety.
- The
subSectionDeliveryfunction is exported, making it reusable across different NextJS apps.- The modular structure of the code, with separate build functions for different field types, promotes reusability and maintainability.
- The use of imported constants and types allows for effective tree-shaking and bundling.
Great job on following the established coding practices!
libs/application/templates/driving-license/src/lib/dataSchema.ts (2)
4-4: LGTM: New import statement is correctly implemented.The import for
Pickupfrom './types' is appropriately placed and follows the correct naming convention for TypeScript types.
Line range hint
1-67: Excellent adherence to coding guidelines for library code.The changes in this file align well with the coding guidelines for
libs/**/*:
Reusability: The addition of the
Pickupenum and its integration into the schema enhances the reusability of this component across different NextJS apps that may require driving license application functionality.TypeScript usage: The changes demonstrate proper TypeScript usage. The
Pickupenum is imported and utilized for type-safe validation in the schema.Tree-shaking and bundling: The specific import of
Pickupfrom './types' allows for effective tree-shaking, ensuring that only the necessary code will be included in the final bundle.These changes maintain the high quality and efficiency standards expected in our shared library code.
libs/api/domains/driving-license/src/lib/drivingLicense.type.ts (1)
Line range hint
1-21: Verify: Code adheres to coding guidelines. Consider adding file-level documentation.The code in this file adheres to the coding guidelines for
libs/**/*files:
- It uses TypeScript for defining props and exporting types.
- The new interface and enum are reusable across different NextJS apps dealing with driving licenses.
To further improve the code's adherence to best practices, consider adding a file-level JSDoc comment explaining the purpose of this file and its contents. This would enhance maintainability and make it easier for other developers to understand the file's role in the project.
Example:
/** * @fileoverview This file contains TypeScript type definitions for driving license-related entities. * It includes interfaces and enums used across the driving license domain of the application. */To verify the impact of these changes on tree-shaking and bundling, we can run the following script:
This script will help us ensure that the file doesn't use default exports (which can hinder tree-shaking) and check how the types are imported in other files, which can affect bundling.
✅ Verification successful
Verified: Code adheres to coding guidelines.
The
drivingLicense.type.tsfile follows the established coding standards:
- No default exports are present, which supports effective tree-shaking.
- Type imports are correctly utilized across multiple files, ensuring proper modularization.
To enhance maintainability and clarity, consider adding a file-level JSDoc comment explaining the purpose and contents of this file. This documentation will aid other developers in understanding the role of these type definitions within the project.
Example:
/** * @fileoverview This file contains TypeScript type definitions for driving license-related entities. * It includes interfaces and enums used across the driving license domain of the application. */🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for default exports and verify import usage # Test 1: Check for default exports (which can hinder tree-shaking) echo "Checking for default exports:" grep -n "export default" libs/api/domains/driving-license/src/lib/drivingLicense.type.ts # Test 2: Verify import usage in other files echo "Checking import usage:" rg --type typescript "import .* from ['\"].*drivingLicense\.type['\"]" libsLength of output: 331
Script:
#!/bin/bash # Description: Check for default exports and verify import usage # Test 1: Check for default exports (which can hinder tree-shaking) echo "Checking for default exports:" grep -n "export default" libs/api/domains/driving-license/src/lib/drivingLicense.type.ts # Test 2: Verify import usage in other files echo "Checking import usage:" rg --type ts "import .* from ['\"].*drivingLicense\.type['\"]" libsLength of output: 740
libs/application/templates/driving-license/src/forms/draft/subSectionSummary.ts (1)
Line range hint
144-149: Approved: Good use of thePickuptype for enhanced type safety.The addition of the
Pickuptype and its usage in determining the pickup location message is well-implemented. It improves type safety and code clarity. The logic is correct and aligns with the reusability principle for components across different NextJS apps.libs/application/templates/driving-license/src/lib/drivingLicenseTemplate.ts (1)
48-48: LGTM: New import adheres to guidelinesThe new import for
Pickupfrom./typesis consistent with the coding guidelines. It supports TypeScript usage for defining types and promotes effective tree-shaking by importing only the necessary type.libs/application/template-api-modules/src/lib/modules/templates/driving-license-submission/driving-license-submission.service.ts (3)
6-6: LGTM: Import of Pickup typeThe addition of the
Pickuptype import is correct and aligns with the new functionality being implemented. It follows good TypeScript practices by explicitly importing the type.
152-152: LGTM: Pickup variable declarationThe declaration of the
pickupvariable is well-implemented. It uses TypeScript for type safety and optional chaining for safe property access. This change effectively supports the new pickup functionality.
Line range hint
1-286: Overall assessment: Well-implemented feature additionThe changes in this file effectively implement the new pickup option for driving license renewal for individuals aged 65 and over. The code adheres to TypeScript best practices, ensures type safety, and maintains good reusability. The implementation aligns well with the PR objectives and follows the provided coding guidelines.
Key points:
- Proper import of the new
Pickuptype.- Effective use of TypeScript for type safety in variable declarations.
- Correct implementation of conditional logic for handling pickup options.
The code is ready for merge, with a minor suggestion for improving readability by extracting the pickup option logic into a separate function.
libs/api/domains/driving-license/src/lib/drivingLicense.service.ts (1)
18-18: LGTM: New import for 65+ renewal functionality.The addition of
PostRenewal65AndOverInputto the import statement is consistent with the existing code structure and relates to the new functionality being implemented.libs/clients/driving-license/src/v5/clientConfig.json (1)
Line range hint
1-3662: Overall, the changes to the OpenAPI specification are well-implemented and valuable.The updates to this file successfully address the PR objectives by adding endpoints and schema updates for 65+ license renewals. The new pickup option is clearly defined in the PostRenewal65AndOver schema.
Additionally, the introduction of statistics endpoints provides valuable functionality for system analysis and reporting. These changes enhance the API's capabilities without disrupting its overall structure.
The modifications are consistent with the existing API design and follow OpenAPI 3.0.1 specifications. They should integrate well with the existing system while providing new features and insights.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16453 +/- ##
==========================================
- Coverage 36.78% 36.73% -0.06%
==========================================
Files 6835 6835
Lines 141418 141262 -156
Branches 40266 40206 -60
==========================================
- Hits 52019 51890 -129
+ Misses 89399 89372 -27
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 20 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 8 Passed Test ServicesThis report shows up to 10 services
|
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Adds pickupPlasticAtDistrict/sendPlasticToPerson to 65+ submissions
Summary by CodeRabbit
New Features
Bug Fixes