Merge pull request CIRC-2184 disable floating collections by default

README.md

@@ -103,6 +103,14 @@ be configured using the following environment variables:
 If a variable is not present, its default values is used as a fallback. If this configuration is
 invalid, the module will start, but Kafka integration will not work.
 
+Module supports so-called floating collections but the feature is disabled by default. Floating
+collections support can be switched on by setting the environment variable ENABLE_FLOATING_COLLECTIONS to TRUE.
+
+| Variable name               | Default value     |
+|-----------------------------|-------------------|
+| ENABLE_FLOATING_COLLECTIONS | FALSE             |
+
+
 ## Design Notes
 
 ### Known Limitations

src/main/java/org/folio/Environment.java

@@ -1,9 +1,13 @@
 package org.folio;
 
+import static java.lang.Boolean.parseBoolean;
 import static java.lang.Integer.parseInt;
 import static org.apache.commons.lang.StringUtils.isBlank;
 
 import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -17,8 +21,12 @@ public static int getScheduledAnonymizationNumberOfLoansToCheck() {
     return getVariable("SCHEDULED_ANONYMIZATION_NUMBER_OF_LOANS_TO_CHECK", 50000);
   }
 
+  public static boolean getEnableFloatingCollections() {
+    return getVariable("ENABLE_FLOATING_COLLECTIONS", false);
+  }
+
   private static int getVariable(String key, int defaultValue) {
-    final var variable = System.getenv().get(key);
+    final var variable = getVar(key);
 
     if (isBlank(variable)) {
       return defaultValue;
@@ -33,4 +41,14 @@ private static int getVariable(String key, int defaultValue) {
       return defaultValue;
     }
   }
+
+  private static boolean getVariable(String key, Boolean defaultValue) {
+      return parseBoolean(Objects.toString(getVar(key),defaultValue.toString()));
+  }
+
+  private static String getVar(String key) {
+    return MOCK_ENV.containsKey(key) ? MOCK_ENV.get(key) : System.getenv().get(key);
+  }
+  // Mock environment variables for unit testing.
+  public static final Map<String,String> MOCK_ENV = new HashMap<>();
 }

src/main/java/org/folio/circulation/domain/Item.java

@@ -20,6 +20,7 @@
 import java.util.UUID;
 import java.util.stream.Stream;
 
+import org.folio.circulation.resources.CheckInByBarcodeResource;
 import org.folio.circulation.storage.mappers.ItemMapper;
 
 import io.vertx.core.json.JsonObject;
@@ -357,7 +358,7 @@ public String getFloatDestinationLocationId() {
   }
 
   public boolean canFloatThroughCheckInServicePoint() {
-    return getLocation() != null
+    return CheckInByBarcodeResource.isFloatingEnabled() && getLocation() != null
       && getLocation().isFloatingCollection()
       && getFloatDestinationLocation() != null
       && getFloatDestinationLocation().getId()  != null;

src/main/java/org/folio/circulation/resources/CheckInByBarcodeResource.java

@@ -8,6 +8,7 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.folio.Environment;
 import org.folio.circulation.domain.CheckInContext;
 import org.folio.circulation.domain.Item;
 import org.folio.circulation.domain.notice.schedule.RequestScheduledNoticeService;
@@ -149,4 +150,8 @@ private ValidationErrorFailure errorWhenInIncorrectStatus(Item item) {
 
     return singleValidationError(message, ITEM_BARCODE, item.getBarcode());
   }
+  public static boolean isFloatingEnabled() {
+    return Environment.getEnableFloatingCollections();
+  }
+
 }

src/main/java/org/folio/circulation/resources/CheckInProcessAdapter.java

