CIRC-1785 Add "service point for the effective location" and "Request Date"as staff slip token #1280
Conversation
rgupta-nla
requested review from
alexanderkurash,
roman-barannyk,
OleksandrVidinieiev and
kentfitch
|
Kudos, SonarCloud Quality Gate passed! |
rgupta-nla
changed the title
| if (entries == null) { | ||
| return new JsonObject(); | ||
| } | ||
| if(primaryServicePoint == null) { |
There was a problem hiding this comment.
Space is missing (different style from line 113)
| @@ -108,6 +109,19 @@ public static JsonObject createStaffSlipContext(Request request) { | |||
| return createStaffSlipContext(request.getItem(), request); | |||
| } | |||
|
|
|||
| public static JsonObject addPrimaryServicePointNameToStaffSlipContext(JsonObject entries, ServicePoint primaryServicePoint) { | |||
There was a problem hiding this comment.
We now have to follow this doc https://wiki.folio.org/display/DD/Logging when creating new code. mod-circulation is still not fully covered with the new-style logging yet, but the new code already needs to meet this criteria. I think it would be nice to at least cover addPrimaryServicePointNameToStaffSlipContext method
| @@ -104,6 +104,8 @@ private void getMany(RoutingContext routingContext) { | |||
| .thenComposeAsync(r -> r.after(addressTypeRepository::findAddressTypesForRequests)) | |||
| .thenComposeAsync(r -> r.after(servicePointRepository::findServicePointsForRequests)) | |||
| .thenApply(flatMapResult(this::mapResultToJson)) | |||
| .thenComposeAsync(a -> a.combineAfter(() -> servicePointRepository.getServicePointById(servicePointId), | |||
There was a problem hiding this comment.
If a is a Result, we typically shorten it to r (lines 98-105)
| optionalRequest | ||
| .map(Request::getRequestDate) | ||
| .ifPresent(value -> write(requestContext, "requestDate", value)); |
There was a problem hiding this comment.
I believe this should a part of another PR because it's not required in CIRC-1785, but anyway, is it covered with tests?
| @@ -184,6 +198,7 @@ private static JsonObject createItemContext(Item item) { | |||
| if (location != null) { | |||
| itemContext | |||
| .put("effectiveLocationSpecific", location.getName()) | |||
| .put("effectiveLocationPrimaryServicePointName", location.getPrimaryServicePoint().getName()) | |||
There was a problem hiding this comment.
If this changes the API (and is looks like it does - we're adding another field to the /circulation/pick-slips/{id} endpoint) we should increase minor version of the corresponding interface in the ModuleDescriptor. Check the 'Version numbers' section here https://dev.folio.org/guidelines/contributing/ specifically this part: If you only add things to the interface – e.g. a new resource or method on existing resources – then you increment the minor number, because the API is backwards compatible.
https://issues.folio.org/browse/UXPROD-4179
Add "service point for the effective location" as staff slip token
https://issues.folio.org/browse/UXPROD-4180
Add "Request date" as staff slip token
Purpose
Purpose of this PR is to add "service point for the effective location" as staff slip token and add "Request date" as staff slip token. The changes would reflect in staff slip produced in the Requests app (pick slip) and staff slip produced in the Check in app (hold, delivery request and transfer slips).
Approach
We are getting ServicePoint object information by fetching it passing the servicePoingId. We then grab the service point location (primary location ) and stick it to the item context. This is done for pick slip resource.
For the checkin app, the location name is already available in the Item object so we just add it to json object.
TODOS and Open Questions
Learning