Skip to content
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

CIRC-1785 Add "service point for the effective location" and "Request Date"as staff slip token #1280

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

CIRC-1785 Add "service point for the effective location" and "Request Date"as staff slip token #1280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2347974
added primary service point field
rgupta-nla Apr 24, 2023
48a4917
added primary service point and request date
rgupta-nla Apr 24, 2023
6be8e3e
UXPROD-4179 fixed unit tests
rgupta-nla Apr 26, 2023
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
4 changes: 4 additions & 0 deletions src/main/java/org/folio/circulation/domain/ServicePoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ public static ServicePoint unknown(String id) {
return new ServicePoint(id, null, null, false, null, null, null, null, null);
}

public static ServicePoint unknown(String id, String name) {
return new ServicePoint(id, name, null, false, null, null, null, null, null);
}

String id;
String name;
String code;
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/org/folio/circulation/domain/notice/TemplateContextUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public class TemplateContextUtil {
private static final String FEE_CHARGE = "feeCharge";
private static final String FEE_ACTION = "feeAction";
private static final String UNLIMITED = "unlimited";
private static final String PICK_SLIPS_KEY = "pickSlips";

private TemplateContextUtil() {
}
Expand Down Expand Up @@ -108,6 +109,19 @@ public static JsonObject createStaffSlipContext(Request request) {
return createStaffSlipContext(request.getItem(), request);
}

public static JsonObject addPrimaryServicePointNameToStaffSlipContext(JsonObject entries, ServicePoint primaryServicePoint) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (entries == null) {
return new JsonObject();
}
if(primaryServicePoint == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space is missing (different style from line 113)

return entries;
}
entries.getJsonArray(PICK_SLIPS_KEY).stream().forEach(pickSlip -> {
((JsonObject)pickSlip).getJsonObject(ITEM).put("effectiveLocationPrimaryServicePointName", primaryServicePoint.getName());
});
return entries;
}

public static JsonObject createStaffSlipContext(
Item item, Request request) {

Expand Down Expand Up @@ -184,6 +198,7 @@ private static JsonObject createItemContext(Item item) {
if (location != null) {
itemContext
.put("effectiveLocationSpecific", location.getName())
.put("effectiveLocationPrimaryServicePointName", location.getPrimaryServicePoint().getName())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.put("effectiveLocationLibrary", location.getLibraryName())
.put("effectiveLocationCampus", location.getCampusName())
.put("effectiveLocationInstitution", location.getInstitutionName())
Expand Down Expand Up @@ -231,6 +246,9 @@ private static JsonObject createRequestContext(Request request) {
.map(Request::getPickupServicePoint)
.map(ServicePoint::getName)
.ifPresent(value -> requestContext.put("servicePointPickup", value));
optionalRequest
.map(Request::getRequestDate)
.ifPresent(value -> write(requestContext, "requestDate", value));
Comment on lines +249 to +251
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should a part of another PR because it's not required in CIRC-1785, but anyway, is it covered with tests?

optionalRequest
.map(Request::getRequestExpirationDate)
.ifPresent(value -> write(requestContext, "requestExpirationDate", value));
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/org/folio/circulation/resources/PickSlipsResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a is a Result, we typically shorten it to r (lines 98-105)

TemplateContextUtil::addPrimaryServicePointNameToStaffSlipContext))
.thenApply(r -> r.map(JsonHttpResponse::ok))
.thenAccept(context::writeResultToHttpResponse);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/circulation/storage/mappers/LocationMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public Location toDomain(JsonObject representation) {
Institution.unknown(getProperty(representation, "institutionId")),
Campus.unknown(getProperty(representation, "campusId")),
Library.unknown(getProperty(representation, "libraryId")),
ServicePoint.unknown(getProperty(representation, "primaryServicePoint")));
ServicePoint.unknown(getProperty(representation, "primaryServicePoint"), getProperty(representation, "primaryServicePointName")));
}

