Fix: Payment Verification Error Handling#234
Conversation
…g in markPaymentAsVerified and markPaymentAsCancelled
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #223 by ensuring payment status update operations no longer fail silently when a paymentId doesn’t exist, improving correctness and observability in the payment verification flow.
Changes:
- Updated
markPaymentAsVerifiedandmarkPaymentAsCancelledto throw when no payment record is found (replacingifPresentwithorElseThrow). - Added warning + success logs around payment status updates.
- Introduced a dedicated
PaymentNotFoundException.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
RestroHub/src/main/java/com/restroly/qrmenu/payment/service/PaymentServiceImpl.java |
Switches from silent no-op to exception-based handling when paymentId is missing; adds logs for not-found and success paths. |
RestroHub/src/main/java/com/restroly/qrmenu/payment/exception/PaymentNotFoundException.java |
Adds a new exception type intended to represent a missing payment record. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| package com.restroly.qrmenu.payment.exception; | ||
|
|
||
| public class PaymentNotFoundException extends RuntimeException { | ||
| public PaymentNotFoundException(String message) { | ||
| super(message); | ||
| } | ||
| } |
|
|
||
| PaymentVerification entity = verificationRepository.findByPaymentId(paymentId) | ||
| .orElseThrow(() ->{ | ||
| log.warn("PaymentId: {} not found in database",paymentId); |
|
|
||
| PaymentVerification entity = verificationRepository.findByPaymentId(paymentId) | ||
| .orElseThrow(() -> { | ||
| log.warn("PaymentId: {} not found in database",paymentId); |
| PaymentVerification entity = verificationRepository.findByPaymentId(paymentId) | ||
| .orElseThrow(() ->{ | ||
| log.warn("PaymentId: {} not found in database",paymentId); | ||
| return new PaymentNotFoundException("Payment record not found for paymentId: " + paymentId); | ||
| }); |
|
Hi @iRahmanG , |
…ception for proper 404 mapping
|
Hi @rdodiya I’ve updated Thanks for the review! |
Summary
This PR fixes the issue in
PaymentServiceImplwheremarkPaymentAsVerifiedandmarkPaymentAsCancelledsilently ignored invalidpaymentIdvalues.Now, both methods throw
PaymentNotFoundExceptionwhen the payment record is missing.Changes
ifPresentwithorElseThrowin both methodsPaymentNotFoundExceptionfor clear error reportingIssue Reference
Fixes: #223