diff --git a/descriptors/ModuleDescriptor-template.json b/descriptors/ModuleDescriptor-template.json index c7eabed65f..37fcbce920 100644 --- a/descriptors/ModuleDescriptor-template.json +++ b/descriptors/ModuleDescriptor-template.json @@ -185,7 +185,7 @@ }, { "id": "circulation", - "version": "14.2", + "version": "14.3", "handlers": [ { "methods": [ diff --git a/src/main/java/org/folio/circulation/domain/RequestServiceUtility.java b/src/main/java/org/folio/circulation/domain/RequestServiceUtility.java index 68dad70a93..35b5c35dd2 100644 --- a/src/main/java/org/folio/circulation/domain/RequestServiceUtility.java +++ b/src/main/java/org/folio/circulation/domain/RequestServiceUtility.java @@ -7,6 +7,7 @@ import static org.folio.circulation.support.ErrorCode.ITEM_ALREADY_REQUESTED; import static org.folio.circulation.support.ErrorCode.ITEM_OF_THIS_INSTANCE_ALREADY_REQUESTED; import static org.folio.circulation.support.ErrorCode.MOVING_REQUEST_TO_THE_SAME_ITEM; +import static org.folio.circulation.support.ErrorCode.REQUEST_NOT_ALLOWED_FOR_PATRON_ITEM_COMBINATION; import static org.folio.circulation.support.ValidationErrorFailure.failedValidation; import static org.folio.circulation.support.results.Result.of; import static org.folio.circulation.support.results.Result.succeeded; @@ -116,7 +117,7 @@ private static Result failureDisallowedForRequestType( return failedValidation( format("%s requests are not allowed for this patron and item combination", requestTypeName), - REQUEST_TYPE, requestTypeName); + REQUEST_TYPE, requestTypeName, REQUEST_NOT_ALLOWED_FOR_PATRON_ITEM_COMBINATION); } static Result refuseWhenInvalidUserAndPatronGroup( diff --git a/src/main/java/org/folio/circulation/domain/validation/AutomatedPatronBlocksValidator.java b/src/main/java/org/folio/circulation/domain/validation/AutomatedPatronBlocksValidator.java index 69a6be54dc..fe6916be30 100644 --- a/src/main/java/org/folio/circulation/domain/validation/AutomatedPatronBlocksValidator.java +++ b/src/main/java/org/folio/circulation/domain/validation/AutomatedPatronBlocksValidator.java @@ -1,6 +1,7 @@ package org.folio.circulation.domain.validation; import static java.util.concurrent.CompletableFuture.completedFuture; +import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_AUTOMATICALLY; import static org.folio.circulation.support.results.Result.ofAsync; import static org.folio.circulation.support.results.Result.succeeded; import static org.folio.circulation.support.utils.LogUtil.listAsString; @@ -18,14 +19,14 @@ import org.apache.logging.log4j.Logger; import org.folio.circulation.domain.AutomatedPatronBlock; import org.folio.circulation.domain.AutomatedPatronBlocks; -import org.folio.circulation.infrastructure.storage.AutomatedPatronBlocksRepository; import org.folio.circulation.domain.LoanAndRelatedRecords; import org.folio.circulation.domain.RequestAndRelatedRecords; +import org.folio.circulation.infrastructure.storage.AutomatedPatronBlocksRepository; import org.folio.circulation.resources.context.RenewalContext; import org.folio.circulation.support.Clients; +import org.folio.circulation.support.ValidationErrorFailure; import org.folio.circulation.support.http.server.ValidationError; import org.folio.circulation.support.results.Result; -import org.folio.circulation.support.ValidationErrorFailure; public class AutomatedPatronBlocksValidator { private static final Logger log = LogManager.getLogger(MethodHandles.lookup().lookupClass()); @@ -43,7 +44,7 @@ public AutomatedPatronBlocksValidator( public AutomatedPatronBlocksValidator(AutomatedPatronBlocksRepository automatedPatronBlocksRepository) { this(automatedPatronBlocksRepository, messages -> new ValidationErrorFailure(messages.stream() - .map(message -> new ValidationError(message, new HashMap<>())) + .map(message -> new ValidationError(message, new HashMap<>(), USER_IS_BLOCKED_AUTOMATICALLY)) .collect(Collectors.toList()))); } diff --git a/src/main/java/org/folio/circulation/domain/validation/UserManualBlocksValidator.java b/src/main/java/org/folio/circulation/domain/validation/UserManualBlocksValidator.java index 5c9332995d..c66f453906 100644 --- a/src/main/java/org/folio/circulation/domain/validation/UserManualBlocksValidator.java +++ b/src/main/java/org/folio/circulation/domain/validation/UserManualBlocksValidator.java @@ -1,5 +1,6 @@ package org.folio.circulation.domain.validation; +import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_MANUALLY; import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError; import static org.folio.circulation.support.fetching.RecordFetching.findWithCqlQuery; import static org.folio.circulation.support.http.client.CqlQuery.exactMatch; @@ -104,7 +105,7 @@ private HttpFailure createUserBlockedValidationError( final String reason = userManualBlocks.getRecords().stream() .map(UserManualBlock::getDesc).collect(Collectors.joining(";")); - return singleValidationError(new ValidationError(message, "reason", reason)); + return singleValidationError(new ValidationError(message, "reason", reason, USER_IS_BLOCKED_MANUALLY)); } private boolean isUserBlockedManually(MultipleRecords userManualBlockMultipleRecords, diff --git a/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java b/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java index 43e78519e1..67c8409e99 100644 --- a/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java +++ b/src/main/java/org/folio/circulation/resources/RequestByInstanceIdResource.java @@ -1,5 +1,6 @@ package org.folio.circulation.resources; +import static java.util.concurrent.CompletableFuture.completedFuture; import static java.util.stream.Collectors.toList; import static org.folio.circulation.domain.InstanceRequestItemsComparer.sortRequestQueues; import static org.folio.circulation.domain.RequestFulfillmentPreference.HOLD_SHELF; @@ -29,11 +30,13 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.stream.Collectors; @@ -159,7 +162,7 @@ private CompletableFuture>> getLoanItems( if (unsortedUnavailableItems == null || unsortedUnavailableItems.isEmpty()) { log.info("getLoanItems:: unsortedUnavailableItems is null or empty"); - return CompletableFuture.completedFuture(succeeded(null)); + return completedFuture(succeeded(null)); } Map>> itemLoanFuturesMap = new HashMap<>(); @@ -301,23 +304,21 @@ updateUponRequest, new RequestLoanValidator(itemFinder, loanRepository), new FailFastErrorHandler()); return placeRequest(requestRepresentations, 0, createRequestService, - clients, new ArrayList<>(), repositories); + clients, new HashSet<>(), repositories); } private CompletableFuture> placeRequest( List itemRequests, int startIndex, CreateRequestService createRequestService, - Clients clients, List errors, RequestRelatedRepositories repositories) { + Clients clients, Set errors, RequestRelatedRepositories repositories) { log.debug("RequestByInstanceIdResource.placeRequest, startIndex: {}, itemRequestSize: {}", startIndex, itemRequests.size()); if (startIndex >= itemRequests.size()) { - String aggregateFailures = String.format("%n%s", String.join("%n", errors)); log.warn("placeRequest:: Failed to place a request for the instance. Reasons: {}", - aggregateFailures); + errors); - return CompletableFuture.completedFuture(failedDueToServerError( - "Failed to place a request for the instance. Reasons: " + aggregateFailures)); + return completedFuture(failedValidation(errors)); } JsonObject currentItemRequest = itemRequests.get(startIndex); @@ -336,12 +337,14 @@ private CompletableFuture> placeRequest( if (r.succeeded()) { log.debug("RequestByInstanceIdResource.placeRequest: succeeded creating request for item {}", currentItemRequest.getString(ITEM_ID)); - return CompletableFuture.completedFuture(r); + return completedFuture(r); } else { - String reason = getErrorMessage(r.cause()); - errors.add(reason); + HttpFailure failure = r.cause(); + errors.addAll(convertToValidationErrors(failure)); + + log.debug("Failed to create request for item {} with cause: {}", + currentItemRequest.getString(ITEM_ID), failure); - log.debug("Failed to create request for item {} with reason: {}", currentItemRequest.getString(ITEM_ID), reason); return placeRequest(itemRequests, startIndex +1, createRequestService, clients, errors, repositories); } @@ -527,4 +530,19 @@ static String getErrorMessage(HttpFailure failure) { } return reason; } + + static Collection convertToValidationErrors(HttpFailure failure) { + Set validationErrors = new HashSet<>(); + if (failure instanceof ServerErrorFailure serverErrorFailure) { + validationErrors.add(new ValidationError(serverErrorFailure.getReason())); + } else if (failure instanceof ValidationErrorFailure validationErrorFailure) { + validationErrors.addAll(validationErrorFailure.getErrors()); + } else if (failure instanceof BadRequestFailure badRequestFailure) { + validationErrors.add(new ValidationError(badRequestFailure.getReason())); + } else if (failure instanceof ForwardOnFailure forwardOnFailure) { + validationErrors.add(new ValidationError(forwardOnFailure.getFailureResponse().getBody())); + } + + return validationErrors; + } } diff --git a/src/main/java/org/folio/circulation/support/ErrorCode.java b/src/main/java/org/folio/circulation/support/ErrorCode.java index 9a2773b6ad..dece89ca03 100644 --- a/src/main/java/org/folio/circulation/support/ErrorCode.java +++ b/src/main/java/org/folio/circulation/support/ErrorCode.java @@ -12,11 +12,14 @@ public enum ErrorCode { ITEM_HAS_OPEN_LOAN, REQUEST_ALREADY_CLOSED, REQUEST_NOT_ALLOWED_FOR_PATRON_TITLE_COMBINATION, + REQUEST_NOT_ALLOWED_FOR_PATRON_ITEM_COMBINATION, REQUEST_PICKUP_SERVICE_POINT_IS_NOT_ALLOWED, SERVICE_POINT_IS_NOT_PICKUP_LOCATION, REQUEST_LEVEL_IS_NOT_ALLOWED, FULFILLMENT_PREFERENCE_IS_NOT_ALLOWED, REQUESTER_ALREADY_HAS_LOAN_FOR_ONE_OF_INSTANCES_ITEMS, + USER_IS_BLOCKED_MANUALLY, + USER_IS_BLOCKED_AUTOMATICALLY, REQUESTER_ALREADY_HAS_THIS_ITEM_ON_LOAN, HOLD_AND_RECALL_TLR_NOT_ALLOWED_PAGEABLE_AVAILABLE_ITEM_FOUND, USER_CANNOT_BE_PROXY_FOR_THEMSELVES, diff --git a/src/test/java/api/requests/InstanceRequestsAPICreationTests.java b/src/test/java/api/requests/InstanceRequestsAPICreationTests.java index a1192a5dba..164821c6f5 100644 --- a/src/test/java/api/requests/InstanceRequestsAPICreationTests.java +++ b/src/test/java/api/requests/InstanceRequestsAPICreationTests.java @@ -1093,6 +1093,39 @@ void canCreateTlrWhenAvailableItemExists(String allowedRequestTypesForCheckedOut is(RequestStatus.OPEN_NOT_YET_FILLED.getValue())); } + @Test + void shouldReturn422StatusIfNoAvailableItemsAndRequesterAlreadyHasOpenRequestForThisInstance() { + UUID pickupServicePointId = servicePointsFixture.cd1().getId(); + + ZonedDateTime instanceRequestDate = ZonedDateTime.of(2024, 6, 7, 10, 22, 54, 0, UTC); + ZonedDateTime instanceRequestDateRequestExpirationDate = instanceRequestDate.plusDays(30); + IndividualResource instance = instancesFixture.basedUponDunkirk(); + IndividualResource holdings = holdingsFixture.defaultWithHoldings(instance.getId()); + IndividualResource locationsResource = locationsFixture.mainFloor(); + + itemsFixture.basedUponDunkirkWithCustomHoldingAndLocation(holdings.getId(), + locationsResource.getId()); + reconfigureTlrFeature(TlrFeatureStatus.ENABLED, true, null, null, null); + + IndividualResource instanceRequester = usersFixture.charlotte(); + JsonObject requestBody = createInstanceRequestObject(instance.getId(), + instanceRequester.getId(), pickupServicePointId, instanceRequestDate, + instanceRequestDateRequestExpirationDate); + + Response postResponse = requestsFixture.attemptToPlaceForInstance(requestBody); + assertEquals(201, postResponse.getStatusCode()); + + Response secondPostResponse = requestsFixture.attemptToPlaceForInstance(requestBody); + assertEquals(422, secondPostResponse.getStatusCode()); + assertThat(secondPostResponse.getJson(), hasErrorWith(allOf( + hasMessage("Cannot create page TLR for this instance ID - no pageable available items found"), + hasParameter("instanceId", instance.getId().toString())))); + assertThat(secondPostResponse.getJson(), hasErrorWith(allOf( + hasMessage("This requester already has an open request for this instance"), + hasParameter("requesterId", instanceRequester.getId().toString()), + hasParameter("instanceId", instance.getId().toString())))); + } + private void validateInstanceRequestResponse(JsonObject representation, UUID pickupServicePointId, UUID instanceId, UUID itemId, RequestType expectedRequestType) { diff --git a/src/test/java/api/requests/RequestsAPICreationTests.java b/src/test/java/api/requests/RequestsAPICreationTests.java index ae49db5a36..0595792bd6 100644 --- a/src/test/java/api/requests/RequestsAPICreationTests.java +++ b/src/test/java/api/requests/RequestsAPICreationTests.java @@ -70,6 +70,8 @@ import static org.folio.circulation.support.ErrorCode.REQUESTER_ALREADY_HAS_LOAN_FOR_ONE_OF_INSTANCES_ITEMS; import static org.folio.circulation.support.ErrorCode.REQUESTER_ALREADY_HAS_THIS_ITEM_ON_LOAN; import static org.folio.circulation.support.ErrorCode.REQUEST_LEVEL_IS_NOT_ALLOWED; +import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_AUTOMATICALLY; +import static org.folio.circulation.support.ErrorCode.USER_IS_BLOCKED_MANUALLY; import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime; import static org.hamcrest.CoreMatchers.allOf; import static org.hamcrest.CoreMatchers.containsString; @@ -2694,7 +2696,8 @@ void cannotCreateRequestWhenRequesterHasActiveRequestManualBlocks() { assertThat(postResponse.getJson(), hasErrors(1)); assertThat(postResponse.getJson(), hasErrorWith(allOf( hasMessage("Patron blocked from requesting"), - hasParameter("reason", "Display description"))) + hasParameter("reason", "Display description"), + hasCode(USER_IS_BLOCKED_MANUALLY))) ); } @@ -2891,6 +2894,8 @@ void requestRefusedWhenAutomatedBlockExistsForPatron() { hasMessage(MAX_NUMBER_OF_ITEMS_CHARGED_OUT_MESSAGE))); assertThat(response.getJson(), hasErrorWith( hasMessage(MAX_OUTSTANDING_FEE_FINE_BALANCE_MESSAGE))); + assertThat(response.getJson(), hasErrorWith( + hasCode(USER_IS_BLOCKED_AUTOMATICALLY))); } @Test diff --git a/src/test/java/org/folio/circulation/resources/RequestByInstanceIdResourceTests.java b/src/test/java/org/folio/circulation/resources/RequestByInstanceIdResourceTests.java index 1c54a1be1c..d6ad2eee50 100644 --- a/src/test/java/org/folio/circulation/resources/RequestByInstanceIdResourceTests.java +++ b/src/test/java/org/folio/circulation/resources/RequestByInstanceIdResourceTests.java @@ -91,6 +91,34 @@ void getExpectedErrorMessages() { assertEquals("fakeResponseFailure", errorMessage); } + @Test + void shouldConvertToValidationErrorForServerErrorFailure() { + ServerErrorFailure serverErrorFailure = new ServerErrorFailure("Server failed"); + Collection errors = RequestByInstanceIdResource.convertToValidationErrors(serverErrorFailure); + + assertEquals(1, errors.size()); + assertTrue(errors.contains(new ValidationError("Server failed"))); + } + + @Test + void shouldConvertToValidationErrorForBadRequestFailure() { + BadRequestFailure badRequestFailure = new BadRequestFailure("Bad request"); + Collection errors = RequestByInstanceIdResource.convertToValidationErrors(badRequestFailure); + + assertEquals(1, errors.size()); + assertTrue(errors.contains(new ValidationError("Bad request"))); + } + + @Test + void shouldConvertToValidationErrorForForwardOnFailure() { + Response failureResponse = new Response(422, "test body", "json"); + ForwardOnFailure forwardFailure = new ForwardOnFailure(failureResponse); + Collection errors = RequestByInstanceIdResource.convertToValidationErrors(forwardFailure); + + assertEquals(1, errors.size()); + assertEquals("test body", errors.stream().toList().get(0).getMessage()); + } + public static JsonObject getJsonInstanceRequest(UUID pickupServicePointId) { ZonedDateTime requestDate = ZonedDateTime.of(2017, 7, 22, 10, 22, 54, 0, UTC); ZonedDateTime requestExpirationDate = requestDate.plusDays(30);