Skip to content

Commit

Permalink
CIRC-1880 update requests error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
roman-barannyk committed Sep 19, 2023
1 parent d741346 commit 235bba9
Show file tree
Hide file tree
Showing 11 changed files with 89 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import static org.folio.circulation.resources.handlers.error.CirculationErrorType.REQUEST_NOT_ALLOWED_FOR_PATRON_TITLE_COMBINATION;
import static org.folio.circulation.resources.handlers.error.CirculationErrorType.TLR_RECALL_WITHOUT_OPEN_LOAN_OR_RECALLABLE_ITEM;
import static org.folio.circulation.resources.handlers.error.CirculationErrorType.USER_IS_INACTIVE;
import static org.folio.circulation.support.ErrorCode.PAGEABLE_AVAILABLE_ITEM_FOUND;
import static org.folio.circulation.support.ValidationErrorFailure.failedValidation;
import static org.folio.circulation.support.results.MappingFunctions.when;
import static org.folio.circulation.support.results.Result.of;
Expand Down Expand Up @@ -145,12 +146,11 @@ private Result<RequestAndRelatedRecords> failValidationWhenPageableItemsExist(
private Result<RequestAndRelatedRecords> failedValidationHoldAndRecallNotAllowed(Request request,
String availableItemId) {

String errorMessage = "Hold/Recall TLR not allowed: pageable available item found for instance";

String errorMessage = "Hold/Recall title level request not allowed: pageable available item found for instance";
log.info("{}. Pageable item(s): {}", errorMessage, availableItemId);

return failedValidation(errorMessage,
Map.of(ITEM_ID, availableItemId, INSTANCE_ID, request.getInstanceId()));
return failedValidation(errorMessage, Map.of(ITEM_ID, availableItemId, INSTANCE_ID,
request.getInstanceId()), PAGEABLE_AVAILABLE_ITEM_FOUND);
}

private CompletableFuture<Result<RequestAndRelatedRecords>> checkInstance(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static java.lang.String.format;
import static org.folio.circulation.domain.representations.RequestProperties.PICKUP_SERVICE_POINT_ID;
import static org.folio.circulation.domain.representations.RequestProperties.REQUEST_TYPE;
import static org.folio.circulation.support.ErrorCode.ALREADY_REQUESTED;
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;
Expand Down Expand Up @@ -218,7 +219,7 @@ private static Result<RequestAndRelatedRecords> alreadyRequestedFailure(
parameters.put(INSTANCE_ID, requestBeingPlaced.getInstanceId());

message = requestBeingPlaced.getOperation() == Operation.MOVE
? "Not allowed to move TLR to the same item"
? "Not allowed to move title level page request to the same item"
: "This requester already has an open request for this instance";
} else {
parameters.put(REQUESTER_ID, requestBeingPlaced.getUserId());
Expand All @@ -234,7 +235,7 @@ private static Result<RequestAndRelatedRecords> alreadyRequestedFailure(
message = "This requester already has an open request for this item";
}

return failedValidation(message, parameters);
return failedValidation(message, parameters, ALREADY_REQUESTED);
}

static boolean isTheSameRequester(RequestAndRelatedRecords it, Request that) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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_CANNOT_BE_PROXY_FOR_THEMSELVES;
import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError;
import static org.folio.circulation.support.http.client.CqlQuery.exactMatch;
import static org.folio.circulation.support.results.Result.failed;
Expand Down Expand Up @@ -51,8 +52,8 @@ public <T extends UserRelatedRecord> CompletableFuture<Result<T>> refuseWhenInva
if (StringUtils.equals(userRelatedRecord.getProxyUserId(), userRelatedRecord.getUserId())) {
log.info("refuseWhenInvalid:: proxy user ID is equal to user ID");
return completedFuture(failed(singleValidationError(
"User cannot be proxy for themself", "proxyUserId",
userRelatedRecord.getProxyUserId())));
"User cannot be proxy for themselves", "proxyUserId",
userRelatedRecord.getProxyUserId(), USER_CANNOT_BE_PROXY_FOR_THEMSELVES)));
}

