Skip to content

MODORDERS-1408. Deleting a holding results in another referenced holding being silently deleted #1248

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.

Sign up for GitHub

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

Open
SerhiiNosko wants to merge 1 commit into
base: master
Choose a base branch
from
Open

MODORDERS-1408. Deleting a holding results in another referenced holding being silently deleted #1248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/main/java/org/folio/config/ApplicationConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -621,8 +621,9 @@ ReceivingEncumbranceStrategy receivingEncumbranceStrategy(EncumbranceService enc
@Bean
PieceUpdateInventoryService pieceUpdateInventoryService(InventoryItemManager inventoryItemManager,
InventoryHoldingManager inventoryHoldingManager,
PieceStorageService pieceStorageService) {
return new PieceUpdateInventoryService(inventoryItemManager, inventoryHoldingManager, pieceStorageService);
PieceStorageService pieceStorageService,
PurchaseOrderLineService purchaseOrderLineService) {
return new PieceUpdateInventoryService(inventoryItemManager, inventoryHoldingManager, pieceStorageService, purchaseOrderLineService);
}

@Bean
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/folio/helper/CheckinHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,10 @@ protected Future<Boolean> receiveInventoryItemAndUpdatePiece(PiecesHolder holder

protected Future<Map<String, List<Piece>>> processInventory(Map<String, List<Piece>> piecesGroupedByPoLine, PiecesHolder holder, boolean deleteHoldings, RequestContext requestContext) {
Map<Pair<String, String>, Set<String>> piecesGroupedByHoldings = PieceUtil.groupPiecesByHoldings(StreamUtils.flatten(piecesGroupedByPoLine.values()));
Set<String> poLineIdsBeingProcessed = piecesGroupedByPoLine.keySet();
return updateInventoryItemsAndHoldings(piecesGroupedByPoLine, holder, requestContext)
.compose(pieces -> deleteHoldings
? pieceUpdateInventoryService.deleteHoldingsConnectedToPieces(piecesGroupedByHoldings, requestContext).map(pieces)
? pieceUpdateInventoryService.deleteHoldingsConnectedToPieces(piecesGroupedByHoldings, poLineIdsBeingProcessed, requestContext).map(pieces)
: Future.succeededFuture(pieces));
}

Expand Down
55 changes: 47 additions & 8 deletions src/main/java/org/folio/service/pieces/PieceUpdateInventoryService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@
import static org.folio.orders.utils.RequestContextUtil.createContextWithNewTenantId;
import static org.folio.service.inventory.InventoryHoldingManager.HOLDING_PERMANENT_LOCATION_ID;

import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;

import lombok.RequiredArgsConstructor;
import lombok.extern.log4j.Log4j2;
import one.util.streamex.StreamEx;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.tuple.Pair;
import org.folio.rest.core.models.RequestContext;
Expand All @@ -19,6 +22,7 @@
import org.folio.rest.jaxrs.model.Piece;
import org.folio.service.inventory.InventoryHoldingManager;
import org.folio.service.inventory.InventoryItemManager;
import org.folio.service.orders.PurchaseOrderLineService;
import org.folio.service.pieces.flows.DefaultPieceFlowsValidator;

import io.vertx.core.Future;
Expand All @@ -33,6 +37,7 @@ public class PieceUpdateInventoryService {
private final InventoryItemManager inventoryItemManager;
private final InventoryHoldingManager inventoryHoldingManager;
private final PieceStorageService pieceStorageService;
private final PurchaseOrderLineService purchaseOrderLineService;

/**
* Return id of created Item
Expand Down Expand Up @@ -60,34 +65,68 @@ public Future<Pair<String, String>> deleteHoldingConnectedToPiece(Piece piece, R
}

public Future<List<Pair<String, String>>> deleteHoldingsConnectedToPieces(List<Piece> pieces, RequestContext requestContext) {
return deleteHoldingsConnectedToPieces(PieceUtil.groupPiecesByHoldings(pieces), requestContext);
return deleteHoldingsConnectedToPieces(PieceUtil.groupPiecesByHoldings(pieces), Collections.emptySet(), requestContext);
}

public Future<List<Pair<String, String>>> deleteHoldingsConnectedToPieces(Map<Pair<String, String>, Set<String>> holdingIdsToPieces, RequestContext requestContext) {
public Future<List<Pair<String, String>>> deleteHoldingsConnectedToPieces(Map<Pair<String, String>, Set<String>> holdingIdsToPieces,
Set<String> poLineIdsBeingProcessed,
RequestContext requestContext) {
var futures = holdingIdsToPieces.entrySet().stream()
.map(entry -> {
var holdingId = entry.getKey().getKey();
var receivingTenantId = entry.getKey().getValue();
var pieceIds = entry.getValue();
var locationContext = createContextWithNewTenantId(requestContext, receivingTenantId);
return inventoryHoldingManager.getHoldingById(holdingId, true, locationContext)
.compose(holding -> getUpdatePossibleForHolding(holding, holdingId, pieceIds, locationContext, requestContext))
.compose(holding -> getUpdatePossibleForHolding(holding, holdingId, pieceIds, poLineIdsBeingProcessed, locationContext, requestContext))
.compose(isUpdatePossibleVsHolding -> deleteHoldingIfPossible(isUpdatePossibleVsHolding, holdingId, locationContext));
}).toList();
return collectResultsOnSuccess(futures);
}

private Future<Pair<Boolean, JsonObject>> getUpdatePossibleForHolding(JsonObject holding, String holdingId, Set<String> pieceIds,
Set<String> poLineIdsBeingProcessed,
RequestContext locationContext, RequestContext requestContext) {
if (holding == null || holding.isEmpty()) {
return Future.succeededFuture(Pair.of(false, new JsonObject()));
}
return pieceStorageService.getPiecesByHoldingId(holdingId, requestContext)
.compose(pieces -> inventoryItemManager.getItemsByHoldingId(holdingId, locationContext)
.map(items -> CollectionUtils.isEmpty(filterPiecesToProcess(pieceIds, pieces)) && CollectionUtils.isEmpty(items)
var piecesFuture = pieceStorageService.getPiecesByHoldingId(holdingId, requestContext);
var itemsFuture = inventoryItemManager.getItemsByHoldingId(holdingId, locationContext);
var poLinesFuture = getPoLinesReferencingHolding(holdingId, poLineIdsBeingProcessed, locationContext, requestContext);

return piecesFuture.compose(pieces -> itemsFuture.compose(items -> poLinesFuture
.map(otherPoLines -> {
var hasOtherPieces = !CollectionUtils.isEmpty(filterPiecesToProcess(pieceIds, pieces));
var hasItems = !CollectionUtils.isEmpty(items);
var hasOtherPoLines = !CollectionUtils.isEmpty(otherPoLines);
if (hasOtherPoLines) {
log.info("getUpdatePossibleForHolding:: Holding '{}' is referenced by other PO lines: {}, skipping deletion",
holdingId, otherPoLines.stream().map(PoLine::getId).toList());
}
return !hasOtherPieces && !hasItems && !hasOtherPoLines
? Pair.of(true, holding)
: Pair.of(false, new JsonObject()))
);
: Pair.of(false, new JsonObject());
})
));
}

private Future<List<PoLine>> getPoLinesReferencingHolding(String holdingId, Set<String> poLineIdsBeingProcessed,
RequestContext locationContext, RequestContext requestContext) {
var holdingIds = List.of(holdingId);
var locationTenantFuture = purchaseOrderLineService.getPoLinesByHoldingIds(holdingIds, locationContext);
var requestTenantFuture = Objects.equals(getTenantId(locationContext), getTenantId(requestContext))
? Future.succeededFuture(List.<PoLine>of())
: purchaseOrderLineService.getPoLinesByHoldingIds(holdingIds, requestContext);

return locationTenantFuture.compose(locationPoLines -> requestTenantFuture
.map(requestPoLines -> StreamEx.of(locationPoLines).append(requestPoLines)
.filter(poLine -> !poLineIdsBeingProcessed.contains(poLine.getId()))
.distinct(PoLine::getId)
.toList()));
}

private String getTenantId(RequestContext requestContext) {
return requestContext.getHeaders().get("x-okapi-tenant");
}

private Future<Pair<String, String>> deleteHoldingIfPossible(Pair<Boolean, JsonObject> isUpdatePossibleVsHolding,
Expand Down
68 changes: 65 additions & 3 deletions src/test/java/org/folio/service/pieces/PieceUpdateInventoryServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import static org.folio.TestConfig.isVerticleNotDeployed;
import static org.folio.service.inventory.InventoryHoldingManager.HOLDING_PERMANENT_LOCATION_ID;
import static org.folio.service.inventory.InventoryHoldingManager.ID;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
Expand All @@ -20,15 +22,19 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;

import org.folio.ApiTestSuite;
import org.folio.rest.core.models.RequestContext;
import org.folio.rest.jaxrs.model.Location;
import org.folio.rest.jaxrs.model.Piece;
import org.folio.rest.jaxrs.model.PoLine;
import org.folio.service.inventory.InventoryHoldingManager;
import org.folio.service.inventory.InventoryItemManager;
import org.folio.service.orders.PurchaseOrderLineService;
import org.folio.service.titles.TitlesService;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -55,6 +61,8 @@ public class PieceUpdateInventoryServiceTest {
private InventoryHoldingManager inventoryHoldingManager;
@Autowired
private PieceStorageService pieceStorageService;
@Autowired
private PurchaseOrderLineService purchaseOrderLineService;
@Mock
private Map<String, String> okapiHeadersMock;

Expand Down Expand Up @@ -111,14 +119,15 @@ void shouldNotDeleteHoldingIfHoldingIdIsNotNullButNotFoundInTheDB() throws Execu


@Test
void shouldDeleteHoldingIfHoldingIdIsProvidedAndFoundInDBAndNoPiecesAndItems() {
void shouldDeleteHoldingIfHoldingIdIsProvidedAndFoundInDBAndNoPiecesAndItemsAndNoOtherPoLines() {
String holdingId = UUID.randomUUID().toString();
JsonObject holding = new JsonObject();
holding.put(ID, holding);
Piece piece = new Piece().withId(UUID.randomUUID().toString()).withHoldingId(holdingId);
doReturn(succeededFuture(Collections.emptyList())).when(pieceStorageService).getPiecesByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(holding)).when(inventoryHoldingManager).getHoldingById(holdingId, true, requestContext);
doReturn(succeededFuture(new ArrayList<>())).when(inventoryItemManager).getItemsByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(Collections.emptyList())).when(purchaseOrderLineService).getPoLinesByHoldingIds(List.of(holdingId), requestContext);

pieceUpdateInventoryService.deleteHoldingConnectedToPiece(piece, requestContext);

Expand All @@ -137,6 +146,7 @@ void shouldNoDeleteHoldingIfHoldingIdIsProvidedAndFoundInDBAndPiecesExistAndNoIt
doReturn(succeededFuture(List.of(piece, piece2))).when(pieceStorageService).getPiecesByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(holding)).when(inventoryHoldingManager).getHoldingById(holdingId, true, requestContext);
doReturn(succeededFuture(new ArrayList<>())).when(inventoryItemManager).getItemsByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(Collections.emptyList())).when(purchaseOrderLineService).getPoLinesByHoldingIds(List.of(holdingId), requestContext);

pieceUpdateInventoryService.deleteHoldingConnectedToPiece(piece, requestContext).result();

Expand All @@ -160,6 +170,52 @@ void shouldNoDeleteHoldingIfHoldingIdIsProvidedAndFoundInDBAndNoPiecesAndItemsEx
verify(inventoryHoldingManager, times(0)).deleteHoldingById(holdingId , true, requestContext);
}

@Test
void shouldNotDeleteHoldingIfOtherPoLinesReferenceIt() {
String holdingId = UUID.randomUUID().toString();
String poLineId = UUID.randomUUID().toString();
String otherPoLineId = UUID.randomUUID().toString();
JsonObject holding = new JsonObject();
holding.put(ID, holdingId);
Piece piece = new Piece().withId(UUID.randomUUID().toString()).withHoldingId(holdingId).withPoLineId(poLineId);

doReturn(succeededFuture(Collections.emptyList())).when(pieceStorageService).getPiecesByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(holding)).when(inventoryHoldingManager).getHoldingById(holdingId, true, requestContext);
doReturn(succeededFuture(new ArrayList<>())).when(inventoryItemManager).getItemsByHoldingId(holdingId, requestContext);

PoLine otherPoLine = new PoLine().withId(otherPoLineId)
.withLocations(List.of(new Location().withHoldingId(holdingId)));
doReturn(succeededFuture(List.of(otherPoLine))).when(purchaseOrderLineService).getPoLinesByHoldingIds(List.of(holdingId), requestContext);

pieceUpdateInventoryService.deleteHoldingConnectedToPiece(piece, requestContext).result();

verify(inventoryHoldingManager, times(0)).deleteHoldingById(holdingId, true, requestContext);
}

@Test
void shouldDeleteHoldingIfOnlyProcessedPoLinesReferenceIt() {
String holdingId = UUID.randomUUID().toString();
String poLineId = UUID.randomUUID().toString();
JsonObject holding = new JsonObject();
holding.put(ID, holdingId);
holding.put(HOLDING_PERMANENT_LOCATION_ID, UUID.randomUUID().toString());
Piece piece = new Piece().withId(UUID.randomUUID().toString()).withHoldingId(holdingId).withPoLineId(poLineId);

doReturn(succeededFuture(Collections.emptyList())).when(pieceStorageService).getPiecesByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(holding)).when(inventoryHoldingManager).getHoldingById(holdingId, true, requestContext);
doReturn(succeededFuture(new ArrayList<>())).when(inventoryItemManager).getItemsByHoldingId(holdingId, requestContext);
doReturn(succeededFuture(null)).when(inventoryHoldingManager).deleteHoldingById(holdingId, true, requestContext);

PoLine currentPoLine = new PoLine().withId(poLineId)
.withLocations(List.of(new Location().withHoldingId(holdingId)));
doReturn(succeededFuture(List.of(currentPoLine))).when(purchaseOrderLineService).getPoLinesByHoldingIds(List.of(holdingId), requestContext);

pieceUpdateInventoryService.deleteHoldingsConnectedToPieces(
PieceUtil.groupPiecesByHoldings(List.of(piece)), Set.of(poLineId), requestContext).result();

verify(inventoryHoldingManager, times(1)).deleteHoldingById(holdingId, true, requestContext);
}


private static class ContextConfiguration {
@Bean
Expand All @@ -182,11 +238,17 @@ PieceStorageService pieceStorageService() {
return mock(PieceStorageService.class);
}

@Bean
PurchaseOrderLineService purchaseOrderLineService() {
return mock(PurchaseOrderLineService.class);
}

@Bean
PieceUpdateInventoryService pieceUpdateInventoryService(InventoryItemManager inventoryItemManager,
InventoryHoldingManager inventoryHoldingManager,
PieceStorageService pieceStorageService) {
return spy(new PieceUpdateInventoryService(inventoryItemManager, inventoryHoldingManager, pieceStorageService));
PieceStorageService pieceStorageService,
PurchaseOrderLineService purchaseOrderLineService) {
return spy(new PieceUpdateInventoryService(inventoryItemManager, inventoryHoldingManager, pieceStorageService, purchaseOrderLineService));
}
}
}