Delete unused payment links on order payment and update related #1578
Conversation
WalkthroughThe pull request introduces enhancements to notification and payment processing functionality across multiple service components. The changes focus on improving the handling of user notifications related to order payments, specifically adding methods to retrieve and manage notification parameters. A new repository method allows fetching specific notification parameters by user notification and key, while the service implementation adds logic to remove payment links after successful order payment processing. Additionally, utility methods and tests have been updated to support these functionalities. Changes
Sequence DiagramsequenceDiagram
participant Client
participant UBSClientService
participant UserNotificationRepo
participant NotificationParameterRepo
Client->>UBSClientService: Process Payment
UBSClientService->>UserNotificationRepo: Find User Notification
UserNotificationRepo-->>UBSClientService: Return Notification
UBSClientService->>NotificationParameterRepo: Remove Payment Link
NotificationParameterRepo-->>UBSClientService: Confirm Removal
UBSClientService-->>Client: Payment Processed
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)
🪧 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
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (3)
service/src/test/java/greencity/ModelUtils.java (1)
2871-2875: Consider usingOptional.of()and extracting constants.
Since there is no scenario where theNotificationParameteris null,Optional.of()is more explicit thanofNullable(). Also, to improve consistency and reduce duplication, consider extracting"payButton"and the URL into constants.- return Optional - .ofNullable(NotificationParameter.builder().key("payButton").value("https://pay.monobank.ua/api").build()); + return Optional.of( + NotificationParameter.builder() + .key(PAY_BUTTON_KEY) + .value(MONOBANK_URL) + .build() + );service/src/main/java/greencity/service/ubs/UBSClientServiceImpl.java (2)
239-239: Prefer descriptive naming for the constant 'PAY_BUTTON'.
WhilePAY_BUTTONis fine, consider a name likePAYMENT_LINK_KEYto clarify that it represents a particular notification key.- private static final String PAY_BUTTON = "payButton"; + private static final String PAYMENT_LINK_KEY = "payButton";
2225-2234: MethodremovePaymentLinkForOrderis well-structured.
- Retrieves the user notification for an unpaid order.
- Looks for the payment link parameter.
- Deletes it if present.
Potential improvement: Log or handle scenarios where the notification or parameter is not found, so there's a clear audit trail.
+ log.debug("Removing payment link for order {}", order.getId()); Optional<UserNotification> userNotification = ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dao/src/main/java/greencity/repository/NotificationParameterRepository.java(1 hunks)service/src/main/java/greencity/service/ubs/UBSClientServiceImpl.java(9 hunks)service/src/test/java/greencity/ModelUtils.java(1 hunks)service/src/test/java/greencity/service/ubs/UBSClientServiceImplTest.java(10 hunks)
🔇 Additional comments (20)
dao/src/main/java/greencity/repository/NotificationParameterRepository.java (2)
13-13: No issues found.
14-15: Ensure null safety in search parameters.
If eitheruserNotificationorkeycan be null, consider using defensive checks or documenting the expected behavior when these inputs are invalid to avoid potentialNullPointerExceptionor unintended queries.service/src/main/java/greencity/service/ubs/UBSClientServiceImpl.java (7)
66-67: Imports look good.
97-97: Minor naming alignment check.
Consider whether the existing enums inNotificationTypeare consistent with naming conventions (e.g., uppercase constants). If everything is in order, proceed.
121-121: New repository import.
This import is used to fetch and delete payment links. No issues found here.
134-134: New import for user notifications.
No concerns; the usage of a separate repository for user notifications keeps responsibilities clear.
276-277: Injected repositories.
Good job injecting bothUserNotificationRepositoryandNotificationParameterRepositoryfor removing unused payment links. No further changes recommended.
1700-1700: Removing payment links on successful order payment.
This call toremovePaymentLinkForOrder(order)ensures the link is cleaned up once the order is paid. No issues found.
2190-2190: MonoBank flow callsremovePaymentLinkForOrder(order).
Ensures consistency between different payment providers. Looks good.service/src/test/java/greencity/service/ubs/UBSClientServiceImplTest.java (11)
51-52: Imports for notification-related entities.
These additions align with new test coverage for removing payment links.
74-74: Confirm test coverage forNotificationType.
Ensure that tests verify the correctNotificationTypeusage (e.g.,UNPAID_ORDER).
96-96: New import forNotificationParameterRepository.
No code smells; properly aligns with the new test implementation.
109-109: New import forUserNotificationRepository.
Ensures the test coverage extends to user notifications references.
195-196: New utility references for test objects.
Ensures consistent test data creation. No issues found.
236-236: Extended test coverage with user notifications.
Good approach to reusing existing model utility methods.
408-413: Mocking new repositories in test.
Defining mocks forUserNotificationRepositoryandNotificationParameterRepositoryensures thorough coverage of new logic.
3808-3813: Tests forvalidatePaymentnow check for user notifications.
- Use of
userNotificationRepositoryto retrieve the notification.- Use of
notificationParameterRepositoryto retrieve the payment link.
No immediate issues; ensures proper coverage.
3824-3827: Verifications for user notifications in success flow.
Ensures the link is found and deleted. Looks correct.
4268-4273: Testing MonoBank flow for user notifications.
Covers new code that deletes notification parameters. No issues found.
4281-4284: Ensures cleanup is triggered in MonoBank paths.
Verifies the link removal logic is called upon payment success. Completes coverage for all providers.
|
Cr1stal423
deleted the
feature/7981/remove-payment-llink-from-DB-when-order-is-paid
branch
😎 GreenCityUBS PR 😎
Issue Link:
7981
Changes:
Added functionality to remove the link to pay for the notification, which was previously with the unpaid status, from the notification_parameters table after it is paid
Update related tests
Summary by CodeRabbit
New Features
Bug Fixes
Tests