diff --git a/src/main/java/org/folio/helper/PurchaseOrderHelper.java b/src/main/java/org/folio/helper/PurchaseOrderHelper.java index 8b2e87b56..defd823c7 100644 --- a/src/main/java/org/folio/helper/PurchaseOrderHelper.java +++ b/src/main/java/org/folio/helper/PurchaseOrderHelper.java @@ -235,12 +235,12 @@ public Future updateOrder(CompositePurchaseOrder compPO, boolean deleteHol boolean isTransitionToOpen = isTransitionToOpen(poFromStorage, compPO); return orderValidationService.validateOrderForUpdate(compPO, poFromStorage, deleteHoldings, requestContext) .compose(v -> { - var poLineFutures = new ArrayList>(); if (isTransitionToOpen) { if (CollectionUtils.isEmpty(compPO.getCompositePoLines())) { compPO.setCompositePoLines(clonedPoFromStorage.getCompositePoLines()); } } + var poLineFutures = new ArrayList>(); logger.info("updateOrder:: POL size: {}", compPO.getCompositePoLines()); if (!compPO.getCompositePoLines().isEmpty()) { compPO.getCompositePoLines().forEach(poLine -> { @@ -249,12 +249,10 @@ public Future updateOrder(CompositePurchaseOrder compPO, boolean deleteHol .findFirst().orElse(null); logger.info("updateOrder:: Stored POL found: {}", Objects.nonNull(compPoLineFromStorage)); if (Objects.nonNull(compPoLineFromStorage)) { - var updatedLocations = poLine.getLocations(); - var storedLocations = compPoLineFromStorage.getLocations(); + var updatedLocations = compPoLineFromStorage.getLocations(); logger.info("updateOrder:: Stored POL poLineId: {}", poLine.getId()); - logger.info("updateOrder:: Stored POL storedLocations: {}", JsonArray.of(storedLocations).encodePrettily()); logger.info("updateOrder:: Stored POL updatedLocations: {}", JsonArray.of(updatedLocations).encodePrettily()); - poLineFutures.add(compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(poLine.getId(), updatedLocations, storedLocations, requestContext)); + poLineFutures.add(compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(poLine.getId(), updatedLocations, requestContext)); } }); } diff --git a/src/main/java/org/folio/helper/PurchaseOrderLineHelper.java b/src/main/java/org/folio/helper/PurchaseOrderLineHelper.java index 9198653dd..2f24c5e9d 100644 --- a/src/main/java/org/folio/helper/PurchaseOrderLineHelper.java +++ b/src/main/java/org/folio/helper/PurchaseOrderLineHelper.java @@ -278,7 +278,7 @@ public Future updateOrderLine(CompositePoLine compOrderLine, RequestContex .compose(compOrder -> protectionService.isOperationRestricted(compOrder.getAcqUnitIds(), UPDATE, requestContext) .compose(v -> purchaseOrderLineService.validateAndNormalizeISBNAndProductType(Collections.singletonList(compOrderLine), requestContext)) .compose(v -> validateAccessProviders(compOrderLine, requestContext)) - .compose(v -> compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(compOrderLine.getId(), compOrderLine.getLocations(), poLineFromStorage.getLocations(), requestContext)) + .compose(v -> compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(compOrderLine.getId(), compOrderLine.getLocations(), requestContext)) .compose(v -> expenseClassValidationService.validateExpenseClassesForOpenedOrder(compOrder, Collections.singletonList(compOrderLine), requestContext)) .compose(v -> processPoLineEncumbrances(compOrder, compOrderLine, poLineFromStorage, requestContext))) .map(v -> compOrderLine.withPoLineNumber(poLineFromStorage.getPoLineNumber())) // PoLine number must not be modified during PoLine update, set original value diff --git a/src/main/java/org/folio/service/orders/CompositePoLineValidationService.java b/src/main/java/org/folio/service/orders/CompositePoLineValidationService.java index e3134a5c8..7d38d4ff6 100644 --- a/src/main/java/org/folio/service/orders/CompositePoLineValidationService.java +++ b/src/main/java/org/folio/service/orders/CompositePoLineValidationService.java @@ -24,8 +24,8 @@ import io.vertx.core.Future; import org.apache.commons.collections4.CollectionUtils; -import org.apache.commons.collections4.SetUtils; import org.apache.commons.lang3.StringUtils; +import org.apache.http.HttpStatus; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.folio.orders.utils.HelperUtils; @@ -345,27 +345,20 @@ private void validateReceivingWorkflowForBindary(CompositePoLine poLine, List validateUserUnaffiliatedLocationUpdates(String poLineId, List updatedPoLineLocations, - List storedPoLineLocations, RequestContext requestContext) { + public Future validateUserUnaffiliatedLocationUpdates(String poLineId, List locations, RequestContext requestContext) { return getUserTenantsIfNeeded(requestContext) .compose(userTenants -> { if (CollectionUtils.isEmpty(userTenants)) { logger.info("validateUserUnaffiliatedLocationUpdates:: User tenants is empty"); return Future.succeededFuture(); } - var storageUnaffiliatedLocations = extractUnaffiliatedLocations(storedPoLineLocations, userTenants); - var updatedUnaffiliatedLocations = extractUnaffiliatedLocations(updatedPoLineLocations, userTenants); - logger.info("validateUserUnaffiliatedLocationUpdates:: Found unaffiliated POL location tenant ids, poLineId: '{}', stored: '{}', updated: '{}'", - poLineId, - storageUnaffiliatedLocations.stream().map(Location::getTenantId).distinct().toList(), - updatedUnaffiliatedLocations.stream().map(Location::getTenantId).distinct().toList()); - if (!SetUtils.isEqualSet(storageUnaffiliatedLocations, updatedUnaffiliatedLocations)) { - logger.info("validateUserUnaffiliatedLocationUpdates:: User is not affiliated with all locations on the POL, poLineId: '{}'", - poLineId); - return Future.failedFuture(new HttpException(422, ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION)); + var uniqueUnAffiliatedLocations = extractUnaffiliatedLocations(locations, userTenants).stream() + .map(Location::getTenantId).distinct().toList(); + if (CollectionUtils.isNotEmpty(uniqueUnAffiliatedLocations)) { + logger.info("validateUserUnaffiliatedLocationUpdates:: User has unaffiliated locations on the POL, poLineId: {}, unique locations: {}", poLineId, uniqueUnAffiliatedLocations); + return Future.failedFuture(new HttpException(HttpStatus.SC_UNPROCESSABLE_ENTITY, ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION)); } - logger.info("validateUserUnaffiliatedLocationUpdates:: User is affiliated with all locations on the POL, poLineId: '{}'", - poLineId); + logger.info("validateUserUnaffiliatedLocationUpdates:: User is affiliated with all {} locations on the POL, poLineId: {}", locations.size(), poLineId); return Future.succeededFuture(); }); } diff --git a/src/test/java/org/folio/helper/PurchaseOrderHelperTest.java b/src/test/java/org/folio/helper/PurchaseOrderHelperTest.java index 3b13e38c7..ba0be3eb2 100644 --- a/src/test/java/org/folio/helper/PurchaseOrderHelperTest.java +++ b/src/test/java/org/folio/helper/PurchaseOrderHelperTest.java @@ -201,7 +201,7 @@ void testPutPendingCompositeOrder() throws IOException { doReturn(succeededFuture(null)) .when(encumbranceService).updateEncumbrancesOrderStatusAndReleaseIfClosed(any(CompositePurchaseOrder.class), eq(requestContext)); doReturn(succeededFuture(null)) - .when(compositePoLineValidationService).validateUserUnaffiliatedLocationUpdates(anyString(), any(), any(), eq(requestContext)); + .when(compositePoLineValidationService).validateUserUnaffiliatedLocationUpdates(anyString(), any(), eq(requestContext)); // When Future future = purchaseOrderHelper.putCompositeOrderById(compPO.getId(), deleteHoldings, compPO, requestContext); diff --git a/src/test/java/org/folio/service/orders/CompositePoLineValidationServiceTest.java b/src/test/java/org/folio/service/orders/CompositePoLineValidationServiceTest.java index 423ba2d09..a04b6f14f 100644 --- a/src/test/java/org/folio/service/orders/CompositePoLineValidationServiceTest.java +++ b/src/test/java/org/folio/service/orders/CompositePoLineValidationServiceTest.java @@ -1,7 +1,6 @@ package org.folio.service.orders; import static io.vertx.core.Future.succeededFuture; -import static org.folio.TestUtils.getLocationsForTenants; import static org.folio.rest.core.exceptions.ErrorCodes.*; import static org.folio.rest.jaxrs.model.CompositePoLine.OrderFormat.OTHER; import static org.folio.rest.jaxrs.model.CompositePoLine.OrderFormat.P_E_MIX; @@ -12,12 +11,11 @@ import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.when; -import java.lang.reflect.InvocationTargetException; import java.util.List; import java.util.Optional; import java.util.Set; @@ -25,6 +23,8 @@ import java.util.stream.Collectors; import io.vertx.core.Future; +import io.vertx.junit5.VertxExtension; +import io.vertx.junit5.VertxTestContext; import org.folio.models.consortium.ConsortiumConfiguration; import org.folio.rest.core.exceptions.ErrorCodes; import org.folio.rest.core.exceptions.HttpException; @@ -35,7 +35,6 @@ import org.folio.rest.jaxrs.model.Error; import org.folio.rest.jaxrs.model.Location; import org.folio.rest.jaxrs.model.Physical; -import org.folio.rest.jaxrs.model.PoLine; import org.folio.service.consortium.ConsortiumConfigurationService; import org.folio.service.consortium.ConsortiumUserTenantsRetriever; import org.folio.service.finance.expenceclass.ExpenseClassValidationService; @@ -43,25 +42,24 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +@ExtendWith(VertxExtension.class) public class CompositePoLineValidationServiceTest { + + @Mock private ExpenseClassValidationService expenseClassValidationService; + @Mock private ConsortiumConfigurationService consortiumConfigurationService; + @Mock private ConsortiumUserTenantsRetriever consortiumUserTenantsRetriever; + @Mock private RequestContext requestContext; + @InjectMocks private CompositePoLineValidationService compositePoLineValidationService; + private AutoCloseable mockitoMocks; - @Mock - private ExpenseClassValidationService expenseClassValidationService; - @Mock - private ConsortiumConfigurationService consortiumConfigurationService; - @Mock - private ConsortiumUserTenantsRetriever consortiumUserTenantsRetriever; - @InjectMocks - private CompositePoLineValidationService compositePoLineValidationService; - @Mock - private RequestContext requestContext; @BeforeEach - public void initMocks(){ + public void initMocks() { mockitoMocks = MockitoAnnotations.openMocks(this); } @@ -169,7 +167,6 @@ void shouldReturnErrorIfIncorrectOrderFormatWhenBindaryActive() { assertEquals(ORDER_FORMAT_INCORRECT_FOR_BINDARY_ACTIVE.getCode(), errors.get(0).getCode()); } - @Test void shouldReturnErrorIfIncorrectCreateInventoryWhenBindaryActive() { var compositePoLine = new CompositePoLine() @@ -382,7 +379,6 @@ void testValidatePoLineWithZeroQuantitiesWithoutLocations() { assertThat(errors, hasSize(0)); } - private Set errorsToCodes(List errors) { return errors .stream() @@ -391,38 +387,91 @@ private Set errorsToCodes(List errors) { } @Test - void testValidateUserUnaffiliatedLocationUpdates() throws NoSuchMethodException, InvocationTargetException, IllegalAccessException { - var locationsStored = getLocationsForTenants(List.of("tenant1", "tenant2", "tenant3")); - var locationsUpdated = getLocationsForTenants(List.of("tenant1", "tenant3")); - var storagePoLine = new PoLine().withLocations(locationsStored); - var updatedPoLine = new CompositePoLine().withLocations(locationsUpdated); + void testValidateUserUnaffiliatedLocationUpdates(VertxTestContext testContext) { + var locationsUpdated = List.of( + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"), + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2")); + var updatedPoLineId = UUID.randomUUID().toString(); + + when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext))) + .thenReturn(Future.succeededFuture(Optional.empty())); + when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext))) + .thenReturn(Future.succeededFuture(List.of("tenant1"))); + + compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext) + .onComplete(testContext.succeeding(result -> testContext.verify(testContext::completeNow))); + } - doReturn(succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId")))) - .when(consortiumConfigurationService).getConsortiumConfiguration(any(RequestContext.class)); - doReturn(succeededFuture(List.of("tenant1", "tenant2"))) - .when(consortiumUserTenantsRetriever).getUserTenants(eq("consortiumId"), eq("centralTenantId"), any(RequestContext.class)); + @Test + void testValidateUserUnaffiliatedLocationUpdatesAllValidLocations(VertxTestContext testContext) { + var locationsUpdated = List.of( + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"), + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2")); + var updatedPoLineId = UUID.randomUUID().toString(); + + when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext))) + .thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId")))); + when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext))) + .thenReturn(Future.succeededFuture(List.of("tenant1", "tenant2"))); + + compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext) + .onComplete(testContext.succeeding(result -> testContext.verify(testContext::completeNow))); + } - var future = compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLine.getId(), updatedPoLine.getLocations(), storagePoLine.getLocations(), requestContext); + @Test + void testValidateUserUnaffiliatedLocationUpdatesAllValidDuplicateTenantLocations(VertxTestContext testContext) { + var locationsUpdated = List.of( + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"), + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2"), + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2")); + var updatedPoLineId = UUID.randomUUID().toString(); + + when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext))) + .thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId")))); + when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext))) + .thenReturn(Future.succeededFuture(List.of("tenant1", "tenant2"))); + + compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext) + .onComplete(testContext.succeeding(result -> testContext.verify(testContext::completeNow))); + } - assertTrue(future.succeeded()); + @Test + void testValidateUserUnaffiliatedLocationUpdatesOneValidAndOneInvalidLocations(VertxTestContext testContext) { + var locationsUpdated = List.of( + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"), + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2")); + var updatedPoLineId = UUID.randomUUID().toString(); + + when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext))) + .thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId")))); + when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext))) + .thenReturn(Future.succeededFuture(List.of("tenant1"))); + + compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext) + .onComplete(testContext.failing(cause -> testContext.verify(() -> { + assertInstanceOf(HttpException.class, cause); + assertTrue(cause.getMessage().contains(ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION.getDescription())); + testContext.completeNow(); + }))); } @Test - void testValidateUserUnaffiliatedLocationUpdatesInvalid() { - var locationsStored = getLocationsForTenants(List.of("tenant1", "tenant2", "tenant3")); - var locationsUpdated = getLocationsForTenants(List.of("tenant1", "tenant3")); - locationsUpdated.get(1).withQuantityPhysical(10); - var storagePoLine = new PoLine().withLocations(locationsStored); - var updatedPoLine = new CompositePoLine().withLocations(locationsUpdated); - - doReturn(succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId")))) - .when(consortiumConfigurationService).getConsortiumConfiguration(any(RequestContext.class)); - doReturn(succeededFuture(List.of("tenant1"))) - .when(consortiumUserTenantsRetriever).getUserTenants(eq("consortiumId"), anyString(), any(RequestContext.class)); - - var future = compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLine.getId(), updatedPoLine.getLocations(), storagePoLine.getLocations(), requestContext); - - assertTrue(future.failed()); - assertInstanceOf(HttpException.class, future.cause()); + void testValidateUserUnaffiliatedLocationUpdatesTwoInvalidLocations(VertxTestContext testContext) { + var locationsUpdated = List.of( + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant1"), + new Location().withLocationId(UUID.randomUUID().toString()).withTenantId("tenant2")); + var updatedPoLineId = UUID.randomUUID().toString(); + + when(consortiumConfigurationService.getConsortiumConfiguration(eq(requestContext))) + .thenReturn(Future.succeededFuture(Optional.of(new ConsortiumConfiguration("tenant1", "consortiumId")))); + when(consortiumUserTenantsRetriever.getUserTenants(eq("consortiumId"), anyString(), eq(requestContext))) + .thenReturn(Future.succeededFuture(List.of("tenant3"))); + + compositePoLineValidationService.validateUserUnaffiliatedLocationUpdates(updatedPoLineId, locationsUpdated, requestContext) + .onComplete(testContext.failing(cause -> testContext.verify(() -> { + assertInstanceOf(HttpException.class, cause); + assertTrue(cause.getMessage().contains(ErrorCodes.LOCATION_UPDATE_WITHOUT_AFFILIATION.getDescription())); + testContext.completeNow(); + }))); } }