From d85b59246eb83a81d6c329bc2f3aad8d46cef773 Mon Sep 17 00:00:00 2001 From: Serhii Nosko Date: Fri, 12 Jul 2024 15:08:09 +0300 Subject: [PATCH] [MODFIN-374]. Update budget validation when updating a budget (#254) * [MODFIN-374]. Update budget validation when updating a budget --- .../folio/services/budget/BudgetService.java | 39 +++++++---- .../services/budget/BudgetServiceTest.java | 69 ++++++++++++++----- 2 files changed, 77 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/folio/services/budget/BudgetService.java b/src/main/java/org/folio/services/budget/BudgetService.java index aeac3b78..3fe8303a 100644 --- a/src/main/java/org/folio/services/budget/BudgetService.java +++ b/src/main/java/org/folio/services/budget/BudgetService.java @@ -129,13 +129,20 @@ private List checkRemainingEncumbrance(SharedBudget budget) { BigDecimal expenditures = BigDecimal.valueOf(budget.getExpenditures()); BigDecimal credits = BigDecimal.valueOf(budget.getCredits()); BigDecimal awaitingPayment = BigDecimal.valueOf(budget.getAwaitingPayment()); - BigDecimal totalFunding = BigDecimal.valueOf(budget.getTotalFunding()); + BigDecimal allocated = BigDecimal.valueOf(budget.getAllocated()); + BigDecimal netTransfers = BigDecimal.valueOf(budget.getNetTransfers()); - //[remaining amount we can encumber] = (allocated * allowableEncumbered) - (encumbered + awaitingPayment + expended) + // [remaining amount we can encumber] = (allocated + netTransfers) * allowableEncumbered - (encumbered + awaitingPayment + expended - credits) + // calculations should be consistent with BatchTransactionChecks in mod-finance-storage if (budget.getAllowableEncumbrance() != null) { log.info("checkRemainingEncumbrance:: Budget '{}' allowable encumbrance is not null", budget.getId()); - BigDecimal newAllowableEncumbrance = BigDecimal.valueOf(budget.getAllowableEncumbrance()).movePointLeft(2); - if (totalFunding.multiply(newAllowableEncumbrance).compareTo(encumbered.add(awaitingPayment).add(expenditures).subtract(credits)) < 0) { + BigDecimal allowableEncumbrance = BigDecimal.valueOf(budget.getAllowableEncumbrance()).movePointLeft(2); + BigDecimal totalFunding = allocated.add(netTransfers); + // unavailable amount shouldn't be negative + BigDecimal unavailable = ensureNonNegative(encumbered.add(awaitingPayment).add(expenditures).subtract(credits)); + + double remaining = totalFunding.multiply(allowableEncumbrance).subtract(unavailable).doubleValue(); + if (remaining < 0) { log.error("checkRemainingEncumbrance:: Allowable encumbrance limit exceeded for budget: {}", budget.getId()); return Collections.singletonList(ALLOWABLE_ENCUMBRANCE_LIMIT_EXCEEDED.toError()); } @@ -146,26 +153,30 @@ private List checkRemainingEncumbrance(SharedBudget budget) { private List checkRemainingExpenditure(SharedBudget budget) { log.debug("checkRemainingExpenditure:: Check remaining expenditure for budget: {}", budget.getId()); BigDecimal allocated = BigDecimal.valueOf(budget.getAllocated()); + BigDecimal netTransfers = BigDecimal.valueOf(budget.getNetTransfers()); BigDecimal expenditures = BigDecimal.valueOf(budget.getExpenditures()); BigDecimal credits = BigDecimal.valueOf(budget.getCredits()); BigDecimal awaitingPayment = BigDecimal.valueOf(budget.getAwaitingPayment()); - BigDecimal available = BigDecimal.valueOf(budget.getAvailable()); - BigDecimal unavailable = BigDecimal.valueOf(budget.getUnavailable()); - //[amount we can expend] = (allocated * allowableExpenditure) - (allocated - (unavailable + available)) - - // (expended - credited + awaitingPayment) + // [remaining amount we can expend] = (allocated + netTransfers) * allowableExpenditure - (awaitingPayment + expended - credited) + // calculations should be consistent with BatchTransactionChecks in mod-finance-storage if (budget.getAllowableExpenditure() != null) { log.info("checkRemainingExpenditure:: Budget '{}' allowable expenditure is not null", budget.getId()); - BigDecimal newAllowableExpenditure = BigDecimal.valueOf(budget.getAllowableExpenditure()) - .movePointLeft(2); - if (allocated.multiply(newAllowableExpenditure) - .subtract(allocated.subtract(available.add(unavailable))) - .subtract(expenditures.subtract(credits).add(awaitingPayment)) - .compareTo(BigDecimal.ZERO) < 0) { + BigDecimal allowableExpenditure = BigDecimal.valueOf(budget.getAllowableExpenditure()).movePointLeft(2); + BigDecimal totalFunding = allocated.add(netTransfers); + // unavailable amount shouldn't be negative + BigDecimal unavailable = ensureNonNegative(awaitingPayment.add(expenditures).subtract(credits)); + + double remaining = totalFunding.multiply(allowableExpenditure).subtract(unavailable).doubleValue(); + if (remaining < 0) { log.error("checkRemainingExpenditure:: Allowable expenditure limit exceed for budget: {}", budget.getId()); return Collections.singletonList(ALLOWABLE_EXPENDITURE_LIMIT_EXCEEDED.toError()); } } return Collections.emptyList(); } + + private BigDecimal ensureNonNegative(BigDecimal amount) { + return amount.max(BigDecimal.ZERO); + } } diff --git a/src/test/java/org/folio/services/budget/BudgetServiceTest.java b/src/test/java/org/folio/services/budget/BudgetServiceTest.java index 5deeb179..5bb9cd81 100644 --- a/src/test/java/org/folio/services/budget/BudgetServiceTest.java +++ b/src/test/java/org/folio/services/budget/BudgetServiceTest.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.UUID; +import io.vertx.core.AsyncResult; import org.folio.rest.core.RestClient; import org.folio.rest.core.models.RequestContext; import org.folio.rest.exception.HttpException; @@ -33,6 +34,7 @@ import org.folio.rest.jaxrs.model.Errors; import org.folio.rest.jaxrs.model.SharedBudget; import org.folio.rest.jaxrs.model.StatusExpenseClass; +import org.folio.rest.util.ErrorCodes; import org.hamcrest.core.IsInstanceOf; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -155,14 +157,27 @@ void testUpdateBudgetWithExceededAllowableEncumbered(VertxTestContext vertxTestC vertxTestContext.assertFailure(future) .onComplete(result -> { - assertThat(result.cause(), IsInstanceOf.instanceOf(HttpException.class)); - HttpException cause = (HttpException) result.cause(); - Errors errors = new Errors().withErrors(Collections.singletonList(ALLOWABLE_ENCUMBRANCE_LIMIT_EXCEEDED.toError())).withTotalRecords(1); - assertEquals(422, cause.getCode()); - assertEquals(errors, cause.getErrors()); - - verify(restClient).get(anyString(), eq(Budget.class), eq(requestContextMock)); - verify(restClient, never()).put(anyString(), any(), any()); + verifyLimitExceeded(result, ALLOWABLE_ENCUMBRANCE_LIMIT_EXCEEDED); + vertxTestContext.completeNow(); + }); + } + + @Test + void testUpdateBudgetWithExceededAllowableEncumberedSimplifiedCase(VertxTestContext vertxTestContext) { + sharedBudget + .withAllowableEncumbrance(90d); + + Budget budgetFromStorage = new Budget() + .withAllocated(10d) + .withEncumbered(10d); + + when(restClient.get(anyString(), eq(Budget.class), any())) + .thenReturn(succeededFuture(budgetFromStorage)); + + Future future = budgetService.updateBudget(sharedBudget, requestContextMock); + vertxTestContext.assertFailure(future) + .onComplete(result -> { + verifyLimitExceeded(result, ALLOWABLE_ENCUMBRANCE_LIMIT_EXCEEDED); vertxTestContext.completeNow(); }); } @@ -232,17 +247,27 @@ void testUpdateBudgetWithExceededAllowableExpenditure(VertxTestContext vertxTest Future future = budgetService.updateBudget(sharedBudget, requestContextMock); vertxTestContext.assertFailure(future) .onComplete(result -> { - var exception = result.cause(); - assertThat(exception, IsInstanceOf.instanceOf(HttpException.class)); - HttpException cause = (HttpException) exception; - Errors errors = new Errors().withErrors(Collections.singletonList(ALLOWABLE_EXPENDITURE_LIMIT_EXCEEDED.toError())).withTotalRecords(1); - assertEquals(422, cause.getCode()); - assertEquals(errors, cause.getErrors()); + verifyLimitExceeded(result, ALLOWABLE_EXPENDITURE_LIMIT_EXCEEDED); + vertxTestContext.completeNow(); + }); + + } - verify(restClient) - .get(assertQueryContains(sharedBudget.getId()), eq(Budget.class), eq(requestContextMock)); - verify(restClient, never()).put(anyString(), any(), any()); + @Test + void testUpdateBudgetWithExceededAllowableExpenditureSimplifiedCase(VertxTestContext vertxTestContext) { + sharedBudget + .withAllowableExpenditure(90d); + + Budget budgetFromStorage = new Budget() + .withAllocated(10d) + .withAwaitingPayment(15d); + when(restClient.get(anyString(), eq(Budget.class), any())).thenReturn(succeededFuture(budgetFromStorage)); + + Future future = budgetService.updateBudget(sharedBudget, requestContextMock); + vertxTestContext.assertFailure(future) + .onComplete(result -> { + verifyLimitExceeded(result, ALLOWABLE_EXPENDITURE_LIMIT_EXCEEDED); vertxTestContext.completeNow(); }); @@ -286,4 +311,14 @@ void testDeleteBudget(VertxTestContext vertxTestContext) { } + private void verifyLimitExceeded(AsyncResult result, ErrorCodes errorCode) { + assertThat(result.cause(), IsInstanceOf.instanceOf(HttpException.class)); + HttpException cause = (HttpException) result.cause(); + Errors errors = new Errors().withErrors(Collections.singletonList(errorCode.toError())).withTotalRecords(1); + assertEquals(422, cause.getCode()); + assertEquals(errors, cause.getErrors()); + + verify(restClient).get(anyString(), eq(Budget.class), eq(requestContextMock)); + verify(restClient, never()).put(anyString(), any(), any()); + } }