return succeeded(userRelatedRecord).failAfter(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package org.folio.circulation.domain.validation;

import static org.folio.circulation.support.ErrorCode.REQUESTER_ALREADY_HAS_LOAN_FOR_INSTANCES_ITEM;
import static org.folio.circulation.support.ErrorCode.REQUESTER_ALREADY_HAS_THIS_ITEM_ON_LOAN;
import static org.folio.circulation.support.ValidationErrorFailure.failedValidation;
import static org.folio.circulation.support.ValidationErrorFailure.singleValidationError;
import static org.folio.circulation.support.http.client.PageLimit.limit;
Expand All @@ -26,6 +28,7 @@
import org.folio.circulation.domain.RequestAndRelatedRecords;
import org.folio.circulation.infrastructure.storage.loans.LoanRepository;
import org.folio.circulation.storage.ItemByInstanceIdFinder;
import org.folio.circulation.support.ErrorCode;
import org.folio.circulation.support.http.client.PageLimit;
import org.folio.circulation.support.http.server.ValidationError;
import org.folio.circulation.support.results.Result;
Expand Down Expand Up @@ -57,9 +60,10 @@ public CompletableFuture<Result<RequestAndRelatedRecords>> refuseWhenUserHasAlre
parameters.put("userId", request.getUserId());
parameters.put("loanId", loan.getId());

String message = "This requester currently has this item on loan.";
String message = "This requester already has this item on loan";

return singleValidationError(new ValidationError(message, parameters));
return singleValidationError(new ValidationError(message, parameters,
REQUESTER_ALREADY_HAS_THIS_ITEM_ON_LOAN));
}).map(loan -> requestAndRelatedRecords));
}

Expand Down Expand Up @@ -112,7 +116,7 @@ private Result<RequestAndRelatedRecords> oneOfTheItemsIsAlreadyLoanedFailure(
parameters.put("userId", requestAndRelatedRecords.getRequest().getUserId());
parameters.put("itemId", loan.getItemId());

return failedValidation("One of the items of the requested title is already loaned to " +
"the requester", parameters);
return failedValidation("This requester already has a loan for one of the instance's items",
parameters, REQUESTER_ALREADY_HAS_LOAN_FOR_INSTANCES_ITEM);
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.folio.circulation.domain.validation;

import static org.folio.circulation.support.ErrorCode.HOLD_SHELF_REQUESTS_REQUIRE_PICKUP_SERVICE_POINT;
import static org.folio.circulation.support.ErrorCode.SERVICE_POINT_IS_NOT_PICKUP_LOCATION;
import static org.folio.circulation.support.ValidationErrorFailure.failedValidation;
import static org.folio.circulation.support.results.Result.succeeded;
Expand Down Expand Up @@ -42,8 +43,8 @@ public Result<RequestAndRelatedRecords> refuseInvalidPickupServicePoint(
log.info("refuseInvalidPickupServicePoint:: Hold Shelf Fulfillment Requests require a " +
"Pickup Service Point");
return failedValidation(
"Hold Shelf Fulfillment Requests require a Pickup Service Point",
"id", request.getId());
"Hold shelf fulfillment requests require a Pickup service point",
"id", request.getId(), HOLD_SHELF_REQUESTS_REQUIRE_PICKUP_SERVICE_POINT);
} else {
log.info("refuseInvalidPickupServicePoint:: No pickup service point specified for request");
return succeeded(requestAndRelatedRecords);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
import static org.folio.circulation.resources.handlers.error.CirculationErrorType.INVALID_PROXY_RELATIONSHIP;
import static org.folio.circulation.resources.handlers.error.CirculationErrorType.NO_AVAILABLE_ITEMS_FOR_TLR;
import static org.folio.circulation.resources.handlers.error.CirculationErrorType.TLR_RECALL_WITHOUT_OPEN_LOAN_OR_RECALLABLE_ITEM;
import static org.folio.circulation.support.ErrorCode.FULFILLMENT_PREFERENCE_IS_NOT_ALLOWED;
import static org.folio.circulation.support.ErrorCode.INSTANCE_HAS_NO_ITEM_ID;
import static org.folio.circulation.support.ErrorCode.REQUEST_LEVEL_IS_NOT_ALLOWED;
import static org.folio.circulation.support.ValidationErrorFailure.failedValidation;
import static org.folio.circulation.support.http.client.PageLimit.limit;
import static org.folio.circulation.support.json.JsonPropertyFetcher.getDateTimeProperty;
Expand Down Expand Up @@ -74,6 +77,7 @@
import org.folio.circulation.services.ItemForTlrService;
import org.folio.circulation.storage.ItemByInstanceIdFinder;
import org.folio.circulation.support.BadRequestFailure;
import org.folio.circulation.support.ErrorCode;
import org.folio.circulation.support.http.client.PageLimit;
import org.folio.circulation.support.request.RequestRelatedRepositories;
import org.folio.circulation.support.results.Result;
Expand Down Expand Up @@ -445,8 +449,8 @@ private Result<Request> validateRequestLevel(Request request) {
.collect(Collectors.joining(", "));

return failedValidation(
"requestLevel must be one of the following: " + allowedStatusesJoined, "requestLevel",
requestLevelRaw);
"Request level must be one of the following: " + allowedStatusesJoined, "requestLevel",
requestLevelRaw, REQUEST_LEVEL_IS_NOT_ALLOWED);
}

