From 46ed0c6abdb161e6d0293cff46ec6ea184ff9166 Mon Sep 17 00:00:00 2001 From: Mykyta_Varenyk Date: Wed, 29 Jun 2022 14:05:44 +0300 Subject: [PATCH 01/19] CIRC-1556 When actual cost is selected, close the loan only if it has been paid or enough time for charging has passed --- .../policy/lostitem/LostItemPolicy.java | 16 ++++- .../RequestFromRepresentationService.java | 5 +- ...anRelatedFeeFineClosedHandlerResource.java | 23 +++++-- .../java/api/loans/DeclareLostAPITests.java | 68 +++++++++---------- 4 files changed, 67 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java index 9c721f8a11..439a896c4c 100644 --- a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java +++ b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java @@ -20,6 +20,7 @@ import org.folio.circulation.domain.policy.lostitem.itemfee.ActualCostFee; import org.folio.circulation.domain.policy.lostitem.itemfee.AutomaticallyChargeableFee; import org.folio.circulation.domain.policy.lostitem.itemfee.ChargeableFee; +import org.folio.circulation.support.utils.ClockUtil; import io.vertx.core.json.JsonObject; @@ -34,6 +35,7 @@ public class LostItemPolicy extends Policy { private final Period patronBilledAfterItemAgedToLostInterval; private final Period recalledItemAgedToLostAfterOverdueInterval; private final Period patronBilledAfterRecalledItemAgedToLostInterval; + private final Period lostItemChargeFeeFine; // There is no separate age to lost processing fee but there is a flag // that turns on/off the fee, but we're modelling it as a separate fee // to simplify logic. @@ -45,7 +47,7 @@ private LostItemPolicy(String id, String name, AutomaticallyChargeableFee declar Period itemAgedToLostAfterOverdueInterval, Period patronBilledAfterItemAgedToLostInterval, Period recalledItemAgedToLostAfterOverdueInterval, Period patronBilledAfterRecalledItemAgedToLostInterval, - AutomaticallyChargeableFee ageToLostProcessingFee) { + AutomaticallyChargeableFee ageToLostProcessingFee, Period lostItemChargeFeeFine) { super(id, name); this.declareLostProcessingFee = declareLostProcessingFee; @@ -60,6 +62,7 @@ private LostItemPolicy(String id, String name, AutomaticallyChargeableFee declar this.patronBilledAfterRecalledItemAgedToLostInterval = patronBilledAfterRecalledItemAgedToLostInterval; this.ageToLostProcessingFee = ageToLostProcessingFee; + this.lostItemChargeFeeFine = lostItemChargeFeeFine; } public static LostItemPolicy from(JsonObject lostItemPolicy) { @@ -76,7 +79,8 @@ public static LostItemPolicy from(JsonObject lostItemPolicy) { getPeriodPropertyOrEmpty(lostItemPolicy, "patronBilledAfterAgedLost"), getPeriodPropertyOrEmpty(lostItemPolicy, "recalledItemAgedLostOverdue"), getPeriodPropertyOrEmpty(lostItemPolicy, "patronBilledAfterRecalledItemAgedLost"), - getProcessingFee(lostItemPolicy, "chargeAmountItemSystem") + getProcessingFee(lostItemPolicy, "chargeAmountItemSystem"), + getPeriodPropertyOrEmpty(lostItemPolicy, "lostItemChargeFeeFine") ); } @@ -139,6 +143,12 @@ public boolean shouldRefundFees(ZonedDateTime lostDateTime) { || feeRefundInterval.isEqualToDateTillNow(lostDateTime); } + public boolean feeFineChargingPeriodHasPassed() { + ZonedDateTime now = ClockUtil.getZonedDateTime(); + return lostItemChargeFeeFine.isEqualToDateTillNow(now) || + lostItemChargeFeeFine.hasPassedSinceDateTillNow(now); + } + public boolean isRefundProcessingFeeWhenReturned() { return refundProcessingFeeWhenReturned; } @@ -189,7 +199,7 @@ private static class UnknownLostItemPolicy extends LostItemPolicy { super(id, null, noAutomaticallyChargeableFee(), noAutomaticallyChargeableFee(), noActualCostFee(), zeroDurationPeriod(), false, false, zeroDurationPeriod(), zeroDurationPeriod(), zeroDurationPeriod(), - zeroDurationPeriod(), noAutomaticallyChargeableFee()); + zeroDurationPeriod(), noAutomaticallyChargeableFee(), zeroDurationPeriod()); } } } diff --git a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java index e9aa92eb54..600bf89670 100644 --- a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java +++ b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java @@ -268,11 +268,12 @@ private CompletableFuture> fetchItemAndLoanForRecallTlrRequest(R RequestQueue requestQueue = records.getRequestQueue(); + List recalledLoansIds = requestQueue.getRecalledLoansIds(); return loanRepository.findLoanWithClosestDueDate(mapToItemIds(request.getInstanceItems()), - requestQueue.getRecalledLoansIds()) + recalledLoansIds) //Loan is null means that we have no items that haven't been recalled. In this case we //take the loan that has been recalled the least times - .thenComposeAsync(r -> r.after(when(loan -> ofAsync(() -> loan == null), + .thenComposeAsync(r -> r.after(when(loan -> ofAsync(() -> loan == null && !recalledLoansIds.isEmpty()), ignored -> ofAsync(requestQueue::getTheLeastRecalledLoan), result -> ofAsync(() -> result)))) .thenApply(resultLoan -> resultLoan.map(request::withLoan)) .thenComposeAsync(requestResult -> requestResult.combineAfter( diff --git a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java index 2742ae6495..9fb68f07b5 100644 --- a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java +++ b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java @@ -2,6 +2,7 @@ import static java.util.concurrent.CompletableFuture.completedFuture; import static org.folio.circulation.domain.EventType.LOAN_RELATED_FEE_FINE_CLOSED; +import static org.folio.circulation.domain.FeeFine.LOST_ITEM_ACTUAL_COST_FEE_TYPE; import static org.folio.circulation.domain.FeeFine.lostItemFeeTypes; import static org.folio.circulation.domain.subscribers.LoanRelatedFeeFineClosedEvent.fromJson; import static org.folio.circulation.support.Clients.create; @@ -16,6 +17,7 @@ import org.folio.circulation.StoreLoanAndItem; import org.folio.circulation.domain.Account; import org.folio.circulation.domain.Loan; +import org.folio.circulation.domain.policy.lostitem.LostItemPolicy; import org.folio.circulation.domain.subscribers.LoanRelatedFeeFineClosedEvent; import org.folio.circulation.infrastructure.storage.feesandfines.AccountRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; @@ -107,7 +109,7 @@ private CompletableFuture> closeLoanWithLostItemIfLostFeesResolved( public CompletableFuture> closeLoanAndUpdateItem(Loan loan, LoanRepository loanRepository, ItemRepository itemRepository, EventPublisher eventPublisher) { - if (!allLostFeesClosed(loan)) { + if (!allLostFeesClosed(loan) && !shouldCloseLoan(loan)) { return completedFuture(succeeded(loan)); } @@ -127,16 +129,25 @@ private CompletableFuture> publishLoanClosedEvent(Loan loan, boolea } private boolean allLostFeesClosed(Loan loan) { - if (loan.getLostItemPolicy().hasActualCostFee()) { - // Actual cost fee is processed manually - return false; - } - return loan.getAccounts().stream() .filter(account -> lostItemFeeTypes().contains(account.getFeeFineType())) .allMatch(Account::isClosed); } + private boolean shouldCloseLoan(Loan loan) { + LostItemPolicy lostItemPolicy = loan.getLostItemPolicy(); + + if (!lostItemPolicy.hasActualCostFee()) { + return true; + } + + if (loan.getAccounts().stream().noneMatch(account -> LOST_ITEM_ACTUAL_COST_FEE_TYPE.equals(account.getFeeFineType()))) { + return lostItemPolicy.feeFineChargingPeriodHasPassed(); + } else { + return true; + } + } + private Result createAndValidateRequest(RoutingContext context) { final LoanRelatedFeeFineClosedEvent eventPayload = fromJson(context.getBodyAsJson()); diff --git a/src/test/java/api/loans/DeclareLostAPITests.java b/src/test/java/api/loans/DeclareLostAPITests.java index b6b3e4e697..5065f48aad 100644 --- a/src/test/java/api/loans/DeclareLostAPITests.java +++ b/src/test/java/api/loans/DeclareLostAPITests.java @@ -827,56 +827,56 @@ public void shouldRefundPartiallyPaidOrTransferredLostItemFeesBeforeApplyingNewF @Test void shouldClearExistingFeesAndCloseLoanAsLostAndPaidIfLostandPaidItemDeclaredLostAndPolicySetNotToChargeFees() { - final double lostItemProcessingFee = 20.0; - UUID servicePointId = servicePointsFixture.cd1().getId(); + final double lostItemProcessingFee = 20.0; + UUID servicePointId = servicePointsFixture.cd1().getId(); - final LostItemFeePolicyBuilder lostPolicyBuilder = lostItemFeePoliciesFixture.ageToLostAfterOneMinutePolicy() - .withName("age to lost with processing fees") - .billPatronImmediatelyWhenAgedToLost() - .withNoFeeRefundInterval() - .withNoChargeAmountItem() - .doNotChargeProcessingFeeWhenDeclaredLost() - .chargeProcessingFeeWhenAgedToLost(lostItemProcessingFee); + final LostItemFeePolicyBuilder lostPolicyBuilder = lostItemFeePoliciesFixture.ageToLostAfterOneMinutePolicy() + .withName("age to lost with processing fees") + .billPatronImmediatelyWhenAgedToLost() + .withNoFeeRefundInterval() + .withNoChargeAmountItem() + .doNotChargeProcessingFeeWhenDeclaredLost() + .chargeProcessingFeeWhenAgedToLost(lostItemProcessingFee); - useLostItemPolicy(lostItemFeePoliciesFixture.create(lostPolicyBuilder).getId()); + useLostItemPolicy(lostItemFeePoliciesFixture.create(lostPolicyBuilder).getId()); - AgeToLostResult agedToLostResult = ageToLostFixture.createLoanAgeToLostAndChargeFees(lostPolicyBuilder); - UUID testLoanId = agedToLostResult.getLoanId(); - UUID itemId = agedToLostResult.getItemId(); + AgeToLostResult agedToLostResult = ageToLostFixture.createLoanAgeToLostAndChargeFees(lostPolicyBuilder); + UUID testLoanId = agedToLostResult.getLoanId(); + UUID itemId = agedToLostResult.getItemId(); - JsonObject AgeToLostItem = itemsFixture.getById(itemId).getJson(); + JsonObject AgeToLostItem = itemsFixture.getById(itemId).getJson(); - assertThat(AgeToLostItem, isAgedToLost()); + assertThat(AgeToLostItem, isAgedToLost()); - JsonObject itemFee = getAccountForLoan(testLoanId, "Lost item processing fee"); + JsonObject itemFee = getAccountForLoan(testLoanId, "Lost item processing fee"); - assertThat(itemFee, hasJsonPath("amount", lostItemProcessingFee)); + assertThat(itemFee, hasJsonPath("amount", lostItemProcessingFee)); - final ZonedDateTime declareLostDate = getZonedDateTime().plusWeeks(1); - mockClockManagerToReturnFixedDateTime(declareLostDate); + final ZonedDateTime declareLostDate = getZonedDateTime().plusWeeks(1); + mockClockManagerToReturnFixedDateTime(declareLostDate); - final DeclareItemLostRequestBuilder builder = new DeclareItemLostRequestBuilder() - .forLoanId(testLoanId) - .withServicePointId(servicePointId) - .on(declareLostDate) - .withNoComment(); + final DeclareItemLostRequestBuilder builder = new DeclareItemLostRequestBuilder() + .forLoanId(testLoanId) + .withServicePointId(servicePointId) + .on(declareLostDate) + .withNoComment(); - FakePubSub.clearPublishedEvents(); + FakePubSub.clearPublishedEvents(); - declareLostFixtures.declareItemLost(builder); + declareLostFixtures.declareItemLost(builder); - JsonObject declareLostLoan = loansClient.getById(testLoanId).getJson(); - JsonObject declareLostItem = itemsFixture.getById(itemId).getJson(); + JsonObject declareLostLoan = loansClient.getById(testLoanId).getJson(); + JsonObject declareLostItem = itemsFixture.getById(itemId).getJson(); - assertThat(declareLostItem, isLostAndPaid()); + assertThat(declareLostItem, isLostAndPaid()); - Double finalAmountRemaining = declareLostLoan.getJsonObject("feesAndFines").getDouble("amountRemainingToPay"); - assertEquals(finalAmountRemaining, 0.0, 0.01); + Double finalAmountRemaining = declareLostLoan.getJsonObject("feesAndFines").getDouble("amountRemainingToPay"); + assertEquals(finalAmountRemaining, 0.0, 0.01); - List accounts = getAccountsForLoan(testLoanId); + List accounts = getAccountsForLoan(testLoanId); - assertThat(accounts, hasSize(1)); - assertThat(getOpenAccounts(accounts), hasSize(0)); + assertThat(accounts, hasSize(1)); + assertThat(getOpenAccounts(accounts), hasSize(0)); verifyNumberOfPublishedEvents(LOAN_CLOSED, 1); verifyNumberOfPublishedEvents(ITEM_DECLARED_LOST, 0); From 11a57a2c93cf5056f622095d540d32442231a80d Mon Sep 17 00:00:00 2001 From: Mykyta_Varenyk Date: Thu, 30 Jun 2022 16:01:30 +0300 Subject: [PATCH 02/19] CIRC-1556 Create actual cost expiration timer --- descriptors/ModuleDescriptor-template.json | 15 ++ .../circulation/CirculationVerticle.java | 2 + .../circulation/domain/ActualCostRecord.java | 1 + .../org/folio/circulation/domain/Loan.java | 34 ++- .../policy/lostitem/LostItemPolicy.java | 13 +- .../storage/ActualCostRecordRepository.java | 42 +++- .../ExpiredActualCostProcessingResource.java | 52 +++++ ...anRelatedFeeFineClosedHandlerResource.java | 95 ++------ .../CloseLoanWithLostItemService.java | 112 ++++++++++ .../services/LostItemFeeChargingService.java | 1 + .../ActualCostRecordExpirationService.java | 58 +++++ .../ActualCostRecordService.java | 6 +- .../ChargeLostFeesWhenAgedToLostService.java | 2 +- .../mappers/ActualCostRecordMapper.java | 8 +- .../support/http/client/CqlQuery.java | 4 + ...LoanWhenLostItemFeesAreClosedApiTests.java | 208 +++++++++++++++++- .../api/support/builders/AccountBuilder.java | 59 +++-- .../builders/LostItemFeePolicyBuilder.java | 9 +- .../fixtures/ExpireActualCostFixture.java | 14 ++ .../fixtures/FeeFineAccountFixture.java | 25 ++- .../fixtures/LostItemFeePoliciesFixture.java | 15 +- .../java/api/support/http/InterfaceUrls.java | 4 + 22 files changed, 647 insertions(+), 132 deletions(-) create mode 100644 src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java create mode 100644 src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java create mode 100644 src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java rename src/main/java/org/folio/circulation/services/{ => actualcostrecord}/ActualCostRecordService.java (95%) create mode 100644 src/test/java/api/support/fixtures/ExpireActualCostFixture.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 6955a729ab..d90482fa30 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -636,6 +636,21 @@ "unit": "minute", "delay": "3" }, + { + "methods": [ + "POST" + ], + "pathPattern": "/circulation/actual-cost-expiration-by-timeout", + "permissionsRequired": [ + "pubsub.events.post" + ], + "modulePermissions": [ + "modperms.circulation.handlers.loan-related-fee-fine-closed.post", + "pubsub.publish.post" + ], + "unit": "minute", + "delay": "20" + }, { "methods": [ "POST" diff --git a/src/main/java/org/folio/circulation/CirculationVerticle.java b/src/main/java/org/folio/circulation/CirculationVerticle.java index 3c9e38403a..d51a8d25e9 100644 --- a/src/main/java/org/folio/circulation/CirculationVerticle.java +++ b/src/main/java/org/folio/circulation/CirculationVerticle.java @@ -13,6 +13,7 @@ import org.folio.circulation.resources.DeclareLostResource; import org.folio.circulation.resources.DueDateNotRealTimeScheduledNoticeProcessingResource; import org.folio.circulation.resources.EndPatronActionSessionResource; +import org.folio.circulation.resources.ExpiredActualCostProcessingResource; import org.folio.circulation.resources.ExpiredSessionProcessingResource; import org.folio.circulation.resources.FeeFineScheduledNoticeProcessingResource; import org.folio.circulation.resources.ItemsInTransitResource; @@ -71,6 +72,7 @@ public void start(Promise startFuture) { new CheckOutByBarcodeResource("/circulation/check-out-by-barcode", client).register(router); new CheckInByBarcodeResource(client).register(router); + new ExpiredActualCostProcessingResource(client).register(router); new RenewByBarcodeResource(client).register(router); new RenewByIdResource(client).register(router); diff --git a/src/main/java/org/folio/circulation/domain/ActualCostRecord.java b/src/main/java/org/folio/circulation/domain/ActualCostRecord.java index 85d919f0f1..a8673814b0 100644 --- a/src/main/java/org/folio/circulation/domain/ActualCostRecord.java +++ b/src/main/java/org/folio/circulation/domain/ActualCostRecord.java @@ -29,4 +29,5 @@ public class ActualCostRecord { private String feeFineOwner; private String feeFineTypeId; private String feeFineType; + private String expirationDate; } diff --git a/src/main/java/org/folio/circulation/domain/Loan.java b/src/main/java/org/folio/circulation/domain/Loan.java index e9fd96f457..4c5786449f 100644 --- a/src/main/java/org/folio/circulation/domain/Loan.java +++ b/src/main/java/org/folio/circulation/domain/Loan.java @@ -88,6 +88,7 @@ public class Loan implements ItemRelatedRecord, UserRelatedRecord { private final Policies policies; private final Collection accounts; + private final ActualCostRecord actualCostRecord; public static Loan from(JsonObject representation) { defaultStatusAndAction(representation); @@ -100,7 +101,7 @@ public static Loan from(JsonObject representation) { return new Loan(representation, null, null, null, null, null, getDateTimeProperty(representation, DUE_DATE), getDateTimeProperty(representation, DUE_DATE), - new Policies(loanPolicy, overdueFinePolicy, lostItemPolicy), emptyList()); + new Policies(loanPolicy, overdueFinePolicy, lostItemPolicy), emptyList(), null); } public JsonObject asJson() { @@ -272,7 +273,7 @@ public Item getItem() { public Loan replaceRepresentation(JsonObject newRepresentation) { return new Loan(newRepresentation, item, user, proxy, checkinServicePoint, - checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts); + checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts, actualCostRecord); } public Loan withItem(Item newItem) { @@ -283,7 +284,7 @@ public Loan withItem(Item newItem) { } return new Loan(newRepresentation, newItem, user, proxy, checkinServicePoint, - checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts); + checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts, actualCostRecord); } public User getUser() { @@ -298,7 +299,16 @@ public Loan withUser(User newUser) { } return new Loan(newRepresentation, item, newUser, proxy, checkinServicePoint, - checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts); + checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts, actualCostRecord); + } + + public Loan withActualCostRecord(ActualCostRecord actualCostRecord) { + return new Loan(representation, item, user, proxy, checkinServicePoint, checkoutServicePoint, + originalDueDate, previousDueDate, policies, accounts, actualCostRecord); + } + + public ActualCostRecord getActualCostRecord() { + return actualCostRecord; } public Loan withPatronGroupAtCheckout(PatronGroup patronGroup) { @@ -325,22 +335,22 @@ Loan withProxy(User newProxy) { } return new Loan(newRepresentation, item, user, newProxy, checkinServicePoint, - checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts); + checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts, actualCostRecord); } public Loan withCheckinServicePoint(ServicePoint newCheckinServicePoint) { return new Loan(representation, item, user, proxy, newCheckinServicePoint, - checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts); + checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts, actualCostRecord); } public Loan withCheckoutServicePoint(ServicePoint newCheckoutServicePoint) { return new Loan(representation, item, user, proxy, checkinServicePoint, - newCheckoutServicePoint, originalDueDate, previousDueDate, policies, accounts); + newCheckoutServicePoint, originalDueDate, previousDueDate, policies, accounts, actualCostRecord); } public Loan withAccounts(Collection newAccounts) { return new Loan(representation, item, user, proxy, checkinServicePoint, - checkoutServicePoint, originalDueDate, previousDueDate, policies, newAccounts); + checkoutServicePoint, originalDueDate, previousDueDate, policies, newAccounts, actualCostRecord); } public Loan withLoanPolicy(LoanPolicy newLoanPolicy) { @@ -348,7 +358,7 @@ public Loan withLoanPolicy(LoanPolicy newLoanPolicy) { return new Loan(representation, item, user, proxy, checkinServicePoint, checkoutServicePoint, originalDueDate, previousDueDate, - policies.withLoanPolicy(newLoanPolicy), accounts); + policies.withLoanPolicy(newLoanPolicy), accounts, actualCostRecord); } public Loan withOverdueFinePolicy(OverdueFinePolicy newOverdueFinePolicy) { @@ -356,7 +366,7 @@ public Loan withOverdueFinePolicy(OverdueFinePolicy newOverdueFinePolicy) { return new Loan(representation, item, user, proxy, checkinServicePoint, checkoutServicePoint, originalDueDate, previousDueDate, - policies.withOverdueFinePolicy(newOverdueFinePolicy), accounts); + policies.withOverdueFinePolicy(newOverdueFinePolicy), accounts, actualCostRecord); } public Loan withLostItemPolicy(LostItemPolicy newLostItemPolicy) { @@ -364,7 +374,7 @@ public Loan withLostItemPolicy(LostItemPolicy newLostItemPolicy) { return new Loan(representation, item, user, proxy, checkinServicePoint, checkoutServicePoint, originalDueDate, previousDueDate, - policies.withLostItemPolicy(newLostItemPolicy), accounts); + policies.withLostItemPolicy(newLostItemPolicy), accounts, actualCostRecord); } public String getLoanPolicyId() { @@ -625,7 +635,7 @@ public void closeLoanAsLostAndPaid() { public Loan copy() { final JsonObject representationCopy = representation.copy(); return new Loan(representationCopy, item, user, proxy, checkinServicePoint, - checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts); + checkoutServicePoint, originalDueDate, previousDueDate, policies, accounts, actualCostRecord); } public Loan ageOverdueItemToLost(ZonedDateTime ageToLostDate) { diff --git a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java index 439a896c4c..c05921aa98 100644 --- a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java +++ b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java @@ -143,10 +143,10 @@ public boolean shouldRefundFees(ZonedDateTime lostDateTime) { || feeRefundInterval.isEqualToDateTillNow(lostDateTime); } - public boolean feeFineChargingPeriodHasPassed() { - ZonedDateTime now = ClockUtil.getZonedDateTime(); - return lostItemChargeFeeFine.isEqualToDateTillNow(now) || - lostItemChargeFeeFine.hasPassedSinceDateTillNow(now); + public boolean feeFineChargingPeriodHasPassed(ZonedDateTime lostDateTime) { + return lostItemChargeFeeFine.hasZeroDuration() + || lostItemChargeFeeFine.isEqualToDateTillNow(lostDateTime) + || lostItemChargeFeeFine.hasPassedSinceDateTillNow(lostDateTime); } public boolean isRefundProcessingFeeWhenReturned() { @@ -172,6 +172,11 @@ public boolean canAgeLoanToLost(boolean isRecalled, ZonedDateTime loanDueDate) { return periodShouldPassSinceOverdue.hasPassedSinceDateTillNow(loanDueDate); } + public ZonedDateTime calculateFeeFineChargingPeriodExpirationDateTime(ZonedDateTime lostTime) { + return lostItemChargeFeeFine.plusDate(lostTime); + } + + public ZonedDateTime calculateDateTimeWhenPatronBilledForAgedToLost( boolean isRecalled, ZonedDateTime ageToLostDate) { diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java index 3a696fcb36..1e9b502467 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java @@ -1,21 +1,37 @@ package org.folio.circulation.infrastructure.storage; +import io.vertx.core.json.JsonObject; +import static java.util.function.Function.identity; import static org.folio.circulation.support.http.ResponseMapping.forwardOnFailure; import static org.folio.circulation.support.http.ResponseMapping.mapUsingJson; +import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; +import static org.folio.circulation.support.http.client.PageLimit.one; +import static org.folio.circulation.support.results.ResultBinding.flatMapResult; +import static org.folio.circulation.support.results.ResultBinding.mapResult; import java.util.concurrent.CompletableFuture; import org.folio.circulation.domain.ActualCostRecord; +import org.folio.circulation.domain.Loan; +import org.folio.circulation.domain.MultipleRecords; import org.folio.circulation.storage.mappers.ActualCostRecordMapper; import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; +import org.folio.circulation.support.fetching.CqlQueryFinder; +import org.folio.circulation.support.fetching.GetManyRecordsRepository; +import org.folio.circulation.support.http.client.CqlQuery; +import org.folio.circulation.support.http.client.Offset; +import org.folio.circulation.support.http.client.PageLimit; +import org.folio.circulation.support.http.client.Response; import org.folio.circulation.support.http.client.ResponseInterpreter; import org.folio.circulation.support.results.Result; -public class ActualCostRecordRepository { +public class ActualCostRecordRepository implements GetManyRecordsRepository { private final CollectionResourceClient actualCostRecordStorageClient; private final ActualCostRecordMapper actualCostRecordMapper = new ActualCostRecordMapper(); + private static final String ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME = "actualCostRecords"; + private static final String LOAN_ID_FIELD_NAME = "loanId"; public ActualCostRecordRepository(Clients clients) { actualCostRecordStorageClient = clients.actualCostRecordsStorage(); @@ -32,4 +48,28 @@ public CompletableFuture> createActualCostRecord( return actualCostRecordStorageClient.post(actualCostRecordMapper.toJson(actualCostRecord)) .thenApply(interpreter::flatMap); } + + public CompletableFuture> findByLoan(Result loanResult) { + return loanResult.after(loan -> createActualCostRecordFinder().findByQuery( + exactMatch(LOAN_ID_FIELD_NAME, loan.getId()), one()) + .thenApply(records -> records.map(MultipleRecords::firstOrNull)) + .thenApply(mapResult(actualCostRecordMapper::toDomain)) + .thenApply(mapResult(loan::withActualCostRecord))); + } + + private CqlQueryFinder createActualCostRecordFinder() { + return new CqlQueryFinder<>(actualCostRecordStorageClient, ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME, + identity()); + } + + public CompletableFuture>> getMany(CqlQuery cqlQuery, + PageLimit pageLimit, Offset offset) { + return actualCostRecordStorageClient.getMany(cqlQuery, pageLimit, offset) + .thenApply(flatMapResult(this::mapResponseToActualCostRecords)); + } + + private Result> mapResponseToActualCostRecords(Response response) { + return MultipleRecords.from(response, actualCostRecordMapper::toDomain, ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME); + } + } diff --git a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java new file mode 100644 index 0000000000..ead899f599 --- /dev/null +++ b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java @@ -0,0 +1,52 @@ +package org.folio.circulation.resources; + +import org.folio.circulation.infrastructure.storage.ActualCostRecordRepository; +import org.folio.circulation.infrastructure.storage.feesandfines.AccountRepository; +import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; +import org.folio.circulation.infrastructure.storage.loans.LoanRepository; +import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; +import org.folio.circulation.infrastructure.storage.users.UserRepository; +import org.folio.circulation.services.CloseLoanWithLostItemService; +import org.folio.circulation.services.EventPublisher; +import org.folio.circulation.services.actualcostrecord.ActualCostRecordExpirationService; +import org.folio.circulation.support.RouteRegistration; +import org.folio.circulation.support.fetching.PageableFetcher; +import org.folio.circulation.support.http.server.NoContentResponse; +import org.folio.circulation.support.http.server.WebContext; + +import io.vertx.core.http.HttpClient; +import io.vertx.ext.web.Router; +import io.vertx.ext.web.RoutingContext; +import static org.folio.circulation.support.Clients.create; +import static org.folio.circulation.support.results.MappingFunctions.toFixedValue; +public class ExpiredActualCostProcessingResource extends Resource { + public ExpiredActualCostProcessingResource(HttpClient client) { + super(client); + } + + @Override + public void register(Router router) { + new RouteRegistration("/circulation/actual-cost-expiration-by-timeout", router) + .create(this::process); + } + + private void process(RoutingContext routingContext) { + final WebContext context = new WebContext(routingContext); + final var clients = create(context, client); + + final var eventPublisher = new EventPublisher(routingContext); + final var itemRepository = new ItemRepository(clients); + final var userRepository = new UserRepository(clients); + final var loanRepository = new LoanRepository(clients, itemRepository, userRepository); + final var closeLoanWithLostItemService = new CloseLoanWithLostItemService(loanRepository, + itemRepository, new AccountRepository(clients), new LostItemPolicyRepository(clients), + eventPublisher, new ActualCostRecordRepository(clients)); + final var loanPageableFetcher = new PageableFetcher<>(loanRepository); + final var actualCostRecordExpirationService = new ActualCostRecordExpirationService( + loanPageableFetcher, closeLoanWithLostItemService, itemRepository); + + actualCostRecordExpirationService.expireActualCostRecords() + .thenApply(r -> r.map(toFixedValue(NoContentResponse::noContent))) + .thenAccept(context::writeResultToHttpResponse); + } +} diff --git a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java index 9fb68f07b5..dd5b819b8c 100644 --- a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java +++ b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java @@ -1,9 +1,6 @@ package org.folio.circulation.resources.handlers; -import static java.util.concurrent.CompletableFuture.completedFuture; import static org.folio.circulation.domain.EventType.LOAN_RELATED_FEE_FINE_CLOSED; -import static org.folio.circulation.domain.FeeFine.LOST_ITEM_ACTUAL_COST_FEE_TYPE; -import static org.folio.circulation.domain.FeeFine.lostItemFeeTypes; import static org.folio.circulation.domain.subscribers.LoanRelatedFeeFineClosedEvent.fromJson; import static org.folio.circulation.support.Clients.create; import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError; @@ -14,17 +11,15 @@ import java.util.concurrent.CompletableFuture; -import org.folio.circulation.StoreLoanAndItem; -import org.folio.circulation.domain.Account; -import org.folio.circulation.domain.Loan; -import org.folio.circulation.domain.policy.lostitem.LostItemPolicy; import org.folio.circulation.domain.subscribers.LoanRelatedFeeFineClosedEvent; +import org.folio.circulation.infrastructure.storage.ActualCostRecordRepository; import org.folio.circulation.infrastructure.storage.feesandfines.AccountRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; import org.folio.circulation.infrastructure.storage.users.UserRepository; import org.folio.circulation.resources.Resource; +import org.folio.circulation.services.CloseLoanWithLostItemService; import org.folio.circulation.services.EventPublisher; import org.folio.circulation.support.Clients; import org.folio.circulation.support.RouteRegistration; @@ -56,13 +51,19 @@ public void register(Router router) { private void handleFeeFineClosedEvent(RoutingContext routingContext) { final WebContext context = new WebContext(routingContext); - final EventPublisher eventPublisher = new EventPublisher(routingContext); + final var eventPublisher = new EventPublisher(routingContext); + final Clients clients = create(context, client); + final var itemRepository = new ItemRepository(clients); + final var userRepository = new UserRepository(clients); + final var loanRepository = new LoanRepository(clients, itemRepository, userRepository); + final var closeLoanWithLostItemService = new CloseLoanWithLostItemService(loanRepository, + itemRepository, new AccountRepository(clients), new LostItemPolicyRepository(clients), + eventPublisher, new ActualCostRecordRepository(clients)); log.info("Event {} received: {}", LOAN_RELATED_FEE_FINE_CLOSED, routingContext.getBodyAsString()); createAndValidateRequest(routingContext) - .after(request -> processEvent(context, request, eventPublisher)) - .thenCompose(r -> r.after(eventPublisher::publishClosedLoanEvent)) + .after(request -> processEvent(loanRepository, request, closeLoanWithLostItemService)) .exceptionally(CommonFailures::failedDueToServerError) .thenApply(r -> r.map(toFixedValue(NoContentResponse::noContent))) .thenAccept(result -> result.applySideEffect(context::write, failure -> { @@ -73,79 +74,11 @@ private void handleFeeFineClosedEvent(RoutingContext routingContext) { })); } - private CompletableFuture> processEvent(WebContext context, - LoanRelatedFeeFineClosedEvent event, EventPublisher eventPublisher) { - - final Clients clients = create(context, client); - final var itemRepository = new ItemRepository(clients); - final var userRepository = new UserRepository(clients); - final var loanRepository = new LoanRepository(clients, itemRepository, userRepository); - final var accountRepository = new AccountRepository(clients); - final var lostItemPolicyRepository = new LostItemPolicyRepository(clients); + private CompletableFuture> processEvent(LoanRepository loanRepository, + LoanRelatedFeeFineClosedEvent event, CloseLoanWithLostItemService closeLoanWithLostItemService) { return loanRepository.getById(event.getLoanId()) - .thenCompose(r -> r.after(loan -> { - if (loan.isItemLost()) { - return closeLoanWithLostItemIfLostFeesResolved(loan, - loanRepository, itemRepository, accountRepository, - lostItemPolicyRepository, eventPublisher); - } - - return completedFuture(succeeded(loan)); - })); - } - - private CompletableFuture> closeLoanWithLostItemIfLostFeesResolved( - Loan loan, LoanRepository loanRepository, - ItemRepository itemRepository, AccountRepository accountRepository, - LostItemPolicyRepository lostItemPolicyRepository, EventPublisher eventPublisher) { - - return accountRepository.findAccountsForLoan(loan) - .thenComposeAsync(lostItemPolicyRepository::findLostItemPolicyForLoan) - .thenCompose(r -> r.after(l -> closeLoanAndUpdateItem(l, loanRepository, - itemRepository, eventPublisher))); - } - - public CompletableFuture> closeLoanAndUpdateItem(Loan loan, - LoanRepository loanRepository, ItemRepository itemRepository, EventPublisher eventPublisher) { - - if (!allLostFeesClosed(loan) && !shouldCloseLoan(loan)) { - return completedFuture(succeeded(loan)); - } - - boolean wasLoanOpen = loan.isOpen(); - loan.closeLoanAsLostAndPaid(); - - return new StoreLoanAndItem(loanRepository, itemRepository).updateLoanAndItemInStorage(loan) - .thenCompose(r -> r.after(l -> publishLoanClosedEvent(l, wasLoanOpen, eventPublisher))); - } - - private CompletableFuture> publishLoanClosedEvent(Loan loan, boolean wasLoanOpen, - EventPublisher eventPublisher) { - - return wasLoanOpen && loan.isClosed() - ? eventPublisher.publishLoanClosedEvent(loan) - : completedFuture(succeeded(loan)); - } - - private boolean allLostFeesClosed(Loan loan) { - return loan.getAccounts().stream() - .filter(account -> lostItemFeeTypes().contains(account.getFeeFineType())) - .allMatch(Account::isClosed); - } - - private boolean shouldCloseLoan(Loan loan) { - LostItemPolicy lostItemPolicy = loan.getLostItemPolicy(); - - if (!lostItemPolicy.hasActualCostFee()) { - return true; - } - - if (loan.getAccounts().stream().noneMatch(account -> LOST_ITEM_ACTUAL_COST_FEE_TYPE.equals(account.getFeeFineType()))) { - return lostItemPolicy.feeFineChargingPeriodHasPassed(); - } else { - return true; - } + .thenCompose(r -> r.after(closeLoanWithLostItemService::tryCloseLoan)); } private Result createAndValidateRequest(RoutingContext context) { diff --git a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java new file mode 100644 index 0000000000..dd9ff946f4 --- /dev/null +++ b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java @@ -0,0 +1,112 @@ +package org.folio.circulation.services; + +import java.util.concurrent.CompletableFuture; + +import org.folio.circulation.StoreLoanAndItem; +import org.folio.circulation.domain.Account; +import org.folio.circulation.domain.ActualCostRecord; +import org.folio.circulation.domain.Loan; +import org.folio.circulation.infrastructure.storage.ActualCostRecordRepository; +import org.folio.circulation.infrastructure.storage.feesandfines.AccountRepository; +import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; +import org.folio.circulation.infrastructure.storage.loans.LoanRepository; +import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; +import org.folio.circulation.support.results.Result; +import org.folio.circulation.support.utils.DateFormatUtil; + +import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.folio.circulation.domain.FeeFine.LOST_ITEM_ACTUAL_COST_FEE_TYPE; +import static org.folio.circulation.domain.FeeFine.lostItemFeeTypes; +import static org.folio.circulation.support.results.Result.*; +import static org.folio.circulation.support.utils.ClockUtil.getZoneId; +import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime; +public class CloseLoanWithLostItemService { + private final LoanRepository loanRepository; + + private final LostItemPolicyRepository lostItemPolicyRepository; + + private final ItemRepository itemRepository; + + private final AccountRepository accountRepository; + + private final EventPublisher eventPublisher; + + private final ActualCostRecordRepository actualCostRecordRepository; + + public CloseLoanWithLostItemService(LoanRepository loanRepository, ItemRepository itemRepository, + AccountRepository accountRepository, LostItemPolicyRepository lostItemPolicyRepository, + EventPublisher eventPublisher, ActualCostRecordRepository actualCostRecordRepository) { + this.loanRepository = loanRepository; + this.itemRepository = itemRepository; + this.accountRepository = accountRepository; + this.lostItemPolicyRepository = lostItemPolicyRepository; + this.eventPublisher = eventPublisher; + this.actualCostRecordRepository = actualCostRecordRepository; + } + + public CompletableFuture> tryCloseLoan(Loan loan) { + if (loan == null || !loan.isItemLost()) { + return completedFuture(Result.succeeded(null)); + } + + return closeLoanWithLostItemIfLostFeesResolved(loan); + } + + private CompletableFuture> closeLoanWithLostItemIfLostFeesResolved(Loan loan) { + return accountRepository.findAccountsForLoan(loan) + .thenComposeAsync(lostItemPolicyRepository::findLostItemPolicyForLoan) + .thenComposeAsync(actualCostRecordRepository::findByLoan) + .thenCompose(r -> r.after(l -> closeLoanAndUpdateItem(l, loanRepository, + itemRepository, eventPublisher))) + .thenCompose(r -> r.after(eventPublisher::publishClosedLoanEvent)); + } + + public CompletableFuture> closeLoanAndUpdateItem(Loan loan, + LoanRepository loanRepository, ItemRepository itemRepository, EventPublisher eventPublisher) { + + if (!shouldCloseLoan(loan)) { + return completedFuture(succeeded(loan)); + } + + boolean wasLoanOpen = loan.isOpen(); + loan.closeLoanAsLostAndPaid(); + + return new StoreLoanAndItem(loanRepository, itemRepository).updateLoanAndItemInStorage(loan) + .thenCompose(r -> r.after(l -> publishLoanClosedEvent(l, wasLoanOpen, eventPublisher))); + } + + private CompletableFuture> publishLoanClosedEvent(Loan loan, boolean wasLoanOpen, + EventPublisher eventPublisher) { + + return wasLoanOpen && loan.isClosed() + ? eventPublisher.publishLoanClosedEvent(loan) + : completedFuture(succeeded(loan)); + } + + private boolean allLostFeesClosed(Loan loan) { + return loan.getAccounts().stream() + .filter(account -> lostItemFeeTypes().contains(account.getFeeFineType())) + .allMatch(Account::isClosed); + } + + private boolean shouldCloseLoan(Loan loan) { + if (allLostFeesClosed(loan)) { + ActualCostRecord actualCostRecord = loan.getActualCostRecord(); + if(actualCostRecord == null) { + return true; + } + if (loan.getAccounts().stream().noneMatch(account -> + LOST_ITEM_ACTUAL_COST_FEE_TYPE.equals(account.getFeeFineType()))) { + + String expirationDate = actualCostRecord.getExpirationDate(); + + return expirationDate != null && getZonedDateTime().isAfter(DateFormatUtil.parseDateTime( + expirationDate, getZoneId())); + } else { + return true; + } + } + + return false; + } +} diff --git a/src/main/java/org/folio/circulation/services/LostItemFeeChargingService.java b/src/main/java/org/folio/circulation/services/LostItemFeeChargingService.java index 3938e5939d..af08f05951 100644 --- a/src/main/java/org/folio/circulation/services/LostItemFeeChargingService.java +++ b/src/main/java/org/folio/circulation/services/LostItemFeeChargingService.java @@ -34,6 +34,7 @@ import org.folio.circulation.infrastructure.storage.feesandfines.FeeFineRepository; import org.folio.circulation.infrastructure.storage.inventory.LocationRepository; import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; +import org.folio.circulation.services.actualcostrecord.ActualCostRecordService; import org.folio.circulation.services.support.CreateAccountCommand; import org.folio.circulation.support.Clients; import org.folio.circulation.support.results.Result; diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java new file mode 100644 index 0000000000..45834a1b1c --- /dev/null +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -0,0 +1,58 @@ +package org.folio.circulation.services.actualcostrecord; + +import java.util.concurrent.CompletableFuture; + +import org.folio.circulation.domain.Loan; +import org.folio.circulation.domain.MultipleRecords; +import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; +import org.folio.circulation.services.CloseLoanWithLostItemService; +import org.folio.circulation.support.fetching.PageableFetcher; +import org.folio.circulation.support.http.client.CqlQuery; +import org.folio.circulation.support.results.Result; + +import static org.folio.circulation.domain.ItemStatus.DECLARED_LOST; +import static org.folio.circulation.domain.representations.LoanProperties.ITEM_STATUS; +import static org.folio.circulation.support.AsyncCoordinationUtil.allOf; +import static org.folio.circulation.support.results.AsynchronousResult.fromFutureResult; +import static org.folio.circulation.support.results.Result.ofAsync; +import static org.folio.circulation.support.results.Result.succeeded; + +public class ActualCostRecordExpirationService { + private final PageableFetcher loanPageableFetcher; + private final CloseLoanWithLostItemService closeLoanWithLostItemService; + private final ItemRepository itemRepository; + + public ActualCostRecordExpirationService(PageableFetcher loanPageableFetcher, + CloseLoanWithLostItemService closeLoanWithLostItemService, ItemRepository itemRepository) { + this.itemRepository = itemRepository; + this.loanPageableFetcher = loanPageableFetcher; + this.closeLoanWithLostItemService = closeLoanWithLostItemService; + } + + public CompletableFuture> expireActualCostRecords() { + return loanFetchQuery() + .after(query -> loanPageableFetcher.processPages(query, this::closeLoans)); + } + + private Result loanFetchQuery() { + return CqlQuery.exactMatch(ITEM_STATUS, DECLARED_LOST.getValue()); + } + + private CompletableFuture> closeLoans(MultipleRecords expiredLoans) { + if (expiredLoans.isEmpty()) { + return ofAsync(() -> null); + } + + return fromFutureResult(itemRepository.fetchItemsFor(succeeded(expiredLoans), Loan::withItem) + .thenApply(r -> r.next(this::excludeLoansWithNonexistentItems))) + .flatMapFuture(loans -> allOf(loans.getRecords(), closeLoanWithLostItemService::tryCloseLoan)) + .toCompletableFuture() + .thenApply(r -> r.map(ignored -> null)); + } + + private Result> excludeLoansWithNonexistentItems( + MultipleRecords loans) { + + return succeeded(loans.filter(loan -> loan.getItem().isFound())); + } +} diff --git a/src/main/java/org/folio/circulation/services/ActualCostRecordService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordService.java similarity index 95% rename from src/main/java/org/folio/circulation/services/ActualCostRecordService.java rename to src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordService.java index 3f126d267b..a24670d1e6 100644 --- a/src/main/java/org/folio/circulation/services/ActualCostRecordService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordService.java @@ -1,4 +1,4 @@ -package org.folio.circulation.services; +package org.folio.circulation.services.actualcostrecord; import java.time.ZonedDateTime; import java.util.Map; @@ -99,6 +99,8 @@ private ActualCostRecord buildActualCostRecord(Loan loan, FeeFineOwner feeFineOw .withFeeFineOwnerId(feeFineOwner.getId()) .withFeeFineOwner(feeFineOwner.getOwner()) .withFeeFineTypeId(feeFine == null ? null : feeFine.getId()) - .withFeeFineType(feeFine == null ? null : feeFine.getFeeFineType()); + .withFeeFineType(feeFine == null ? null : feeFine.getFeeFineType()) + .withExpirationDate(loan.getLostItemPolicy() + .calculateFeeFineChargingPeriodExpirationDateTime(dateOfLoss).toString()); } } 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 e490bc4c08..ecfe9b3cf3 100644 --- a/src/main/java/org/folio/circulation/services/agedtolost/ChargeLostFeesWhenAgedToLostService.java +++ b/src/main/java/org/folio/circulation/services/agedtolost/ChargeLostFeesWhenAgedToLostService.java @@ -51,7 +51,7 @@ import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; import org.folio.circulation.infrastructure.storage.users.UserRepository; -import org.folio.circulation.services.ActualCostRecordService; +import org.folio.circulation.services.actualcostrecord.ActualCostRecordService; import org.folio.circulation.services.EventPublisher; import org.folio.circulation.services.FeeFineFacade; import org.folio.circulation.services.support.CreateAccountCommand; diff --git a/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java b/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java index 489fa296ee..1f5ea575df 100644 --- a/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java +++ b/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java @@ -31,6 +31,7 @@ public JsonObject toJson(ActualCostRecord actualCostRecord) { write(json, "feeFineTypeId", actualCostRecord.getFeeFineTypeId()); write(json, "feeFineType", actualCostRecord.getFeeFineType()); write(json,"permanentItemLocation", actualCostRecord.getPermanentItemLocation()); + write(json, "expirationDate", actualCostRecord.getExpirationDate()); if (actualCostRecord.getAccountId() != null) { write(json, "accountId", actualCostRecord.getAccountId()); @@ -40,6 +41,10 @@ public JsonObject toJson(ActualCostRecord actualCostRecord) { } public ActualCostRecord toDomain(JsonObject representation) { + if (representation == null ) { + return null; + } + return new ActualCostRecord(getProperty(representation, "id"), getProperty(representation, "userId"), getProperty(representation, "accountId"), @@ -56,7 +61,8 @@ public ActualCostRecord toDomain(JsonObject representation) { getProperty(representation, "feeFineOwnerId"), getProperty(representation, "feeFineOwner"), getProperty(representation, "feeFineTypeId"), - getProperty(representation, "feeFineType") + getProperty(representation, "feeFineType"), + getProperty(representation, "expirationDate") ); } } diff --git a/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java b/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java index e8f87d6e0a..14cff63686 100644 --- a/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java +++ b/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java @@ -42,6 +42,10 @@ public static Result hasValue(String index) { return of(() -> new CqlQuery(String.format("%s=\"\"", index), none())); } + public static Result valueUndefined(String index) { + return of(() -> new CqlQuery(format("cql.allRecords=1 NOT %s=\"\" ", index), none())); + } + public static Result match(String index, String value) { return Result.of(() -> new CqlQuery(format("%s=\"%s\"", index, value), none())); } diff --git a/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java b/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java index dcd0806654..13f0f3607a 100644 --- a/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java +++ b/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java @@ -1,7 +1,14 @@ package api.handlers; +import static api.support.APITestContext.getOkapiHeadersFromContext; +import api.support.builders.AccountBuilder; +import api.support.builders.FeefineActionsBuilder; +import api.support.builders.LostItemFeePolicyBuilder; import static api.support.fakes.FakePubSub.getPublishedEventsAsList; import static api.support.fakes.PublishedEvents.byEventType; +import static api.support.http.InterfaceUrls.scheduledActualCostExpiration; +import static api.support.http.InterfaceUrls.scheduledAgeToLostFeeChargingUrl; +import api.support.http.TimedTaskClient; import static api.support.matchers.EventMatchers.isValidLoanClosedEvent; import static api.support.matchers.ItemMatchers.isAvailable; import static api.support.matchers.ItemMatchers.isCheckedOut; @@ -10,7 +17,12 @@ import static api.support.matchers.JsonObjectMatcher.hasJsonPath; import static api.support.matchers.LoanMatchers.isClosed; import static api.support.matchers.LoanMatchers.isOpen; +import static java.time.Clock.fixed; +import static java.time.ZoneOffset.UTC; import static org.folio.circulation.domain.EventType.LOAN_CLOSED; +import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime; +import static org.folio.circulation.support.utils.ClockUtil.setClock; +import static org.folio.circulation.support.utils.ClockUtil.setDefaultClock; import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; @@ -20,7 +32,9 @@ import java.util.List; import java.util.UUID; +import org.folio.circulation.domain.policy.Period; import org.folio.circulation.support.http.client.Response; +import org.folio.circulation.support.utils.ClockUtil; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -33,8 +47,11 @@ class CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests extends APITests { private IndividualResource loan; private IndividualResource item; + private final TimedTaskClient timedTaskClient = new TimedTaskClient(getOkapiHeadersFromContext()); + @BeforeEach public void createLoanAndDeclareItemLost() { + mockClockManagerToReturnDefaultDateTime(); UUID servicePointId = servicePointsFixture.cd1().getId(); useLostItemPolicy(lostItemFeePoliciesFixture.chargeFee().getId()); @@ -97,17 +114,175 @@ void shouldNotCloseLoanIfSetCostFeeIsNotClosed() { assertThat(itemsClient.getById(item.getId()).getJson(), isDeclaredLost()); } + @Test + void shouldCloseLoanIfActualCostFeeHasBeenPaidWithoutProcessingFee() { + UUID servicePointId = servicePointsFixture.cd2().getId(); + UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( + new LostItemFeePolicyBuilder().withName("test") + .doNotChargeProcessingFeeWhenDeclaredLost() + .withActualCost(10.0) + .withLostItemChargeFeeFine(Period.weeks(2))).getId(); + useLostItemPolicy(actualCostLostItemFeePolicyId); + + item = itemsFixture.basedUponNod(); + + loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); + + declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() + .withServicePointId(servicePointId) + .forLoanId(loan.getId())); + + createLostItemFeeActualCostAccount(10.0); + + feeFineAccountFixture.payLostItemActualCostFee(loan.getId()); + + eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); + + assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); + assertThat(itemsClient.getById(item.getId()).getJson(), isLostAndPaid()); + } + + @Test + void shouldCloseLoanIfActualCostFeeAndProcessingFeeHaveBeenPaid() { + UUID servicePointId = servicePointsFixture.cd2().getId(); + UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( + new LostItemFeePolicyBuilder().withName("test") + .chargeProcessingFeeWhenDeclaredLost(10.00) + .withActualCost(10.0) + .withLostItemChargeFeeFine(Period.weeks(2))).getId(); + useLostItemPolicy(actualCostLostItemFeePolicyId); + + item = itemsFixture.basedUponNod(); + + loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); + + declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() + .withServicePointId(servicePointId) + .forLoanId(loan.getId())); + createLostItemFeeActualCostAccount(10.0); + + feeFineAccountFixture.payLostItemActualCostFee(loan.getId()); + + feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); + + eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); + + assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); + assertThat(itemsClient.getById(item.getId()).getJson(), isLostAndPaid()); + } + @Test void shouldNotCloseLoanIfActualCostFeeShouldBeCharged() { - useLostItemPolicy(lostItemFeePoliciesFixture.create( - lostItemFeePoliciesFixture.facultyStandardPolicy() + UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( + new LostItemFeePolicyBuilder().withName("test") + .chargeProcessingFeeWhenDeclaredLost(10.00) + .withActualCost(10.0) + .withLostItemChargeFeeFine(Period.weeks(2))).getId(); + useLostItemPolicy(actualCostLostItemFeePolicyId); + + loansFixture.replaceLoan(loan.getId(), loan.getJson().put("lostItemPolicyId", + actualCostLostItemFeePolicyId)); + + feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); + + eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); + + assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isOpen()); + assertThat(itemsClient.getById(item.getId()).getJson(), isDeclaredLost()); + } + + @Test + void shouldCloseLoanIfActualCostFeeShouldBeChargedButChargingPeriodElapsedAndProcessingFeeHasBeenPaid() { + UUID servicePointId = servicePointsFixture.cd2().getId(); + + UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( + new LostItemFeePolicyBuilder().withName("test") + .chargeProcessingFeeWhenDeclaredLost(10.00) + .withActualCost(10.0) + .withLostItemChargeFeeFine(Period.weeks(2))).getId(); + useLostItemPolicy(actualCostLostItemFeePolicyId); + + item = itemsFixture.basedUponNod(); + + loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); + + declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() + .withServicePointId(servicePointId) + .forLoanId(loan.getId())); + + loansFixture.replaceLoan(loan.getId(), loan.getJson().put("lostItemPolicyId", + actualCostLostItemFeePolicyId)); + + mockClockManagerToReturnFixedDateTime(ClockUtil.getZonedDateTime().plusWeeks(3)); + + feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); + + eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); + + assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); + assertThat(itemsClient.getById(item.getId()).getJson(), isLostAndPaid()); + } + + @Test + void shouldCloseLoanDuringScheduledExpirationIfChargingPeriodElapsedAndProcessingFeeHasBeenPaid() { + UUID servicePointId = servicePointsFixture.cd2().getId(); + + UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( + new LostItemFeePolicyBuilder().withName("test") + .chargeProcessingFeeWhenDeclaredLost(10.00) + .withActualCost(10.0) + .withLostItemChargeFeeFine(Period.weeks(2))).getId(); + useLostItemPolicy(actualCostLostItemFeePolicyId); + + item = itemsFixture.basedUponNod(); + + loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); + + declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() + .withServicePointId(servicePointId) + .forLoanId(loan.getId())); + + feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); + + eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); + + mockClockManagerToReturnFixedDateTime(ClockUtil.getZonedDateTime().plusWeeks(3)); + + timedTaskClient.start(scheduledActualCostExpiration(), 204, + "scheduled-actual-cost-expiration"); + + assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); + assertThat(itemsClient.getById(item.getId()).getJson(), isLostAndPaid()); + } + + @Test + void shouldNotCloseLoanDuringScheduledExpirationIfChargingPeriodHasNotElapsedAndProcessingFeeHasBeenPaid() { + UUID servicePointId = servicePointsFixture.cd2().getId(); + + UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( + new LostItemFeePolicyBuilder().withName("test") .chargeProcessingFeeWhenDeclaredLost(10.00) - .withActualCost(10.0)).getId()); + .withActualCost(10.0) + .withLostItemChargeFeeFine(Period.weeks(2))).getId(); + useLostItemPolicy(actualCostLostItemFeePolicyId); + + item = itemsFixture.basedUponNod(); + + loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); + + declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() + .withServicePointId(servicePointId) + .forLoanId(loan.getId())); feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); + mockClockManagerToReturnFixedDateTime(ClockUtil.getZonedDateTime().plusWeeks(1)); + + timedTaskClient.start(scheduledActualCostExpiration(), 204, + "scheduled-actual-cost-expiration"); + assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isOpen()); assertThat(itemsClient.getById(item.getId()).getJson(), isDeclaredLost()); } @@ -177,4 +352,31 @@ void shouldNotPublishLoanClosedEventWhenLoanIsOriginallyClosed() { assertThat(getPublishedEventsAsList(byEventType(LOAN_CLOSED)), empty()); } + + protected void createLostItemFeeActualCostAccount(double amount) { + IndividualResource account = accountsClient.create(new AccountBuilder() + .withLoan(loan) + .withAmount(amount) + .withRemainingFeeFine(amount) + .feeFineStatusOpen() + .withFeeFineActualCostType() + .withFeeFine(feeFineTypeFixture.lostItemActualCostFee()) + .withOwner(feeFineOwnerFixture.cd1Owner()) + .withPaymentStatus("Outstanding")); + + feeFineActionsClient.create(new FeefineActionsBuilder() + .forAccount(account.getId()) + .withBalance(amount) + .withActionAmount(amount) + .withActionType("Lost item fee (actual cost)")); + } + + public void expireActualCost() { + setClock(fixed(getZonedDateTime().plusWeeks(3).toInstant(), UTC)); + + timedTaskClient.start(scheduledActualCostExpiration(), 204, + "scheduled-actual-cost-expiration"); + + setDefaultClock(); + } } diff --git a/src/test/java/api/support/builders/AccountBuilder.java b/src/test/java/api/support/builders/AccountBuilder.java index bbb974ca2a..c9c1da4a05 100644 --- a/src/test/java/api/support/builders/AccountBuilder.java +++ b/src/test/java/api/support/builders/AccountBuilder.java @@ -1,28 +1,26 @@ package api.support.builders; - - import static org.folio.circulation.support.json.JsonPropertyWriter.write; - import java.util.UUID; import api.support.http.IndividualResource; - import io.vertx.core.json.JsonObject; public class AccountBuilder extends JsonBuilder implements Builder { - private String id; private String loanId; private Double remainingAmount; private Double amount; private String status; private String feeFineType; + private String feeFineId; + private String ownerId; + private String paymentStatus; public AccountBuilder() { } AccountBuilder(String loanId, Double amount, Double remainingAmount, - String status, String feeFineType) { + String status, String feeFineType, String feeFineId, String ownerId, String paymentStatus) { this.loanId = loanId; this.amount = amount; @@ -30,47 +28,80 @@ public AccountBuilder() { this.status = status; this.id = UUID.randomUUID().toString(); this.feeFineType = feeFineType; + this.feeFineId = feeFineId; + this.ownerId = ownerId; + this.paymentStatus = paymentStatus; } @Override public JsonObject create() { JsonObject accountRequest = new JsonObject(); - write(accountRequest, "id", id); write(accountRequest, "loanId", loanId); write(accountRequest, "amount", amount); write(accountRequest, "remaining", remainingAmount); write(accountRequest, "feeFineType", feeFineType); + write(accountRequest, "feeFineId", feeFineId); + write(accountRequest, "ownerId", ownerId); JsonObject statusObject = new JsonObject(); write(statusObject, "name", status); write(accountRequest, "status", statusObject); + JsonObject paymentStatusObject = new JsonObject(); + write(paymentStatusObject, "name", paymentStatus); + write(accountRequest, "paymentStatus", paymentStatusObject); + return accountRequest; } public AccountBuilder withLoan(IndividualResource loan) { return new AccountBuilder(loan.getId().toString(), amount, remainingAmount, - status, feeFineType); + status, feeFineType, feeFineId, ownerId, paymentStatus); } public AccountBuilder withRemainingFeeFine(double remaining) { - return new AccountBuilder(loanId, amount, remaining, status, feeFineType); + return new AccountBuilder(loanId, amount, remaining, status, feeFineType, + feeFineId, ownerId, paymentStatus); } public AccountBuilder withAmount(double amount) { - return new AccountBuilder(loanId, amount, remainingAmount, status, feeFineType); + return new AccountBuilder(loanId, amount, remainingAmount, status, feeFineType, + feeFineId, ownerId, paymentStatus); } public AccountBuilder feeFineStatusOpen() { - return new AccountBuilder(loanId, amount, remainingAmount, "Open", feeFineType); + return new AccountBuilder(loanId, amount, remainingAmount, "Open", feeFineType, + feeFineId, ownerId, paymentStatus); } public AccountBuilder feeFineStatusClosed() { - return new AccountBuilder(loanId, amount, remainingAmount, "Closed", feeFineType); + return new AccountBuilder(loanId, amount, remainingAmount, "Closed", feeFineType, + feeFineId, ownerId, paymentStatus); } public AccountBuilder manualFeeFine() { - return new AccountBuilder(loanId, amount, remainingAmount, status, "Manual fee fine"); + return new AccountBuilder(loanId, amount, remainingAmount, status, "Manual fee fine", + feeFineId, ownerId, paymentStatus); + } + + public AccountBuilder withFeeFineActualCostType() { + return new AccountBuilder(loanId, amount, remainingAmount, status, + "Lost item fee (actual cost)", feeFineId, ownerId, paymentStatus); + } + + public AccountBuilder withFeeFine(IndividualResource feeFine) { + return new AccountBuilder(loanId, amount, remainingAmount, status, feeFineType, + feeFine.getId().toString(), ownerId, paymentStatus); + } + + public AccountBuilder withOwner(IndividualResource owner) { + return new AccountBuilder(loanId, amount, remainingAmount, status, feeFineType, feeFineId, + owner.getId().toString(), paymentStatus); + } + + public AccountBuilder withPaymentStatus(String paymentStatus) { + return new AccountBuilder(loanId, amount, remainingAmount, status, feeFineType, feeFineId, + ownerId, paymentStatus); } -} +} \ No newline at end of file diff --git a/src/test/java/api/support/builders/LostItemFeePolicyBuilder.java b/src/test/java/api/support/builders/LostItemFeePolicyBuilder.java index a91787994d..64bcb5d077 100644 --- a/src/test/java/api/support/builders/LostItemFeePolicyBuilder.java +++ b/src/test/java/api/support/builders/LostItemFeePolicyBuilder.java @@ -24,7 +24,7 @@ public class LostItemFeePolicyBuilder extends JsonBuilder implements Builder { private final Double lostItemProcessingFee; private final boolean chargeAmountItemPatron; private final boolean chargeAmountItemSystem; - private final JsonObject lostItemChargeFeeFine; + private final Period lostItemChargeFeeFine; private final boolean returnedLostItemProcessingFee; private final boolean replacedLostItemProcessingFee; private final double replacementProcessingFee; @@ -44,7 +44,7 @@ public LostItemFeePolicyBuilder() { null, false, false, - new JsonObject(), + null, false, false, 0.0, @@ -156,13 +156,16 @@ public JsonObject create() { request.put("feesFinesShallRefunded", this.feeRefundInterval.asJson()); } + if (lostItemChargeFeeFine != null) { + request.put("lostItemChargeFeeFine", this.lostItemChargeFeeFine.asJson()); + } + put(request, "name", this.name); put(request, "description", this.description); put(request, "chargeAmountItem", this.chargeAmountItem); put(request, "lostItemProcessingFee", this.lostItemProcessingFee); put(request, "chargeAmountItemPatron", this.chargeAmountItemPatron); put(request, "chargeAmountItemSystem", this.chargeAmountItemSystem); - put(request, "lostItemChargeFeeFine", this.lostItemChargeFeeFine); put(request, "returnedLostItemProcessingFee", this.returnedLostItemProcessingFee); put(request, "replacedLostItemProcessingFee", this.replacedLostItemProcessingFee); put(request, "replacementProcessingFee", String.valueOf(this.replacementProcessingFee)); diff --git a/src/test/java/api/support/fixtures/ExpireActualCostFixture.java b/src/test/java/api/support/fixtures/ExpireActualCostFixture.java new file mode 100644 index 0000000000..b9ed022e20 --- /dev/null +++ b/src/test/java/api/support/fixtures/ExpireActualCostFixture.java @@ -0,0 +1,14 @@ +/* +package api.support.fixtures; + +import api.support.http.TimedTaskClient; +public final class ExpireActualCostFixture { + private final TimedTaskClient timedTaskClient; + + public ExpireActualCostFixture(TimedTaskClient timedTaskClient) { + this.timedTaskClient = timedTaskClient; + } + + public void +} +*/ diff --git a/src/test/java/api/support/fixtures/FeeFineAccountFixture.java b/src/test/java/api/support/fixtures/FeeFineAccountFixture.java index 6c66d7f4b7..b0327ca839 100644 --- a/src/test/java/api/support/fixtures/FeeFineAccountFixture.java +++ b/src/test/java/api/support/fixtures/FeeFineAccountFixture.java @@ -12,6 +12,9 @@ import api.support.builders.FeefineActionsBuilder; import api.support.http.ResourceClient; import io.vertx.core.json.JsonObject; +import static org.folio.circulation.domain.FeeFine.LOST_ITEM_ACTUAL_COST_FEE_TYPE; +import static org.folio.circulation.domain.FeeFine.LOST_ITEM_FEE_TYPE; +import static org.folio.circulation.domain.FeeFine.LOST_ITEM_PROCESSING_FEE_TYPE; public final class FeeFineAccountFixture { private final ResourceClient accountsClient = forAccounts(); @@ -59,20 +62,30 @@ public void payLostItemFee(UUID loanId) { } public void payLostItemFee(UUID loanId, double amount) { - final String accountId = getLostItemFeeAccount(loanId).getString("id"); + final String accountId = getAccount(loanId, LOST_ITEM_FEE_TYPE) + .getString("id"); pay(accountId, amount); } + public void payLostItemActualCostFee(UUID loanId) { + final JsonObject lostItemFeeActualCostAccount = getAccount(loanId, LOST_ITEM_ACTUAL_COST_FEE_TYPE); + final String accountId = lostItemFeeActualCostAccount.getString("id"); + + pay(accountId, lostItemFeeActualCostAccount.getDouble("amount")); + } + public void payLostItemProcessingFee(UUID loanId) { - final JsonObject lostItemProcessingFeeAccount = getLostItemProcessingFeeAccount(loanId); - final String accountId = lostItemProcessingFeeAccount.getString("id"); + final JsonObject lostItemProcessingFeeAccount = getAccount( + loanId, LOST_ITEM_PROCESSING_FEE_TYPE); + final String accountId = lostItemProcessingFeeAccount.getString("id"); pay(accountId, lostItemProcessingFeeAccount.getDouble("amount")); } public void payLostItemProcessingFee(UUID loanId, double amount) { - final String accountId = getLostItemProcessingFeeAccount(loanId).getString("id"); + final String accountId = getAccount( + loanId, LOST_ITEM_PROCESSING_FEE_TYPE).getString("id"); pay(accountId, amount); } @@ -122,9 +135,9 @@ private JsonObject getLostItemFeeAccount(UUID loanId) { .getFirst(); } - private JsonObject getLostItemProcessingFeeAccount(UUID loanId) { + private JsonObject getAccount(UUID loanId, String feeFineType) { return accountsClient.getMany(exactMatch("loanId", loanId.toString()) - .and(exactMatch("feeFineType", "Lost item processing fee"))) + .and(exactMatch("feeFineType", feeFineType))) .getFirst(); } } diff --git a/src/test/java/api/support/fixtures/LostItemFeePoliciesFixture.java b/src/test/java/api/support/fixtures/LostItemFeePoliciesFixture.java index 204860cb55..db6bca4683 100644 --- a/src/test/java/api/support/fixtures/LostItemFeePoliciesFixture.java +++ b/src/test/java/api/support/fixtures/LostItemFeePoliciesFixture.java @@ -2,6 +2,7 @@ import static api.support.http.ResourceClient.forLostItemFeePolicies; import static org.folio.circulation.domain.policy.Period.minutes; +import static org.folio.circulation.domain.policy.lostitem.ChargeAmountType.ACTUAL_COST; import static org.folio.circulation.domain.policy.lostitem.ChargeAmountType.SET_COST; import static org.folio.circulation.support.json.JsonPropertyFetcher.getProperty; @@ -34,19 +35,19 @@ public IndividualResource facultyStandard() { public IndividualResource chargeFee() { createReferenceData(); - return create(chargeFeePolicy(10.0, 5.0)); + return create(chargeSetCostFeePolicy(10.0, 5.0)); } public IndividualResource chargeFeeWithZeroLostItemFee() { createReferenceData(); - return create(chargeFeePolicy(0.0, 5.0)); + return create(chargeSetCostFeePolicy(0.0, 5.0)); } public IndividualResource chargeFeeWithZeroLostItemProcessingFee() { createReferenceData(); - return create(chargeFeePolicy(10.0, 0.0)); + return create(chargeSetCostFeePolicy(10.0, 0.0)); } public IndividualResource ageToLostAfterOneMinute() { @@ -86,12 +87,18 @@ public LostItemFeePolicyBuilder facultyStandardPolicy() { .chargeOverdueFineWhenReturned(); } - private LostItemFeePolicyBuilder chargeFeePolicy(double lostItemFeeCost, + private LostItemFeePolicyBuilder chargeSetCostFeePolicy(double lostItemFeeCost, double lostItemProcessingFeeCost) { return chargeFeePolicy(lostItemFeeCost, lostItemProcessingFeeCost, SET_COST); } + private LostItemFeePolicyBuilder chargeActualCostFeePolicy(double lostItemFeeCost, + double lostItemProcessingFeeCost) { + + return chargeFeePolicy(lostItemFeeCost, lostItemProcessingFeeCost, ACTUAL_COST); + } + private LostItemFeePolicyBuilder chargeFeePolicy(double lostItemFeeCost, double lostItemProcessingFeeCost, ChargeAmountType costType) { diff --git a/src/test/java/api/support/http/InterfaceUrls.java b/src/test/java/api/support/http/InterfaceUrls.java index 11bcce7759..793ee2d787 100644 --- a/src/test/java/api/support/http/InterfaceUrls.java +++ b/src/test/java/api/support/http/InterfaceUrls.java @@ -297,6 +297,10 @@ public static URL scheduledAgeToLostFeeChargingUrl() { return circulationModuleUrl("/circulation/scheduled-age-to-lost-fee-charging"); } + public static URL scheduledActualCostExpiration() { + return circulationModuleUrl("/circulation/actual-cost-expiration-by-timeout"); + } + public static URL actualCostRecordsStorageUrl(String subPath) { return APITestContext.viaOkapiModuleUrl("/actual-cost-record-storage/actual-cost-records" + subPath); } From 03d72da9a6ddb44eb6343603d57cd7abc5368eff Mon Sep 17 00:00:00 2001 From: Mykyta_Varenyk Date: Thu, 30 Jun 2022 16:18:23 +0300 Subject: [PATCH 03/19] Fix conflicts --- .../infrastructure/storage/ActualCostRecordRepository.java | 6 +----- .../circulation/storage/mappers/ActualCostRecordMapper.java | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java index 25d71d58a0..9166f93c1b 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java @@ -72,7 +72,7 @@ public CompletableFuture> findByLoan(Result loanResult) { return loanResult.after(loan -> createActualCostRecordFinder().findByQuery( exactMatch(LOAN_ID_FIELD_NAME, loan.getId()), one()) .thenApply(records -> records.map(MultipleRecords::firstOrNull)) - .thenApply(mapResult(actualCostRecordMapper::toDomain)) + .thenApply(mapResult(ActualCostRecordMapper::toDomain)) .thenApply(mapResult(loan::withActualCostRecord))); } @@ -87,8 +87,4 @@ public CompletableFuture>> getMany(CqlQ .thenApply(flatMapResult(this::mapResponseToActualCostRecords)); } - private Result> mapResponseToActualCostRecords(Response response) { - return MultipleRecords.from(response, actualCostRecordMapper::toDomain, ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME); - } - } diff --git a/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java b/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java index 06b4821b3f..4e8c775c72 100644 --- a/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java +++ b/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java @@ -44,7 +44,7 @@ public static JsonObject toJson(ActualCostRecord actualCostRecord) { return json; } - public ActualCostRecord toDomain(JsonObject representation) { + public static ActualCostRecord toDomain(JsonObject representation) { if (representation == null ) { return null; } @@ -66,7 +66,7 @@ public ActualCostRecord toDomain(JsonObject representation) { getProperty(representation, "feeFineOwner"), getProperty(representation, "feeFineTypeId"), getProperty(representation, "feeFineType"), - getNestedDateTimeProperty(representation, "metadata", "createdDate") + getNestedDateTimeProperty(representation, "metadata", "createdDate"), getProperty(representation, "expirationDate") ); } From 4097f26e97679a1cd066823a14f981c2ec590e02 Mon Sep 17 00:00:00 2001 From: Mykyta_Varenyk Date: Mon, 4 Jul 2022 14:00:55 +0300 Subject: [PATCH 04/19] CIRC-1556 Fix comments --- .../policy/lostitem/LostItemPolicy.java | 7 -- .../storage/ActualCostRecordRepository.java | 11 +-- .../ExpiredActualCostProcessingResource.java | 2 + .../CloseLoanWithLostItemService.java | 2 +- .../ActualCostRecordExpirationService.java | 4 +- .../support/http/client/CqlQuery.java | 4 - ...LoanWhenLostItemFeesAreClosedApiTests.java | 78 ------------------- 7 files changed, 6 insertions(+), 102 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java index c05921aa98..8650dcfe08 100644 --- a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java +++ b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java @@ -143,12 +143,6 @@ public boolean shouldRefundFees(ZonedDateTime lostDateTime) { || feeRefundInterval.isEqualToDateTillNow(lostDateTime); } - public boolean feeFineChargingPeriodHasPassed(ZonedDateTime lostDateTime) { - return lostItemChargeFeeFine.hasZeroDuration() - || lostItemChargeFeeFine.isEqualToDateTillNow(lostDateTime) - || lostItemChargeFeeFine.hasPassedSinceDateTillNow(lostDateTime); - } - public boolean isRefundProcessingFeeWhenReturned() { return refundProcessingFeeWhenReturned; } @@ -176,7 +170,6 @@ public ZonedDateTime calculateFeeFineChargingPeriodExpirationDateTime(ZonedDateT return lostItemChargeFeeFine.plusDate(lostTime); } - public ZonedDateTime calculateDateTimeWhenPatronBilledForAgedToLost( boolean isRecalled, ZonedDateTime ageToLostDate) { diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java index 9166f93c1b..07417df66d 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java @@ -6,7 +6,6 @@ import static org.folio.circulation.support.http.ResponseMapping.mapUsingJson; import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; import static org.folio.circulation.support.http.client.PageLimit.one; -import static org.folio.circulation.support.results.ResultBinding.flatMapResult; import static org.folio.circulation.support.results.ResultBinding.mapResult; import static org.folio.circulation.support.results.Result.ofAsync; @@ -19,15 +18,13 @@ import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; import org.folio.circulation.support.fetching.CqlQueryFinder; -import org.folio.circulation.support.fetching.GetManyRecordsRepository; import org.folio.circulation.support.http.client.CqlQuery; -import org.folio.circulation.support.http.client.Offset; import org.folio.circulation.support.http.client.PageLimit; import org.folio.circulation.support.http.client.Response; import org.folio.circulation.support.http.client.ResponseInterpreter; import org.folio.circulation.support.results.Result; -public class ActualCostRecordRepository implements GetManyRecordsRepository { +public class ActualCostRecordRepository { private final CollectionResourceClient actualCostRecordStorageClient; private static final String ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME = "actualCostRecords"; @@ -81,10 +78,4 @@ private CqlQueryFinder createActualCostRecordFinder() { identity()); } - public CompletableFuture>> getMany(CqlQuery cqlQuery, - PageLimit pageLimit, Offset offset) { - return actualCostRecordStorageClient.getMany(cqlQuery, pageLimit, offset) - .thenApply(flatMapResult(this::mapResponseToActualCostRecords)); - } - } diff --git a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java index ead899f599..7681655c62 100644 --- a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java +++ b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java @@ -49,4 +49,6 @@ itemRepository, new AccountRepository(clients), new LostItemPolicyRepository(cli .thenApply(r -> r.map(toFixedValue(NoContentResponse::noContent))) .thenAccept(context::writeResultToHttpResponse); } + + } diff --git a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java index dd9ff946f4..ab3d002ccd 100644 --- a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java +++ b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java @@ -92,7 +92,7 @@ private boolean allLostFeesClosed(Loan loan) { private boolean shouldCloseLoan(Loan loan) { if (allLostFeesClosed(loan)) { ActualCostRecord actualCostRecord = loan.getActualCostRecord(); - if(actualCostRecord == null) { + if (actualCostRecord == null) { return true; } if (loan.getAccounts().stream().noneMatch(account -> diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index 45834a1b1c..3bd6fe4e83 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -30,11 +30,11 @@ public ActualCostRecordExpirationService(PageableFetcher loanPageableFetch } public CompletableFuture> expireActualCostRecords() { - return loanFetchQuery() + return fetchLoansForLostItemsQuery() .after(query -> loanPageableFetcher.processPages(query, this::closeLoans)); } - private Result loanFetchQuery() { + private Result fetchLoansForLostItemsQuery() { return CqlQuery.exactMatch(ITEM_STATUS, DECLARED_LOST.getValue()); } diff --git a/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java b/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java index 14cff63686..e8f87d6e0a 100644 --- a/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java +++ b/src/main/java/org/folio/circulation/support/http/client/CqlQuery.java @@ -42,10 +42,6 @@ public static Result hasValue(String index) { return of(() -> new CqlQuery(String.format("%s=\"\"", index), none())); } - public static Result valueUndefined(String index) { - return of(() -> new CqlQuery(format("cql.allRecords=1 NOT %s=\"\" ", index), none())); - } - public static Result match(String index, String value) { return Result.of(() -> new CqlQuery(format("%s=\"%s\"", index, value), none())); } diff --git a/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java b/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java index 13f0f3607a..d42f8c68f0 100644 --- a/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java +++ b/src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java @@ -125,17 +125,13 @@ void shouldCloseLoanIfActualCostFeeHasBeenPaidWithoutProcessingFee() { useLostItemPolicy(actualCostLostItemFeePolicyId); item = itemsFixture.basedUponNod(); - loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); - declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() .withServicePointId(servicePointId) .forLoanId(loan.getId())); - createLostItemFeeActualCostAccount(10.0); feeFineAccountFixture.payLostItemActualCostFee(loan.getId()); - eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); @@ -153,44 +149,20 @@ void shouldCloseLoanIfActualCostFeeAndProcessingFeeHaveBeenPaid() { useLostItemPolicy(actualCostLostItemFeePolicyId); item = itemsFixture.basedUponNod(); - loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); - declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() .withServicePointId(servicePointId) .forLoanId(loan.getId())); createLostItemFeeActualCostAccount(10.0); feeFineAccountFixture.payLostItemActualCostFee(loan.getId()); - feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); - eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); assertThat(itemsClient.getById(item.getId()).getJson(), isLostAndPaid()); } - @Test - void shouldNotCloseLoanIfActualCostFeeShouldBeCharged() { - UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( - new LostItemFeePolicyBuilder().withName("test") - .chargeProcessingFeeWhenDeclaredLost(10.00) - .withActualCost(10.0) - .withLostItemChargeFeeFine(Period.weeks(2))).getId(); - useLostItemPolicy(actualCostLostItemFeePolicyId); - - loansFixture.replaceLoan(loan.getId(), loan.getJson().put("lostItemPolicyId", - actualCostLostItemFeePolicyId)); - - feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); - - eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); - - assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isOpen()); - assertThat(itemsClient.getById(item.getId()).getJson(), isDeclaredLost()); - } - @Test void shouldCloseLoanIfActualCostFeeShouldBeChargedButChargingPeriodElapsedAndProcessingFeeHasBeenPaid() { UUID servicePointId = servicePointsFixture.cd2().getId(); @@ -203,58 +175,19 @@ void shouldCloseLoanIfActualCostFeeShouldBeChargedButChargingPeriodElapsedAndPro useLostItemPolicy(actualCostLostItemFeePolicyId); item = itemsFixture.basedUponNod(); - loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); - declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() .withServicePointId(servicePointId) .forLoanId(loan.getId())); - loansFixture.replaceLoan(loan.getId(), loan.getJson().put("lostItemPolicyId", - actualCostLostItemFeePolicyId)); - mockClockManagerToReturnFixedDateTime(ClockUtil.getZonedDateTime().plusWeeks(3)); - feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); - eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); assertThat(itemsClient.getById(item.getId()).getJson(), isLostAndPaid()); } - @Test - void shouldCloseLoanDuringScheduledExpirationIfChargingPeriodElapsedAndProcessingFeeHasBeenPaid() { - UUID servicePointId = servicePointsFixture.cd2().getId(); - - UUID actualCostLostItemFeePolicyId = lostItemFeePoliciesFixture.create( - new LostItemFeePolicyBuilder().withName("test") - .chargeProcessingFeeWhenDeclaredLost(10.00) - .withActualCost(10.0) - .withLostItemChargeFeeFine(Period.weeks(2))).getId(); - useLostItemPolicy(actualCostLostItemFeePolicyId); - - item = itemsFixture.basedUponNod(); - - loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); - - declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() - .withServicePointId(servicePointId) - .forLoanId(loan.getId())); - - feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); - - eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); - - mockClockManagerToReturnFixedDateTime(ClockUtil.getZonedDateTime().plusWeeks(3)); - - timedTaskClient.start(scheduledActualCostExpiration(), 204, - "scheduled-actual-cost-expiration"); - - assertThat(loansFixture.getLoanById(loan.getId()).getJson(), isClosed()); - assertThat(itemsClient.getById(item.getId()).getJson(), isLostAndPaid()); - } - @Test void shouldNotCloseLoanDuringScheduledExpirationIfChargingPeriodHasNotElapsedAndProcessingFeeHasBeenPaid() { UUID servicePointId = servicePointsFixture.cd2().getId(); @@ -267,15 +200,12 @@ void shouldNotCloseLoanDuringScheduledExpirationIfChargingPeriodHasNotElapsedAnd useLostItemPolicy(actualCostLostItemFeePolicyId); item = itemsFixture.basedUponNod(); - loan = checkOutFixture.checkOutByBarcode(item, usersFixture.jessica()); - declareLostFixtures.declareItemLost(new DeclareItemLostRequestBuilder() .withServicePointId(servicePointId) .forLoanId(loan.getId())); feeFineAccountFixture.payLostItemProcessingFee(loan.getId()); - eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId()); mockClockManagerToReturnFixedDateTime(ClockUtil.getZonedDateTime().plusWeeks(1)); @@ -371,12 +301,4 @@ protected void createLostItemFeeActualCostAccount(double amount) { .withActionType("Lost item fee (actual cost)")); } - public void expireActualCost() { - setClock(fixed(getZonedDateTime().plusWeeks(3).toInstant(), UTC)); - - timedTaskClient.start(scheduledActualCostExpiration(), 204, - "scheduled-actual-cost-expiration"); - - setDefaultClock(); - } } From f3ff403a261d9040f046b70cfcb88be76c48bfa1 Mon Sep 17 00:00:00 2001 From: Mykyta_Varenyk Date: Mon, 4 Jul 2022 19:28:50 +0300 Subject: [PATCH 05/19] CIRC-1556 Refactor --- descriptors/ModuleDescriptor-template.json | 3 ++ .../policy/lostitem/LostItemPolicy.java | 1 - .../storage/ActualCostRecordRepository.java | 47 +++++++++++++++++-- .../storage/inventory/ItemRepository.java | 12 +++-- .../storage/loans/LoanRepository.java | 2 +- .../storage/requests/RequestRepository.java | 2 +- .../ExpiredActualCostProcessingResource.java | 10 ++-- .../resources/LoanCollectionResource.java | 4 +- .../RequestFromRepresentationService.java | 6 +-- .../RequestHoldShelfClearanceResource.java | 2 +- ...anRelatedFeeFineClosedHandlerResource.java | 2 +- .../CloseLoanWithLostItemService.java | 23 ++++++--- .../services/LostItemFeeRefundService.java | 2 +- .../ActualCostRecordExpirationService.java | 24 ++++++++-- .../fixtures/ExpireActualCostFixture.java | 14 ------ 15 files changed, 110 insertions(+), 44 deletions(-) delete mode 100644 src/test/java/api/support/fixtures/ExpireActualCostFixture.java diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index d90482fa30..41025236c4 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -645,6 +645,9 @@ "pubsub.events.post" ], "modulePermissions": [ + "accounts.collection.get", + "circulation.internal.fetch-items", + "circulation-storage.loans.collection.get", "modperms.circulation.handlers.loan-related-fee-fine-closed.post", "pubsub.publish.post" ], diff --git a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java index 8650dcfe08..c4072ca308 100644 --- a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java +++ b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java @@ -20,7 +20,6 @@ import org.folio.circulation.domain.policy.lostitem.itemfee.ActualCostFee; import org.folio.circulation.domain.policy.lostitem.itemfee.AutomaticallyChargeableFee; import org.folio.circulation.domain.policy.lostitem.itemfee.ChargeableFee; -import org.folio.circulation.support.utils.ClockUtil; import io.vertx.core.json.JsonObject; diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java index 07417df66d..ac232f23bb 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java @@ -1,22 +1,36 @@ package org.folio.circulation.infrastructure.storage; import io.vertx.core.json.JsonObject; +import static java.util.concurrent.CompletableFuture.completedFuture; import static java.util.function.Function.identity; +import static org.folio.circulation.support.fetching.MultipleCqlIndexValuesCriteria.byIndex; +import static org.folio.circulation.support.fetching.RecordFetching.findWithMultipleCqlIndexValues; import static org.folio.circulation.support.http.ResponseMapping.forwardOnFailure; import static org.folio.circulation.support.http.ResponseMapping.mapUsingJson; import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; import static org.folio.circulation.support.http.client.PageLimit.one; +import static org.folio.circulation.support.results.Result.succeeded; import static org.folio.circulation.support.results.ResultBinding.mapResult; import static org.folio.circulation.support.results.Result.ofAsync; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.stream.Collectors; +import org.folio.circulation.domain.Account; import org.folio.circulation.domain.ActualCostRecord; import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.MultipleRecords; +import org.folio.circulation.domain.policy.lostitem.LostItemPolicy; import org.folio.circulation.storage.mappers.ActualCostRecordMapper; import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; +import org.folio.circulation.support.FindWithMultipleCqlIndexValues; import org.folio.circulation.support.fetching.CqlQueryFinder; import org.folio.circulation.support.http.client.CqlQuery; import org.folio.circulation.support.http.client.PageLimit; @@ -62,18 +76,45 @@ public CompletableFuture> getActualCostRecordByAccountI } private Result> mapResponseToActualCostRecords(Response response) { - return MultipleRecords.from(response, ActualCostRecordMapper::toDomain, "actualCostRecords"); + return MultipleRecords.from(response, ActualCostRecordMapper::toDomain, ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME); } public CompletableFuture> findByLoan(Result loanResult) { - return loanResult.after(loan -> createActualCostRecordFinder().findByQuery( + return loanResult.after(loan -> createActualCostRecordCqlFinder().findByQuery( exactMatch(LOAN_ID_FIELD_NAME, loan.getId()), one()) .thenApply(records -> records.map(MultipleRecords::firstOrNull)) .thenApply(mapResult(ActualCostRecordMapper::toDomain)) .thenApply(mapResult(loan::withActualCostRecord))); } - private CqlQueryFinder createActualCostRecordFinder() { + public CompletableFuture>> findActualCostRecordsForLoans( + MultipleRecords multipleLoans) { + + if (multipleLoans.getRecords().isEmpty()) { + return completedFuture(succeeded(multipleLoans)); + } + + return getActualCostRecordsForLoans(multipleLoans.getRecords()) + .thenApply(r -> r.map(accountMap -> multipleLoans.mapRecords( + loan -> loan.withActualCostRecord(accountMap.getOrDefault(loan.getId(),null))))); + } + + private CompletableFuture>> getActualCostRecordsForLoans(Collection loans) { + + final Set loanIds = + loans.stream() + .filter(Objects::nonNull) + .map(Loan::getId) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + return findWithMultipleCqlIndexValues(actualCostRecordStorageClient, + ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME, ActualCostRecordMapper::toDomain) + .find(byIndex(LOAN_ID_FIELD_NAME, loanIds)) + .thenApply(mapResult(r -> r.toMap(ActualCostRecord::getLoanId))); + } + + private CqlQueryFinder createActualCostRecordCqlFinder() { return new CqlQueryFinder<>(actualCostRecordStorageClient, ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME, identity()); } diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java index 609ba1d0a3..60ade9bf00 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java @@ -71,7 +71,7 @@ public ItemRepository(Clients clients) { new LoanTypeRepository(clients.loanTypesStorage())); } - public CompletableFuture> fetchFor(ItemRelatedRecord itemRelatedRecord) { + public CompletableFuture> fetchItemsWithRelatedRecordsFor(ItemRelatedRecord itemRelatedRecord) { if (itemRelatedRecord.getItemId() == null) { return completedFuture(succeeded(Item.from(null))); } @@ -252,7 +252,7 @@ private CompletableFuture> fetchItemByBarcode(String barcode) { public CompletableFuture>> fetchItemsFor(Result> result, BiFunction includeItemMap) { - return fetchItemsFor(result, includeItemMap, this::fetchFor); + return fetchItemsFor(result, includeItemMap, this::fetchItemsWithRelatedRecordsFor); } public CompletableFuture>> @@ -261,6 +261,12 @@ private CompletableFuture> fetchItemByBarcode(String barcode) { return fetchItemsFor(result, includeItemMap, this::fetchItemsWithHoldingsRecords); } + public CompletableFuture>> + fetchItems(Result> result, BiFunction includeItemMap) { + + return fetchItemsFor(result, includeItemMap, this::fetchItems); + } + public CompletableFuture>> fetchItemsFor(Result> result, BiFunction includeItemMap, Function, CompletableFuture>>> fetcher) { @@ -302,7 +308,7 @@ public CompletableFuture>> findByIndexNameAndQuery( .thenComposeAsync(this::fetchItemsRelatedRecords); } - private CompletableFuture>> fetchFor( + private CompletableFuture>> fetchItemsWithRelatedRecordsFor( Collection itemIds) { return fetchItems(itemIds) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java index 555df80332..07d730e3a3 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java @@ -185,7 +185,7 @@ private CompletableFuture> refreshLoanRepresentation(Loan loan) { } private CompletableFuture> fetchItem(Result result) { - return result.combineAfter(itemRepository::fetchFor, Loan::withItem); + return result.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Loan::withItem); } private CompletableFuture> fetchUser(Result result) { diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java index f13e9ec6e7..833b7f33ab 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java @@ -150,7 +150,7 @@ private CompletableFuture> exists(String id) { public CompletableFuture> getById(String id) { return getByIdWithoutItem(id) - .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchFor, + .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Request::withItem)) .thenComposeAsync(result -> result.combineAfter(instanceRepository::fetch, Request::withInstance)) diff --git a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java index 7681655c62..0352d6b724 100644 --- a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java +++ b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java @@ -38,12 +38,16 @@ private void process(RoutingContext routingContext) { final var itemRepository = new ItemRepository(clients); final var userRepository = new UserRepository(clients); final var loanRepository = new LoanRepository(clients, itemRepository, userRepository); + AccountRepository accountRepository = new AccountRepository(clients); + LostItemPolicyRepository lostItemPolicyRepository = new LostItemPolicyRepository(clients); + ActualCostRecordRepository actualCostRecordRepository = new ActualCostRecordRepository(clients); final var closeLoanWithLostItemService = new CloseLoanWithLostItemService(loanRepository, - itemRepository, new AccountRepository(clients), new LostItemPolicyRepository(clients), - eventPublisher, new ActualCostRecordRepository(clients)); + itemRepository, accountRepository, lostItemPolicyRepository, + eventPublisher, actualCostRecordRepository); final var loanPageableFetcher = new PageableFetcher<>(loanRepository); final var actualCostRecordExpirationService = new ActualCostRecordExpirationService( - loanPageableFetcher, closeLoanWithLostItemService, itemRepository); + loanPageableFetcher, closeLoanWithLostItemService, itemRepository, accountRepository, lostItemPolicyRepository, + actualCostRecordRepository); actualCostRecordExpirationService.expireActualCostRecords() .thenApply(r -> r.map(toFixedValue(NoContentResponse::noContent))) diff --git a/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java b/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java index 93555f6d06..fc5961f6fa 100644 --- a/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java +++ b/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java @@ -113,7 +113,7 @@ void create(RoutingContext routingContext) { .thenApply(this::refuseWhenNotOpenOrClosed) .thenApply(this::refuseWhenOpenAndNoUserId) .thenApply(spLoanLocationValidator::checkServicePointLoanLocation) - .thenCombineAsync(itemRepository.fetchFor(loan), this::addItem) + .thenCombineAsync(itemRepository.fetchItemsWithRelatedRecordsFor(loan), this::addItem) .thenApply(itemNotFoundValidator::refuseWhenItemNotFound) .thenApply(alreadyCheckedOutValidator::refuseWhenItemIsAlreadyCheckedOut) .thenApply(itemStatusValidator::refuseWhenItemIsMissing) @@ -183,7 +183,7 @@ void replace(RoutingContext routingContext) { .thenApply(this::refuseWhenOpenAndNoUserId) .thenApply(spLoanLocationValidator::checkServicePointLoanLocation) .thenApply(this::refuseWhenClosedAndNoCheckInServicePointId) - .thenCombineAsync(itemRepository.fetchFor(loan), this::addItem) + .thenCombineAsync(itemRepository.fetchItemsWithRelatedRecordsFor(loan), this::addItem) .thenApply(itemNotFoundValidator::refuseWhenItemNotFound) .thenCompose(changeDueDateValidator::refuseChangeDueDateForItemInDisallowedStatus) .thenCombineAsync(userRepository.getUser(loan.getUserId()), this::addUser) diff --git a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java index f0b9846cb8..8fc145bf20 100644 --- a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java +++ b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java @@ -316,7 +316,7 @@ private Result findRecallableItemOrFail(Request request) { } private CompletableFuture> fetchItemForLoan(Request request) { - return itemRepository.fetchFor(request.getLoan()) + return itemRepository.fetchItemsWithRelatedRecordsFor(request.getLoan()) .thenApply(r -> r.map(request::withItem)); } @@ -338,7 +338,7 @@ private CompletableFuture> fetchLoan(Request request) { // of the title's items return fetchFirstLoanForUserWithTheSameInstanceId(request) - .thenCompose(r -> r.combineAfter(itemRepository::fetchFor, Request::withItem)); + .thenCompose(r -> r.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Request::withItem)); } else { return loanRepository.findOpenLoanForRequest(request) @@ -562,7 +562,7 @@ private Request removeProcessingParameters(Request request) { private CompletableFuture> findItemForRequest(Request request) { return succeeded(request) - .combineAfter(itemRepository::fetchFor, Request::withItem); + .combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Request::withItem); } private CompletableFuture> fetchUserForLoan(Request request) { diff --git a/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java b/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java index eb2774b81b..8281ded9d6 100644 --- a/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java @@ -282,7 +282,7 @@ private JsonObject toRequestsResponse(List requests) { private CompletableFuture> fetchItem(ItemRepository itemRepository, Request request) { return CompletableFuture.completedFuture(Result.succeeded(request)) - .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchFor, Request::withItem)); + .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Request::withItem)); } private Result> mapResponseToRequest(Response response) { diff --git a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java index dd5b819b8c..26f28c3ce2 100644 --- a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java +++ b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java @@ -78,7 +78,7 @@ private CompletableFuture> processEvent(LoanRepository loanReposito LoanRelatedFeeFineClosedEvent event, CloseLoanWithLostItemService closeLoanWithLostItemService) { return loanRepository.getById(event.getLoanId()) - .thenCompose(r -> r.after(closeLoanWithLostItemService::tryCloseLoan)); + .thenCompose(r -> r.after(closeLoanWithLostItemService::tryCloseLoanForLoanRelatedFeeFineClosedEvent)); } private Result createAndValidateRequest(RoutingContext context) { diff --git a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java index ab3d002ccd..0552a18f81 100644 --- a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java +++ b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java @@ -44,21 +44,30 @@ public CloseLoanWithLostItemService(LoanRepository loanRepository, ItemRepositor this.actualCostRecordRepository = actualCostRecordRepository; } - public CompletableFuture> tryCloseLoan(Loan loan) { + public CompletableFuture> tryCloseLoanForActualCostExpiration(Loan loan) { + return closeLoanWithLostItemIfLostFeesResolvedAndPublishLoanClosedEventAndLogEvent(loan); + } + + public CompletableFuture> tryCloseLoanForLoanRelatedFeeFineClosedEvent(Loan loan) { if (loan == null || !loan.isItemLost()) { return completedFuture(Result.succeeded(null)); } - return closeLoanWithLostItemIfLostFeesResolved(loan); + return fetchLoanRelatedRecords(loan) + .thenCompose(r -> r.after( + this::closeLoanWithLostItemIfLostFeesResolvedAndPublishLoanClosedEventAndLogEvent)); + } + + private CompletableFuture> + closeLoanWithLostItemIfLostFeesResolvedAndPublishLoanClosedEventAndLogEvent(Loan loan) { + return closeLoanAndUpdateItem(loan, loanRepository, itemRepository, eventPublisher) + .thenCompose(r -> r.after(eventPublisher::publishClosedLoanEvent)); } - private CompletableFuture> closeLoanWithLostItemIfLostFeesResolved(Loan loan) { + private CompletableFuture> fetchLoanRelatedRecords(Loan loan) { return accountRepository.findAccountsForLoan(loan) .thenComposeAsync(lostItemPolicyRepository::findLostItemPolicyForLoan) - .thenComposeAsync(actualCostRecordRepository::findByLoan) - .thenCompose(r -> r.after(l -> closeLoanAndUpdateItem(l, loanRepository, - itemRepository, eventPublisher))) - .thenCompose(r -> r.after(eventPublisher::publishClosedLoanEvent)); + .thenComposeAsync(actualCostRecordRepository::findByLoan); } public CompletableFuture> closeLoanAndUpdateItem(Loan loan, diff --git a/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java b/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java index 6c2b5065fc..0c72e4fcd3 100644 --- a/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java +++ b/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java @@ -205,7 +205,7 @@ private CompletableFuture> fetchUserAndItem(Loan loan) { } return userRepository.findUserForLoan(succeeded(loan)) - .thenCompose(r -> r.combineAfter(itemRepository::fetchFor, Loan::withItem)); + .thenCompose(r -> r.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Loan::withItem)); } private CompletableFuture> fetchAccountsAndActionsForLoan( diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index 3bd6fe4e83..dfd90c5430 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -2,10 +2,15 @@ import java.util.concurrent.CompletableFuture; +import org.folio.circulation.domain.Account; import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.MultipleRecords; +import org.folio.circulation.infrastructure.storage.ActualCostRecordRepository; +import org.folio.circulation.infrastructure.storage.feesandfines.AccountRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; +import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; import org.folio.circulation.services.CloseLoanWithLostItemService; +import org.folio.circulation.support.Clients; import org.folio.circulation.support.fetching.PageableFetcher; import org.folio.circulation.support.http.client.CqlQuery; import org.folio.circulation.support.results.Result; @@ -21,12 +26,21 @@ public class ActualCostRecordExpirationService { private final PageableFetcher loanPageableFetcher; private final CloseLoanWithLostItemService closeLoanWithLostItemService; private final ItemRepository itemRepository; + private final AccountRepository accountRepository; + private final LostItemPolicyRepository lostItemPolicyRepository; + private final ActualCostRecordRepository actualCostRecordRepository; public ActualCostRecordExpirationService(PageableFetcher loanPageableFetcher, - CloseLoanWithLostItemService closeLoanWithLostItemService, ItemRepository itemRepository) { + CloseLoanWithLostItemService closeLoanWithLostItemService, ItemRepository itemRepository, + AccountRepository accountRepository, LostItemPolicyRepository lostItemPolicyRepository, + ActualCostRecordRepository actualCostRecordRepository) { + this.itemRepository = itemRepository; this.loanPageableFetcher = loanPageableFetcher; this.closeLoanWithLostItemService = closeLoanWithLostItemService; + this.accountRepository = accountRepository; + this.lostItemPolicyRepository = lostItemPolicyRepository; + this.actualCostRecordRepository = actualCostRecordRepository; } public CompletableFuture> expireActualCostRecords() { @@ -43,9 +57,13 @@ private CompletableFuture> closeLoans(MultipleRecords expired return ofAsync(() -> null); } - return fromFutureResult(itemRepository.fetchItemsFor(succeeded(expiredLoans), Loan::withItem) + return fromFutureResult(itemRepository.fetchItems(succeeded(expiredLoans), Loan::withItem) .thenApply(r -> r.next(this::excludeLoansWithNonexistentItems))) - .flatMapFuture(loans -> allOf(loans.getRecords(), closeLoanWithLostItemService::tryCloseLoan)) + .flatMapFuture(accountRepository::findAccountsForLoans) + .flatMapFuture(lostItemPolicyRepository::findLostItemPoliciesForLoans) + .flatMapFuture(actualCostRecordRepository::findActualCostRecordsForLoans) + .flatMapFuture(loans -> allOf(loans.getRecords(), + closeLoanWithLostItemService::tryCloseLoanForActualCostExpiration)) .toCompletableFuture() .thenApply(r -> r.map(ignored -> null)); } diff --git a/src/test/java/api/support/fixtures/ExpireActualCostFixture.java b/src/test/java/api/support/fixtures/ExpireActualCostFixture.java deleted file mode 100644 index b9ed022e20..0000000000 --- a/src/test/java/api/support/fixtures/ExpireActualCostFixture.java +++ /dev/null @@ -1,14 +0,0 @@ -/* -package api.support.fixtures; - -import api.support.http.TimedTaskClient; -public final class ExpireActualCostFixture { - private final TimedTaskClient timedTaskClient; - - public ExpireActualCostFixture(TimedTaskClient timedTaskClient) { - this.timedTaskClient = timedTaskClient; - } - - public void -} -*/ From 3e2b0dcfcbec5ca182d478ee949e8ddcc1820492 Mon Sep 17 00:00:00 2001 From: Mykyta_Varenyk Date: Mon, 4 Jul 2022 19:35:25 +0300 Subject: [PATCH 06/19] CIRC-1556 Add module permissions --- descriptors/ModuleDescriptor-template.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 41025236c4..82916b86d6 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -645,10 +645,13 @@ "pubsub.events.post" ], "modulePermissions": [ + "circulation-storage.loans.item.put", + "inventory-storage.items.item.put", + "actual-cost-record-storage.actual-cost-records.collection.get", "accounts.collection.get", "circulation.internal.fetch-items", + "lost-item-fees-policies.collection.get", "circulation-storage.loans.collection.get", - "modperms.circulation.handlers.loan-related-fee-fine-closed.post", "pubsub.publish.post" ], "unit": "minute", From 7353375de566cc6649b9b2d1d91f05b6ab8a5d63 Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Wed, 6 Jul 2022 16:11:51 +0300 Subject: [PATCH 07/19] CIRC-1556 refactoring --- descriptors/ModuleDescriptor-template.json | 2 +- .../policy/lostitem/LostItemPolicy.java | 8 ++--- .../storage/ActualCostRecordRepository.java | 29 ++++++++----------- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index 82916b86d6..eb058e651c 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -642,7 +642,7 @@ ], "pathPattern": "/circulation/actual-cost-expiration-by-timeout", "permissionsRequired": [ - "pubsub.events.post" + "pubsub.events.post" ], "modulePermissions": [ "circulation-storage.loans.item.put", diff --git a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java index c4072ca308..d5a56363ce 100644 --- a/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java +++ b/src/main/java/org/folio/circulation/domain/policy/lostitem/LostItemPolicy.java @@ -34,7 +34,7 @@ public class LostItemPolicy extends Policy { private final Period patronBilledAfterItemAgedToLostInterval; private final Period recalledItemAgedToLostAfterOverdueInterval; private final Period patronBilledAfterRecalledItemAgedToLostInterval; - private final Period lostItemChargeFeeFine; + private final Period lostItemChargeFeeFineInterval; // There is no separate age to lost processing fee but there is a flag // that turns on/off the fee, but we're modelling it as a separate fee // to simplify logic. @@ -46,7 +46,7 @@ private LostItemPolicy(String id, String name, AutomaticallyChargeableFee declar Period itemAgedToLostAfterOverdueInterval, Period patronBilledAfterItemAgedToLostInterval, Period recalledItemAgedToLostAfterOverdueInterval, Period patronBilledAfterRecalledItemAgedToLostInterval, - AutomaticallyChargeableFee ageToLostProcessingFee, Period lostItemChargeFeeFine) { + AutomaticallyChargeableFee ageToLostProcessingFee, Period lostItemChargeFeeFineInterval) { super(id, name); this.declareLostProcessingFee = declareLostProcessingFee; @@ -61,7 +61,7 @@ private LostItemPolicy(String id, String name, AutomaticallyChargeableFee declar this.patronBilledAfterRecalledItemAgedToLostInterval = patronBilledAfterRecalledItemAgedToLostInterval; this.ageToLostProcessingFee = ageToLostProcessingFee; - this.lostItemChargeFeeFine = lostItemChargeFeeFine; + this.lostItemChargeFeeFineInterval = lostItemChargeFeeFineInterval; } public static LostItemPolicy from(JsonObject lostItemPolicy) { @@ -166,7 +166,7 @@ public boolean canAgeLoanToLost(boolean isRecalled, ZonedDateTime loanDueDate) { } public ZonedDateTime calculateFeeFineChargingPeriodExpirationDateTime(ZonedDateTime lostTime) { - return lostItemChargeFeeFine.plusDate(lostTime); + return lostItemChargeFeeFineInterval.plusDate(lostTime); } public ZonedDateTime calculateDateTimeWhenPatronBilledForAgedToLost( diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java index ac232f23bb..850f027c51 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java @@ -13,24 +13,19 @@ import static org.folio.circulation.support.results.ResultBinding.mapResult; import static org.folio.circulation.support.results.Result.ofAsync; -import java.util.ArrayList; import java.util.Collection; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; -import org.folio.circulation.domain.Account; import org.folio.circulation.domain.ActualCostRecord; import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.MultipleRecords; -import org.folio.circulation.domain.policy.lostitem.LostItemPolicy; import org.folio.circulation.storage.mappers.ActualCostRecordMapper; import org.folio.circulation.support.Clients; import org.folio.circulation.support.CollectionResourceClient; -import org.folio.circulation.support.FindWithMultipleCqlIndexValues; import org.folio.circulation.support.fetching.CqlQueryFinder; import org.folio.circulation.support.http.client.CqlQuery; import org.folio.circulation.support.http.client.PageLimit; @@ -41,7 +36,7 @@ public class ActualCostRecordRepository { private final CollectionResourceClient actualCostRecordStorageClient; - private static final String ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME = "actualCostRecords"; + private static final String ACTUAL_COST_RECORDS = "actualCostRecords"; private static final String LOAN_ID_FIELD_NAME = "loanId"; public ActualCostRecordRepository(Clients clients) { @@ -76,7 +71,7 @@ public CompletableFuture> getActualCostRecordByAccountI } private Result> mapResponseToActualCostRecords(Response response) { - return MultipleRecords.from(response, ActualCostRecordMapper::toDomain, ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME); + return MultipleRecords.from(response, ActualCostRecordMapper::toDomain, ACTUAL_COST_RECORDS); } public CompletableFuture> findByLoan(Result loanResult) { @@ -96,26 +91,26 @@ public CompletableFuture>> findActualCostRecordsFor return getActualCostRecordsForLoans(multipleLoans.getRecords()) .thenApply(r -> r.map(accountMap -> multipleLoans.mapRecords( - loan -> loan.withActualCostRecord(accountMap.getOrDefault(loan.getId(),null))))); + loan -> loan.withActualCostRecord(accountMap.getOrDefault(loan.getId(), null))))); } - private CompletableFuture>> getActualCostRecordsForLoans(Collection loans) { + private CompletableFuture>> getActualCostRecordsForLoans( + Collection loans) { - final Set loanIds = - loans.stream() - .filter(Objects::nonNull) - .map(Loan::getId) - .filter(Objects::nonNull) - .collect(Collectors.toSet()); + final Set loanIds = loans.stream() + .filter(Objects::nonNull) + .map(Loan::getId) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); return findWithMultipleCqlIndexValues(actualCostRecordStorageClient, - ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME, ActualCostRecordMapper::toDomain) + ACTUAL_COST_RECORDS, ActualCostRecordMapper::toDomain) .find(byIndex(LOAN_ID_FIELD_NAME, loanIds)) .thenApply(mapResult(r -> r.toMap(ActualCostRecord::getLoanId))); } private CqlQueryFinder createActualCostRecordCqlFinder() { - return new CqlQueryFinder<>(actualCostRecordStorageClient, ACTUAL_COST_RECORDS_COLLECTION_PROPERTY_NAME, + return new CqlQueryFinder<>(actualCostRecordStorageClient, ACTUAL_COST_RECORDS, identity()); } From b6003342e99fd1f365d0f9bc98c17bcdbbb208e2 Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Wed, 6 Jul 2022 16:18:29 +0300 Subject: [PATCH 08/19] CIRC-1556 fix code smells --- .../storage/ActualCostRecordRepository.java | 5 +++-- .../ActualCostRecordExpirationService.java | 16 +++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java index 850f027c51..26fee10c23 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java @@ -1,6 +1,5 @@ package org.folio.circulation.infrastructure.storage; -import io.vertx.core.json.JsonObject; import static java.util.concurrent.CompletableFuture.completedFuture; import static java.util.function.Function.identity; import static org.folio.circulation.support.fetching.MultipleCqlIndexValuesCriteria.byIndex; @@ -9,9 +8,9 @@ import static org.folio.circulation.support.http.ResponseMapping.mapUsingJson; import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; import static org.folio.circulation.support.http.client.PageLimit.one; +import static org.folio.circulation.support.results.Result.ofAsync; import static org.folio.circulation.support.results.Result.succeeded; import static org.folio.circulation.support.results.ResultBinding.mapResult; -import static org.folio.circulation.support.results.Result.ofAsync; import java.util.Collection; import java.util.Map; @@ -33,6 +32,8 @@ import org.folio.circulation.support.http.client.ResponseInterpreter; import org.folio.circulation.support.results.Result; +import io.vertx.core.json.JsonObject; + public class ActualCostRecordRepository { private final CollectionResourceClient actualCostRecordStorageClient; diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index dfd90c5430..361fa887cf 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -1,8 +1,14 @@ package org.folio.circulation.services.actualcostrecord; +import static org.folio.circulation.domain.ItemStatus.DECLARED_LOST; +import static org.folio.circulation.domain.representations.LoanProperties.ITEM_STATUS; +import static org.folio.circulation.support.AsyncCoordinationUtil.allOf; +import static org.folio.circulation.support.results.AsynchronousResult.fromFutureResult; +import static org.folio.circulation.support.results.Result.ofAsync; +import static org.folio.circulation.support.results.Result.succeeded; + import java.util.concurrent.CompletableFuture; -import org.folio.circulation.domain.Account; import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.MultipleRecords; import org.folio.circulation.infrastructure.storage.ActualCostRecordRepository; @@ -10,18 +16,10 @@ import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; import org.folio.circulation.services.CloseLoanWithLostItemService; -import org.folio.circulation.support.Clients; import org.folio.circulation.support.fetching.PageableFetcher; import org.folio.circulation.support.http.client.CqlQuery; import org.folio.circulation.support.results.Result; -import static org.folio.circulation.domain.ItemStatus.DECLARED_LOST; -import static org.folio.circulation.domain.representations.LoanProperties.ITEM_STATUS; -import static org.folio.circulation.support.AsyncCoordinationUtil.allOf; -import static org.folio.circulation.support.results.AsynchronousResult.fromFutureResult; -import static org.folio.circulation.support.results.Result.ofAsync; -import static org.folio.circulation.support.results.Result.succeeded; - public class ActualCostRecordExpirationService { private final PageableFetcher loanPageableFetcher; private final CloseLoanWithLostItemService closeLoanWithLostItemService; From 574d5de53957b2080c6057e15f1edc634ccd3233 Mon Sep 17 00:00:00 2001 From: alexanderkurash Date: Wed, 6 Jul 2022 19:01:30 +0300 Subject: [PATCH 09/19] CIRC-1556 Extend ItemRelatedRecord --- .../org/folio/circulation/domain/ItemRelatedRecord.java | 1 + .../infrastructure/storage/inventory/ItemRepository.java | 9 +++++---- .../ActualCostRecordExpirationService.java | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/ItemRelatedRecord.java b/src/main/java/org/folio/circulation/domain/ItemRelatedRecord.java index 45b88be89d..9a3cf88053 100644 --- a/src/main/java/org/folio/circulation/domain/ItemRelatedRecord.java +++ b/src/main/java/org/folio/circulation/domain/ItemRelatedRecord.java @@ -2,4 +2,5 @@ public interface ItemRelatedRecord { String getItemId(); + ItemRelatedRecord withItem(Item item); } diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java index 60ade9bf00..2c75044330 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java @@ -252,7 +252,7 @@ private CompletableFuture> fetchItemByBarcode(String barcode) { public CompletableFuture>> fetchItemsFor(Result> result, BiFunction includeItemMap) { - return fetchItemsFor(result, includeItemMap, this::fetchItemsWithRelatedRecordsFor); + return fetchItemsFor(result, includeItemMap, this::fetchFor); } public CompletableFuture>> @@ -262,9 +262,10 @@ private CompletableFuture> fetchItemByBarcode(String barcode) { } public CompletableFuture>> - fetchItems(Result> result, BiFunction includeItemMap) { + fetchItems(Result> result) { - return fetchItemsFor(result, includeItemMap, this::fetchItems); + return fetchItemsFor(result, (itemRelatedRecord, item) -> (T) itemRelatedRecord.withItem(item), + this::fetchItems); } public CompletableFuture>> @@ -308,7 +309,7 @@ public CompletableFuture>> findByIndexNameAndQuery( .thenComposeAsync(this::fetchItemsRelatedRecords); } - private CompletableFuture>> fetchItemsWithRelatedRecordsFor( + private CompletableFuture>> fetchFor( Collection itemIds) { return fetchItems(itemIds) diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index 361fa887cf..2c72f6a4b6 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -55,7 +55,7 @@ private CompletableFuture> closeLoans(MultipleRecords expired return ofAsync(() -> null); } - return fromFutureResult(itemRepository.fetchItems(succeeded(expiredLoans), Loan::withItem) + return fromFutureResult(itemRepository.fetchItems(succeeded(expiredLoans)) .thenApply(r -> r.next(this::excludeLoansWithNonexistentItems))) .flatMapFuture(accountRepository::findAccountsForLoans) .flatMapFuture(lostItemPolicyRepository::findLostItemPoliciesForLoans) From 55acdb5891f5751c1abdf83c6b4b3bd52168c201 Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Thu, 7 Jul 2022 12:30:59 +0300 Subject: [PATCH 10/19] CIRC-1556 refactoring according to code review --- descriptors/ModuleDescriptor-template.json | 3 --- .../folio/circulation/CirculationVerticle.java | 2 +- .../storage/ActualCostRecordRepository.java | 10 +++++----- .../storage/inventory/ItemRepository.java | 16 +++++++--------- .../storage/loans/LoanRepository.java | 2 +- .../storage/requests/RequestRepository.java | 2 +- .../ExpiredActualCostProcessingResource.java | 4 ++-- .../resources/LoanCollectionResource.java | 4 ++-- .../RequestFromRepresentationService.java | 6 +++--- .../RequestHoldShelfClearanceResource.java | 2 +- .../services/LostItemFeeRefundService.java | 4 ++-- .../ActualCostRecordExpirationService.java | 2 +- 12 files changed, 26 insertions(+), 31 deletions(-) diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index eb058e651c..c460669204 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -641,9 +641,6 @@ "POST" ], "pathPattern": "/circulation/actual-cost-expiration-by-timeout", - "permissionsRequired": [ - "pubsub.events.post" - ], "modulePermissions": [ "circulation-storage.loans.item.put", "inventory-storage.items.item.put", diff --git a/src/main/java/org/folio/circulation/CirculationVerticle.java b/src/main/java/org/folio/circulation/CirculationVerticle.java index d51a8d25e9..4bc1d7821a 100644 --- a/src/main/java/org/folio/circulation/CirculationVerticle.java +++ b/src/main/java/org/folio/circulation/CirculationVerticle.java @@ -72,7 +72,6 @@ public void start(Promise startFuture) { new CheckOutByBarcodeResource("/circulation/check-out-by-barcode", client).register(router); new CheckInByBarcodeResource(client).register(router); - new ExpiredActualCostProcessingResource(client).register(router); new RenewByBarcodeResource(client).register(router); new RenewByIdResource(client).register(router); @@ -140,6 +139,7 @@ public void start(Promise startFuture) { startFuture.fail(result.cause()); } }); + new ExpiredActualCostProcessingResource(client).register(router); } @Override diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java index 26fee10c23..c3223aca34 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/ActualCostRecordRepository.java @@ -83,19 +83,19 @@ public CompletableFuture> findByLoan(Result loanResult) { .thenApply(mapResult(loan::withActualCostRecord))); } - public CompletableFuture>> findActualCostRecordsForLoans( + public CompletableFuture>> fetchActualCostRecords( MultipleRecords multipleLoans) { if (multipleLoans.getRecords().isEmpty()) { return completedFuture(succeeded(multipleLoans)); } - return getActualCostRecordsForLoans(multipleLoans.getRecords()) - .thenApply(r -> r.map(accountMap -> multipleLoans.mapRecords( - loan -> loan.withActualCostRecord(accountMap.getOrDefault(loan.getId(), null))))); + return buildLoanIdToActualCostRecordMap(multipleLoans.getRecords()) + .thenApply(r -> r.map(actualCostRecordMap -> multipleLoans.mapRecords( + loan -> loan.withActualCostRecord(actualCostRecordMap.getOrDefault(loan.getId(), null))))); } - private CompletableFuture>> getActualCostRecordsForLoans( + private CompletableFuture>> buildLoanIdToActualCostRecordMap( Collection loans) { final Set loanIds = loans.stream() diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java index 60ade9bf00..07c93797c3 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java @@ -71,7 +71,7 @@ public ItemRepository(Clients clients) { new LoanTypeRepository(clients.loanTypesStorage())); } - public CompletableFuture> fetchItemsWithRelatedRecordsFor(ItemRelatedRecord itemRelatedRecord) { + public CompletableFuture> fetchFor(ItemRelatedRecord itemRelatedRecord) { if (itemRelatedRecord.getItemId() == null) { return completedFuture(succeeded(Item.from(null))); } @@ -252,19 +252,19 @@ private CompletableFuture> fetchItemByBarcode(String barcode) { public CompletableFuture>> fetchItemsFor(Result> result, BiFunction includeItemMap) { - return fetchItemsFor(result, includeItemMap, this::fetchItemsWithRelatedRecordsFor); + return fetchItemsFor(result, includeItemMap, this::fetchFor); } public CompletableFuture>> - fetchItemsWithHoldings(Result> result, BiFunction includeItemMap) { + fetchItemsWithHoldings(Result> result, BiFunction withItemMapper) { - return fetchItemsFor(result, includeItemMap, this::fetchItemsWithHoldingsRecords); + return fetchItemsFor(result, withItemMapper, this::fetchItemsWithHoldingsRecords); } public CompletableFuture>> - fetchItems(Result> result, BiFunction includeItemMap) { + fetchItems(Result> result, BiFunction withItemMapper) { - return fetchItemsFor(result, includeItemMap, this::fetchItems); + return fetchItemsFor(result, withItemMapper, this::fetchItems); } public CompletableFuture>> @@ -308,9 +308,7 @@ public CompletableFuture>> findByIndexNameAndQuery( .thenComposeAsync(this::fetchItemsRelatedRecords); } - private CompletableFuture>> fetchItemsWithRelatedRecordsFor( - Collection itemIds) { - + private CompletableFuture>> fetchFor(Collection itemIds) { return fetchItems(itemIds) .thenComposeAsync(this::fetchItemsRelatedRecords); } diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java index 07d730e3a3..555df80332 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/loans/LoanRepository.java @@ -185,7 +185,7 @@ private CompletableFuture> refreshLoanRepresentation(Loan loan) { } private CompletableFuture> fetchItem(Result result) { - return result.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Loan::withItem); + return result.combineAfter(itemRepository::fetchFor, Loan::withItem); } private CompletableFuture> fetchUser(Result result) { diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java index 833b7f33ab..f13e9ec6e7 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/requests/RequestRepository.java @@ -150,7 +150,7 @@ private CompletableFuture> exists(String id) { public CompletableFuture> getById(String id) { return getByIdWithoutItem(id) - .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, + .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchFor, Request::withItem)) .thenComposeAsync(result -> result.combineAfter(instanceRepository::fetch, Request::withInstance)) diff --git a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java index 0352d6b724..6c3d973d7f 100644 --- a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java +++ b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java @@ -46,8 +46,8 @@ private void process(RoutingContext routingContext) { eventPublisher, actualCostRecordRepository); final var loanPageableFetcher = new PageableFetcher<>(loanRepository); final var actualCostRecordExpirationService = new ActualCostRecordExpirationService( - loanPageableFetcher, closeLoanWithLostItemService, itemRepository, accountRepository, lostItemPolicyRepository, - actualCostRecordRepository); + loanPageableFetcher, closeLoanWithLostItemService, itemRepository, accountRepository, + lostItemPolicyRepository, actualCostRecordRepository); actualCostRecordExpirationService.expireActualCostRecords() .thenApply(r -> r.map(toFixedValue(NoContentResponse::noContent))) diff --git a/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java b/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java index fc5961f6fa..93555f6d06 100644 --- a/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java +++ b/src/main/java/org/folio/circulation/resources/LoanCollectionResource.java @@ -113,7 +113,7 @@ void create(RoutingContext routingContext) { .thenApply(this::refuseWhenNotOpenOrClosed) .thenApply(this::refuseWhenOpenAndNoUserId) .thenApply(spLoanLocationValidator::checkServicePointLoanLocation) - .thenCombineAsync(itemRepository.fetchItemsWithRelatedRecordsFor(loan), this::addItem) + .thenCombineAsync(itemRepository.fetchFor(loan), this::addItem) .thenApply(itemNotFoundValidator::refuseWhenItemNotFound) .thenApply(alreadyCheckedOutValidator::refuseWhenItemIsAlreadyCheckedOut) .thenApply(itemStatusValidator::refuseWhenItemIsMissing) @@ -183,7 +183,7 @@ void replace(RoutingContext routingContext) { .thenApply(this::refuseWhenOpenAndNoUserId) .thenApply(spLoanLocationValidator::checkServicePointLoanLocation) .thenApply(this::refuseWhenClosedAndNoCheckInServicePointId) - .thenCombineAsync(itemRepository.fetchItemsWithRelatedRecordsFor(loan), this::addItem) + .thenCombineAsync(itemRepository.fetchFor(loan), this::addItem) .thenApply(itemNotFoundValidator::refuseWhenItemNotFound) .thenCompose(changeDueDateValidator::refuseChangeDueDateForItemInDisallowedStatus) .thenCombineAsync(userRepository.getUser(loan.getUserId()), this::addUser) diff --git a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java index 8fc145bf20..f0b9846cb8 100644 --- a/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java +++ b/src/main/java/org/folio/circulation/resources/RequestFromRepresentationService.java @@ -316,7 +316,7 @@ private Result findRecallableItemOrFail(Request request) { } private CompletableFuture> fetchItemForLoan(Request request) { - return itemRepository.fetchItemsWithRelatedRecordsFor(request.getLoan()) + return itemRepository.fetchFor(request.getLoan()) .thenApply(r -> r.map(request::withItem)); } @@ -338,7 +338,7 @@ private CompletableFuture> fetchLoan(Request request) { // of the title's items return fetchFirstLoanForUserWithTheSameInstanceId(request) - .thenCompose(r -> r.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Request::withItem)); + .thenCompose(r -> r.combineAfter(itemRepository::fetchFor, Request::withItem)); } else { return loanRepository.findOpenLoanForRequest(request) @@ -562,7 +562,7 @@ private Request removeProcessingParameters(Request request) { private CompletableFuture> findItemForRequest(Request request) { return succeeded(request) - .combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Request::withItem); + .combineAfter(itemRepository::fetchFor, Request::withItem); } private CompletableFuture> fetchUserForLoan(Request request) { diff --git a/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java b/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java index 8281ded9d6..eb2774b81b 100644 --- a/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestHoldShelfClearanceResource.java @@ -282,7 +282,7 @@ private JsonObject toRequestsResponse(List requests) { private CompletableFuture> fetchItem(ItemRepository itemRepository, Request request) { return CompletableFuture.completedFuture(Result.succeeded(request)) - .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Request::withItem)); + .thenComposeAsync(result -> result.combineAfter(itemRepository::fetchFor, Request::withItem)); } private Result> mapResponseToRequest(Response response) { diff --git a/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java b/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java index 0c72e4fcd3..9cbbdb1775 100644 --- a/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java +++ b/src/main/java/org/folio/circulation/services/LostItemFeeRefundService.java @@ -205,7 +205,7 @@ private CompletableFuture> fetchUserAndItem(Loan loan) { } return userRepository.findUserForLoan(succeeded(loan)) - .thenCompose(r -> r.combineAfter(itemRepository::fetchItemsWithRelatedRecordsFor, Loan::withItem)); + .thenCompose(r -> r.combineAfter(itemRepository::fetchFor, Loan::withItem)); } private CompletableFuture> fetchAccountsAndActionsForLoan( @@ -329,4 +329,4 @@ private Result noLoanFoundForLostItem(String itemId) { "Item is lost however there is no aged to lost nor declared lost loan found", "itemId", itemId)); } -} \ No newline at end of file +} diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index 361fa887cf..8766815725 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -59,7 +59,7 @@ private CompletableFuture> closeLoans(MultipleRecords expired .thenApply(r -> r.next(this::excludeLoansWithNonexistentItems))) .flatMapFuture(accountRepository::findAccountsForLoans) .flatMapFuture(lostItemPolicyRepository::findLostItemPoliciesForLoans) - .flatMapFuture(actualCostRecordRepository::findActualCostRecordsForLoans) + .flatMapFuture(actualCostRecordRepository::fetchActualCostRecords) .flatMapFuture(loans -> allOf(loans.getRecords(), closeLoanWithLostItemService::tryCloseLoanForActualCostExpiration)) .toCompletableFuture() From 69f0cad7db9ece780880e84cd47fa1b1dda36adb Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Thu, 7 Jul 2022 14:51:59 +0300 Subject: [PATCH 11/19] CIRC-1556 refactoring according to code review --- .../infrastructure/storage/inventory/ItemRepository.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java index c9c91f3499..ec04e0f5e0 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java @@ -258,11 +258,11 @@ private CompletableFuture> fetchItemByBarcode(String barcode) { public CompletableFuture>> fetchItemsWithHoldings(Result> result, BiFunction withItemMapper) { - return fetchItemsFor(result, withItemMapper, this::fetchItemsWithHoldingsRecords); + return fetchItemsFor(result, withItemMapper, this::fetchItems); } public CompletableFuture>> - fetchItems(Result> result, BiFunction withItemMapper) { + fetchItems(Result> result) { return fetchItemsFor(result, (itemRelatedRecord, item) -> (T) itemRelatedRecord.withItem(item), this::fetchItems); @@ -309,7 +309,6 @@ public CompletableFuture>> findByIndexNameAndQuery( .thenComposeAsync(this::fetchItemsRelatedRecords); } - private CompletableFuture>> fetchFor(Collection itemIds) { private CompletableFuture>> fetchFor( Collection itemIds) { From 382f4577f4ca1d13de4bb003209b4bda3f1ab9b8 Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Thu, 7 Jul 2022 15:33:10 +0300 Subject: [PATCH 12/19] CIRC-1556 fix code smell --- .../infrastructure/storage/inventory/ItemRepository.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java index ec04e0f5e0..f46b043b73 100644 --- a/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java +++ b/src/main/java/org/folio/circulation/infrastructure/storage/inventory/ItemRepository.java @@ -316,13 +316,6 @@ private CompletableFuture>> fetchFor( .thenComposeAsync(this::fetchItemsRelatedRecords); } - private CompletableFuture>> fetchItemsWithHoldingsRecords( - Collection itemIds) { - - return fetchItems(itemIds) - .thenComposeAsync(this::fetchHoldingsRecords); - } - public CompletableFuture> fetchItemRelatedRecords(Result itemResult) { return itemResult.combineAfter(this::fetchHoldingsRecord, Item::withHoldings) .thenComposeAsync(combineAfter(this::fetchInstance, Item::withInstance)) From 827439265090c9bcbd9d6843ae5a9db6e93341af Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Thu, 7 Jul 2022 15:47:45 +0300 Subject: [PATCH 13/19] CIRC-1556 fix code smell --- .../ExpiredActualCostProcessingResource.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java index 6c3d973d7f..7daa9079c6 100644 --- a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java +++ b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java @@ -19,6 +19,7 @@ import io.vertx.ext.web.RoutingContext; import static org.folio.circulation.support.Clients.create; import static org.folio.circulation.support.results.MappingFunctions.toFixedValue; + public class ExpiredActualCostProcessingResource extends Resource { public ExpiredActualCostProcessingResource(HttpClient client) { super(client); @@ -31,21 +32,21 @@ public void register(Router router) { } private void process(RoutingContext routingContext) { - final WebContext context = new WebContext(routingContext); - final var clients = create(context, client); - - final var eventPublisher = new EventPublisher(routingContext); - final var itemRepository = new ItemRepository(clients); - final var userRepository = new UserRepository(clients); - final var loanRepository = new LoanRepository(clients, itemRepository, userRepository); - AccountRepository accountRepository = new AccountRepository(clients); - LostItemPolicyRepository lostItemPolicyRepository = new LostItemPolicyRepository(clients); - ActualCostRecordRepository actualCostRecordRepository = new ActualCostRecordRepository(clients); - final var closeLoanWithLostItemService = new CloseLoanWithLostItemService(loanRepository, + var context = new WebContext(routingContext); + var clients = create(context, client); + + var eventPublisher = new EventPublisher(routingContext); + var itemRepository = new ItemRepository(clients); + var userRepository = new UserRepository(clients); + var loanRepository = new LoanRepository(clients, itemRepository, userRepository); + var accountRepository = new AccountRepository(clients); + var lostItemPolicyRepository = new LostItemPolicyRepository(clients); + var actualCostRecordRepository = new ActualCostRecordRepository(clients); + var closeLoanWithLostItemService = new CloseLoanWithLostItemService(loanRepository, itemRepository, accountRepository, lostItemPolicyRepository, eventPublisher, actualCostRecordRepository); - final var loanPageableFetcher = new PageableFetcher<>(loanRepository); - final var actualCostRecordExpirationService = new ActualCostRecordExpirationService( + var loanPageableFetcher = new PageableFetcher<>(loanRepository); + var actualCostRecordExpirationService = new ActualCostRecordExpirationService( loanPageableFetcher, closeLoanWithLostItemService, itemRepository, accountRepository, lostItemPolicyRepository, actualCostRecordRepository); From b202b6e0398c378674f5c6a61d8f3c0efaf9447f Mon Sep 17 00:00:00 2001 From: alexanderkurash Date: Thu, 7 Jul 2022 18:52:17 +0300 Subject: [PATCH 14/19] CIRC-1557 Refactor close loan service --- pom.xml | 2 +- ...anRelatedFeeFineClosedHandlerResource.java | 2 +- .../CloseLoanWithLostItemService.java | 20 +++++++------------ .../ActualCostRecordExpirationService.java | 5 +---- 4 files changed, 10 insertions(+), 19 deletions(-) diff --git a/pom.xml b/pom.xml index 79db53b5c9..bd56d6154e 100644 --- a/pom.xml +++ b/pom.xml @@ -30,7 +30,7 @@ 2.17.1 UTF-8 UTF-8 - 1.18.16 + 1.18.22 5.2.22.RELEASE 3.9.1 diff --git a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java index 26f28c3ce2..f285691223 100644 --- a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java +++ b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java @@ -78,7 +78,7 @@ private CompletableFuture> processEvent(LoanRepository loanReposito LoanRelatedFeeFineClosedEvent event, CloseLoanWithLostItemService closeLoanWithLostItemService) { return loanRepository.getById(event.getLoanId()) - .thenCompose(r -> r.after(closeLoanWithLostItemService::tryCloseLoanForLoanRelatedFeeFineClosedEvent)); + .thenCompose(r -> r.after(closeLoanWithLostItemService::closeLoanWithLostItemFeesPaid)); } private Result createAndValidateRequest(RoutingContext context) { diff --git a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java index 0552a18f81..602cc0269a 100644 --- a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java +++ b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java @@ -44,33 +44,27 @@ public CloseLoanWithLostItemService(LoanRepository loanRepository, ItemRepositor this.actualCostRecordRepository = actualCostRecordRepository; } - public CompletableFuture> tryCloseLoanForActualCostExpiration(Loan loan) { - return closeLoanWithLostItemIfLostFeesResolvedAndPublishLoanClosedEventAndLogEvent(loan); - } - - public CompletableFuture> tryCloseLoanForLoanRelatedFeeFineClosedEvent(Loan loan) { + public CompletableFuture> closeLoanWithLostItemFeesPaid(Loan loan) { if (loan == null || !loan.isItemLost()) { return completedFuture(Result.succeeded(null)); } - return fetchLoanRelatedRecords(loan) - .thenCompose(r -> r.after( - this::closeLoanWithLostItemIfLostFeesResolvedAndPublishLoanClosedEventAndLogEvent)); + return fetchLoanFeeFineData(loan) + .thenCompose(r -> r.after(this::closeLoanWithLostItemFeesPaidAndPublishEvents)); } - private CompletableFuture> - closeLoanWithLostItemIfLostFeesResolvedAndPublishLoanClosedEventAndLogEvent(Loan loan) { - return closeLoanAndUpdateItem(loan, loanRepository, itemRepository, eventPublisher) + private CompletableFuture> closeLoanWithLostItemFeesPaidAndPublishEvents(Loan loan) { + return closeLoanWithLostItemFeesPaid(loan, loanRepository, itemRepository, eventPublisher) .thenCompose(r -> r.after(eventPublisher::publishClosedLoanEvent)); } - private CompletableFuture> fetchLoanRelatedRecords(Loan loan) { + private CompletableFuture> fetchLoanFeeFineData(Loan loan) { return accountRepository.findAccountsForLoan(loan) .thenComposeAsync(lostItemPolicyRepository::findLostItemPolicyForLoan) .thenComposeAsync(actualCostRecordRepository::findByLoan); } - public CompletableFuture> closeLoanAndUpdateItem(Loan loan, + private CompletableFuture> closeLoanWithLostItemFeesPaid(Loan loan, LoanRepository loanRepository, ItemRepository itemRepository, EventPublisher eventPublisher) { if (!shouldCloseLoan(loan)) { diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index 4d4fb82267..cbd21511ef 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -57,11 +57,8 @@ private CompletableFuture> closeLoans(MultipleRecords expired return fromFutureResult(itemRepository.fetchItems(succeeded(expiredLoans)) .thenApply(r -> r.next(this::excludeLoansWithNonexistentItems))) - .flatMapFuture(accountRepository::findAccountsForLoans) - .flatMapFuture(lostItemPolicyRepository::findLostItemPoliciesForLoans) - .flatMapFuture(actualCostRecordRepository::fetchActualCostRecords) .flatMapFuture(loans -> allOf(loans.getRecords(), - closeLoanWithLostItemService::tryCloseLoanForActualCostExpiration)) + closeLoanWithLostItemService::closeLoanWithLostItemFeesPaid)) .toCompletableFuture() .thenApply(r -> r.map(ignored -> null)); } From 62051704b3d9644f426c1a880dbc2d02d1ace7cc Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Tue, 12 Jul 2022 15:12:30 +0300 Subject: [PATCH 15/19] CIRC-1557 refactoring after code review --- .../handlers/LoanRelatedFeeFineClosedHandlerResource.java | 2 +- .../circulation/services/CloseLoanWithLostItemService.java | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java index f285691223..1e51b6e623 100644 --- a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java +++ b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java @@ -52,7 +52,7 @@ public void register(Router router) { private void handleFeeFineClosedEvent(RoutingContext routingContext) { final WebContext context = new WebContext(routingContext); final var eventPublisher = new EventPublisher(routingContext); - final Clients clients = create(context, client); + final var clients = create(context, client); final var itemRepository = new ItemRepository(clients); final var userRepository = new UserRepository(clients); final var loanRepository = new LoanRepository(clients, itemRepository, userRepository); diff --git a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java index 602cc0269a..60cfe5e967 100644 --- a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java +++ b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java @@ -21,6 +21,7 @@ import static org.folio.circulation.support.utils.ClockUtil.getZoneId; import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime; public class CloseLoanWithLostItemService { + private final LoanRepository loanRepository; private final LostItemPolicyRepository lostItemPolicyRepository; @@ -36,6 +37,7 @@ public class CloseLoanWithLostItemService { public CloseLoanWithLostItemService(LoanRepository loanRepository, ItemRepository itemRepository, AccountRepository accountRepository, LostItemPolicyRepository lostItemPolicyRepository, EventPublisher eventPublisher, ActualCostRecordRepository actualCostRecordRepository) { + this.loanRepository = loanRepository; this.itemRepository = itemRepository; this.accountRepository = accountRepository; @@ -53,7 +55,9 @@ public CompletableFuture> closeLoanWithLostItemFeesPaid(Loan loan) .thenCompose(r -> r.after(this::closeLoanWithLostItemFeesPaidAndPublishEvents)); } - private CompletableFuture> closeLoanWithLostItemFeesPaidAndPublishEvents(Loan loan) { + private CompletableFuture> closeLoanWithLostItemFeesPaidAndPublishEvents( + Loan loan) { + return closeLoanWithLostItemFeesPaid(loan, loanRepository, itemRepository, eventPublisher) .thenCompose(r -> r.after(eventPublisher::publishClosedLoanEvent)); } From ad237f3d6beac698656a6b743b5f888407f1b9b5 Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Tue, 12 Jul 2022 15:15:45 +0300 Subject: [PATCH 16/19] CIRC-1557 remove code smells --- .../resources/ExpiredActualCostProcessingResource.java | 3 +-- .../ActualCostRecordExpirationService.java | 10 +--------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java index 7daa9079c6..57b1b61224 100644 --- a/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java +++ b/src/main/java/org/folio/circulation/resources/ExpiredActualCostProcessingResource.java @@ -47,8 +47,7 @@ private void process(RoutingContext routingContext) { eventPublisher, actualCostRecordRepository); var loanPageableFetcher = new PageableFetcher<>(loanRepository); var actualCostRecordExpirationService = new ActualCostRecordExpirationService( - loanPageableFetcher, closeLoanWithLostItemService, itemRepository, accountRepository, - lostItemPolicyRepository, actualCostRecordRepository); + loanPageableFetcher, closeLoanWithLostItemService, itemRepository); actualCostRecordExpirationService.expireActualCostRecords() .thenApply(r -> r.map(toFixedValue(NoContentResponse::noContent))) diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index cbd21511ef..f8a8b1c34d 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -24,21 +24,13 @@ public class ActualCostRecordExpirationService { private final PageableFetcher loanPageableFetcher; private final CloseLoanWithLostItemService closeLoanWithLostItemService; private final ItemRepository itemRepository; - private final AccountRepository accountRepository; - private final LostItemPolicyRepository lostItemPolicyRepository; - private final ActualCostRecordRepository actualCostRecordRepository; public ActualCostRecordExpirationService(PageableFetcher loanPageableFetcher, - CloseLoanWithLostItemService closeLoanWithLostItemService, ItemRepository itemRepository, - AccountRepository accountRepository, LostItemPolicyRepository lostItemPolicyRepository, - ActualCostRecordRepository actualCostRecordRepository) { + CloseLoanWithLostItemService closeLoanWithLostItemService, ItemRepository itemRepository) { this.itemRepository = itemRepository; this.loanPageableFetcher = loanPageableFetcher; this.closeLoanWithLostItemService = closeLoanWithLostItemService; - this.accountRepository = accountRepository; - this.lostItemPolicyRepository = lostItemPolicyRepository; - this.actualCostRecordRepository = actualCostRecordRepository; } public CompletableFuture> expireActualCostRecords() { From 01e300c7d1e4fa9ad17f0c9320a6a54bed05aac9 Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Tue, 12 Jul 2022 15:48:40 +0300 Subject: [PATCH 17/19] CIRC-1557 remove unused imports --- .../handlers/LoanRelatedFeeFineClosedHandlerResource.java | 5 ++--- .../actualcostrecord/ActualCostRecordExpirationService.java | 3 --- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java index 1e51b6e623..c82109fa49 100644 --- a/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java +++ b/src/main/java/org/folio/circulation/resources/handlers/LoanRelatedFeeFineClosedHandlerResource.java @@ -11,6 +11,8 @@ import java.util.concurrent.CompletableFuture; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.subscribers.LoanRelatedFeeFineClosedEvent; import org.folio.circulation.infrastructure.storage.ActualCostRecordRepository; import org.folio.circulation.infrastructure.storage.feesandfines.AccountRepository; @@ -21,15 +23,12 @@ import org.folio.circulation.resources.Resource; import org.folio.circulation.services.CloseLoanWithLostItemService; import org.folio.circulation.services.EventPublisher; -import org.folio.circulation.support.Clients; import org.folio.circulation.support.RouteRegistration; import org.folio.circulation.support.http.server.NoContentResponse; import org.folio.circulation.support.http.server.ValidationError; import org.folio.circulation.support.http.server.WebContext; import org.folio.circulation.support.results.CommonFailures; import org.folio.circulation.support.results.Result; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import io.vertx.core.http.HttpClient; import io.vertx.ext.web.Router; diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java index f8a8b1c34d..9c55e81c30 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordExpirationService.java @@ -11,10 +11,7 @@ import org.folio.circulation.domain.Loan; import org.folio.circulation.domain.MultipleRecords; -import org.folio.circulation.infrastructure.storage.ActualCostRecordRepository; -import org.folio.circulation.infrastructure.storage.feesandfines.AccountRepository; import org.folio.circulation.infrastructure.storage.inventory.ItemRepository; -import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; import org.folio.circulation.services.CloseLoanWithLostItemService; import org.folio.circulation.support.fetching.PageableFetcher; import org.folio.circulation.support.http.client.CqlQuery; From 2b0ddf61dbaf5733e5eade3f150a5a7005388e16 Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Tue, 12 Jul 2022 16:44:18 +0300 Subject: [PATCH 18/19] CIRC-1557 refactoring after code review --- .../java/org/folio/circulation/domain/ActualCostRecord.java | 4 ++-- .../circulation/services/CloseLoanWithLostItemService.java | 6 +++--- .../services/actualcostrecord/ActualCostRecordService.java | 4 ++-- .../circulation/storage/mappers/ActualCostRecordMapper.java | 5 +++-- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/folio/circulation/domain/ActualCostRecord.java b/src/main/java/org/folio/circulation/domain/ActualCostRecord.java index d76d411887..0d975f8272 100644 --- a/src/main/java/org/folio/circulation/domain/ActualCostRecord.java +++ b/src/main/java/org/folio/circulation/domain/ActualCostRecord.java @@ -19,7 +19,7 @@ public class ActualCostRecord { private String userBarcode; private String loanId; private ItemLossType itemLossType; - private String dateOfLoss; + private ZonedDateTime dateOfLoss; private String title; private Collection identifiers; private String itemBarcode; @@ -31,5 +31,5 @@ public class ActualCostRecord { private String feeFineTypeId; private String feeFineType; private ZonedDateTime creationDate; - private String expirationDate; + private ZonedDateTime expirationDate; } diff --git a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java index 60cfe5e967..d1f7178485 100644 --- a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java +++ b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java @@ -1,5 +1,6 @@ package org.folio.circulation.services; +import java.time.ZonedDateTime; import java.util.concurrent.CompletableFuture; import org.folio.circulation.StoreLoanAndItem; @@ -105,10 +106,9 @@ private boolean shouldCloseLoan(Loan loan) { if (loan.getAccounts().stream().noneMatch(account -> LOST_ITEM_ACTUAL_COST_FEE_TYPE.equals(account.getFeeFineType()))) { - String expirationDate = actualCostRecord.getExpirationDate(); + ZonedDateTime expirationDate = actualCostRecord.getExpirationDate(); - return expirationDate != null && getZonedDateTime().isAfter(DateFormatUtil.parseDateTime( - expirationDate, getZoneId())); + return expirationDate != null && getZonedDateTime().isAfter(expirationDate); } else { return true; } diff --git a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordService.java b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordService.java index a24670d1e6..657a0d37f8 100644 --- a/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordService.java +++ b/src/main/java/org/folio/circulation/services/actualcostrecord/ActualCostRecordService.java @@ -89,7 +89,7 @@ private ActualCostRecord buildActualCostRecord(Loan loan, FeeFineOwner feeFineOw .withUserBarcode(loan.getUser().getBarcode()) .withLoanId(loan.getId()) .withItemLossType(itemLossType) - .withDateOfLoss(dateOfLoss.toString()) + .withDateOfLoss(dateOfLoss) .withTitle(item.getTitle()) .withIdentifiers(item.getIdentifiers().collect(Collectors.toList())) .withItemBarcode(item.getBarcode()) @@ -101,6 +101,6 @@ private ActualCostRecord buildActualCostRecord(Loan loan, FeeFineOwner feeFineOw .withFeeFineTypeId(feeFine == null ? null : feeFine.getId()) .withFeeFineType(feeFine == null ? null : feeFine.getFeeFineType()) .withExpirationDate(loan.getLostItemPolicy() - .calculateFeeFineChargingPeriodExpirationDateTime(dateOfLoss).toString()); + .calculateFeeFineChargingPeriodExpirationDateTime(dateOfLoss)); } } diff --git a/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java b/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java index 4e8c775c72..8a88af09bd 100644 --- a/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java +++ b/src/main/java/org/folio/circulation/storage/mappers/ActualCostRecordMapper.java @@ -7,6 +7,7 @@ import io.vertx.core.json.JsonObject; import static org.folio.circulation.domain.representations.CallNumberComponentsRepresentation.createCallNumberComponents; +import static org.folio.circulation.support.json.JsonPropertyFetcher.getDateTimeProperty; import static org.folio.circulation.support.json.JsonPropertyFetcher.getNestedDateTimeProperty; import static org.folio.circulation.support.json.JsonPropertyFetcher.getProperty; import static org.folio.circulation.support.json.JsonPropertyWriter.write; @@ -55,7 +56,7 @@ public static ActualCostRecord toDomain(JsonObject representation) { getProperty(representation, "userBarcode"), getProperty(representation, "loanId"), ItemLossType.from(getProperty(representation, "itemLossType")), - getProperty(representation, "dateOfLoss"), + getDateTimeProperty(representation, "dateOfLoss"), getProperty(representation, "title"), IdentifierMapper.mapIdentifiers(representation), getProperty(representation, "itemBarcode"), @@ -67,7 +68,7 @@ public static ActualCostRecord toDomain(JsonObject representation) { getProperty(representation, "feeFineTypeId"), getProperty(representation, "feeFineType"), getNestedDateTimeProperty(representation, "metadata", "createdDate"), - getProperty(representation, "expirationDate") + getDateTimeProperty(representation, "expirationDate") ); } } From 0f3057cb8c965f4fe073c9b59c447cb6fbffda4f Mon Sep 17 00:00:00 2001 From: Roman_Barannyk Date: Tue, 12 Jul 2022 16:56:37 +0300 Subject: [PATCH 19/19] CIRC-1557 remove unused import --- .../services/CloseLoanWithLostItemService.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java index d1f7178485..31c799ec53 100644 --- a/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java +++ b/src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java @@ -1,5 +1,11 @@ package org.folio.circulation.services; +import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.folio.circulation.domain.FeeFine.LOST_ITEM_ACTUAL_COST_FEE_TYPE; +import static org.folio.circulation.domain.FeeFine.lostItemFeeTypes; +import static org.folio.circulation.support.results.Result.succeeded; +import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime; + import java.time.ZonedDateTime; import java.util.concurrent.CompletableFuture; @@ -13,14 +19,6 @@ import org.folio.circulation.infrastructure.storage.loans.LoanRepository; import org.folio.circulation.infrastructure.storage.loans.LostItemPolicyRepository; import org.folio.circulation.support.results.Result; -import org.folio.circulation.support.utils.DateFormatUtil; - -import static java.util.concurrent.CompletableFuture.completedFuture; -import static org.folio.circulation.domain.FeeFine.LOST_ITEM_ACTUAL_COST_FEE_TYPE; -import static org.folio.circulation.domain.FeeFine.lostItemFeeTypes; -import static org.folio.circulation.support.results.Result.*; -import static org.folio.circulation.support.utils.ClockUtil.getZoneId; -import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime; public class CloseLoanWithLostItemService { private final LoanRepository loanRepository;