chore(application-system): Payments updates to lifecycle on callback, UX fixes. #15883
Conversation
WalkthroughThe changes involve enhancements to the API documentation by adding a new tag for payment callbacks, restructuring the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
obmagnusson
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
libs/application/ui-components/src/components/PaymentPending/PaymentPending.tsx (1)
92-112: Enhance error handling by using localized messages.The changes to the error handling section are approved. The
AlertMessagecomponent provides a structured and visually distinct error notification, enhancing the user experience.Consider using localized messages for the error message text instead of hardcoding it in Icelandic. This will make it easier to support multiple languages in the future. You can use the
useMsghook and define the error message in the message catalog.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/application-system/api/src/openApi.ts (1 hunks)
- libs/api/domains/payment/src/lib/api-domains-payment.types.ts (2 hunks)
- libs/application/api/core/src/lib/application/application.service.ts (1 hunks)
- libs/application/api/payment/src/lib/payment-callback.controller.ts (2 hunks)
- libs/application/core/src/lib/messages.ts (1 hunks)
- libs/application/template-api-modules/src/lib/modules/templates/example-payment-actions/examplePaymentActions.service.ts (1 hunks)
- libs/application/ui-components/src/components/PaymentPending/PaymentPending.tsx (2 hunks)
Additional context used
Path-based instructions (7)
apps/application-system/api/src/openApi.ts (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."
libs/api/domains/payment/src/lib/api-domains-payment.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/application/api/payment/src/lib/payment-callback.controller.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/example-payment-actions/examplePaymentActions.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/ui-components/src/components/PaymentPending/PaymentPending.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/api/core/src/lib/application/application.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/core/src/lib/messages.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."
Additional comments not posted (9)
apps/application-system/api/src/openApi.ts (1)
11-11: LGTM!The addition of the
payment-callbacktag to theopenApiconfiguration is a valuable improvement that enhances the organization and clarity of the API documentation. The tag is added in the correct order, maintaining the logical grouping of related tags, and adheres to the existing code style and structure.libs/api/domains/payment/src/lib/api-domains-payment.types.ts (3)
1-3: LGTM!The new imports from
class-validatorand@nestjs/swaggerare necessary for adding validation decorators and API documentation annotations to theCallbackclass.
21-26: LGTM!The new enum
paidStatusimproves type safety and readability by defining a set of allowed values for thestatusproperty of theCallbackclass.
28-42: Excellent refactoring!The changes to the
Callbackinterface, transforming it into a class, provide several benefits:
The validation decorators from
class-validatorensure that the properties are not empty and are of the correct type, improving data integrity.The API documentation annotations from
@nestjs/swaggerenrich the API documentation, making it easier for consumers to understand the structure and constraints of theCallbackentity.The use of the
paidStatusenum for thestatusproperty improves type safety by restricting the allowed values to a predefined set.Overall, these changes enhance the structure, validation capabilities, and documentation of the
Callbackentity.libs/application/api/payment/src/lib/payment-callback.controller.ts (2)
10-13: LGTM!The code changes are approved.
Line range hint
15-42: LGTM!The code changes are approved.
libs/application/ui-components/src/components/PaymentPending/PaymentPending.tsx (1)
8-14: LGTM!The changes to the import statements are approved. The imported components are used to enhance the user interface and feedback during payment processing. The import statements follow the correct syntax and adhere to the TypeScript usage for defining props and exporting types.
libs/application/api/core/src/lib/application/application.service.ts (1)
302-302: LGTM!The code change is approved.
libs/application/core/src/lib/messages.ts (1)
424-429: LGTM!The new
paymentSubmitFailedDescriptionentry incoreErrorMessageslooks good. It follows the existing pattern and provides a clear error message to display when a payment submission fails.
...i-modules/src/lib/modules/templates/example-payment-actions/examplePaymentActions.service.ts
Outdated
Show resolved
Hide resolved
libs/application/api/payment/src/lib/payment-callback.controller.ts
Outdated
Show resolved
Hide resolved
libs/application/ui-components/src/components/PaymentPending/PaymentPending.tsx
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15883 +/- ##
==========================================
+ Coverage 36.65% 36.68% +0.02%
==========================================
Files 6750 6759 +9
Lines 138925 138993 +68
Branches 39472 39481 +9
==========================================
+ Hits 50926 50983 +57
- Misses 87999 88010 +11 Flags with carried forward coverage won't be shown. Click here to find out more.
... and 9 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 17 Total Test Services: 0 Failed, 17 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- libs/api/domains/payment/src/lib/api-domains-payment.types.ts (2 hunks)
- libs/application/api/payment/src/lib/payment-callback.controller.ts (2 hunks)
- libs/application/ui-components/src/components/PaymentPending/PaymentPending.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- libs/application/api/payment/src/lib/payment-callback.controller.ts
- libs/application/ui-components/src/components/PaymentPending/PaymentPending.tsx
Additional context used
Path-based instructions (1)
libs/api/domains/payment/src/lib/api-domains-payment.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."
Additional comments not posted (3)
libs/api/domains/payment/src/lib/api-domains-payment.types.ts (3)
1-3: LGTM!The new imports from
class-validatorand@nestjs/swaggerlibraries are necessary for using validation decorators and API documentation annotations in theCallbackclass.
21-26: LGTM!The new enum
PaidStatusprovides a clear and type-safe way to represent the possible payment statuses. It is used to define the type of thestatusproperty in theCallbackclass, which improves type safety and readability.
28-42: Excellent improvements to theCallbackentity! 👍The conversion of the
Callbackinterface to a class with validation decorators and API documentation annotations is a significant enhancement that brings several benefits:
Improved structure: The class structure provides a clearer and more organized representation of the
Callbackentity.Enhanced type safety: The use of validation decorators from
class-validatorensures that the properties are not empty and are of the correct type. This helps prevent errors and improves the robustness of the code.Better API documentation: The API documentation annotations from
@nestjs/swaggerprovide clear and informative documentation for theCallbackentity in the API. This makes it easier for developers to understand and use the API.Overall, these changes greatly improve the clarity, type safety, and documentation of the
Callbackentity. Well done!
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (4)
libs/api/domains/payment/src/lib/api-domains-payment.types.ts (2)
20-25: LGTM: ThePaidStatusenum is well-defined and enhances type safety.The enum provides a clear set of possible payment statuses, which is good for type safety and code clarity. It's also exported, allowing for reuse across different parts of the application, which aligns with our reusability guidelines.
Consider adding JSDoc comments to describe each status, especially for
recreatedandrecreatedAndPaid, to improve documentation:export enum PaidStatus { /** Payment successfully completed */ paid = 'paid', /** Payment was cancelled */ cancelled = 'cancelled', /** Payment was recreated (please add more context) */ recreated = 'recreated', /** Payment was recreated and then paid (please add more context) */ recreatedAndPaid = 'recreatedAndPaid', }
27-38: LGTM: TheCallbackclass is well-structured and follows best practices.The conversion of the
Callbackinterface to a class with decorators enhances type safety, provides runtime validation, and improves API documentation. This aligns well with NestJS best practices and our TypeScript usage guidelines. The use of readonly properties ensures immutability, which is a good practice for data objects.For consistency, consider adding an
@ApiProperty()decorator to thestatusproperty that includes a description of the enum:@IsEnum(PaidStatus) @ApiProperty({ enum: PaidStatus, description: 'The status of the payment callback' }) readonly status!: PaidStatusThis addition would provide more context in the API documentation, improving clarity for API consumers.
apps/application-system/api/src/app/modules/application/e2e/payment/payment-callback.spec.ts (2)
19-28: LGTM! Consider using a constant for the UUID.The changes improve the test case by simplifying the payload structure and using a more realistic UUID format. The new expectation syntax is also more idiomatic for Jest.
Consider defining the UUID as a constant at the top of the file or in a separate test utilities file. This would make it easier to reuse and update if needed:
const SAMPLE_UUID = '123e4567-e89b-12d3-a456-426614174000';Then use it in the test case:
receptionID: SAMPLE_UUID,
38-40: LGTM! Consider updating the expectation syntax for consistency.The changes in this test case are consistent with the improvements made in the first test case, which is good for maintaining overall test consistency and quality.
For consistency with the first test case, consider updating the expectation syntax:
- .expect(400) + const response = await server + .post( + '/application-payment/32eee126-6b7f-4fca-b9a0-a3618b3e42bf/missing-id', + ) + .send({ + receptionID: SAMPLE_UUID, // Use the constant suggested earlier + chargeItemSubject: 'nice subject.. not', + status: 'paid', + }); + expect(response.status).toBe(400);This change would make both test cases use the same expectation style, improving overall consistency.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- apps/application-system/api/src/app/modules/application/e2e/payment/payment-callback.spec.ts (2 hunks)
- libs/api/domains/payment/src/lib/api-domains-payment.types.ts (2 hunks)
Additional context used
Path-based instructions (2)
apps/application-system/api/src/app/modules/application/e2e/payment/payment-callback.spec.ts (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."
libs/api/domains/payment/src/lib/api-domains-payment.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."
Additional comments not posted (2)
libs/api/domains/payment/src/lib/api-domains-payment.types.ts (2)
1-2: LGTM: Import statements are appropriate and align with TypeScript usage guidelines.The new imports from 'class-validator' and '@nestjs/swagger' are necessary for the class decorators and API documentation, which enhances type safety and API clarity.
Line range hint
1-38: Great job adhering to the coding guidelines forlibsdirectory!The changes in this file align well with the specified guidelines:
- Reusability: Both the
PaidStatusenum andCallbackclass are exported, allowing for reuse across different NextJS apps.- TypeScript usage: The code makes excellent use of TypeScript features, including enums, classes, and decorators for defining props and exporting types.
- Tree-shaking: The use of named exports (enum and class) allows for effective tree-shaking and bundling.
These changes contribute to creating a robust, reusable, and maintainable codebase.
What
Why
Better UX.
Problem: Application users may experience disruptions after a successful payment due to a service failure. Currently, the payment state lifecycle is limited to 24 hours, which may not be enough time to resolve the issue.
Solution: We will extend the payment state lifecycle, allowing users to continue with the application once the fault is resolved.
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
Callbackstructure with validation and documentation annotations.Bug Fixes