return succeeded(request);
Expand All @@ -457,9 +461,9 @@ private Result<Request> validatefulfillmentPreference(Request request) {
.filter(value -> value.equals(request.getfulfillmentPreferenceName()))
.findFirst()
.map(value -> succeeded(request))
.orElseGet(() -> failedValidation("fulfillmentPreference must be one of the following: " +
.orElseGet(() -> failedValidation("Fulfillment preference must be one of the following: " +
join(", ", RequestFulfillmentPreference.allowedValues()), "fulfillmentPreference",
request.getfulfillmentPreferenceName()));
request.getfulfillmentPreferenceName(), FULFILLMENT_PREFERENCE_IS_NOT_ALLOWED));
}

private Result<Request> refuseWhenNoInstanceId(Result<Request> result) {
Expand Down Expand Up @@ -535,8 +539,8 @@ private Result<Request> validateAbsenceOfItemLinkInTlr(Request request) {
Map<String, String> errorParameters = new HashMap<>();
errorParameters.put("itemId", itemId);
errorParameters.put("holdingsRecordId", holdingsRecordId);
return failedValidation("Attempt to create TLR request linked to an item",
errorParameters);
return failedValidation("Cannot create a title level page request " +
"for this instance ID with no item ID", errorParameters, INSTANCE_HAS_NO_ITEM_ID);
}
else {
return of(() -> request);
Expand Down
12 changes: 11 additions & 1 deletion src/main/java/org/folio/circulation/support/ErrorCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,15 @@ public enum ErrorCode {
REQUEST_ALREADY_CLOSED,
REQUEST_NOT_ALLOWED_FOR_PATRON_TITLE_COMBINATION,
REQUEST_PICKUP_SERVICE_POINT_IS_NOT_ALLOWED,
SERVICE_POINT_IS_NOT_PICKUP_LOCATION
SERVICE_POINT_IS_NOT_PICKUP_LOCATION,
REQUEST_LEVEL_IS_NOT_ALLOWED,
FULFILLMENT_PREFERENCE_IS_NOT_ALLOWED,
REQUESTER_ALREADY_HAS_LOAN_FOR_INSTANCES_ITEM,
REQUESTER_ALREADY_HAS_THIS_ITEM_ON_LOAN,
INSTANCE_HAS_NO_ITEM_ID,
PAGEABLE_AVAILABLE_ITEM_FOUND,
USER_CANNOT_BE_PROXY_FOR_THEMSELVES,
MOVING_REQUEST_TO_THE_SAME_ITEM,
ALREADY_REQUESTED,
HOLD_SHELF_REQUESTS_REQUIRE_PICKUP_SERVICE_POINT
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import static api.support.matchers.PatronNoticeMatcher.hasEmailNoticeProperties;
import static api.support.matchers.RequestMatchers.isOpenAwaitingPickup;
import static api.support.matchers.TextDateTimeMatcher.isEquivalentTo;
import static api.support.matchers.ValidationErrorMatchers.hasCode;
import static api.support.matchers.ValidationErrorMatchers.hasErrorWith;
import static api.support.matchers.ValidationErrorMatchers.hasMessage;
import static api.support.utl.PatronNoticeTestHelper.verifyNumberOfPublishedEvents;
Expand All @@ -25,6 +26,7 @@
import static org.folio.circulation.domain.notice.NoticeEventType.REQUEST_EXPIRATION;
import static org.folio.circulation.domain.representations.logs.LogEventType.NOTICE;
import static org.folio.circulation.domain.representations.logs.LogEventType.NOTICE_ERROR;
import static org.folio.circulation.support.ErrorCode.REQUEST_LEVEL_IS_NOT_ALLOWED;
import static org.folio.circulation.support.json.JsonPropertyFetcher.getDateTimeProperty;
import static org.folio.circulation.support.utils.ClockUtil.getLocalDate;
import static org.folio.circulation.support.utils.ClockUtil.getZonedDateTime;
Expand All @@ -33,6 +35,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.nullValue;

Expand All @@ -45,6 +48,7 @@
import java.util.stream.Stream;

import org.folio.circulation.domain.policy.Period;
import org.folio.circulation.support.ErrorCode;
import org.folio.circulation.support.http.client.Response;
import org.hamcrest.Matcher;
import org.junit.FixMethodOrder;
Expand Down Expand Up @@ -634,8 +638,9 @@ void titleLevelRequestExpirationNoticeShouldNotBeCreatedWithDisabledTlr(UUID exp
Response response = requestsFixture.attemptPlace(buildTitleLevelRequest(requestExpiration));

assertThat(response.getStatusCode(), is(422));
assertThat(response.getJson(), hasErrorWith(
hasMessage("requestLevel must be one of the following: \"Item\"")));
assertThat(response.getJson(), hasErrorWith(allOf(
hasMessage("Request level must be one of the following: \"Item\""),
hasCode(REQUEST_LEVEL_IS_NOT_ALLOWED))));
verifyNumberOfScheduledNotices(0);
}

Expand Down
Loading

0 comments on commit 235bba9

Please sign in to comment.