Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MODORDER-1087]-Delete received pieces in bulk #952

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
34 changes: 33 additions & 1 deletion ramls/pieces.raml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ protocols: [ HTTP, HTTPS ]

documentation:
- title: Orders Business Logic API
content: <b>API for managing pieces</b>
content: <b>API for managing pieces including batch operations for deletion.</b>

types:
piece: !include acq-models/mod-orders-storage/schemas/piece.json
Expand Down Expand Up @@ -47,6 +47,38 @@ resourceTypes:
example: true
required: false
default: false
/batch:
delete:
description: Batch delete pieces
body:
application/json:
type: piece-collection
responses:
204:
description: "Batch delete of pieces successfully completed"
400:
description: "Bad request"
body:
application/json:
example:
strict: false
value: !include examples/errors_400.sample
text/plain:
example: "unable to delete Piece -- Bad request"
404:
description: "One or more pieces not found"
body:
application/json:
type: errors
text/plain:
example: "Error - one or more pieces not found."
500:
description: "Internal server error, e.g. due to misconfiguration"
body:
application/json:
type: errors
text/plain:
example: "Internal server error - due to misconfiguration."
/{id}:
uriParameters:
id:
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/org/folio/rest/impl/PiecesAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import org.folio.rest.annotations.Validate;
import org.folio.rest.core.models.RequestContext;
import org.folio.rest.jaxrs.model.Piece;
import org.folio.rest.jaxrs.model.PieceCollection;
import org.folio.rest.jaxrs.resource.OrdersPieces;
import org.folio.service.pieces.PieceStorageService;
import org.folio.service.pieces.flows.create.PieceCreateFlowManager;
Expand Down Expand Up @@ -96,4 +97,10 @@ public void deleteOrdersPiecesById(String pieceId, boolean deleteHolding, Map<St
.onSuccess(ok -> asyncResultHandler.handle(succeededFuture(buildNoContentResponse())))
.onFailure(fail -> handleErrorResponse(asyncResultHandler, fail));
}