private Collection<UUID> getServicePointIds(JsonObject representation) {
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/api/requests/PickSlipsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ void responseContainsPickSlipWithAllAvailableTokens() {
JsonObject itemContext = pickSlip.getJsonObject(ITEM_KEY);

ZonedDateTime requestCheckinDateTime = getDateTimeProperty(itemContext, "lastCheckedInDateTime");

Item item = Item.from(itemResource.getJson())
.withInstance(new InstanceMapper().toDomain(itemResource.getInstance().getJson()));

Expand Down Expand Up @@ -206,7 +206,7 @@ void responseContainsPickSlipWithAllAvailableTokens() {
assertEquals(item.getDescriptionOfPieces(), itemContext.getString("descriptionOfPieces"));
assertDatetimeEquivalent(actualCheckinDateTime, requestCheckinDateTime);
assertEquals(location.getName(), itemContext.getString("effectiveLocationSpecific"));

assertEquals(location.getPrimaryServicePoint().getName(), itemContext.getString("effectiveLocationPrimaryServicePointName"));
CallNumberComponents callNumberComponents = item.getCallNumberComponents();
assertEquals(callNumberComponents.getCallNumber(), itemContext.getString("callNumber"));
assertEquals(callNumberComponents.getPrefix(), itemContext.getString("callNumberPrefix"));
Expand Down
47 changes: 37 additions & 10 deletions src/test/java/api/support/builders/LocationBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import io.vertx.core.json.JsonArray;
import io.vertx.core.json.JsonObject;
import org.folio.circulation.domain.ServicePoint;

public class LocationBuilder extends JsonBuilder implements Builder {

Expand All @@ -21,9 +22,10 @@ public class LocationBuilder extends JsonBuilder implements Builder {
private final UUID libraryId;
private final UUID primaryServicePointId;
private final Set<UUID> otherServicePointIds;
private final String primaryServicePointName;

public LocationBuilder() {
this(null, null, null, null, null, null, null, new HashSet<>());
this(null, null, null, null, null, null, null, new HashSet<>(), null);
}

private LocationBuilder(
Expand All @@ -34,7 +36,8 @@ private LocationBuilder(
UUID campusId,
UUID libraryId,
UUID primaryServicePointId,
Set<UUID> otherServicePointIds) {
Set<UUID> otherServicePointIds,
String primaryServicePointName) {

if (otherServicePointIds == null) {
otherServicePointIds = new HashSet<>();
Expand All @@ -48,6 +51,7 @@ private LocationBuilder(
this.libraryId = libraryId;
this.primaryServicePointId = primaryServicePointId;
this.otherServicePointIds = otherServicePointIds;
this.primaryServicePointName = primaryServicePointName;
}

@Override
Expand All @@ -70,6 +74,7 @@ public JsonObject create() {
.collect(Collectors.toList()));

write(representation, "servicePointIds", mappedServicePointIds);
write(representation, "primaryServicePointName", primaryServicePointName);
}

return representation;
Expand All @@ -84,7 +89,8 @@ public LocationBuilder withName(String name) {
this.campusId,
this.libraryId,
this.primaryServicePointId,
this.otherServicePointIds);
this.otherServicePointIds,
this.primaryServicePointName);
}

public LocationBuilder withCode(String code) {
Expand All @@ -96,7 +102,8 @@ public LocationBuilder withCode(String code) {
this.campusId,
this.libraryId,
this.primaryServicePointId,
this.otherServicePointIds);
this.otherServicePointIds,
this.primaryServicePointName);
}

public LocationBuilder forInstitution(UUID institutionId) {
Expand All @@ -108,7 +115,8 @@ public LocationBuilder forInstitution(UUID institutionId) {
this.campusId,
this.libraryId,
this.primaryServicePointId,
this.otherServicePointIds);
this.otherServicePointIds,
this.primaryServicePointName);
}

public LocationBuilder forCampus(UUID campusId) {
Expand All @@ -120,7 +128,8 @@ public LocationBuilder forCampus(UUID campusId) {
campusId,
this.libraryId,
this.primaryServicePointId,
this.otherServicePointIds);
this.otherServicePointIds,
this.primaryServicePointName);
}

public LocationBuilder forLibrary(UUID libraryId) {
Expand All @@ -132,7 +141,8 @@ public LocationBuilder forLibrary(UUID libraryId) {
this.campusId,
libraryId,
this.primaryServicePointId,
this.otherServicePointIds);
this.otherServicePointIds,
this.primaryServicePointName);
}

public LocationBuilder withPrimaryServicePoint(UUID primaryServicePointId) {
Expand All @@ -144,7 +154,22 @@ public LocationBuilder withPrimaryServicePoint(UUID primaryServicePointId) {
this.campusId,
this.libraryId,
primaryServicePointId,
this.otherServicePointIds)
this.otherServicePointIds,
this.primaryServicePointName)
.servedBy(primaryServicePointId);
}

public LocationBuilder withPrimaryServicePointName(String primaryServicePointName) {
return new LocationBuilder(
this.name,
this.code,
this.discoveryDisplayName,
this.institutionId,
this.campusId,
this.libraryId,
primaryServicePointId,
this.otherServicePointIds,
primaryServicePointName)
.servedBy(primaryServicePointId);
}

Expand All @@ -157,7 +182,8 @@ public LocationBuilder withDiscoveryDisplayName(String discoveryDisplayName) {
this.campusId,
this.libraryId,
this.primaryServicePointId,
this.otherServicePointIds);
this.otherServicePointIds,
this.primaryServicePointName);
}

public LocationBuilder servedBy(UUID servicePointId) {
Expand All @@ -181,6 +207,7 @@ public LocationBuilder servedBy(Set<UUID> servicePoints) {
this.campusId,
this.libraryId,
this.primaryServicePointId,
newServicePointIds);
newServicePointIds,
this.primaryServicePointName);
}
}
1 change: 1 addition & 0 deletions src/test/java/api/support/examples/LocationExamples.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public LocationBuilder thirdFloor() {
.forCampus(jubileeCampusId)
.forLibrary(djanoglyLibraryId)
.withPrimaryServicePoint(primaryServicePointId)
.withPrimaryServicePointName("Circ Desk 1")
.servedBy(otherServicePointIds);
}

Expand Down