From e04e5b9fc901213a6154d59bd8a5925d7eeb5b35 Mon Sep 17 00:00:00 2001 From: Julian Ladisch Date: Sat, 23 Nov 2024 18:12:21 +0100 Subject: [PATCH] CIRC-2181: Missing owner throws NullPointerException on age to lost with actual cost https://folio-org.atlassian.net/browse/CIRC-2181 Gracefully handle a missing owner on age to lost with actual cost. --- .../ChargeLostFeesWhenAgedToLostService.java | 32 ++++++++++--------- .../ScheduledAgeToLostFeeChargingApiTest.java | 22 +++++++++++++ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/folio/circulation/services/agedtolost/ChargeLostFeesWhenAgedToLostService.java b/src/main/java/org/folio/circulation/services/agedtolost/ChargeLostFeesWhenAgedToLostService.java index 4f6f2372cf..46a65bc00e 100644 --- a/src/main/java/org/folio/circulation/services/agedtolost/ChargeLostFeesWhenAgedToLostService.java +++ b/src/main/java/org/folio/circulation/services/agedtolost/ChargeLostFeesWhenAgedToLostService.java @@ -1,5 +1,6 @@ package org.folio.circulation.services.agedtolost; +import static java.util.concurrent.CompletableFuture.completedFuture; import static java.util.stream.Collectors.toSet; import static org.folio.circulation.domain.FeeFine.LOST_ITEM_FEE_TYPE; import static org.folio.circulation.domain.FeeFine.LOST_ITEM_PROCESSING_FEE_TYPE; @@ -36,6 +37,7 @@ import org.apache.logging.log4j.Logger; import org.folio.circulation.StoreLoanAndItem; import org.folio.circulation.domain.FeeFine; +import org.folio.circulation.domain.FeeFineAction; import org.folio.circulation.domain.FeeFineOwner; import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.MultipleRecords; @@ -136,7 +138,6 @@ private CompletableFuture> chargeLostFees( LoanToChargeFees loanToChargeFees) { return ofAsync(() -> loanToChargeFees) - .thenCompose(r -> r.after(actualCostRecordService::createIfNecessaryForAgedToLostItem)) .thenCompose(r -> r.after(this::chargeLostFeesForLoan)) .thenCompose(r -> r.after(eventPublisher::publishClosedLoanEvent)) .thenApply(r -> r.mapFailure(failure -> handleFailure(loanToChargeFees, failure.toString()))) @@ -149,18 +150,27 @@ private static Result handleFailure(LoanToChargeFees loan, String errorMes } private CompletableFuture> chargeLostFeesForLoan(LoanToChargeFees loanToChargeFees) { + Loan loan = loanToChargeFees.getLoan(); + // we can close loans that have no fee to charge // and billed immediately if (loanToChargeFees.shouldCloseLoan()) { - log.info("No age to lost fees/fines to charge immediately, closing loan [{}]", - loanToChargeFees.getLoan().getId()); - + log.info("No age to lost fees/fines to charge immediately, closing loan [{}]", loan.getId()); return closeLoanAsLostAndPaid(loanToChargeFees); } - Loan loan = loanToChargeFees.getLoan(); - return createAccountsForLoan(loanToChargeFees) - .after(feeFineFacade::createAccounts) + if (loanToChargeFees.hasNoFeeFineOwner()) { + log.warn("No fee/fine owner present for primary service point {}, skipping loan {}", + loanToChargeFees.getPrimaryServicePointId(), loan.getId()); + + return completedFuture(failed(singleValidationError( + "No fee/fine owner found for item's permanent location", + "servicePointId", loanToChargeFees.getPrimaryServicePointId()))); + } + + return actualCostRecordService.createIfNecessaryForAgedToLostItem(loanToChargeFees) + .thenApply(r -> r.next(this::createAccountsForLoan)) + .thenCompose(r -> r.after(feeFineFacade::createAccounts)) .thenCompose(r -> r.after(actions -> feeFineScheduledNoticeService.scheduleNoticesForAgedLostFeeFineCharged(loan, actions))) .thenCompose(r -> r.after(notUsed -> updateLoanBillingInfo(loanToChargeFees))); @@ -279,14 +289,6 @@ private Result loanFetchQuery() { } private Result validateCanCreateAccountForLoan(LoanToChargeFees loanToChargeFees) { - if (loanToChargeFees.hasNoFeeFineOwner()) { - log.warn("No fee/fine owner present for service point {}, skipping loan {}", - loanToChargeFees.getPrimaryServicePointId(), loanToChargeFees.getLoan().getId()); - - return failed(singleValidationError("No fee/fine owner found for item's effective location", - "servicePointId", loanToChargeFees.getPrimaryServicePointId())); - } - final LostItemPolicy lostItemPolicy = loanToChargeFees.getLoan().getLostItemPolicy(); if (lostItemPolicy.getSetCostFee().isChargeable() && loanToChargeFees.hasNoLostItemFee()) { diff --git a/src/test/java/api/loans/agetolost/ScheduledAgeToLostFeeChargingApiTest.java b/src/test/java/api/loans/agetolost/ScheduledAgeToLostFeeChargingApiTest.java index 87fb375c29..2189356023 100644 --- a/src/test/java/api/loans/agetolost/ScheduledAgeToLostFeeChargingApiTest.java +++ b/src/test/java/api/loans/agetolost/ScheduledAgeToLostFeeChargingApiTest.java @@ -298,6 +298,28 @@ void cannotChargeFeesWhenNoItemFeeType() { assertThatPublishedLoanLogRecordEventsAreValid(loanAfter.getJson()); } + @Test + void cannotChargeActualCostWithoutOwner() { + feeFineOwnerFixture.cleanUp(); // remove owner + + var policy = lostItemFeePoliciesFixture + .ageToLostAfterOneMinutePolicy() + .withActualCost(19.99) + .withLostItemProcessingFee(9.99); + useLostItemPolicy(lostItemFeePoliciesFixture.create(policy).getId()); + val item = itemsFixture.basedUponNod(ItemBuilder::withRandomBarcode); + val loanBefore = checkOutFixture.checkOutByBarcode(item, usersFixture.steve()); + + ageToLostFixture.ageToLostAndChargeFees(); + + IndividualResource loanAfter = loansFixture.getLoanById(loanBefore.getId()); + assertThat(loanAfter, hasNoLostItemFee()); + assertThat(loanAfter, hasNoLostItemProcessingFee()); + assertThat(accountsClient.getAll(), hasSize(0)); + assertThat(scheduledNoticesClient.getAll(), hasSize(0)); + assertThatPublishedLoanLogRecordEventsAreValid(loanAfter.getJson()); + } + @Test void shouldNotChargeFeesWhenDelayedBillingPeriodHasNotPassed() { val lostItemFeePolicy = lostItemFeePoliciesFixture