diff --git a/src/main/java/org/folio/services/transactions/BaseTransactionService.java b/src/main/java/org/folio/services/transactions/BaseTransactionService.java index d726a032..f3e088df 100644 --- a/src/main/java/org/folio/services/transactions/BaseTransactionService.java +++ b/src/main/java/org/folio/services/transactions/BaseTransactionService.java @@ -31,6 +31,7 @@ public class BaseTransactionService implements TransactionService { private static final Logger log = LogManager.getLogger(); private static final int MAX_FUND_PER_QUERY = 5; + private static final int MAX_TRANSACTIONS_PER_QUERY = 15; private static final String ALLOCATION_TYPE_TRANSACTIONS_QUERY = "(fiscalYearId==%s AND %s) AND %s"; private static final String AWAITING_PAYMENT_WITH_ENCUMBRANCE = "awaitingPayment.encumbranceId==%s"; @@ -92,6 +93,15 @@ public Future> retrieveToTransactions(List fundIds, St .collect(Collectors.toList())).map(lists -> lists.stream().flatMap(Collection::stream).collect(Collectors.toList())); } + public Future> retrievePendingPaymentsByEncumbranceIds(List encumbranceIds, + RequestContext requestContext) { + return collectResultsOnSuccess( + ofSubLists(new ArrayList<>(encumbranceIds), MAX_TRANSACTIONS_PER_QUERY) + .map(ids -> retrievePendingPaymentsByEncumbranceIdsChunk(ids, requestContext)) + .toList()) + .map(lists -> lists.stream().flatMap(Collection::stream).toList()); + } + public Future isConnectedToInvoice(String transactionId, RequestContext requestContext) { // We want to know if the order with the given encumbrance is connected to an invoice. // To avoid adding a dependency to mod-invoice, we check if there is a related awaitingPayment transaction @@ -114,4 +124,11 @@ private List filterFundIdsByAllocationDirection(List fundId .filter(transaction -> !fundIds.contains(allocationDirection.apply(transaction))) .collect(Collectors.toList()); } + + private Future> retrievePendingPaymentsByEncumbranceIdsChunk(List encumbranceIds, + RequestContext requestContext) { + String query = convertIdsToCqlQuery(encumbranceIds, "awaitingPayment.encumbranceId", "==", " OR "); + return retrieveTransactions(query, 0, Integer.MAX_VALUE, requestContext) + .map(TransactionCollection::getTransactions); + } } diff --git a/src/main/java/org/folio/services/transactions/BatchTransactionService.java b/src/main/java/org/folio/services/transactions/BatchTransactionService.java index 60db8cc1..6a9fb53b 100644 --- a/src/main/java/org/folio/services/transactions/BatchTransactionService.java +++ b/src/main/java/org/folio/services/transactions/BatchTransactionService.java @@ -6,22 +6,19 @@ import org.apache.logging.log4j.Logger; import org.folio.rest.core.RestClient; import org.folio.rest.core.models.RequestContext; -import org.folio.rest.core.models.RequestEntry; import org.folio.rest.exception.HttpException; import org.folio.rest.jaxrs.model.Batch; import org.folio.rest.jaxrs.model.Encumbrance; import org.folio.rest.jaxrs.model.Transaction; -import org.folio.rest.jaxrs.model.TransactionCollection; import java.util.List; +import java.util.Optional; import static io.vertx.core.Future.succeededFuture; -import static java.lang.Boolean.FALSE; +import static java.lang.Boolean.TRUE; import static java.util.Collections.singletonList; import static org.folio.rest.util.ErrorCodes.DELETE_CONNECTED_TO_INVOICE; -import static org.folio.rest.util.HelperUtils.convertIdsToCqlQuery; import static org.folio.rest.util.ResourcePathResolver.BATCH_TRANSACTIONS_STORAGE; -import static org.folio.rest.util.ResourcePathResolver.TRANSACTIONS; import static org.folio.rest.util.ResourcePathResolver.resourcesPath; public class BatchTransactionService { @@ -81,34 +78,40 @@ private Future updateTransaction(Transaction transaction, RequestContext r return processBatch(batch, requestContext); } + /** + * Check transaction deletions are allowed. + * Usually it is not allowed to delete an encumbrance connected to an approved invoice. + * To avoid adding a dependency to mod-invoice, we check if there is a related awaitingPayment transaction. + * It is OK to delete an encumbrance connected to a *cancelled* invoice *if* the batch includes a change to the + * matching pending payment to remove the link to the encumbrance. + */ private Future checkDeletions(Batch batch, RequestContext requestContext) { - if (batch.getIdsOfTransactionsToDelete().isEmpty()) { + List ids = batch.getIdsOfTransactionsToDelete(); + if (ids.isEmpty()) { return succeededFuture(); } - return anyConnectedToInvoice(batch.getIdsOfTransactionsToDelete(), requestContext) - .map(connected -> { - if (FALSE.equals(connected)) { + return baseTransactionService.retrievePendingPaymentsByEncumbranceIds(ids, requestContext) + .map(pendingPayments -> { + if (pendingPayments.isEmpty()) { return null; } - logger.warn("validateDeletion:: Tried to delete transactions but one is connected to an invoice, ids={}", - batch.getIdsOfTransactionsToDelete()); - throw new HttpException(422, DELETE_CONNECTED_TO_INVOICE.toError()); + ids.forEach(id -> { + Optional existingPP = pendingPayments.stream() + .filter(pp -> id.equals(pp.getAwaitingPayment().getEncumbranceId())).findFirst(); + if (existingPP.isEmpty()) { + return; + } + if (TRUE.equals(existingPP.get().getInvoiceCancelled())) { + Optional matchingPPInBatch = batch.getTransactionsToUpdate().stream() + .filter(pp -> existingPP.get().getId().equals(pp.getId())).findFirst(); + if (matchingPPInBatch.isPresent() && matchingPPInBatch.get().getAwaitingPayment().getEncumbranceId() == null) { + return; + } + } + logger.warn("validateDeletion:: Tried to delete transactions but one is connected to an invoice, id={}", id); + throw new HttpException(422, DELETE_CONNECTED_TO_INVOICE.toError()); + }); + return null; }); } - - private Future anyConnectedToInvoice(List ids, RequestContext requestContext) { - // We want to know if the order with the given encumbrance is connected to an invoice. - // To avoid adding a dependency to mod-invoice, we check if there is a related awaitingPayment transaction - String query = convertIdsToCqlQuery(ids, "awaitingPayment.encumbranceId", "==", " OR "); - return getTransactions(query, requestContext) - .map(collection -> collection.getTotalRecords() > 0); - } - - private Future getTransactions(String query, RequestContext requestContext) { - var requestEntry = new RequestEntry(resourcesPath(TRANSACTIONS)) - .withOffset(0) - .withLimit(Integer.MAX_VALUE) - .withQuery(query); - return restClient.get(requestEntry.buildEndpoint(), TransactionCollection.class, requestContext); - } } diff --git a/src/test/java/org/folio/ApiTestSuite.java b/src/test/java/org/folio/ApiTestSuite.java index d03be1fc..d21b9726 100644 --- a/src/test/java/org/folio/ApiTestSuite.java +++ b/src/test/java/org/folio/ApiTestSuite.java @@ -50,6 +50,7 @@ import org.folio.services.protection.AcqUnitMembershipsServiceTest; import org.folio.services.protection.AcqUnitsServiceTest; import org.folio.services.protection.ProtectionServiceTest; +import org.folio.services.transactions.BatchTransactionServiceTest; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Nested; @@ -234,4 +235,7 @@ class GroupServiceNested extends GroupServiceTest {} @Nested class RecalculateBudgetServiceTestNested extends RecalculateBudgetServiceTest {} + @Nested + class BatchTransactionServiceTestNested extends BatchTransactionServiceTest {} + } diff --git a/src/test/java/org/folio/services/transactions/BatchTransactionServiceTest.java b/src/test/java/org/folio/services/transactions/BatchTransactionServiceTest.java new file mode 100644 index 00000000..2d640110 --- /dev/null +++ b/src/test/java/org/folio/services/transactions/BatchTransactionServiceTest.java @@ -0,0 +1,83 @@ +package org.folio.services.transactions; + +import io.vertx.core.Future; +import io.vertx.core.json.JsonObject; +import org.folio.rest.jaxrs.model.AwaitingPayment; +import org.folio.rest.jaxrs.model.Transaction; +import org.folio.rest.core.RestClient; +import org.folio.rest.core.models.RequestContext; +import org.folio.rest.jaxrs.model.Batch; +import org.folio.rest.jaxrs.model.TransactionCollection; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.List; +import java.util.UUID; + +import static org.folio.rest.jaxrs.model.Transaction.TransactionType.PENDING_PAYMENT; +import static org.folio.rest.util.ResourcePathResolver.BATCH_TRANSACTIONS_STORAGE; +import static org.folio.rest.util.ResourcePathResolver.resourcesPath; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +public class BatchTransactionServiceTest { + @Mock + private RestClient restClient; + @Mock + private RequestContext requestContext; + + private BatchTransactionService batchTransactionService; + + @BeforeEach + void init() { + BaseTransactionService baseTransactionService = new BaseTransactionService(restClient); + batchTransactionService = new BatchTransactionService(restClient, baseTransactionService); + } + + @Test + void testRemovingEncumbranceWithPendingPaymentUpdate() { + String encumbranceId = UUID.randomUUID().toString(); + String pendingPaymentId = UUID.randomUUID().toString(); + String invoiceId = UUID.randomUUID().toString(); + String invoiceLineId = UUID.randomUUID().toString(); + String fundId = UUID.randomUUID().toString(); + String fiscalYearId = UUID.randomUUID().toString(); + Transaction existingPendingPayment = new Transaction() + .withId(pendingPaymentId) + .withTransactionType(PENDING_PAYMENT) + .withSourceInvoiceId(invoiceId) + .withSourceInvoiceLineId(invoiceLineId) + .withFromFundId(fundId) + .withFiscalYearId(fiscalYearId) + .withCurrency("USD") + .withInvoiceCancelled(true) + .withAwaitingPayment(new AwaitingPayment() + .withEncumbranceId(encumbranceId)); + TransactionCollection existingPendingPaymentCollection = new TransactionCollection() + .withTransactions(List.of(existingPendingPayment)) + .withTotalRecords(1); + Transaction newPendingPayment = JsonObject.mapFrom(existingPendingPayment).mapTo(Transaction.class); + newPendingPayment.getAwaitingPayment().setEncumbranceId(null); + Batch batch = new Batch() + .withIdsOfTransactionsToDelete(List.of(encumbranceId)) + .withTransactionsToUpdate(List.of(newPendingPayment)); + + String expectedQuery = URLEncoder.encode("awaitingPayment.encumbranceId==(" + encumbranceId + ")", StandardCharsets.UTF_8); + when(restClient.get(contains(expectedQuery), any(), eq(requestContext))) + .thenReturn(Future.succeededFuture(existingPendingPaymentCollection)); + when(restClient.postEmptyResponse(eq(resourcesPath(BATCH_TRANSACTIONS_STORAGE)), any(Batch.class), eq(requestContext))) + .thenReturn(Future.succeededFuture()); + + Future result = batchTransactionService.processBatch(batch, requestContext); + assertTrue(result.succeeded()); + } +}