From 2701dbafb5e0006d96377b0d9e8594b055f7b1dc Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Thu, 2 Apr 2026 17:02:14 +0800 Subject: [PATCH 1/4] Add bill refund request and approval workflow --- .../module/billing/api/BillService.java | 49 ++++++ .../hibernate/ImmutableBillInterceptor.java | 3 +- .../billing/api/impl/BillServiceImpl.java | 58 +++++++ .../module/billing/api/model/Bill.java | 22 +++ .../module/billing/api/model/BillStatus.java | 5 +- .../billing/validator/BillValidator.java | 14 ++ api/src/main/resources/Bill.hbm.xml | 16 ++ api/src/main/resources/messages.properties | 2 + .../module/billing/api/model/BillTest.java | 69 +++++++++ .../billing/impl/BillServiceImplTest.java | 144 ++++++++++++++++++ .../billing/validator/BillValidatorTest.java | 52 +++++++ .../web/rest/resource/BillResource.java | 27 +++- omod/src/main/resources/liquibase.xml | 42 +++++ 13 files changed, 500 insertions(+), 3 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/billing/api/BillService.java b/api/src/main/java/org/openmrs/module/billing/api/BillService.java index bf51980d..a1e91c95 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/BillService.java +++ b/api/src/main/java/org/openmrs/module/billing/api/BillService.java @@ -144,4 +144,53 @@ public interface BillService extends OpenmrsService { @Authorized(PrivilegeConstants.VIEW_BILLS) boolean isBillEditable(Bill bill); + /** + * Requests a refund for a paid bill. + *

+ * The bill must have status {@code PAID}. On success, the bill's status transitions to + * {@code REFUND_REQUESTED} and the refund request metadata (reason, requestedBy, date) is recorded. + *

+ * + * @param bill the bill to request a refund for + * @param refundReason the reason for requesting the refund (required, cannot be blank) + * @return the updated bill with status {@code REFUND_REQUESTED} + * @throws org.openmrs.api.APIAuthenticationException if the user lacks MANAGE_BILLS privilege + * @throws IllegalArgumentException if the bill is null, not in PAID status, or refundReason is + * blank + */ + @Authorized(PrivilegeConstants.MANAGE_BILLS) + Bill requestRefund(Bill bill, String refundReason); + + /** + * Approves a pending refund request. + *

+ * The bill must have status {@code REFUND_REQUESTED}. On success, the bill's status transitions to + * {@code REFUNDED} and the approval metadata (approvedBy, date) is recorded. + *

+ * + * @param bill the bill whose refund request is being approved + * @return the updated bill with status {@code REFUNDED} + * @throws org.openmrs.api.APIAuthenticationException if the user lacks REFUND_MONEY privilege + * @throws IllegalArgumentException if the bill is null or not in REFUND_REQUESTED status + */ + @Authorized(PrivilegeConstants.REFUND_MONEY) + Bill approveRefund(Bill bill); + + /** + * Rejects a pending refund request. + *

+ * The bill must have status {@code REFUND_REQUESTED}. On success, the bill's status transitions to + * {@code REFUND_DENIED} and the rejection metadata (reason, rejectedBy, date) is recorded. + *

+ * + * @param bill the bill whose refund request is being rejected + * @param denialReason the reason for denying the refund (required, cannot be blank) + * @return the updated bill with status {@code REFUND_DENIED} + * @throws org.openmrs.api.APIAuthenticationException if the user lacks REFUND_MONEY privilege + * @throws IllegalArgumentException if the bill is null, not in REFUND_REQUESTED status, or + * denialReason is blank + */ + @Authorized(PrivilegeConstants.REFUND_MONEY) + Bill rejectRefund(Bill bill, String denialReason); + } diff --git a/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/ImmutableBillInterceptor.java b/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/ImmutableBillInterceptor.java index 8360cbff..46ffd021 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/ImmutableBillInterceptor.java +++ b/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/ImmutableBillInterceptor.java @@ -29,7 +29,8 @@ public class ImmutableBillInterceptor extends ImmutableEntityInterceptor { private static final String[] MUTABLE_PROPERTY_NAMES = new String[] { "changedBy", "dateChanged", "voided", "dateVoided", "voidedBy", "voidReason", "payment", "billAdjusted", "adjustmentReason", "adjustedBy", "receiptPrinted", - "status", "receiptNumber" }; + "status", "receiptNumber", "refundReason", "refundRequestedBy", "dateRefundRequested", "refundApprovedBy", + "dateRefundApproved", "refundDenialReason", "refundRejectedBy", "dateRefundRejected" }; @Override protected Class getSupportedType() { diff --git a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java index f6766a9c..12dee746 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java +++ b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java @@ -21,12 +21,14 @@ import org.openmrs.module.billing.api.base.PagingInfo; import org.openmrs.module.billing.api.db.BillDAO; import org.openmrs.module.billing.api.model.Bill; +import org.openmrs.module.billing.api.model.BillStatus; import org.openmrs.module.billing.api.search.BillSearch; import org.openmrs.module.billing.util.ReceiptGenerator; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.transaction.annotation.Transactional; import java.util.Collections; +import java.util.Date; import java.util.List; /** @@ -175,4 +177,60 @@ public boolean isBillEditable(Bill bill) { return true; } + @Override + @Transactional + public Bill requestRefund(Bill bill, String refundReason) { + if (bill == null) { + throw new IllegalArgumentException("The bill must be defined."); + } + if (StringUtils.isBlank(refundReason)) { + throw new IllegalArgumentException("refundReason cannot be null or empty"); + } + if (bill.getStatus() != BillStatus.PAID) { + throw new IllegalArgumentException( + "Only PAID bills can have a refund requested. Current status: " + bill.getStatus()); + } + bill.setRefundReason(refundReason); + bill.setRefundRequestedBy(Context.getAuthenticatedUser()); + bill.setDateRefundRequested(new Date()); + bill.setStatus(BillStatus.REFUND_REQUESTED); + return billDAO.saveBill(bill); + } + + @Override + @Transactional + public Bill approveRefund(Bill bill) { + if (bill == null) { + throw new IllegalArgumentException("The bill must be defined."); + } + if (bill.getStatus() != BillStatus.REFUND_REQUESTED) { + throw new IllegalArgumentException( + "Only bills with REFUND_REQUESTED status can be approved. Current status: " + bill.getStatus()); + } + bill.setRefundApprovedBy(Context.getAuthenticatedUser()); + bill.setDateRefundApproved(new Date()); + bill.setStatus(BillStatus.REFUNDED); + return billDAO.saveBill(bill); + } + + @Override + @Transactional + public Bill rejectRefund(Bill bill, String denialReason) { + if (bill == null) { + throw new IllegalArgumentException("The bill must be defined."); + } + if (StringUtils.isBlank(denialReason)) { + throw new IllegalArgumentException("denialReason cannot be null or empty"); + } + if (bill.getStatus() != BillStatus.REFUND_REQUESTED) { + throw new IllegalArgumentException( + "Only bills with REFUND_REQUESTED status can be rejected. Current status: " + bill.getStatus()); + } + bill.setRefundDenialReason(denialReason); + bill.setRefundRejectedBy(Context.getAuthenticatedUser()); + bill.setDateRefundRejected(new Date()); + bill.setStatus(BillStatus.REFUND_DENIED); + return billDAO.saveBill(bill); + } + } diff --git a/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java b/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java index bf65b11b..c963651b 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java +++ b/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java @@ -16,6 +16,7 @@ import java.math.BigDecimal; import java.security.AccessControlException; import java.util.ArrayList; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -25,6 +26,7 @@ import org.openmrs.BaseOpenmrsData; import org.openmrs.Patient; import org.openmrs.Provider; +import org.openmrs.User; import org.openmrs.api.context.Context; import org.openmrs.module.billing.api.util.PrivilegeConstants; import org.openmrs.module.stockmanagement.api.model.StockItem; @@ -63,6 +65,22 @@ public class Bill extends BaseOpenmrsData { private String adjustmentReason; + private String refundReason; + + private User refundRequestedBy; + + private Date dateRefundRequested; + + private User refundApprovedBy; + + private Date dateRefundApproved; + + private String refundDenialReason; + + private User refundRejectedBy; + + private Date dateRefundRejected; + public BigDecimal getTotal() { BigDecimal total = BigDecimal.ZERO; @@ -169,6 +187,10 @@ public void addPayment(Payment payment) { } public void synchronizeBillStatus() { + if (this.status == BillStatus.REFUND_REQUESTED || this.status == BillStatus.REFUNDED + || this.status == BillStatus.REFUND_DENIED) { + return; + } if (!this.getPayments().isEmpty() && getTotalPayments().compareTo(BigDecimal.ZERO) > 0) { boolean billFullySettled = getTotalPayments().compareTo(getTotal()) >= 0; if (billFullySettled) { diff --git a/api/src/main/java/org/openmrs/module/billing/api/model/BillStatus.java b/api/src/main/java/org/openmrs/module/billing/api/model/BillStatus.java index 29e17d2e..ff6698cd 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/model/BillStatus.java +++ b/api/src/main/java/org/openmrs/module/billing/api/model/BillStatus.java @@ -23,7 +23,10 @@ public enum BillStatus { PAID(), CANCELLED(), ADJUSTED(), - EXEMPTED(); + EXEMPTED(), + REFUND_REQUESTED(), + REFUNDED(), + REFUND_DENIED(); BillStatus() { } diff --git a/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java b/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java index 67947550..02ce712c 100644 --- a/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java +++ b/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java @@ -13,6 +13,7 @@ import org.openmrs.module.billing.api.BillLineItemService; import org.openmrs.module.billing.api.model.Bill; import org.openmrs.module.billing.api.model.BillLineItem; +import org.openmrs.module.billing.api.model.BillStatus; import org.openmrs.module.billing.api.model.Payment; import org.springframework.validation.Errors; import org.springframework.validation.Validator; @@ -38,6 +39,7 @@ public void validate(@Nonnull Object target, @Nonnull Errors errors) { validateNewPaymentsHaveCashier(bill, errors); validateLineItemsNotModified(bill, errors); + validateRefundFields(bill, errors); } } @@ -84,6 +86,18 @@ private void validateLineItemsNotModified(Bill bill, Errors errors) { } } + /** + * Validates that refund-related fields are provided when transitioning to refund statuses. + */ + private void validateRefundFields(Bill bill, Errors errors) { + if (bill.getStatus() == BillStatus.REFUND_REQUESTED && StringUtils.isBlank(bill.getRefundReason())) { + errors.reject("billing.error.refundReasonRequired"); + } + if (bill.getStatus() == BillStatus.REFUND_DENIED && StringUtils.isBlank(bill.getRefundDenialReason())) { + errors.reject("billing.error.denialReasonRequired"); + } + } + /** * Validates that any new (unsaved) non-voided payment has a cashier. Existing persisted payments * (id != null) are exempt to allow legacy data. diff --git a/api/src/main/resources/Bill.hbm.xml b/api/src/main/resources/Bill.hbm.xml index 2a3ea05e..d856a0db 100644 --- a/api/src/main/resources/Bill.hbm.xml +++ b/api/src/main/resources/Bill.hbm.xml @@ -139,6 +139,22 @@ + + + + + + + + diff --git a/api/src/main/resources/messages.properties b/api/src/main/resources/messages.properties index 4db22f78..6f926341 100644 --- a/api/src/main/resources/messages.properties +++ b/api/src/main/resources/messages.properties @@ -181,6 +181,8 @@ openhmis.cashier.payment.error.amountType=Amount needs to be a number openhmis.cashier.payment.error.amountRequired=Amount is required. openhmis.cashier.payment.confirm.paymentProcess=Are you sure you want to process a %s payment of %s? billing.error.paymentCashierRequired=Each payment must have an associated cashier. +billing.error.refundReasonRequired=A reason is required when requesting a refund. +billing.error.denialReasonRequired=A reason is required when denying a refund. #setting page openhmis.cashier.setting.header=Cashier Settings openhmis.cashier.setting.adjustmentReason.field.header=Require Adjustment Reason diff --git a/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java b/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java index 17a1240a..b046c971 100644 --- a/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java +++ b/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java @@ -218,6 +218,75 @@ public void synchronizeBillStatus_shouldUpdateAllNonVoidedLineItemsToPaidWhenBil assertNull(voidedLineItem.getPaymentStatus()); } + @Test + public void synchronizeBillStatus_shouldNotOverwriteRefundRequestedStatus() { + Bill bill = new Bill(); + bill.setLineItems(new ArrayList<>()); + bill.setPayments(new HashSet<>()); + bill.setStatus(BillStatus.REFUND_REQUESTED); + + BillLineItem lineItem = new BillLineItem(); + lineItem.setPrice(BigDecimal.valueOf(100)); + lineItem.setQuantity(1); + lineItem.setVoided(false); + bill.getLineItems().add(lineItem); + + Payment payment = new Payment(); + payment.setAmountTendered(BigDecimal.valueOf(100)); + payment.setVoided(false); + bill.getPayments().add(payment); + + bill.synchronizeBillStatus(); + + assertEquals(BillStatus.REFUND_REQUESTED, bill.getStatus()); + } + + @Test + public void synchronizeBillStatus_shouldNotOverwriteRefundedStatus() { + Bill bill = new Bill(); + bill.setLineItems(new ArrayList<>()); + bill.setPayments(new HashSet<>()); + bill.setStatus(BillStatus.REFUNDED); + + BillLineItem lineItem = new BillLineItem(); + lineItem.setPrice(BigDecimal.valueOf(100)); + lineItem.setQuantity(1); + lineItem.setVoided(false); + bill.getLineItems().add(lineItem); + + Payment payment = new Payment(); + payment.setAmountTendered(BigDecimal.valueOf(100)); + payment.setVoided(false); + bill.getPayments().add(payment); + + bill.synchronizeBillStatus(); + + assertEquals(BillStatus.REFUNDED, bill.getStatus()); + } + + @Test + public void synchronizeBillStatus_shouldNotOverwriteRefundDeniedStatus() { + Bill bill = new Bill(); + bill.setLineItems(new ArrayList<>()); + bill.setPayments(new HashSet<>()); + bill.setStatus(BillStatus.REFUND_DENIED); + + BillLineItem lineItem = new BillLineItem(); + lineItem.setPrice(BigDecimal.valueOf(100)); + lineItem.setQuantity(1); + lineItem.setVoided(false); + bill.getLineItems().add(lineItem); + + Payment payment = new Payment(); + payment.setAmountTendered(BigDecimal.valueOf(100)); + payment.setVoided(false); + bill.getPayments().add(payment); + + bill.synchronizeBillStatus(); + + assertEquals(BillStatus.REFUND_DENIED, bill.getStatus()); + } + @Test public void setLineItems_shouldAllowSettingLineItemsOnNewBill() { Bill bill = new Bill(); diff --git a/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java b/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java index 4f2602f9..847919f8 100644 --- a/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java +++ b/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java @@ -509,4 +509,148 @@ public void saveBill_shouldGenerateReceiptNumberWhenNotProvided() { assertNotNull(savedBill.getReceiptNumber()); assertFalse(savedBill.getReceiptNumber().isEmpty()); } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#requestRefund(Bill, String) + */ + @Test + public void requestRefund_shouldThrowIllegalArgumentExceptionIfBillIsNull() { + assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(null, "reason")); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#requestRefund(Bill, String) + */ + @Test + public void requestRefund_shouldThrowIllegalArgumentExceptionIfReasonIsBlank() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + assertEquals(BillStatus.PAID, paidBill.getStatus()); + + assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(paidBill, "")); + assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(paidBill, null)); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#requestRefund(Bill, String) + */ + @Test + public void requestRefund_shouldThrowIllegalArgumentExceptionIfBillIsNotPaid() { + Bill pendingBill = billService.getBill(2); + assertNotNull(pendingBill); + assertEquals(BillStatus.PENDING, pendingBill.getStatus()); + + assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(pendingBill, "Equipment failure")); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#requestRefund(Bill, String) + */ + @Test + public void requestRefund_shouldTransitionPaidBillToRefundRequested() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + assertEquals(BillStatus.PAID, paidBill.getStatus()); + + Bill result = billService.requestRefund(paidBill, "Equipment failure"); + + assertNotNull(result); + assertEquals(BillStatus.REFUND_REQUESTED, result.getStatus()); + assertEquals("Equipment failure", result.getRefundReason()); + assertNotNull(result.getRefundRequestedBy()); + assertNotNull(result.getDateRefundRequested()); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#approveRefund(Bill) + */ + @Test + public void approveRefund_shouldThrowIllegalArgumentExceptionIfBillIsNull() { + assertThrows(IllegalArgumentException.class, () -> billService.approveRefund(null)); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#approveRefund(Bill) + */ + @Test + public void approveRefund_shouldThrowIllegalArgumentExceptionIfBillIsNotRefundRequested() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + assertEquals(BillStatus.PAID, paidBill.getStatus()); + + assertThrows(IllegalArgumentException.class, () -> billService.approveRefund(paidBill)); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#approveRefund(Bill) + */ + @Test + public void approveRefund_shouldTransitionRefundRequestedBillToRefunded() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + + Bill requestedBill = billService.requestRefund(paidBill, "Equipment failure"); + assertEquals(BillStatus.REFUND_REQUESTED, requestedBill.getStatus()); + + Bill result = billService.approveRefund(requestedBill); + + assertNotNull(result); + assertEquals(BillStatus.REFUNDED, result.getStatus()); + assertEquals("Equipment failure", result.getRefundReason()); + assertNotNull(result.getRefundApprovedBy()); + assertNotNull(result.getDateRefundApproved()); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#rejectRefund(Bill, String) + */ + @Test + public void rejectRefund_shouldThrowIllegalArgumentExceptionIfBillIsNull() { + assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(null, "reason")); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#rejectRefund(Bill, String) + */ + @Test + public void rejectRefund_shouldThrowIllegalArgumentExceptionIfDenialReasonIsBlank() { + Bill paidBill = billService.getBill(1); + Bill requestedBill = billService.requestRefund(paidBill, "Equipment failure"); + + assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(requestedBill, "")); + assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(requestedBill, null)); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#rejectRefund(Bill, String) + */ + @Test + public void rejectRefund_shouldThrowIllegalArgumentExceptionIfBillIsNotRefundRequested() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + assertEquals(BillStatus.PAID, paidBill.getStatus()); + + assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(paidBill, "Not eligible")); + } + + /** + * @see org.openmrs.module.billing.api.impl.BillServiceImpl#rejectRefund(Bill, String) + */ + @Test + public void rejectRefund_shouldTransitionRefundRequestedBillToRefundDenied() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + + Bill requestedBill = billService.requestRefund(paidBill, "Equipment failure"); + assertEquals(BillStatus.REFUND_REQUESTED, requestedBill.getStatus()); + + Bill result = billService.rejectRefund(requestedBill, "Service was already provided"); + + assertNotNull(result); + assertEquals(BillStatus.REFUND_DENIED, result.getStatus()); + assertEquals("Equipment failure", result.getRefundReason()); + assertEquals("Service was already provided", result.getRefundDenialReason()); + assertNotNull(result.getRefundRejectedBy()); + assertNotNull(result.getDateRefundRejected()); + } } diff --git a/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java b/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java index cdb3aaa6..3eafd50e 100644 --- a/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java +++ b/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java @@ -141,4 +141,56 @@ public void validate_shouldTolerateVoidedNewPaymentWithNoCashier() { assertFalse(errors.hasErrors()); } + @Test + public void validate_shouldRejectRefundRequestedBillWithNoRefundReason() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + paidBill.setStatus(BillStatus.REFUND_REQUESTED); + // refundReason intentionally NOT set + + Errors errors = new BindException(paidBill, "bill"); + billValidator.validate(paidBill, errors); + + assertTrue(errors.hasErrors()); + } + + @Test + public void validate_shouldNotRejectRefundRequestedBillWithRefundReason() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + paidBill.setStatus(BillStatus.REFUND_REQUESTED); + paidBill.setRefundReason("Equipment failure"); + + Errors errors = new BindException(paidBill, "bill"); + billValidator.validate(paidBill, errors); + + assertFalse(errors.hasErrors()); + } + + @Test + public void validate_shouldRejectRefundDeniedBillWithNoDenialReason() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + paidBill.setStatus(BillStatus.REFUND_DENIED); + // denialReason intentionally NOT set + + Errors errors = new BindException(paidBill, "bill"); + billValidator.validate(paidBill, errors); + + assertTrue(errors.hasErrors()); + } + + @Test + public void validate_shouldNotRejectRefundDeniedBillWithDenialReason() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + paidBill.setStatus(BillStatus.REFUND_DENIED); + paidBill.setRefundDenialReason("Service was already provided"); + + Errors errors = new BindException(paidBill, "bill"); + billValidator.validate(paidBill, errors); + + assertFalse(errors.hasErrors()); + } + } diff --git a/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/BillResource.java b/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/BillResource.java index 7f57ec5a..3c7165cf 100644 --- a/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/BillResource.java +++ b/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/BillResource.java @@ -75,6 +75,14 @@ public DelegatingResourceDescription getRepresentationDescription(Representation description.addProperty("receiptNumber"); description.addProperty("status"); description.addProperty("adjustmentReason"); + description.addProperty("refundReason"); + description.addProperty("refundRequestedBy", Representation.REF); + description.addProperty("dateRefundRequested"); + description.addProperty("refundApprovedBy", Representation.REF); + description.addProperty("dateRefundApproved"); + description.addProperty("refundDenialReason"); + description.addProperty("refundRejectedBy", Representation.REF); + description.addProperty("dateRefundRejected"); description.addProperty("uuid"); return description; } @@ -131,6 +139,11 @@ public void setBillStatus(Bill instance, BillStatus status) { instance.setStatus(status); } else if (instance.getStatus() == BillStatus.PENDING && status == BillStatus.POSTED) { instance.setStatus(status); + } else if (instance.getStatus() == BillStatus.PAID && status == BillStatus.REFUND_REQUESTED) { + instance.setStatus(status); + } else if (instance.getStatus() == BillStatus.REFUND_REQUESTED + && (status == BillStatus.REFUNDED || status == BillStatus.REFUND_DENIED)) { + instance.setStatus(status); } if (status == BillStatus.POSTED) { RoundingUtil.handleRoundingLineItem(instance); @@ -148,6 +161,18 @@ public void setAdjustReason(Bill instance, String adjustReason) { public Bill save(Bill bill) { //TODO: Test all the ways that this could fail + BillService service = Context.getService(BillService.class); + + if (bill.getStatus() == BillStatus.REFUND_REQUESTED) { + return service.requestRefund(bill, bill.getRefundReason()); + } + if (bill.getStatus() == BillStatus.REFUNDED) { + return service.approveRefund(bill); + } + if (bill.getStatus() == BillStatus.REFUND_DENIED) { + return service.rejectRefund(bill, bill.getRefundDenialReason()); + } + if (bill.getId() == null) { if (bill.getCashier() == null) { Provider cashier = getCurrentCashier(); @@ -171,7 +196,7 @@ public Bill save(Bill bill) { } } - return Context.getService(BillService.class).saveBill(bill); + return service.saveBill(bill); } @Override diff --git a/omod/src/main/resources/liquibase.xml b/omod/src/main/resources/liquibase.xml index 43ae8ee8..9ac2ea90 100644 --- a/omod/src/main/resources/liquibase.xml +++ b/omod/src/main/resources/liquibase.xml @@ -1109,4 +1109,46 @@ referencedTableName="provider" referencedColumnNames="provider_id" onDelete="SET NULL" onUpdate="CASCADE"/> + + + Widen status columns and add refund audit fields to cashier_bill + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 4a5757f415680618d71281c70dabd6c03aacdf2e Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Mon, 6 Apr 2026 17:22:09 +0800 Subject: [PATCH 2/4] fixup --- .../openmrs/module/billing/api/impl/BillServiceImpl.java | 9 +++------ .../java/org/openmrs/module/billing/api/model/Bill.java | 3 +-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java index 12dee746..c3590224 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java +++ b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java @@ -178,7 +178,6 @@ public boolean isBillEditable(Bill bill) { } @Override - @Transactional public Bill requestRefund(Bill bill, String refundReason) { if (bill == null) { throw new IllegalArgumentException("The bill must be defined."); @@ -194,11 +193,10 @@ public Bill requestRefund(Bill bill, String refundReason) { bill.setRefundRequestedBy(Context.getAuthenticatedUser()); bill.setDateRefundRequested(new Date()); bill.setStatus(BillStatus.REFUND_REQUESTED); - return billDAO.saveBill(bill); + return Context.getService(BillService.class).saveBill(bill); } @Override - @Transactional public Bill approveRefund(Bill bill) { if (bill == null) { throw new IllegalArgumentException("The bill must be defined."); @@ -210,11 +208,10 @@ public Bill approveRefund(Bill bill) { bill.setRefundApprovedBy(Context.getAuthenticatedUser()); bill.setDateRefundApproved(new Date()); bill.setStatus(BillStatus.REFUNDED); - return billDAO.saveBill(bill); + return Context.getService(BillService.class).saveBill(bill); } @Override - @Transactional public Bill rejectRefund(Bill bill, String denialReason) { if (bill == null) { throw new IllegalArgumentException("The bill must be defined."); @@ -230,7 +227,7 @@ public Bill rejectRefund(Bill bill, String denialReason) { bill.setRefundRejectedBy(Context.getAuthenticatedUser()); bill.setDateRefundRejected(new Date()); bill.setStatus(BillStatus.REFUND_DENIED); - return billDAO.saveBill(bill); + return Context.getService(BillService.class).saveBill(bill); } } diff --git a/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java b/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java index c963651b..aec1ea19 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java +++ b/api/src/main/java/org/openmrs/module/billing/api/model/Bill.java @@ -187,8 +187,7 @@ public void addPayment(Payment payment) { } public void synchronizeBillStatus() { - if (this.status == BillStatus.REFUND_REQUESTED || this.status == BillStatus.REFUNDED - || this.status == BillStatus.REFUND_DENIED) { + if (this.status == BillStatus.REFUND_REQUESTED || this.status == BillStatus.REFUNDED) { return; } if (!this.getPayments().isEmpty() && getTotalPayments().compareTo(BigDecimal.ZERO) > 0) { From 58d1c29a7a35490315478563265fd0d849bdb600 Mon Sep 17 00:00:00 2001 From: Ujjawal Prabhat Date: Wed, 8 Apr 2026 13:25:59 +0800 Subject: [PATCH 3/4] update tests --- .../java/org/openmrs/module/billing/api/model/BillTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java b/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java index b046c971..82b74c4a 100644 --- a/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java +++ b/api/src/test/java/org/openmrs/module/billing/api/model/BillTest.java @@ -265,7 +265,7 @@ public void synchronizeBillStatus_shouldNotOverwriteRefundedStatus() { } @Test - public void synchronizeBillStatus_shouldNotOverwriteRefundDeniedStatus() { + public void synchronizeBillStatus_shouldSynchronizeRefundDeniedBillToPaid() { Bill bill = new Bill(); bill.setLineItems(new ArrayList<>()); bill.setPayments(new HashSet<>()); @@ -284,7 +284,7 @@ public void synchronizeBillStatus_shouldNotOverwriteRefundDeniedStatus() { bill.synchronizeBillStatus(); - assertEquals(BillStatus.REFUND_DENIED, bill.getStatus()); + assertEquals(BillStatus.PAID, bill.getStatus()); } @Test From 95dee67aadaa90a4ef391a502216dd8b1e39d769 Mon Sep 17 00:00:00 2001 From: NethmiRodrigo Date: Tue, 21 Apr 2026 11:55:28 +0530 Subject: [PATCH 4/4] (fix) O3-5413: Validate refund status transitions against persisted DB state --- .../module/billing/api/BillService.java | 12 ++++ .../module/billing/api/db/BillDAO.java | 12 ++++ .../api/db/hibernate/HibernateBillDAO.java | 15 +++++ .../billing/api/impl/BillServiceImpl.java | 21 +++---- .../billing/validator/BillValidator.java | 25 +++++++- api/src/main/resources/messages.properties | 1 + .../billing/impl/BillServiceImplTest.java | 30 ++++++---- .../billing/validator/BillValidatorTest.java | 58 ++++++++++++++++++- omod/src/main/resources/liquibase.xml | 4 +- 9 files changed, 148 insertions(+), 30 deletions(-) diff --git a/api/src/main/java/org/openmrs/module/billing/api/BillService.java b/api/src/main/java/org/openmrs/module/billing/api/BillService.java index a1e91c95..9a7d4ab8 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/BillService.java +++ b/api/src/main/java/org/openmrs/module/billing/api/BillService.java @@ -4,6 +4,7 @@ import org.openmrs.api.OpenmrsService; import org.openmrs.module.billing.api.base.PagingInfo; import org.openmrs.module.billing.api.model.Bill; +import org.openmrs.module.billing.api.model.BillStatus; import org.openmrs.module.billing.api.search.BillSearch; import org.openmrs.module.billing.api.util.PrivilegeConstants; @@ -144,6 +145,17 @@ public interface BillService extends OpenmrsService { @Authorized(PrivilegeConstants.VIEW_BILLS) boolean isBillEditable(Bill bill); + /** + * Reads the bill's status directly from the database, bypassing Hibernate's session cache. Intended + * for status-transition validation where an in-memory entity may have a pending target status and + * the caller needs the untouched persisted value. + * + * @param billId the database ID of the bill + * @return the persisted status, or null if no bill with that ID exists + */ + @Authorized(PrivilegeConstants.VIEW_BILLS) + BillStatus getPersistedBillStatus(Integer billId); + /** * Requests a refund for a paid bill. *

diff --git a/api/src/main/java/org/openmrs/module/billing/api/db/BillDAO.java b/api/src/main/java/org/openmrs/module/billing/api/db/BillDAO.java index a5d08e98..3892204e 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/db/BillDAO.java +++ b/api/src/main/java/org/openmrs/module/billing/api/db/BillDAO.java @@ -2,6 +2,7 @@ import org.openmrs.module.billing.api.base.PagingInfo; import org.openmrs.module.billing.api.model.Bill; +import org.openmrs.module.billing.api.model.BillStatus; import org.openmrs.module.billing.api.search.BillSearch; import javax.annotation.Nonnull; @@ -99,4 +100,15 @@ public interface BillDAO { */ void purgeBill(@Nonnull Bill bill); + /** + * Reads the bill's status directly from the database, bypassing Hibernate's session cache. This + * returns the persisted (pre-mutation) value even when the managed entity in the current session + * has been modified, so callers can compare the DB truth against in-memory changes (e.g., for + * status-transition validation). + * + * @param billId the database ID of the bill (must not be null) + * @return the persisted status, or null if no bill with that ID exists + */ + BillStatus getPersistedBillStatus(@Nonnull Integer billId); + } diff --git a/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/HibernateBillDAO.java b/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/HibernateBillDAO.java index 1a9bca4e..74d61b3a 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/HibernateBillDAO.java +++ b/api/src/main/java/org/openmrs/module/billing/api/db/hibernate/HibernateBillDAO.java @@ -12,6 +12,7 @@ import org.openmrs.module.billing.api.base.PagingInfo; import org.openmrs.module.billing.api.db.BillDAO; import org.openmrs.module.billing.api.model.Bill; +import org.openmrs.module.billing.api.model.BillStatus; import org.openmrs.module.billing.api.search.BillSearch; import javax.annotation.Nonnull; @@ -133,6 +134,20 @@ public void purgeBill(@Nonnull Bill bill) { sessionFactory.getCurrentSession().remove(bill); } + /** + * {@inheritDoc} + */ + @Override + public BillStatus getPersistedBillStatus(@Nonnull Integer billId) { + List results = sessionFactory.getCurrentSession() + .createNativeQuery("SELECT status FROM cashier_bill WHERE bill_id = :billId").setParameter("billId", billId) + .getResultList(); + if (results.isEmpty() || results.get(0) == null) { + return null; + } + return BillStatus.valueOf(results.get(0).toString()); + } + private List buildBillSearchPredicate(CriteriaBuilder cb, Root root, BillSearch billSearch) { List predicates = new ArrayList<>(); diff --git a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java index c3590224..ece0f0d7 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java +++ b/api/src/main/java/org/openmrs/module/billing/api/impl/BillServiceImpl.java @@ -177,6 +177,15 @@ public boolean isBillEditable(Bill bill) { return true; } + @Override + @Transactional(readOnly = true) + public BillStatus getPersistedBillStatus(Integer billId) { + if (billId == null) { + return null; + } + return billDAO.getPersistedBillStatus(billId); + } + @Override public Bill requestRefund(Bill bill, String refundReason) { if (bill == null) { @@ -185,10 +194,6 @@ public Bill requestRefund(Bill bill, String refundReason) { if (StringUtils.isBlank(refundReason)) { throw new IllegalArgumentException("refundReason cannot be null or empty"); } - if (bill.getStatus() != BillStatus.PAID) { - throw new IllegalArgumentException( - "Only PAID bills can have a refund requested. Current status: " + bill.getStatus()); - } bill.setRefundReason(refundReason); bill.setRefundRequestedBy(Context.getAuthenticatedUser()); bill.setDateRefundRequested(new Date()); @@ -201,10 +206,6 @@ public Bill approveRefund(Bill bill) { if (bill == null) { throw new IllegalArgumentException("The bill must be defined."); } - if (bill.getStatus() != BillStatus.REFUND_REQUESTED) { - throw new IllegalArgumentException( - "Only bills with REFUND_REQUESTED status can be approved. Current status: " + bill.getStatus()); - } bill.setRefundApprovedBy(Context.getAuthenticatedUser()); bill.setDateRefundApproved(new Date()); bill.setStatus(BillStatus.REFUNDED); @@ -219,10 +220,6 @@ public Bill rejectRefund(Bill bill, String denialReason) { if (StringUtils.isBlank(denialReason)) { throw new IllegalArgumentException("denialReason cannot be null or empty"); } - if (bill.getStatus() != BillStatus.REFUND_REQUESTED) { - throw new IllegalArgumentException( - "Only bills with REFUND_REQUESTED status can be rejected. Current status: " + bill.getStatus()); - } bill.setRefundDenialReason(denialReason); bill.setRefundRejectedBy(Context.getAuthenticatedUser()); bill.setDateRefundRejected(new Date()); diff --git a/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java b/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java index 02ce712c..19ae5edd 100644 --- a/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java +++ b/api/src/main/java/org/openmrs/module/billing/validator/BillValidator.java @@ -11,6 +11,7 @@ import org.openmrs.annotation.Handler; import org.openmrs.api.context.Context; import org.openmrs.module.billing.api.BillLineItemService; +import org.openmrs.module.billing.api.BillService; import org.openmrs.module.billing.api.model.Bill; import org.openmrs.module.billing.api.model.BillLineItem; import org.openmrs.module.billing.api.model.BillStatus; @@ -87,9 +88,31 @@ private void validateLineItemsNotModified(Bill bill, Errors errors) { } /** - * Validates that refund-related fields are provided when transitioning to refund statuses. + * Validates refund-related fields and that the persisted status is a valid predecessor for the + * requested refund-workflow target. Reads the persisted status via + * {@link BillService#getPersistedBillStatus(Integer)} (bypasses the Hibernate session cache) + * because {@code BillResource.setBillStatus} and the refund service methods mutate the in-memory + * status to the target before validation runs — so the managed entity's status is not usable as the + * source of truth here. */ private void validateRefundFields(Bill bill, Errors errors) { + if (bill.getId() == null || bill.getStatus() == null) { + return; + } + + BillStatus requiredSource; + if (bill.getStatus() == BillStatus.REFUND_REQUESTED) { + requiredSource = BillStatus.PAID; + } else if (bill.getStatus() == BillStatus.REFUNDED || bill.getStatus() == BillStatus.REFUND_DENIED) { + requiredSource = BillStatus.REFUND_REQUESTED; + } else { + return; + } + BillStatus persisted = Context.getService(BillService.class).getPersistedBillStatus(bill.getId()); + if (persisted != requiredSource) { + errors.reject("billing.error.invalidBillStatusTransition"); + } + if (bill.getStatus() == BillStatus.REFUND_REQUESTED && StringUtils.isBlank(bill.getRefundReason())) { errors.reject("billing.error.refundReasonRequired"); } diff --git a/api/src/main/resources/messages.properties b/api/src/main/resources/messages.properties index 6f926341..be5bbea6 100644 --- a/api/src/main/resources/messages.properties +++ b/api/src/main/resources/messages.properties @@ -183,6 +183,7 @@ openhmis.cashier.payment.confirm.paymentProcess=Are you sure you want to process billing.error.paymentCashierRequired=Each payment must have an associated cashier. billing.error.refundReasonRequired=A reason is required when requesting a refund. billing.error.denialReasonRequired=A reason is required when denying a refund. +billing.error.invalidBillStatusTransition=The bill status transition is not allowed. #setting page openhmis.cashier.setting.header=Cashier Settings openhmis.cashier.setting.adjustmentReason.field.header=Require Adjustment Reason diff --git a/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java b/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java index 847919f8..43b0937f 100644 --- a/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java +++ b/api/src/test/java/org/openmrs/module/billing/impl/BillServiceImplTest.java @@ -527,20 +527,21 @@ public void requestRefund_shouldThrowIllegalArgumentExceptionIfReasonIsBlank() { assertNotNull(paidBill); assertEquals(BillStatus.PAID, paidBill.getStatus()); - assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(paidBill, "")); - assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(paidBill, null)); + assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(billService.getBill(1), "")); + Context.clearSession(); + assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(billService.getBill(1), null)); } /** * @see org.openmrs.module.billing.api.impl.BillServiceImpl#requestRefund(Bill, String) */ @Test - public void requestRefund_shouldThrowIllegalArgumentExceptionIfBillIsNotPaid() { + public void requestRefund_shouldThrowValidationExceptionIfBillIsNotPaid() { Bill pendingBill = billService.getBill(2); assertNotNull(pendingBill); assertEquals(BillStatus.PENDING, pendingBill.getStatus()); - assertThrows(IllegalArgumentException.class, () -> billService.requestRefund(pendingBill, "Equipment failure")); + assertThrows(ValidationException.class, () -> billService.requestRefund(pendingBill, "Equipment failure")); } /** @@ -573,12 +574,12 @@ public void approveRefund_shouldThrowIllegalArgumentExceptionIfBillIsNull() { * @see org.openmrs.module.billing.api.impl.BillServiceImpl#approveRefund(Bill) */ @Test - public void approveRefund_shouldThrowIllegalArgumentExceptionIfBillIsNotRefundRequested() { + public void approveRefund_shouldThrowValidationExceptionIfBillIsNotRefundRequested() { Bill paidBill = billService.getBill(1); assertNotNull(paidBill); assertEquals(BillStatus.PAID, paidBill.getStatus()); - assertThrows(IllegalArgumentException.class, () -> billService.approveRefund(paidBill)); + assertThrows(ValidationException.class, () -> billService.approveRefund(paidBill)); } /** @@ -591,6 +592,8 @@ public void approveRefund_shouldTransitionRefundRequestedBillToRefunded() { Bill requestedBill = billService.requestRefund(paidBill, "Equipment failure"); assertEquals(BillStatus.REFUND_REQUESTED, requestedBill.getStatus()); + // Flush so the validator's native-SQL read of the persisted status sees REFUND_REQUESTED. + Context.flushSession(); Bill result = billService.approveRefund(requestedBill); @@ -614,23 +617,25 @@ public void rejectRefund_shouldThrowIllegalArgumentExceptionIfBillIsNull() { */ @Test public void rejectRefund_shouldThrowIllegalArgumentExceptionIfDenialReasonIsBlank() { - Bill paidBill = billService.getBill(1); - Bill requestedBill = billService.requestRefund(paidBill, "Equipment failure"); + Integer billId = billService.requestRefund(billService.getBill(1), "Equipment failure").getId(); + Context.flushSession(); + Context.clearSession(); - assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(requestedBill, "")); - assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(requestedBill, null)); + assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(billService.getBill(billId), "")); + Context.clearSession(); + assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(billService.getBill(billId), null)); } /** * @see org.openmrs.module.billing.api.impl.BillServiceImpl#rejectRefund(Bill, String) */ @Test - public void rejectRefund_shouldThrowIllegalArgumentExceptionIfBillIsNotRefundRequested() { + public void rejectRefund_shouldThrowValidationExceptionIfBillIsNotRefundRequested() { Bill paidBill = billService.getBill(1); assertNotNull(paidBill); assertEquals(BillStatus.PAID, paidBill.getStatus()); - assertThrows(IllegalArgumentException.class, () -> billService.rejectRefund(paidBill, "Not eligible")); + assertThrows(ValidationException.class, () -> billService.rejectRefund(paidBill, "Not eligible")); } /** @@ -643,6 +648,7 @@ public void rejectRefund_shouldTransitionRefundRequestedBillToRefundDenied() { Bill requestedBill = billService.requestRefund(paidBill, "Equipment failure"); assertEquals(BillStatus.REFUND_REQUESTED, requestedBill.getStatus()); + Context.flushSession(); Bill result = billService.rejectRefund(requestedBill, "Service was already provided"); diff --git a/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java b/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java index 3eafd50e..5e33e51d 100644 --- a/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java +++ b/api/src/test/java/org/openmrs/module/billing/validator/BillValidatorTest.java @@ -182,15 +182,67 @@ public void validate_shouldRejectRefundDeniedBillWithNoDenialReason() { @Test public void validate_shouldNotRejectRefundDeniedBillWithDenialReason() { + // REFUND_DENIED requires the persisted status to be REFUND_REQUESTED, so transition the bill + // through requestRefund first. Bill paidBill = billService.getBill(1); assertNotNull(paidBill); - paidBill.setStatus(BillStatus.REFUND_DENIED); - paidBill.setRefundDenialReason("Service was already provided"); + billService.requestRefund(paidBill, "Equipment failure"); + Context.flushSession(); + Bill requestedBill = billService.getBill(1); + requestedBill.setStatus(BillStatus.REFUND_DENIED); + requestedBill.setRefundDenialReason("Service was already provided"); + + Errors errors = new BindException(requestedBill, "bill"); + billValidator.validate(requestedBill, errors); + + assertFalse(errors.hasErrors()); + } + + @Test + public void validate_shouldRejectRefundRequestedTransitionFromNonPaidBill() { + Bill pendingBill = billService.getBill(2); + assertNotNull(pendingBill); + assertEquals(BillStatus.PENDING, pendingBill.getStatus()); + pendingBill.setStatus(BillStatus.REFUND_REQUESTED); + pendingBill.setRefundReason("Attempt from PENDING"); + + Errors errors = new BindException(pendingBill, "bill"); + billValidator.validate(pendingBill, errors); + + assertTrue(errors.hasErrors()); + } + + @Test + public void validate_shouldRejectSecondRefundRequestOnAlreadyRefundRequestedBill() { + // Core audit-field-clobber guard: a second request on an already-REFUND_REQUESTED bill must + // be rejected so audit fields (refundRequestedBy / dateRefundRequested) aren't overwritten. + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + billService.requestRefund(paidBill, "First reason"); + // Flush so the DB reflects REFUND_REQUESTED — simulates a second HTTP request arriving after + // the prior transaction committed. + Context.flushSession(); + Bill requestedBill = billService.getBill(1); + assertEquals(BillStatus.REFUND_REQUESTED, requestedBill.getStatus()); + // Caller is trying to re-submit a refund request; in-memory status is still REFUND_REQUESTED. + requestedBill.setRefundReason("Second reason"); + + Errors errors = new BindException(requestedBill, "bill"); + billValidator.validate(requestedBill, errors); + + assertTrue(errors.hasErrors()); + } + + @Test + public void validate_shouldRejectApproveRefundOnNonRequestedBill() { + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + paidBill.setStatus(BillStatus.REFUNDED); Errors errors = new BindException(paidBill, "bill"); billValidator.validate(paidBill, errors); - assertFalse(errors.hasErrors()); + assertTrue(errors.hasErrors()); } } diff --git a/omod/src/main/resources/liquibase.xml b/omod/src/main/resources/liquibase.xml index 9ac2ea90..0414eb70 100644 --- a/omod/src/main/resources/liquibase.xml +++ b/omod/src/main/resources/liquibase.xml @@ -1113,8 +1113,8 @@ Widen status columns and add refund audit fields to cashier_bill - - + ALTER TABLE cashier_bill MODIFY COLUMN status varchar(50) + ALTER TABLE cashier_bill_line_item MODIFY COLUMN payment_status varchar(50)