Modify bind pieces endpoint to accept holdingId and locationId (#959)

* Modify bind pieces endpoint to accept holdingId and locationId

* Fix tests

* Remove unnecessary test

* Add testcase for holding creation

* Remove unused import

* Update acq-models

src/main/java/org/folio/helper/BindHelper.java

@@ -16,8 +16,6 @@
 import org.folio.rest.jaxrs.model.BindPiecesCollection;
 import org.folio.rest.jaxrs.model.BindPiecesResult;
 import org.folio.rest.jaxrs.model.CompositePoLine;
-import org.folio.rest.jaxrs.model.Error;
-import org.folio.rest.jaxrs.model.Parameter;
 import org.folio.rest.jaxrs.model.Piece;
 import org.folio.rest.jaxrs.model.ReceivedItem;
 import org.folio.rest.jaxrs.model.Title;
@@ -167,14 +165,10 @@ private Future<BindPiecesHolder> updateItemStatus(BindPiecesHolder holder, Reque
   private Future<BindPiecesHolder> createItemForPieces(BindPiecesHolder holder, RequestContext requestContext) {
     var bindPiecesCollection = holder.getBindPiecesCollection();
     var poLineId = holder.getPoLineId();
-    var holdingIds = holder.getPieces()
-      .map(Piece::getHoldingId).distinct().toList();
-    validateHoldingIds(holdingIds, bindPiecesCollection);
     logger.debug("createItemForPiece:: Trying to get poLine by id '{}'", poLineId);
-
     return purchaseOrderLineService.getOrderLineById(poLineId, requestContext)
       .map(PoLineCommonUtil::convertToCompositePoLine)
-      .compose(compPOL -> createInventoryObjects(compPOL, holdingIds.get(0), bindPiecesCollection.getBindItem(), requestContext))
+      .compose(compPOL -> createInventoryObjects(compPOL, bindPiecesCollection.getBindItem(), requestContext))
       .map(newItemId -> {
         // Move requests if requestsAction is TRANSFER, otherwise do nothing
         if (TRANSFER.equals(bindPiecesCollection.getRequestsAction())) {
@@ -187,32 +181,27 @@ private Future<BindPiecesHolder> createItemForPieces(BindPiecesHolder holder, Re
       });
   }
 
-  private void validateHoldingIds(List<String> holdingIds, BindPiecesCollection bindPiecesCollection) {
-    if (holdingIds.size() != 1) {
-      var holdingParam = new Parameter().withKey("holdingIds").withValue(holdingIds.toString());
-      var pieceParam = new Parameter().withKey("pieceIds").withValue(bindPiecesCollection.getBindPieceIds().toString());
-      var error = new Error().withParameters(List.of(holdingParam, pieceParam))
-        .withMessage("Holding Id must not be null or different for pieces");
-      throw new HttpException(400, error);
-    }
-  }
-
-  private Future<String> createInventoryObjects(CompositePoLine compPOL, String holdingId, BindItem bindItem, RequestContext requestContext) {
+  private Future<String> createInventoryObjects(CompositePoLine compPOL, BindItem bindItem, RequestContext requestContext) {
     var locationContext = createContextWithNewTenantId(requestContext, bindItem.getTenantId());
-    return createShadowInstanceAndHoldingIfNeeded(compPOL, bindItem, holdingId, locationContext, requestContext)
-      .compose(targetHoldingId -> inventoryItemManager.createBindItem(compPOL, targetHoldingId, bindItem, locationContext));
+    return handleInstance(compPOL.getInstanceId(), bindItem.getTenantId(), locationContext, requestContext)
+      .compose(instanceId -> handleHolding(bindItem, instanceId, locationContext))
+      .compose(holdingId -> inventoryItemManager.createBindItem(compPOL, holdingId, bindItem, locationContext));
   }
 
-  private Future<String> createShadowInstanceAndHoldingIfNeeded(CompositePoLine compPOL, BindItem bindItem, String holdingId,
-                                                                RequestContext locationContext, RequestContext requestContext) {
-    // No need to create inventory objects if BindItem tenantId is not different
-    var targetTenantId = bindItem.getTenantId();
+  private Future<String> handleInstance(String instanceId, String targetTenantId,
+                                      RequestContext locationContext, RequestContext requestContext) {
     if (StringUtils.isEmpty(targetTenantId) || targetTenantId.equals(TenantTool.tenantId(requestContext.getHeaders()))) {
-      return Future.succeededFuture(holdingId);
+      return Future.succeededFuture(instanceId);
     }
-    var instanceId = compPOL.getInstanceId();
     return inventoryInstanceManager.createShadowInstanceIfNeeded(instanceId, locationContext)
-      .compose(s -> inventoryHoldingManager.createHoldingAndReturnId(instanceId, bindItem.getPermanentLocationId(), locationContext));
+      .map(s -> instanceId);
+  }
+
+  private Future<String> handleHolding(BindItem bindItem, String instanceId, RequestContext locationContext) {
+    if (bindItem.getHoldingId() != null) {
+      return Future.succeededFuture(bindItem.getHoldingId());
+    }
+    return inventoryHoldingManager.createHoldingAndReturnId(instanceId, bindItem.getLocationId(), locationContext);
   }
 
   private Future<BindPiecesHolder> storeUpdatedPieces(BindPiecesHolder holder, RequestContext requestContext) {

src/main/java/org/folio/rest/core/exceptions/ErrorCodes.java

@@ -126,6 +126,7 @@ public enum ErrorCodes {
   ORDER_FORMAT_INCORRECT_FOR_BINDARY_ACTIVE("orderFormatIncorrectForBindaryActive", "When PoLine is bindery active, its format must be 'P/E Mix' or 'Physical Resource'"),
   CREATE_INVENTORY_INCORRECT_FOR_BINDARY_ACTIVE("createInventoryIncorrectForBindaryActive", "When PoLine is bindery active, Create Inventory must be 'Instance, Holding, Item'"),
   RECEIVING_WORKFLOW_INCORRECT_FOR_BINDARY_ACTIVE("receivingWorkflowIncorrectForBindaryActive", "When PoLine is bindery active, its receiving workflow must be set to 'Independent order and receipt quantity'"),
+  BIND_ITEM_MUST_INCLUDE_EITHER_HOLDING_ID_OR_LOCATION_ID("bindItemMustIncludeEitherHoldingIdOrLocationId", "During binding pieces, the bindItem object must have either holdingId or locationId field populated"),
   BUDGET_NOT_FOUND_FOR_FISCAL_YEAR("budgetNotFoundForFiscalYear", "Could not find an active budget for a fund with the current fiscal year of another fund in the fund distribution");
 
   private final String code;

src/main/java/org/folio/rest/impl/ReceivingAPI.java

@@ -2,18 +2,22 @@
 
 import static io.vertx.core.Future.succeededFuture;
 import static org.folio.orders.utils.HelperUtils.handleErrorResponse;
+import static org.folio.rest.core.exceptions.ErrorCodes.BIND_ITEM_MUST_INCLUDE_EITHER_HOLDING_ID_OR_LOCATION_ID;
 
 import java.util.Map;
 
 import javax.ws.rs.core.Response;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.folio.helper.BindHelper;
 import org.folio.helper.CheckinHelper;
 import org.folio.helper.ExpectHelper;
 import org.folio.helper.ReceivingHelper;
+import org.folio.rest.RestConstants;
 import org.folio.rest.annotations.Validate;
+import org.folio.rest.core.exceptions.HttpException;
 import org.folio.rest.core.models.RequestContext;
 import org.folio.rest.jaxrs.model.BindPiecesCollection;
 import org.folio.rest.jaxrs.model.CheckinCollection;
@@ -68,6 +72,7 @@ public void postOrdersExpect(ExpectCollection entity, Map<String, String> okapiH
   @Override
   public void postOrdersBindPieces(BindPiecesCollection entity, Map<String, String> okapiHeaders, Handler<AsyncResult<Response>> asyncResultHandler, Context vertxContext) {
     logger.info("Bind {} pieces", entity.getBindPieceIds());
+    validateRequiredFields(entity);
     BindHelper helper = new BindHelper(entity, okapiHeaders, vertxContext);
     helper.bindPieces(entity, new RequestContext(vertxContext, okapiHeaders))
       .onSuccess(result -> asyncResultHandler.handle(succeededFuture(helper.buildOkResponse(result))))
@@ -90,4 +95,16 @@ public void getOrdersReceivingHistory(String totalRecords, int offset, int limit
       })
       .onFailure(t -> handleErrorResponse(asyncResultHandler, helper, t));
   }
+
+  private void validateRequiredFields(BindPiecesCollection bindPiecesCollection) {
+    var bindItem = bindPiecesCollection.getBindItem();
+    var isHoldingIdPresent = !StringUtils.isEmpty(bindItem.getHoldingId());
+    var isLocationIdPresent = !StringUtils.isEmpty(bindItem.getLocationId());
+    // BindItem must have either locationId or holdingId field populated
+    // If both or neither are present throw validation error
+    if (isLocationIdPresent == isHoldingIdPresent) {
+      throw new HttpException(RestConstants.VALIDATION_ERROR, BIND_ITEM_MUST_INCLUDE_EITHER_HOLDING_ID_OR_LOCATION_ID.toError());
+    }
+  }
+
 }

src/test/java/org/folio/TestUtils.java

@@ -197,7 +197,7 @@ public static BindItem getMinimalContentBindItem() {
       .withCallNumber("TK5105.88815 . A58 2004 FT MEADE")
       .withMaterialTypeId("1a54b431-2e4f-452d-9cae-9cee66c9a892")
       .withPermanentLoanTypeId("2b94c631-fca9-4892-a730-03ee529ffe27")
-      .withPermanentLocationId("fcd64ce1-6995-48f0-840e-89ffa2288371");
+      .withLocationId("fcd64ce1-6995-48f0-840e-89ffa2288371");
   }
   public static String encodePrettily(Object entity) {
     return JsonObject.mapFrom(entity).encodePrettily();

src/test/java/org/folio/rest/impl/CheckinReceivingApiTest.java

@@ -1004,7 +1004,9 @@ void testBindPiecesToTitleWithItem() {
     var pieceIds = List.of(bindingPiece1.getId(), bindingPiece2.getId());
     var bindPiecesCollection = new BindPiecesCollection()
       .withPoLineId(poLine.getId())
-      .withBindItem(getMinimalContentBindItem())
+      .withBindItem(getMinimalContentBindItem()
+        .withLocationId(null)
+        .withHoldingId(holdingId))
       .withBindPieceIds(pieceIds);
 
     var response = verifyPostResponse(ORDERS_BIND_ENDPOINT, JsonObject.mapFrom(bindPiecesCollection).encode(),
@@ -1032,42 +1034,55 @@ void testBindPiecesToTitleWithItem() {
   }
 
   @Test
-  void testBindPiecesWithDifferentHoldingIdAndThrowError() {
-    logger.info("=== Test POST Bind with different holdingId to Title With and throw error");
+  void testBindPiecesWithLocationIdOnly() {
+    logger.info("=== Test POST Bind to Title With Item using locationId");
 
-    var holdingId = "849241fa-4a14-4df5-b951-846dcd6cfc4d";
     var receivingStatus = Piece.ReceivingStatus.UNRECEIVABLE;
     var format = Piece.Format.ELECTRONIC;
-
     var order = getMinimalContentCompositePurchaseOrder()
       .withWorkflowStatus(CompositePurchaseOrder.WorkflowStatus.OPEN);
     var poLine = getMinimalContentCompositePoLine(order.getId());
-    var bindingPiece1 = getMinimalContentPiece(poLine.getId())
-      .withHoldingId(holdingId)
-      .withReceivingStatus(receivingStatus)
-      .withFormat(format);
-    var bindingPiece2 = getMinimalContentPiece(poLine.getId())
-      .withId(UUID.randomUUID().toString())
-      .withHoldingId("64ee33f2-b2b5-4912-942a-50ddea063663")
+    var bindingPiece = getMinimalContentPiece(poLine.getId())
       .withReceivingStatus(receivingStatus)
       .withFormat(format);
 
     addMockEntry(PURCHASE_ORDER_STORAGE, order);
     addMockEntry(PO_LINES_STORAGE, poLine);
-    addMockEntry(PIECES_STORAGE, bindingPiece1);
-    addMockEntry(PIECES_STORAGE, bindingPiece2);
+    addMockEntry(PIECES_STORAGE, bindingPiece);
     addMockEntry(TITLES, getTitle(poLine));
 
+
+    var locationId = UUID.randomUUID().toString();
+    var pieceIds = List.of(bindingPiece.getId());
     var bindPiecesCollection = new BindPiecesCollection()
       .withPoLineId(poLine.getId())
-      .withBindItem(getMinimalContentBindItem())
-      .withBindPieceIds(List.of(bindingPiece1.getId(), bindingPiece2.getId()));
+      .withBindItem(getMinimalContentBindItem()
+        .withLocationId(locationId))
+      .withBindPieceIds(pieceIds);
 
-    var errors = verifyPostResponse(ORDERS_BIND_ENDPOINT, JsonObject.mapFrom(bindPiecesCollection).encode(),
-      prepareHeaders(EXIST_CONFIG_X_OKAPI_TENANT_LIMIT_10), APPLICATION_JSON, HttpStatus.HTTP_BAD_REQUEST.toInt())
-      .as(Errors.class);
+    var response = verifyPostResponse(ORDERS_BIND_ENDPOINT, JsonObject.mapFrom(bindPiecesCollection).encode(),
+      prepareHeaders(EXIST_CONFIG_X_OKAPI_TENANT_LIMIT_10), APPLICATION_JSON, HttpStatus.HTTP_OK.toInt())
+      .as(BindPiecesResult.class);
+
+    assertThat(response.getPoLineId(), is(poLine.getId()));
+    assertThat(response.getBoundPieceIds(), is(pieceIds));
+    assertThat(response.getItemId(), notNullValue());
+
+    var pieceUpdates = getPieceUpdates();
+    assertThat(pieceUpdates, notNullValue());
+    assertThat(pieceUpdates, hasSize(bindPiecesCollection.getBindPieceIds().size()));
 
-    assertEquals(errors.getErrors().get(0).getMessage(), "Holding Id must not be null or different for pieces");
+    var pieceList = pieceUpdates.stream().filter(pol -> {
+      Piece piece = pol.mapTo(Piece.class);
+      String pieceId = piece.getId();
+      return Objects.equals(bindingPiece.getId(), pieceId);
+    }).toList();
+    assertThat(pieceList.size(), is(1));
+
+    var createdHoldings = getCreatedHoldings();
+    assertThat(createdHoldings, notNullValue());
+    assertThat(createdHoldings, hasSize(1));
+    assertThat(createdHoldings.get(0).getString(HOLDING_PERMANENT_LOCATION_ID), is(locationId));
   }
 
   @Test
@@ -1085,7 +1100,9 @@ void testBindPiecesToTitleWithItemWithOutstandingRequest() {
       .withFormat(org.folio.rest.jaxrs.model.Piece.Format.ELECTRONIC);
     var bindPiecesCollection = new BindPiecesCollection()
       .withPoLineId(poLine.getId())
-      .withBindItem(getMinimalContentBindItem())
+      .withBindItem(getMinimalContentBindItem()
+        .withLocationId(null)
+        .withHoldingId(UUID.randomUUID().toString()))
       .withBindPieceIds(List.of(bindingPiece.getId()));
 
     addMockEntry(PURCHASE_ORDER_STORAGE, order);
@@ -1117,7 +1134,9 @@ void testBindPiecesToTitleWithBindItemWithDifferentTenantId() {
     var bindPiecesCollection = new BindPiecesCollection()
       .withPoLineId(poLine.getId())
       .withBindItem(getMinimalContentBindItem()
-        .withTenantId("differentTenantId"))
+        .withTenantId("differentTenantId")
+        .withLocationId(null)
+        .withHoldingId(UUID.randomUUID().toString()))
       .withBindPieceIds(List.of(bindingPiece.getId()))
       .withRequestsAction(BindPiecesCollection.RequestsAction.TRANSFER);
 
@@ -1149,7 +1168,9 @@ void testBindPiecesToTitleWithTransferRequestsAction() {
       .withFormat(org.folio.rest.jaxrs.model.Piece.Format.ELECTRONIC);
     var bindPiecesCollection = new BindPiecesCollection()
       .withPoLineId(poLine.getId())
-      .withBindItem(getMinimalContentBindItem())
+      .withBindItem(getMinimalContentBindItem()
+        .withLocationId(null)
+        .withHoldingId(UUID.randomUUID().toString()))
       .withBindPieceIds(List.of(bindingPiece.getId()))
       .withRequestsAction(BindPiecesCollection.RequestsAction.TRANSFER);