Merge pull request #1461 from folio-org/tmp-release-24.2.2

[CIRC-2074] Release 24.2.2

NEWS.md

@@ -1,3 +1,8 @@
+## 24.2.2 2024-04-19
+
+* Fix alternate loan period applying when multiple requests in fulfillment in progress (CIRC-2026)
+* Fix issue with closing a fee/fine for a previous loan if item is checked out again and lost (CIRC-2066)
+
 ## 24.2.1 2024-03-29
 
 * Fix item details not fully populated when response contains more than 50 loans (CIRC-2059)

pom.xml

@@ -2,7 +2,7 @@
   <modelVersion>4.0.0</modelVersion>
   <artifactId>mod-circulation</artifactId>
   <groupId>org.folio</groupId>
-  <version>24.2.2-SNAPSHOT</version>
+  <version>24.2.3-SNAPSHOT</version>
   <licenses>
     <license>
       <name>Apache License 2.0</name>

src/main/java/org/folio/circulation/domain/policy/LoanPolicy.java

@@ -21,7 +21,6 @@
 
 import org.apache.commons.lang3.StringUtils;
 import org.folio.circulation.domain.Loan;
-import org.folio.circulation.domain.Request;
 import org.folio.circulation.domain.RequestQueue;
 import org.folio.circulation.domain.RequestStatus;
 import org.folio.circulation.domain.RequestType;