@Override
@Validate
public void deleteOrdersPiecesBatch(PieceCollection entity, Map<String, String> okapiHeaders, Handler<AsyncResult<Response>> asyncResultHandler, Context vertxContext) {
pieceDeleteFlowManager.batchDeletePiece(entity, new RequestContext(vertxContext, okapiHeaders));
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
package org.folio.service.pieces.flows.delete;

import static org.folio.orders.utils.ProtectedOperationType.DELETE;
import static org.folio.service.orders.utils.HelperUtils.collectResultsOnSuccess;

import java.util.ArrayList;
import java.util.List;

import org.apache.commons.collections4.CollectionUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.models.pieces.PieceDeletionHolder;
Expand All @@ -13,6 +18,7 @@
import org.folio.service.ProtectionService;
import org.folio.service.pieces.PieceStorageService;
import org.folio.service.pieces.flows.BasePieceFlowHolderBuilder;
import org.folio.rest.jaxrs.model.PieceCollection;

import io.vertx.core.Future;

Expand Down Expand Up @@ -79,4 +85,26 @@ protected Future<Void> updatePoLine(PieceDeletionHolder holder, RequestContext r
: pieceDeleteFlowPoLineService.updatePoLine(holder, requestContext);
}

public Future<List<Void>> batchDeletePiece (PieceCollection entity, RequestContext requestContext) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is some formatting issue, also we need to accept list of ids to delete instead of full PieceCollection object

Suggested change
public Future<List<Void>> batchDeletePiece (PieceCollection entity, RequestContext requestContext) {
public Future<List<Void>> batchDeletePieces(List<String> pieceIds, RequestContext requestContext) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example how you can define the an endpoint:
DELETE /<endpoint_name>>
Content-Type: application/json

{
"ids": [1, 2, 3]
}

List <String> ids = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should keep an existing logic that Delete by Id endpoint uses with many existing checks.
In the simplest way you can invoke this method in cycle:
Future deletePiece(String pieceId, boolean deleteHolding, RequestContext requestContext)
It is not optimezed solution of course, but can be as starting point

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thank you for pointing it out, will do! :)

entity.getPieces().stream().forEach(v -> ids.add(v.getId()));
List<Future<PieceDeletionHolder>> deletionHolders = ids.stream()
.map(pieceId -> {
PieceDeletionHolder holder = new PieceDeletionHolder().withDeleteHolding(true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could not hardcode deleteHolding to true, it should be fetched from request param as doing in Delete By Id endpoint

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for pointing it out, I will correct.

return pieceStorageService.getPieceById(pieceId, requestContext)
.map(pieceToDelete -> {
holder.withPieceToDelete(pieceToDelete);
return holder;
});
})
.toList();
return collectResultsOnSuccess(deletionHolders)
.compose(holders -> {
List<Future<Void>> deleteFutures = holders.stream()
.map(holder -> pieceStorageService.deletePiece(holder.getPieceToDelete().getId(), true, requestContext))
.toList();
return collectResultsOnSuccess(deleteFutures);
});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.folio.rest.jaxrs.model.Eresource;
import org.folio.rest.jaxrs.model.Location;
import org.folio.rest.jaxrs.model.Piece;
import org.folio.rest.jaxrs.model.PieceCollection;
import org.folio.rest.jaxrs.model.PoLine;
import org.folio.rest.jaxrs.model.PurchaseOrder;
import org.folio.rest.jaxrs.model.Title;
Expand Down Expand Up @@ -368,6 +369,67 @@ void shouldUpdateLineQuantityIfPoLineIsNotPackageAndManualPieceCreateFalseAndInv
verify(basePieceFlowHolderBuilder).updateHolderWithOrderInformation(holder, requestContext);
}

@Test
void shouldDeletePiecesInBatch() {
String orderId = UUID.randomUUID().toString();
String holdingId = UUID.randomUUID().toString();
String lineId = UUID.randomUUID().toString();
String itemId = UUID.randomUUID().toString();
String locationId = UUID.randomUUID().toString();
String titleId = UUID.randomUUID().toString();
JsonObject holding = new JsonObject();
holding.put(ID, holdingId);
holding.put(HOLDING_PERMANENT_LOCATION_ID, locationId);
JsonObject item = new JsonObject().put(ID, itemId);
item.put(ITEM_STATUS, new JsonObject().put(ITEM_STATUS_NAME, ItemStatus.ON_ORDER.value()));
Piece piece = new Piece().withId(UUID.randomUUID().toString()).withPoLineId(lineId)
.withHoldingId(holdingId).withFormat(Piece.Format.ELECTRONIC);
Location loc = new Location().withHoldingId(holdingId).withQuantityElectronic(1).withQuantity(1);
Cost cost = new Cost().withQuantityElectronic(1)
.withListUnitPriceElectronic(1d).withExchangeRate(1d).withCurrency("USD")
.withPoLineEstimatedPrice(1d);
PoLine poLine = new PoLine().withIsPackage(false).withCheckinItems(false).withOrderFormat(PoLine.OrderFormat.ELECTRONIC_RESOURCE)
.withEresource(new Eresource().withCreateInventory(Eresource.CreateInventory.INSTANCE_HOLDING))
.withPurchaseOrderId(orderId).withId(lineId).withLocations(List.of(loc)).withCost(cost);
PurchaseOrder purchaseOrder = new PurchaseOrder().withId(orderId).withWorkflowStatus(PurchaseOrder.WorkflowStatus.OPEN);
Title title = new Title().withId(titleId);
List<Piece> ids = new ArrayList<>();
ids.add(piece);
PieceCollection pieceCollection = new PieceCollection();
pieceCollection.withPieces(ids);
doReturn(succeededFuture(piece)).when(pieceStorageService).getPieceById(piece.getId(), requestContext);
doReturn(succeededFuture(null)).when(protectionService).isOperationRestricted(any(), any(ProtectedOperationType.class), eq(requestContext));
doReturn(succeededFuture(null)).when(pieceStorageService).deletePiece(eq(piece.getId()), eq(true), eq(requestContext));
doReturn(succeededFuture(null)).when(circulationRequestsRetriever).getNumberOfRequestsByItemId(eq(piece.getItemId()), eq(requestContext));
doReturn(succeededFuture(holding)).when(inventoryHoldingManager).getHoldingById(holdingId, false, requestContext);
doReturn(succeededFuture(null)).when(inventoryItemManager).getItemsByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(null)).when(inventoryHoldingManager).deleteHoldingById(piece.getHoldingId(), true, requestContext);
doReturn(succeededFuture(null)).when(inventoryItemManager).getItemRecordById(itemId, true, requestContext);
doReturn(succeededFuture(null)).when(inventoryItemManager).deleteItem(itemId, true, requestContext);
doReturn(succeededFuture(holding)).when(inventoryHoldingManager).getHoldingById(holdingId, true, requestContext);
doReturn(succeededFuture(null)).when(pieceUpdateInventoryService).deleteHoldingConnectedToPiece(piece, requestContext);
doReturn(succeededFuture(new ArrayList<JsonObject>())).when(inventoryItemManager).getItemsByHoldingId(holdingId, requestContext);
final ArgumentCaptor<PieceDeletionHolder> PieceDeletionHolderCapture = ArgumentCaptor.forClass(PieceDeletionHolder.class);
doAnswer((Answer<Future<Void>>) invocation -> {
PieceDeletionHolder answerHolder = invocation.getArgument(0);
answerHolder.withOrderInformation(purchaseOrder, poLine);
return succeededFuture(null);
}).when(basePieceFlowHolderBuilder).updateHolderWithOrderInformation(PieceDeletionHolderCapture.capture(), eq(requestContext));
doAnswer((Answer<Future<Void>>) invocation -> {
PieceDeletionHolder answerHolder = invocation.getArgument(0);
answerHolder.withTitleInformation(title);
return succeededFuture(null);
}).when(basePieceFlowHolderBuilder).updateHolderWithTitleInformation(PieceDeletionHolderCapture.capture(), eq(requestContext));

final ArgumentCaptor<PieceDeletionHolder> pieceDeletionHolderCapture = ArgumentCaptor.forClass(PieceDeletionHolder.class);
doReturn(succeededFuture(null)).when(pieceDeleteFlowPoLineService).updatePoLine(pieceDeletionHolderCapture.capture(), eq(requestContext));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also separate declaration doReturn and action and verification part to be easy to read

//When
pieceDeleteFlowManager.batchDeletePiece(pieceCollection, requestContext).result();
verify(pieceStorageService).deletePiece(eq(piece.getId()), eq(true), eq(requestContext));
verify(inventoryItemManager, times(0)).deleteItem(itemId, true, requestContext);
verify(pieceStorageService, times(1)).deletePiece(eq(piece.getId()), eq(true), eq(requestContext));
}

private static class ContextConfiguration {

@Bean
Expand Down
Loading