diff --git a/api/src/main/java/org/openmrs/module/billing/api/model/Payment.java b/api/src/main/java/org/openmrs/module/billing/api/model/Payment.java index 7e2bef26..385fcc9f 100644 --- a/api/src/main/java/org/openmrs/module/billing/api/model/Payment.java +++ b/api/src/main/java/org/openmrs/module/billing/api/model/Payment.java @@ -13,14 +13,16 @@ */ package org.openmrs.module.billing.api.model; +import java.math.BigDecimal; + import lombok.AllArgsConstructor; import lombok.Builder; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; -import org.openmrs.module.billing.api.base.entity.model.BaseInstanceCustomizableData; -import java.math.BigDecimal; +import org.openmrs.Provider; +import org.openmrs.module.billing.api.base.entity.model.BaseInstanceCustomizableData; /** * Model class that represents the {@link Bill} payment information. @@ -46,6 +48,10 @@ public class Payment extends BaseInstanceCustomizableData + diff --git a/api/src/main/resources/messages.properties b/api/src/main/resources/messages.properties index 2f661acd..4db22f78 100644 --- a/api/src/main/resources/messages.properties +++ b/api/src/main/resources/messages.properties @@ -180,6 +180,7 @@ openhmis.cashier.payment.error.paymentMode.required=Payment mode is required. 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. #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 aa8b62ef..71df52e7 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 @@ -328,18 +328,37 @@ public void saveBill_shouldAllowPaymentsForPostedBill() { Bill postedBill = billService.getBill(0); assertNotNull(postedBill); assertEquals(BillStatus.POSTED, postedBill.getStatus()); - + PaymentMode paymentMode = paymentModeService.getPaymentMode(0); - + Payment payment = Payment.builder().amount(BigDecimal.valueOf(10.0)).amountTendered(BigDecimal.valueOf(10.0)) .build(); payment.setInstanceType(paymentMode); - + payment.setCashier(providerService.getProvider(0)); + postedBill.addPayment(payment); billService.saveBill(postedBill); - + assertDoesNotThrow(Context::flushSession); } + + @Test + public void saveBill_shouldThrowValidationExceptionWhenPaymentHasNoCashier() { + Bill postedBill = billService.getBill(0); + assertNotNull(postedBill); + assertEquals(BillStatus.POSTED, postedBill.getStatus()); + + PaymentMode paymentMode = paymentModeService.getPaymentMode(0); + + Payment payment = Payment.builder().amount(BigDecimal.valueOf(10.0)).amountTendered(BigDecimal.valueOf(10.0)) + .build(); + payment.setInstanceType(paymentMode); + // cashier intentionally NOT set — BillValidator must reject this + + postedBill.addPayment(payment); + + assertThrows(ValidationException.class, () -> billService.saveBill(postedBill)); + } @Test public void saveBill_shouldNotAllowModifyingLineItemPropertiesOnPaidBill() { 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 b65bfb5b..cdb3aaa6 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 @@ -16,13 +16,18 @@ import static org.junit.jupiter.api.Assertions.*; +import java.math.BigDecimal; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.openmrs.api.context.Context; import org.openmrs.module.billing.TestConstants; import org.openmrs.module.billing.api.BillService; +import org.openmrs.module.billing.api.PaymentModeService; import org.openmrs.module.billing.api.model.Bill; import org.openmrs.module.billing.api.model.BillStatus; +import org.openmrs.module.billing.api.model.Payment; +import org.openmrs.module.billing.api.model.PaymentMode; import org.openmrs.test.jupiter.BaseModuleContextSensitiveTest; import org.springframework.validation.BindException; import org.springframework.validation.Errors; @@ -36,10 +41,13 @@ public class BillValidatorTest extends BaseModuleContextSensitiveTest { private BillService billService; + private PaymentModeService paymentModeService; + @BeforeEach public void setup() { billValidator = new BillValidator(); billService = Context.getService(BillService.class); + paymentModeService = Context.getService(PaymentModeService.class); executeDataSet(TestConstants.CORE_DATASET2); executeDataSet(TestConstants.BASE_DATASET_DIR + "StockOperationType.xml"); @@ -73,4 +81,64 @@ public void validate_shouldNotRejectUnmodifiedPaidBill() { // attempting to modify line items assertFalse(errors.hasErrors()); } + + @Test + public void validate_shouldRejectNewPaymentWithNoCashier() { + Bill postedBill = billService.getBill(0); + assertNotNull(postedBill); + assertEquals(BillStatus.POSTED, postedBill.getStatus()); + + PaymentMode paymentMode = paymentModeService.getPaymentMode(0); + + Payment payment = Payment.builder().amount(BigDecimal.valueOf(10.0)).amountTendered(BigDecimal.valueOf(10.0)) + .build(); + payment.setInstanceType(paymentMode); + // cashier intentionally NOT set + + postedBill.addPayment(payment); + + Errors errors = new BindException(postedBill, "bill"); + billValidator.validate(postedBill, errors); + + assertTrue(errors.hasErrors()); + } + + @Test + public void validate_shouldTolerateExistingPaymentsWithNoCashier() { + // Legacy payments (with ID) that have no cashier must be tolerated + Bill paidBill = billService.getBill(1); + assertNotNull(paidBill); + assertFalse(paidBill.getPayments().isEmpty()); + + Payment existingPayment = paidBill.getPayments().iterator().next(); + assertNotNull(existingPayment.getId(), "Existing payment should have an ID"); + + Errors errors = new BindException(paidBill, "bill"); + billValidator.validate(paidBill, errors); + + assertFalse(errors.hasErrors()); + } + + @Test + public void validate_shouldTolerateVoidedNewPaymentWithNoCashier() { + Bill postedBill = billService.getBill(0); + assertNotNull(postedBill); + assertEquals(BillStatus.POSTED, postedBill.getStatus()); + + PaymentMode paymentMode = paymentModeService.getPaymentMode(0); + + Payment payment = Payment.builder().amount(BigDecimal.valueOf(10.0)).amountTendered(BigDecimal.valueOf(10.0)) + .build(); + payment.setInstanceType(paymentMode); + payment.setVoided(true); + // cashier intentionally NOT set + + postedBill.addPayment(payment); + + Errors errors = new BindException(postedBill, "bill"); + billValidator.validate(postedBill, 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 df393725..ff8a7bdf 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 @@ -106,6 +106,14 @@ public void setBillPayments(Bill instance, Set payments) { } BaseRestDataResource.syncCollection(instance.getPayments(), payments); for (Payment payment : instance.getPayments()) { + if (payment.getId() == null && payment.getCashier() == null) { + Provider cashier = getCurrentCashier(); + if (cashier == null) { + throw new RestClientException("Couldn't find Provider for the current user (" + + Context.getAuthenticatedUser().getUsername() + ")"); + } + payment.setCashier(cashier); + } instance.addPayment(payment); } } diff --git a/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/PaymentResource.java b/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/PaymentResource.java index 7e84e0ce..89547af3 100644 --- a/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/PaymentResource.java +++ b/omod/src/main/java/org/openmrs/module/billing/web/rest/resource/PaymentResource.java @@ -13,8 +13,11 @@ */ package org.openmrs.module.billing.web.rest.resource; +import org.openmrs.Provider; +import org.openmrs.api.APIException; import org.openmrs.api.context.Context; import org.openmrs.module.billing.api.BillService; +import org.openmrs.module.billing.api.base.ProviderUtil; import org.openmrs.module.billing.api.PaymentModeService; import org.openmrs.module.billing.web.base.resource.BaseRestDataResource; import org.openmrs.module.billing.api.model.Bill; @@ -55,6 +58,7 @@ public DelegatingResourceDescription getRepresentationDescription(Representation description.addProperty("attributes"); description.addProperty("amount"); description.addProperty("amountTendered"); + description.addProperty("cashier", Representation.REF); description.addProperty("dateCreated"); description.addProperty("voided"); return description; @@ -70,10 +74,20 @@ public DelegatingResourceDescription getCreatableProperties() { description.addProperty("attributes"); description.addProperty("amount"); description.addProperty("amountTendered"); + description.addProperty("cashier"); return description; } + @PropertySetter("cashier") + public void setCashier(Payment instance, String uuid) { + Provider provider = Context.getProviderService().getProviderByUuid(uuid); + if (provider == null) { + throw new ObjectNotFoundException(); + } + instance.setCashier(provider); + } + // Work around TypeVariable issue on base generic property (BaseCustomizableInstanceData.getInstanceType) @PropertySetter("instanceType") public void setPaymentMode(Payment instance, String uuid) { @@ -132,6 +146,15 @@ public Long getPaymentDate(Payment instance) { @Override public Payment save(Payment delegate) { + if (delegate.getCashier() == null) { + Provider cashier = ProviderUtil.getCurrentProvider(); + if (cashier == null) { + throw new APIException( + "The authenticated user is not associated with a Provider and cannot process payments."); + } + delegate.setCashier(cashier); + } + BillService service = Context.getService(BillService.class); Bill bill = delegate.getBill(); bill.addPayment(delegate); diff --git a/omod/src/main/resources/liquibase.xml b/omod/src/main/resources/liquibase.xml index 118d17c3..6d584506 100644 --- a/omod/src/main/resources/liquibase.xml +++ b/omod/src/main/resources/liquibase.xml @@ -1095,4 +1095,17 @@ baseTableName="bill_exemption_rule" baseColumnNames="voided_by" referencedTableName="users" referencedColumnNames="user_id"/> + + + Add cashier (provider) for each payment to track which cashier processed the payment + + + + + + + \ No newline at end of file diff --git a/omod/src/test/java/org/openmrs/module/billing/web/rest/resource/PaymentResourceTest.java b/omod/src/test/java/org/openmrs/module/billing/web/rest/resource/PaymentResourceTest.java new file mode 100644 index 00000000..61d7e437 --- /dev/null +++ b/omod/src/test/java/org/openmrs/module/billing/web/rest/resource/PaymentResourceTest.java @@ -0,0 +1,170 @@ +/* + * The contents of this file are subject to the OpenMRS Public License + * Version 1.1 (the "License"); you may not use this file except in + * compliance with the License. You may obtain a copy of the License at + * http://license.openmrs.org + * + * Software distributed under the License is distributed on an "AS IS" + * basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the + * License for the specific language governing rights and limitations + * under the License. + * + * Copyright (C) OpenMRS, LLC. All Rights Reserved. + */ +package org.openmrs.module.billing.web.rest.resource; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.math.BigDecimal; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.MockedStatic; +import org.openmrs.Provider; +import org.openmrs.api.APIException; +import org.openmrs.api.context.Context; +import org.openmrs.module.billing.api.BillService; +import org.openmrs.module.billing.api.base.ProviderUtil; +import org.openmrs.module.billing.api.model.Bill; +import org.openmrs.module.billing.api.model.Payment; + +/** + * Unit tests for {@link PaymentResource} + */ +public class PaymentResourceTest { + + private PaymentResource resource; + + private BillService billService; + + private MockedStatic contextMock; + + private MockedStatic providerUtilMock; + + @Before + public void setUp() { + resource = new PaymentResource(); + billService = mock(BillService.class); + + contextMock = mockStatic(Context.class); + contextMock.when(() -> Context.getService(BillService.class)).thenReturn(billService); + + providerUtilMock = mockStatic(ProviderUtil.class); + } + + @After + public void tearDown() { + if (contextMock != null) { + contextMock.close(); + } + if (providerUtilMock != null) { + providerUtilMock.close(); + } + } + + @Test + public void save_shouldSetCashierFromAuthenticatedProvider() { + Provider cashier = new Provider(); + cashier.setId(1); + providerUtilMock.when(ProviderUtil::getCurrentProvider).thenReturn(cashier); + + Bill bill = new Bill(); + bill.setId(1); + Payment payment = new Payment(); + payment.setBill(bill); + payment.setAmount(BigDecimal.TEN); + payment.setAmountTendered(BigDecimal.TEN); + + when(billService.saveBill(bill)).thenReturn(bill); + + resource.save(payment); + + assertNotNull("Cashier should be set on the payment", payment.getCashier()); + assertSame("Cashier should be the current provider", cashier, payment.getCashier()); + verify(billService).saveBill(bill); + } + + @Test(expected = APIException.class) + public void save_shouldThrowAPIExceptionWhenNoProviderLinkedToUser() { + providerUtilMock.when(ProviderUtil::getCurrentProvider).thenReturn(null); + + Bill bill = new Bill(); + bill.setId(1); + Payment payment = new Payment(); + payment.setBill(bill); + + resource.save(payment); + } + + @Test + public void save_shouldUseClientProvidedCashierWhenSet() { + Provider clientCashier = new Provider(); + clientCashier.setId(2); + + Provider authenticatedCashier = new Provider(); + authenticatedCashier.setId(99); + providerUtilMock.when(ProviderUtil::getCurrentProvider).thenReturn(authenticatedCashier); + + Bill bill = new Bill(); + bill.setId(1); + Payment payment = new Payment(); + payment.setBill(bill); + payment.setAmount(BigDecimal.TEN); + payment.setAmountTendered(BigDecimal.TEN); + payment.setCashier(clientCashier); + + when(billService.saveBill(bill)).thenReturn(bill); + + resource.save(payment); + + assertSame("Client-provided cashier should not be overwritten", clientCashier, payment.getCashier()); + } + + @Test + public void save_shouldFallbackToAuthenticatedUserWhenNoCashierProvided() { + Provider cashier = new Provider(); + cashier.setId(1); + providerUtilMock.when(ProviderUtil::getCurrentProvider).thenReturn(cashier); + + Bill bill = new Bill(); + bill.setId(1); + Payment payment = new Payment(); + payment.setBill(bill); + payment.setAmount(BigDecimal.TEN); + payment.setAmountTendered(BigDecimal.TEN); + + when(billService.saveBill(bill)).thenReturn(bill); + + resource.save(payment); + + assertSame("Authenticated user's provider should be used as fallback", cashier, payment.getCashier()); + } + + @Test + public void save_shouldSetCashierBeforeAddingPaymentToBill() { + Provider cashier = new Provider(); + cashier.setId(1); + providerUtilMock.when(ProviderUtil::getCurrentProvider).thenReturn(cashier); + + Bill bill = new Bill(); + bill.setId(1); + Payment payment = new Payment(); + payment.setBill(bill); + payment.setAmount(BigDecimal.TEN); + payment.setAmountTendered(BigDecimal.TEN); + + when(billService.saveBill(bill)).thenReturn(bill); + + resource.save(payment); + + assertNotNull(payment.getCashier()); + assertTrue(bill.getPayments().contains(payment)); + } +}