@@ -306,7 +306,7 @@ CompletableFuture<Result<CheckInContext>> findFulfillableRequest(CheckInContext
 
   CompletableFuture<Result<Item>> findFloatingDestination(CheckInContext context) {
     Item item = context.getItem();
-    if (item.getLocation().isFloatingCollection()) {
+    if (CheckInByBarcodeResource.isFloatingEnabled() && item.getLocation().isFloatingCollection()) {
       return locationRepository.fetchLocationsForServicePoint(context.getCheckInServicePointId().toString())
         .thenApply(rLocations -> rLocations.map(locations -> locations.stream()
           .filter(Location::isFloatingCollection).findFirst()

src/test/java/api/loans/ReminderFeeTests.java

@@ -823,7 +823,6 @@ void willResetRemindersRescheduleNoticeOnManualDueDateChange() {
     JsonObject loanAfterReminder = loansClient.getById(UUID.fromString(loan.getString("id"))).getJson();
     assertThat("loan should have first reminder", loanAfterReminder.encode(),
       hasJsonPath("reminders.lastFeeBilled.number", is(1)));
-    System.out.println(loanAfterReminder.encodePrettily());
 
     ChangeDueDateRequestBuilder changeDueDate =
       new ChangeDueDateRequestBuilder()
@@ -833,7 +832,6 @@ void willResetRemindersRescheduleNoticeOnManualDueDateChange() {
     waitAtMost(1, SECONDS).until(scheduledNoticesClient::getAll, hasSize(1));
     JsonObject rescheduledNotice = scheduledNoticesClient.getAll().get(0);
     JsonObject loanAfterDueDateChange = loansClient.getById(UUID.fromString(loan.getString("id"))).getJson();
-    System.out.println("loan after due date change: " + loanAfterDueDateChange.encodePrettily());
     assertThat("Loan should have no reminders after due date change",
       !loanAfterDueDateChange.containsKey("reminders"));
     assertThat("rescheduled reminder should have different runtime than previous, second reminder",

src/test/java/api/loans/scenarios/FloatingItemsTests.java

@@ -8,9 +8,11 @@
 import api.support.http.IndividualResource;
 import api.support.http.ItemResource;
 import io.vertx.core.json.JsonObject;
+import org.folio.Environment;
 import org.folio.circulation.support.utils.ClockUtil;
 import org.hamcrest.CoreMatchers;
 import org.hamcrest.core.IsNull;
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import java.util.UUID;
@@ -20,8 +22,17 @@
 
 public class FloatingItemsTests extends APITests {
 
+  private static final String ENV_VAR_ENABLE_FLOATING_COLLECTIONS = "ENABLE_FLOATING_COLLECTIONS";
+
+  // Floating logic only applies when explicitly enabled through system env var.
+  @BeforeEach
+  void resetEnvVar() {
+    Environment.MOCK_ENV.remove(ENV_VAR_ENABLE_FLOATING_COLLECTIONS);
+  }
+
   @Test
   void willSetFloatingItemsTemporaryLocationToFloatingCollectionAtServicePoint() {
+    Environment.MOCK_ENV.put(ENV_VAR_ENABLE_FLOATING_COLLECTIONS,"TRUE");
 
     // Floating collection served by service point 'cd1'.
     final IndividualResource floatingCollection = locationsFixture.floatingCollection();
@@ -114,8 +125,104 @@ void willSetFloatingItemsTemporaryLocationToFloatingCollectionAtServicePoint() {
 
   }
 
+  @Test
+  void willNotApplyFloatingIfFloatingNotExplicitlyEnabled() {
+    Environment.MOCK_ENV.remove(ENV_VAR_ENABLE_FLOATING_COLLECTIONS);
+
+    // Floating collection served by service point 'cd1'.
+    final IndividualResource floatingCollection = locationsFixture.floatingCollection();
+
+    // Another floating collection serviced by another service point
+    final IndividualResource servicePointTwo = servicePointsFixture.cd2();
+    locationsFixture.createLocation(
+      new LocationBuilder()
+        .withName("Floating collection 2")
+        .forInstitution(UUID.randomUUID())
+        .forCampus(UUID.randomUUID())
+        .forLibrary(UUID.randomUUID())
+        .withCode("FLOAT2")
+        .isFloatingCollection(true)
+        .withPrimaryServicePoint(servicePointTwo.getId()));
+
+    final IndividualResource james = usersFixture.james();
+
+    final IndividualResource holdingsInFloatingLocation =
+      holdingsFixture.createHoldingsRecord(UUID.randomUUID(), floatingCollection.getId());
+
+    IndividualResource nod = itemsClient.create(ItemExamples.basedUponNod(
+        materialTypesFixture.book().getId(),
+        loanTypesFixture.canCirculate().getId())
+      .withBarcode("565578437802")
+      .forHolding(holdingsInFloatingLocation.getId()));
+
+    final IndividualResource loan = checkOutFixture.checkOutByBarcode(nod, james);
+
+    final CheckInByBarcodeResponse checkInResponse = checkInFixture.checkInByBarcode(
+      new CheckInByBarcodeRequestBuilder().forItem(nod).at(servicePointTwo.getId()));
+
+    JsonObject itemRepresentation = checkInResponse.getItem();
+
+    assertThat("item should be present in response",
+      itemRepresentation, IsNull.notNullValue());
+
+    assertThat("ID should be included for item",
+      itemRepresentation.getString("id"), is(nod.getId()));
+
+    assertThat("barcode should be included for item",
+      itemRepresentation.getString("barcode"), CoreMatchers.is("565578437802"));
+
+    assertThat("item status should be 'In transit'",
+      itemRepresentation.getJsonObject("status").getString("name"), CoreMatchers.is("In transit"));
+
+    assertThat("item should have a destination",
+      itemRepresentation.containsKey("inTransitDestinationServicePointId"),
+      CoreMatchers.is(true));
+
+    assertThat( "The check-in response should display the item's original location.",
+      itemRepresentation.getJsonObject("location").getString("name"), CoreMatchers.is("Floating collection"));
+
+    JsonObject staffSlipContext = checkInResponse.getStaffSlipContext();
+
+    assertThat( "The staff slip context should display the item's original location.",
+      staffSlipContext.getJsonObject("item").getString("effectiveLocationSpecific"), CoreMatchers.is("Floating collection"));
+
+    JsonObject loanRepresentation = checkInResponse.getLoan();
+
+    assertThat("closed loan should be present in response",
+      loanRepresentation, IsNull.notNullValue());
+
+    assertThat("item (in loan) should have a destination",
+      loanRepresentation.getJsonObject("item")
+        .containsKey("inTransitDestinationServicePointId"), CoreMatchers.is(true));
+
+    JsonObject updatedNod = itemsClient.getById(nod.getId()).getJson();
+
+    assertThat("stored item status should be 'In transit'",
+      updatedNod.getJsonObject("status").getString("name"), CoreMatchers.is("In transit"));
+
+    assertThat("item in storage should have a destination",
+      updatedNod.containsKey("inTransitDestinationServicePointId"), CoreMatchers.is(true));
+
+    assertThat("item's temporary location is not set",
+      updatedNod.getString("temporaryLocationId"), CoreMatchers.nullValue(String.class));
+
+    final JsonObject storedLoan = loansStorageClient.getById(loan.getId()).getJson();
+
+    assertThat("stored loan status should be 'Closed'",
+      storedLoan.getJsonObject("status").getString("name"), CoreMatchers.is("Closed"));
+
+    assertThat("item status snapshot in storage should be 'In transit'",
+      storedLoan.getString("itemStatus"), CoreMatchers.is("In transit"));
+
+    assertThat("Checkin Service Point Id should be stored",
+      storedLoan.getString("checkinServicePointId"), is(servicePointTwo.getId()));
+
+  }
+
   @Test
   void willPutFloatingItemInTransitWhenCheckInServicePointServesNoFloatingCollection() {
+    Environment.MOCK_ENV.put(ENV_VAR_ENABLE_FLOATING_COLLECTIONS,"TRUE");
+
     final IndividualResource floatingCollection = locationsFixture.floatingCollection();
 
     final IndividualResource servicePointTwo = servicePointsFixture.cd2();
@@ -151,8 +258,6 @@ void willPutFloatingItemInTransitWhenCheckInServicePointServesNoFloatingCollecti
 
     JsonObject itemRepresentation = checkInResponse.getItem();
 
-    System.out.println(itemsClient.get(nod).getJson().encodePrettily());
-
     assertThat("item should be present in response",
       itemRepresentation, IsNull.notNullValue());
 
@@ -201,6 +306,8 @@ void willPutFloatingItemInTransitWhenCheckInServicePointServesNoFloatingCollecti
 
   @Test
   void willPutFloatingItemInTransitWhenHoldRequestWasIssuedAtDifferentServicePoint() {
+    Environment.MOCK_ENV.put(ENV_VAR_ENABLE_FLOATING_COLLECTIONS,"TRUE");
+
     final IndividualResource floatingCollection = locationsFixture.floatingCollection();
 
     final IndividualResource servicePointTwo = servicePointsFixture.cd3();
@@ -257,8 +364,6 @@ void willPutFloatingItemInTransitWhenHoldRequestWasIssuedAtDifferentServicePoint
 
     JsonObject itemRepresentation = checkInResponse.getItem();
 
-    System.out.println(itemsClient.get(nod).getJson().encodePrettily());
-
     assertThat("item should be present in response",
       itemRepresentation, IsNull.notNullValue());
 
@@ -323,6 +428,8 @@ void willPutFloatingItemInTransitWhenHoldRequestWasIssuedAtDifferentServicePoint
 
   @Test
   void willPutItemInTransitIfFloatingLocationWasOverriddenByNonFloatingLocation() {
+    Environment.MOCK_ENV.put(ENV_VAR_ENABLE_FLOATING_COLLECTIONS,"TRUE");
+
     // floating collection served by service point cd1.
     final IndividualResource floatingCollection = locationsFixture.floatingCollection();
 
@@ -372,9 +479,7 @@ void willPutItemInTransitIfFloatingLocationWasOverriddenByNonFloatingLocation()
 
     JsonObject itemRepresentation = checkInResponse.getItem();
 
-    System.out.println(itemsClient.get(nod).getJson().encodePrettily());
-
-    assertThat("item should be present in response",
+    assertThat("item should be psetresent in response",
       itemRepresentation, IsNull.notNullValue());
 
     assertThat("ID should be included for item",