@@ -88,7 +87,8 @@ public static LoanPolicy unknown(String id) {
 
   public Result<ZonedDateTime> calculateInitialDueDate(Loan loan, RequestQueue requestQueue) {
     final ZonedDateTime systemTime = ClockUtil.getZonedDateTime();
-    return determineStrategy(requestQueue, false, false, systemTime).calculateDueDate(loan);
+    return determineStrategy(requestQueue, false, false, systemTime, loan.getItemId())
+      .calculateDueDate(loan);
   }
 
   public boolean hasRenewalPeriod() {
@@ -139,11 +139,8 @@ public Integer getRenewalLimit() {
     return getIntegerProperty(getRenewalsPolicy(), "numberAllowed", 0);
   }
 
-  public DueDateStrategy determineStrategy(
-    RequestQueue requestQueue,
-    boolean isRenewal,
-    boolean isRenewalWithHoldRequest,
-    ZonedDateTime systemDate) {
+  public DueDateStrategy determineStrategy(RequestQueue requestQueue, boolean isRenewal,
+    boolean isRenewalWithHoldRequest, ZonedDateTime systemDate, String itemId) {
 
     final JsonObject loansPolicy = getLoansPolicy();
     final JsonObject renewalsPolicy = getRenewalsPolicy();
@@ -163,7 +160,7 @@ systemDate, getRenewFrom(), getRenewalPeriod(loansPolicy, renewalsPolicy, isRene
       else {
         boolean useAlternatePeriod = false;
         Period rollingPeriod = getPeriod(loansPolicy);
-        if(isAlternatePeriod(requestQueue)) {
+        if (isAlternatePeriod(requestQueue, itemId)) {
           rollingPeriod = getPeriod(holds, ALTERNATE_CHECKOUT_LOAN_PERIOD_KEY);
           useAlternatePeriod = true;
         }
@@ -178,7 +175,7 @@ else if(isFixed(loansPolicy)) {
           getRenewalFixedDueDateSchedules(), systemDate, this::loanPolicyValidationError);
       }
       else {
-        if(isAlternatePeriod(requestQueue)) {
+        if (isAlternatePeriod(requestQueue, itemId)) {
           return new RollingCheckOutDueDateStrategy(getId(), getName(),
             getPeriod(holds, ALTERNATE_CHECKOUT_LOAN_PERIOD_KEY),
               fixedDueDateSchedules, this::loanPolicyValidationError, false);
@@ -199,23 +196,17 @@ private ValidationError loanPolicyValidationError(String message) {
     return RenewalValidator.loanPolicyValidationError(this, message);
   }
 
-  private boolean isAlternatePeriod(RequestQueue requestQueue) {
-    final JsonObject holds = getHolds();
-    if(Objects.isNull(requestQueue)
-      || !holds.containsKey(ALTERNATE_CHECKOUT_LOAN_PERIOD_KEY)) {
+  private boolean isAlternatePeriod(RequestQueue requestQueue, String itemId) {
+    if (Objects.isNull(requestQueue) || !getHolds().containsKey(
+      ALTERNATE_CHECKOUT_LOAN_PERIOD_KEY)) {
+
       return false;
     }
-    Optional<Request> potentialRequest = requestQueue.getRequests().stream().skip(1).findFirst();
-    boolean isAlternateDueDateSchedule = false;
-    if(potentialRequest.isPresent()) {
-      Request request = potentialRequest.get();
-      boolean isHold = request.getRequestType() == RequestType.HOLD;
-      boolean isOpenNotYetFilled = request.getStatus() == RequestStatus.OPEN_NOT_YET_FILLED;
-      if(isHold && isOpenNotYetFilled) {
-        isAlternateDueDateSchedule = true;
-      }
-    }
-    return isAlternateDueDateSchedule;
+
+    return requestQueue.getRequests().stream()
+      .anyMatch(request -> request.getRequestType() == RequestType.HOLD &&
+        request.getStatus() == RequestStatus.OPEN_NOT_YET_FILLED &&
+        (!request.hasItem() || itemId.equals(request.getItemId())));
   }
 
   private JsonObject getLoansPolicy() {

src/main/java/org/folio/circulation/resources/renewal/RenewalResource.java

@@ -537,7 +537,8 @@ private Result<ZonedDateTime> calculateNewDueDate(ZonedDateTime overrideDueDate,
 
   private Result<ZonedDateTime> calculateProposedDueDate(Loan loan, ZonedDateTime systemDate) {
     return loan.getLoanPolicy()
-      .determineStrategy(null, true, false, systemDate).calculateDueDate(loan);
+      .determineStrategy(null, true, false, systemDate, loan.getItemId())
+      .calculateDueDate(loan);
   }
 
   private boolean newDueDateAfterCurrentDueDate(Loan loan, Result<ZonedDateTime> proposedDueDateResult) {
@@ -665,7 +666,7 @@ private Result<ZonedDateTime> calculateNewDueDate(Loan loan, RequestQueue reques
     final var loanPolicy = loan.getLoanPolicy();
     final var isRenewalWithHoldRequest = firstRequestForLoanedItemIsHold(requestQueue, loan);
 
-    return loanPolicy.determineStrategy(null, true, isRenewalWithHoldRequest, systemDate)
+    return loanPolicy.determineStrategy(null, true, isRenewalWithHoldRequest, systemDate, loan.getItemId())
       .calculateDueDate(loan);
   }
 

src/main/java/org/folio/circulation/services/CloseLoanWithLostItemService.java

@@ -49,7 +49,7 @@ public CloseLoanWithLostItemService(LoanRepository loanRepository, ItemRepositor
   }
 
   public CompletableFuture<Result<Void>> closeLoanAsLostAndPaid(Loan loan) {
-    if (loan == null || !loan.isItemLost()) {
+    if (loan == null || loan.isClosed() || !loan.isItemLost()) {
       return emptyAsync();
     }
 

src/test/java/api/handlers/CloseDeclaredLostLoanWhenLostItemFeesAreClosedApiTests.java

@@ -241,14 +241,8 @@ void shouldIgnoreErrorWhenNonExistentLoanIdProvided() {
   void shouldNotPublishLoanClosedEventWhenLoanIsOriginallyClosed() {
     feeFineAccountFixture.payLostItemFee(loan.getId());
     feeFineAccountFixture.payLostItemProcessingFee(loan.getId());
-
-    JsonObject loanToClose = loansStorageClient.get(loan).getJson();
-    loanToClose.getJsonObject("status").put("name", "Closed");
-    loansStorageClient.replace(loan.getId(), loanToClose);
-
+    checkInFixture.checkInByBarcode(item);
     eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(loan.getId());
-    assertThatLoanIsClosedAsLostAndPaid();
-
     assertThat(getPublishedEventsAsList(byEventType(LOAN_CLOSED)), empty());
   }
 

src/test/java/api/loans/agetolost/ScheduledAgeToLostFeeChargingApiTest.java

@@ -2,9 +2,13 @@
 
 import static api.support.PubsubPublisherTestUtils.assertThatPublishedLoanLogRecordEventsAreValid;
 import static api.support.builders.DeclareItemLostRequestBuilder.forLoan;
+import static api.support.http.CqlQuery.queryFromTemplate;
 import static api.support.matchers.AccountMatchers.isOpen;
+import static api.support.matchers.AccountMatchers.isPaidFully;
 import static api.support.matchers.ActualCostRecordMatchers.isActualCostRecord;
 import static api.support.matchers.ItemMatchers.isAgedToLost;
+import static api.support.matchers.ItemMatchers.isAvailable;
+import static api.support.matchers.ItemMatchers.isCheckedOut;
 import static api.support.matchers.ItemMatchers.isDeclaredLost;
 import static api.support.matchers.ItemMatchers.isLostAndPaid;
 import static api.support.matchers.JsonObjectMatcher.hasJsonPath;
@@ -25,6 +29,7 @@
 import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.allOf;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.iterableWithSize;
@@ -40,10 +45,12 @@
 import org.awaitility.Awaitility;
 import org.folio.circulation.domain.ItemLossType;
 import org.folio.circulation.domain.policy.Period;
+import org.folio.circulation.domain.policy.lostitem.ChargeAmountType;
 import org.hamcrest.Matcher;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
+import api.support.MultipleJsonRecords;
 import api.support.builders.FeeFineOwnerBuilder;
 import api.support.builders.ItemBuilder;
 import api.support.builders.LostItemFeePolicyBuilder;
@@ -617,6 +624,77 @@ void shouldNotScheduleNoticeIfFeeFineBalanceChangedEventWithoutLoanId() {
     assertThat(scheduledNoticesClient.getAll(), hasSize(0));
   }
 
+  @Test
+  void closingLostItemFeeDoesNotAffectMoreRecentLoanForSameItem() {
+    ItemResource item = itemsFixture.basedUponNod();
+    UUID itemId = item.getId();
+    UserResource firstPatron = usersFixture.steve();
+    UserResource secondPatron = usersFixture.james();
+
+    policiesActivation.use(PoliciesToActivate.builder().lostItemPolicy(
+      lostItemFeePoliciesFixture.create(
+        new LostItemFeePolicyBuilder()
+          .withName("Processing fee only, no refund on check-in")
+          .withItemAgedToLostAfterOverdue(Period.minutes(1))
+          .withPatronBilledAfterItemAgedToLost(Period.minutes(1))
+          .withNoChargeAmountItem()
+          .withLostItemProcessingFee(1.0)
+          .withChargeAmountItemPatron(true)
+          .doNotRefundProcessingFeeWhenReturned()
+      )));
+
+    // create first loan and declare item lost
+    CheckOutResource firstLoan = checkOutFixture.checkOutByBarcode(item, firstPatron);
+    UUID firstLoanId = firstLoan.getId();
+    assertThat(itemsFixture.getById(itemId).getJson(), isCheckedOut());
+    declareLostFixtures.declareItemLost(firstLoanId);
+
+    assertThat(feeFineAccountFixture.getAccounts(firstLoanId), hasSize(1));
+    assertThat(firstLoan, hasLostItemProcessingFee(isOpen(1.0)));
+    assertThat(itemsFixture.getById(itemId).getJson(), isDeclaredLost());
+    assertThat(loansFixture.getLoanById(firstLoanId).getJson(), isOpen());
+
+    // check in first loan
+    checkInFixture.checkInByBarcode(item);
+    int firstLoanHistorySizeAfterCheckIn = getLoanHistory(firstLoanId).size();
+
+    assertThat(feeFineAccountFixture.getAccounts(firstLoanId), hasSize(1));
+    assertThat(firstLoan, hasLostItemProcessingFee(isOpen(1.0)));
+    assertThat(itemsFixture.getById(itemId).getJson(), isAvailable());
+    assertThat(loansFixture.getLoanById(firstLoanId).getJson(), isClosed());
+
+    // create second loan and declare item lost
+    CheckOutResource secondLoan = checkOutFixture.checkOutByBarcode(item, secondPatron);
+    UUID secondLoanId = secondLoan.getId();
+    assertThat(itemsFixture.getById(itemId).getJson(), isCheckedOut());
+    declareLostFixtures.declareItemLost(secondLoanId);
+
+    assertThat(feeFineAccountFixture.getAccounts(secondLoanId), hasSize(1));
+    assertThat(secondLoan, hasLostItemProcessingFee(isOpen(1.0)));
+    assertThat(itemsFixture.getById(itemId).getJson(), isDeclaredLost());
+    assertThat(loansFixture.getLoanById(firstLoanId).getJson(), isClosed());
+    assertThat(loansFixture.getLoanById(secondLoanId).getJson(), isOpen());
+
+    // pay lost item fee for first loan
+    feeFineAccountFixture.payLostItemProcessingFee(firstLoanId);
+    eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(firstLoanId);
+
+    assertThat(getLoanHistory(firstLoanId).size(), equalTo(firstLoanHistorySizeAfterCheckIn));
+    assertThat(firstLoan, hasLostItemProcessingFee(allOf(isClosed(), isPaidFully())));
+    assertThat(itemsFixture.getById(itemId).getJson(), isDeclaredLost());
+    assertThat(loansFixture.getLoanById(firstLoanId).getJson(), isClosed());
+    assertThat(loansFixture.getLoanById(secondLoanId).getJson(), isOpen());
+
+    // pay lost item fee for second loan
+    feeFineAccountFixture.payLostItemProcessingFee(secondLoanId);
+    eventSubscribersFixture.publishLoanRelatedFeeFineClosedEvent(secondLoanId);
+
+    assertThat(secondLoan, hasLostItemProcessingFee(allOf(isClosed(), isPaidFully())));
+    assertThat(itemsFixture.getById(itemId).getJson(), isLostAndPaid());
+    assertThat(loansFixture.getLoanById(firstLoanId).getJson(), isClosed());
+    assertThat(loansFixture.getLoanById(secondLoanId).getJson(), isClosed());
+  }
+
   private Map<IndividualResource, Double> checkoutTenItems() {
     val loanToFeeMap = new LinkedHashMap<IndividualResource, Double>();
 
@@ -681,4 +759,9 @@ private NoticePolicyBuilder createNoticePolicyWithAgedToLostChargedNotice() {
           .withUponAtTiming()
           .create()));
   }
+
+  private MultipleJsonRecords getLoanHistory(UUID loanId) {
+    return loanHistoryClient.getMany(
+      queryFromTemplate("loan.id=\"%s\" sortBy createdDate/sort.descending", loanId));
+  }
 }