diff --git a/doc/release-notes/12524-sort-guestbook-responses.md b/doc/release-notes/12524-sort-guestbook-responses.md new file mode 100644 index 00000000000..6c2a402436d --- /dev/null +++ b/doc/release-notes/12524-sort-guestbook-responses.md @@ -0,0 +1,2 @@ +## Feature ## +This feature adds the ability to sort the Guestbook Responses in the API /api/guestbooks/{id}/responses. Responses can be sorted by Event Type(type), File Name(file), User Name(user), and Response Date(date) diff --git a/doc/sphinx-guides/source/api/native-api.rst b/doc/sphinx-guides/source/api/native-api.rst index 582891e0861..79d4d2eb2e1 100644 --- a/doc/sphinx-guides/source/api/native-api.rst +++ b/doc/sphinx-guides/source/api/native-api.rst @@ -1442,7 +1442,16 @@ Retrieve Guestbook Responses for a Guestbook For more about guestbooks, see :ref:`dataset-guestbooks` in the User Guide. -In order to retrieve the Guestbook Responses for a Guestbook within a Dataverse collection, you must know the ID if the Guestbook. This API also supports pagination by passing a page limit and an optional offset (starting point). The resulting Json will include 'Next' and 'Prev' urls for navigation as well as the total number of responses. +In order to retrieve the Guestbook Responses for a Guestbook within a Dataverse collection, you must know the ID if the Guestbook. This API also supports pagination by passing a page limit and an optional offset (starting point). The resulting Json will include the total number of responses as `responseCount`. +The Responses can be sorted by specifying one of the following in query parameter 'sort' and 'order': + +* ``type``: Event Type +* ``file``: File Name +* ``user``: Entered Name +* ``date``: Response Date (Default) + +To sort in reverse order you can add ``&order=desc``. + The resulting Json will be more detailed than that of the :ref:`download-guestbook-api` CSV response file by including Guestbook metadata as well as Guestbook Response metadata. .. note:: See :ref:`curl-examples-and-environment-variables` if you are unfamiliar with the use of ``export`` below. @@ -1454,14 +1463,14 @@ The resulting Json will be more detailed than that of the :ref:`download-guestbo export ID=1 curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/guestbooks/$ID/responses" - curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/guestbooks/$ID/responses?limit10&offset=0" + curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/guestbooks/$ID/responses?limit10&offset=0&sort=file&order=asc" The fully expanded example above (without environment variables) looks like this: .. code-block:: bash curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" "https://demo.dataverse.org/api/guestbooks/1/responses" - curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" "https://demo.dataverse.org/api/guestbooks/1/responses?limit10&offset=0" + curl -H "X-Dataverse-key:xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" "https://demo.dataverse.org/api/guestbooks/1/responses?limit10&offset=0&sort=type&order=desc" .. _collection-attributes-api: diff --git a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java index 79cfee32cc8..9a82a3f15af 100644 --- a/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/GuestbookResponseServiceBean.java @@ -8,6 +8,7 @@ import edu.harvard.iq.dataverse.authorization.users.AuthenticatedUser; import edu.harvard.iq.dataverse.authorization.users.User; import edu.harvard.iq.dataverse.externaltools.ExternalTool; +import edu.harvard.iq.dataverse.search.SortBy; import edu.harvard.iq.dataverse.util.StringUtil; import jakarta.ejb.EJB; import jakarta.ejb.Stateless; @@ -19,6 +20,7 @@ import jakarta.persistence.PersistenceContext; import jakarta.persistence.Query; import jakarta.persistence.TypedQuery; +import jakarta.persistence.criteria.*; import org.apache.commons.text.StringEscapeUtils; import java.io.IOException; @@ -112,24 +114,53 @@ public List findAllIds(Long dataverseId) { return em.createQuery("select o.id from GuestbookResponse o, Dataset d where o.dataset.id = d.id and d.owner.id = " + dataverseId + " order by o.responseTime desc", Long.class).getResultList(); } - public List findAllByGuestbookId(Long guestbookId) { - return findAllByGuestbookId(guestbookId, null, null); + private Order getOrderBy(CriteriaBuilder cb, Path pathObj, boolean isDescending) { + return isDescending ? cb.desc(pathObj) : cb.asc(pathObj); } - public List findAllByGuestbookId(Long guestbookId, Integer offset, Integer limit) { + public List findAllByGuestbookId(Long guestbookId, String sortField, String sortOrder, Integer offset, Integer limit) { if (guestbookId != null) { - TypedQuery query = em.createQuery("select o from GuestbookResponse as o where o.guestbook.id = " + guestbookId + " order by o.responseTime desc", GuestbookResponse.class); - if (offset != null) { - query.setFirstResult(offset); + CriteriaBuilder cb = em.getCriteriaBuilder(); + CriteriaQuery cq = cb.createQuery(GuestbookResponse.class); + Root guestbookResponseRoot = cq.from(GuestbookResponse.class); + + boolean isDescending = sortOrder != null && sortOrder.equalsIgnoreCase(SortBy.DESCENDING); + Order order; + String orderField = (sortField == null) ? "" : sortField.toLowerCase(); + switch(orderField) { + case "date": + order = getOrderBy(cb, guestbookResponseRoot.get("responseTime"), isDescending); + break; + case "type": + order = getOrderBy(cb, guestbookResponseRoot.get("eventType"), isDescending); + break; + case "file": + Join dataFileJoin = guestbookResponseRoot.join("dataFile", JoinType.INNER); + order = getOrderBy(cb, dataFileJoin.get("fileMetadatas").get("label"), isDescending); + break; + case "user": + order = getOrderBy(cb, guestbookResponseRoot.get("name"), isDescending); + break; + default: + order = null; } - if (limit != null) { - query.setMaxResults(limit); + + cq.where(cb.equal(guestbookResponseRoot.get("guestbook").get("id"), guestbookId)); + if (order != null) { + cq.orderBy(order, getOrderBy(cb, guestbookResponseRoot.get("id"), isDescending)); } + cq.distinct(true); + + int firstResult = offset == null ? 0 : offset; + int pageSize = limit == null ? Integer.MAX_VALUE : limit; - return query.getResultList(); + return em.createQuery(cq) + .setFirstResult(firstResult) + .setMaxResults(pageSize) + .getResultList(); } return null; } - + /* This method is used for streaming downloads of guestbook responses, in CSV format, both for individual guestbooks, and for entire dataverses diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Guestbooks.java b/src/main/java/edu/harvard/iq/dataverse/api/Guestbooks.java index 73b864f817f..0d8c1cd783f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Guestbooks.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Guestbooks.java @@ -21,6 +21,7 @@ import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.Response; +import java.security.InvalidParameterException; import java.sql.Timestamp; import java.time.Instant; import java.util.List; @@ -174,12 +175,11 @@ public Response updateGuestbook(@Context ContainerRequestContext crc, @Operation(summary = "Lists guestbook responses", description = "Returns guestbook metadata and response records, with pagination links when a limit is supplied.") public Response getResponses(@Context ContainerRequestContext crc, - @Parameter(description = "Numeric id of the guestbook whose responses are listed.", required = true) - @PathParam("id") Long id, - @Parameter(description = "Maximum number of response records to return.") - @QueryParam("limit") Integer limit, - @Parameter(description = "Response record offset.") - @QueryParam("offset") Integer offset) { + @Parameter(description = "Numeric id of the guestbook whose responses are listed.", required = true) @PathParam("id") Long id, + @Parameter(description = "Sort Field. One of: 'date'; 'type'; 'file'; 'user'") @QueryParam("sort") String sortField, + @Parameter(description = "Sort order. ('asc' or 'desc')") @QueryParam("order") String sortOrder, + @Parameter(description = "Maximum number of response records to return.") @QueryParam("limit") Integer limit, + @Parameter(description = "Response record offset.") @QueryParam("offset") Integer offset) { return response( req -> { Guestbook guestbook = guestbookService.find(id); @@ -190,12 +190,15 @@ public Response getResponses(@Context ContainerRequestContext crc, if (!permissionSvc.request(req).on(dataverse).has(Permission.EditDataverse)) { return error(Response.Status.FORBIDDEN, "Not authorized"); } + + validateFindGuestbookResponsesParameters(sortField, sortOrder, offset, limit); + Long totalUsageCount = guestbookService.findCountUsages(guestbook.getId(), null); Long totalResponseCount = guestbookResponseService.findCountByGuestbookId(guestbook.getId(), null); guestbook.setUsageCount(totalUsageCount); guestbook.setResponseCount(totalResponseCount); - List responses = guestbookResponseService.findAllByGuestbookId(guestbook.getId(), offset, limit); + List responses = guestbookResponseService.findAllByGuestbookId(guestbook.getId(), sortField, sortOrder, offset, limit); JsonObjectBuilder guestbookResponseObject = jsonObjectBuilder(); guestbookResponseObject.add("guestbook", JsonPrinter.json(guestbook)); @@ -206,23 +209,6 @@ public Response getResponses(@Context ContainerRequestContext crc, } guestbookResponseObject.add("responses", responseObjects); - if (limit != null) { - JsonObjectBuilder guestbookPageObject = jsonObjectBuilder(); - int thisOffset = offset != null ? offset : 0; - int next = thisOffset + limit; - int prev = thisOffset - limit; - - String baseUrl = crc.getUriInfo().getAbsolutePath() + "?limit=" + limit + "&offset=" ; - if (prev >= 0) { - guestbookPageObject.add("previous",baseUrl + prev); - } - if (next < totalResponseCount) { - guestbookPageObject.add("next", baseUrl + next); - } - guestbookPageObject.add("totalResponses", totalResponseCount); - - guestbookResponseObject.add("pagination", guestbookPageObject); - } return ok(guestbookResponseObject); }, getRequestUser(crc)); } @@ -268,6 +254,22 @@ public Response enableGuestbook(@Context ContainerRequestContext crc, return notFound("Guestbook " + guestbookId + " not found."); }, getRequestUser(crc)); } + + private void validateFindGuestbookResponsesParameters(String sortField, String sortOrder, Integer offset, Integer limit) throws WrappedResponse { + if (sortField != null && !List.of("date","type","file","user").contains(sortField.toLowerCase())) { + throw new WrappedResponse(error( Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("guestbookResponses.invalidSortField"))); + } + if (sortOrder != null && !List.of("asc","desc").contains(sortOrder.toLowerCase())) { + throw new WrappedResponse(error( Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("guestbookResponses.invalidSortOrder"))); + } + if (offset != null && offset < 0) { + throw new WrappedResponse(error( Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("guestbookResponses.invalidOffset"))); + } + if (limit != null && limit < 1) { + throw new WrappedResponse(error( Response.Status.BAD_REQUEST, BundleUtil.getStringFromBundle("guestbookResponses.invalidLimit"))); + } + } + private Response handleWrappedResponse(WrappedResponse ww) { String error = ConstraintViolationUtil.getErrorStringForConstraintViolations(ww.getCause()); if (!error.isEmpty()) { diff --git a/src/main/java/propertyFiles/Bundle.properties b/src/main/java/propertyFiles/Bundle.properties index 9cba188a7e6..8ebd2f43751 100644 --- a/src/main/java/propertyFiles/Bundle.properties +++ b/src/main/java/propertyFiles/Bundle.properties @@ -1524,6 +1524,10 @@ dataset.guestbookResponse.noResponse=(No Response) dataset.guestbookResponse.requestor.id=authenticatedUserId dataset.guestbookResponse.requestor.identifier=authenticatedUserIdentifier +guestbookResponses.invalidSortField=Sort field must be one of the following: 'date', 'type', 'file', 'user' +guestbookResponses.invalidSortOrder=Sort order must be either 'asc' or 'desc' +guestbookResponses.invalidOffset=Offset cannot be negative +guestbookResponses.invalidLimit=Limit must be greater than 0 # dataset.xhtml dataset.configureBtn=Configure diff --git a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java index ca8af25044c..b9a35171b41 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/FilesIT.java @@ -16,7 +16,6 @@ import io.restassured.path.json.JsonPath; import io.restassured.path.xml.XmlPath; import io.restassured.response.Response; -import jakarta.json.Json; import jakarta.json.JsonObject; import jakarta.json.JsonObjectBuilder; import jakarta.ws.rs.core.Response.Status; @@ -3953,7 +3952,7 @@ public void testDownloadFileWithGuestbookResponse() throws IOException, JsonPars Response publishResponse = UtilIT.publishDataverseViaNativeApi(parentDataverseAlias, ownerApiToken); assertEquals(200, publishResponse.getStatusCode()); // Create a Parent Guestbook - Guestbook parentGuestbook = UtilIT.createRandomGuestbook(parentDataverseAlias, null, ownerApiToken); + UtilIT.createRandomGuestbook(parentDataverseAlias, null, ownerApiToken); // Create Dataverse String dataverseAlias = createDataverseGetAlias(ownerApiToken); @@ -4181,21 +4180,32 @@ public void testDownloadFileWithGuestbookResponse() throws IOException, JsonPars .statusCode(OK.getStatusCode()); JsonPath jsonPath = JsonPath.from(guestbookListResponses.body().asString()); int totalCount = jsonPath.getList("data.responses").size(); + int totalCountFromJson = jsonPath.getInt("data.guestbook.responseCount"); assertTrue(totalCount > 0); assertNotNull(jsonPath.getString("data.responses[0].name")); + // Test Get All Responses Sorted + testSortByField(guestbook.getId(), "file", "asc", 0, Integer.MAX_VALUE, null, ownerApiToken); + testSortByField(guestbook.getId(), "user", "asc", null, null, null, ownerApiToken); + testSortByField(guestbook.getId(), "user", "desc",null, null, null, ownerApiToken); + testSortByField(guestbook.getId(), "date", "asc", null, null, null, ownerApiToken); + testSortByField(guestbook.getId(), "date", "desc",null, null, null, ownerApiToken); + // Test Get All Responses Sorted with errors + testSortByField(guestbook.getId(), "bad", null, null, null, BundleUtil.getStringFromBundle("guestbookResponses.invalidSortField"), ownerApiToken); + testSortByField(guestbook.getId(), null, "bad", null, null, BundleUtil.getStringFromBundle("guestbookResponses.invalidSortOrder"), ownerApiToken); + testSortByField(guestbook.getId(), null, null, -1, null, BundleUtil.getStringFromBundle("guestbookResponses.invalidOffset"), ownerApiToken); + testSortByField(guestbook.getId(), null, null, null, 0, BundleUtil.getStringFromBundle("guestbookResponses.invalidLimit"), ownerApiToken); + // Test Get Responses with pagination int pages = 4; // total should be 17. set to 4 pages int limit = (totalCount / pages) + 1; // should be 5 per page. we should see 5, 5, 5, 2 int pagedTotalCount = 0; - int totalCountFromJson = 0; for (int i = 0; i < pages; i++) { int offset = limit * i; - guestbookListResponses = UtilIT.getGuestbooksResponses(guestbook.getId(), offset, limit, ownerApiToken); + guestbookListResponses = UtilIT.getGuestbooksResponses(guestbook.getId(), "date", null, offset, limit, ownerApiToken); guestbookListResponses.prettyPrint(); jsonPath = JsonPath.from(guestbookListResponses.body().asString()); pagedTotalCount += jsonPath.getList("data.responses").size(); - totalCountFromJson = jsonPath.getInt("data.pagination.totalResponses"); // 'No duplicate ids' was manually verified. Just make sure the count is good. If there were duplicates the count would be high } // verify all counts are good and equal @@ -4203,6 +4213,30 @@ public void testDownloadFileWithGuestbookResponse() throws IOException, JsonPars assertEquals(pagedTotalCount, totalCountFromJson); } + private void testSortByField(Long id, String sortField, String order, Integer offset, Integer limit, String errorMsg, String token) { + Response guestbookListResponses = UtilIT.getGuestbooksResponses(id, sortField, order, offset, limit, token); + Map fieldMap = Map.of("type", "type", "user", "name", "file", "fileName", "date", "date"); + String fieldName = sortField != null ? fieldMap.get(sortField) : null; + boolean isDescending = order != null && order.equalsIgnoreCase("desc"); + guestbookListResponses.prettyPrint(); + if (errorMsg == null) { + guestbookListResponses.then().assertThat().statusCode(OK.getStatusCode()); + JsonPath jsonPath = JsonPath.from(guestbookListResponses.body().asString()); + int totalCount = jsonPath.getList("data.responses").size(); + assertTrue(totalCount > 0); + String lastFieldValue = jsonPath.getString("data.responses[0]." + fieldName).toLowerCase(); // The sort seems to be case-insensitive + for (int i = 1; i < totalCount; i++) { + String fieldValue = jsonPath.getString("data.responses[" + i + "]." + fieldName).toLowerCase(); + assertTrue(isDescending ? fieldValue.compareTo(lastFieldValue) <= 0 : fieldValue.compareTo(lastFieldValue) >= 0); + lastFieldValue = fieldValue; + } + } else { + guestbookListResponses.then().assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", equalTo(errorMsg)); + } + } + @Test public void testGetFileCitationFormatted() { Response createUser = UtilIT.createRandomUser(); diff --git a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java index edec890b2e9..11667910db2 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/UtilIT.java @@ -641,6 +641,9 @@ static Response getGuestbooks(String dataverseAlias, String apiToken, boolean in } static Response getGuestbooksResponses(Long guestbookId, Integer offset, Integer limit, String apiToken) { + return getGuestbooksResponses(guestbookId, null, null, offset, limit, apiToken); + } + static Response getGuestbooksResponses(Long guestbookId, String sortField, String sortOrder, Integer offset, Integer limit, String apiToken) { RequestSpecification requestSpec = given(); if (apiToken != null) { requestSpec.header(API_TOKEN_HTTP_HEADER, apiToken); @@ -651,6 +654,12 @@ static Response getGuestbooksResponses(Long guestbookId, Integer offset, Integer if (limit != null) { requestSpec.queryParam("limit", limit); } + if (sortField != null) { + requestSpec.queryParam("sort", sortField); + } + if (sortOrder != null) { + requestSpec.queryParam("order", sortOrder); + } return requestSpec.get("/api/guestbooks/" + guestbookId + "/responses"); }