Added: order featured items results by display order and missing test…

…s added
IQSS · Jan 11, 2025 · b4f5ce9 · b4f5ce9
1 parent b2eca4b
commit b4f5ce9
Show file tree

Hide file tree

Showing 5 changed files with 116 additions and 28 deletions.
diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseFeaturedItem.java b/src/main/java/edu/harvard/iq/dataverse/DataverseFeaturedItem.java
@@ -8,9 +8,12 @@
 
 @NamedQueries({
         @NamedQuery(name = "DataverseFeaturedItem.deleteById",
-                query = "DELETE FROM DataverseFeaturedItem item WHERE item.id=:id")
+                query = "DELETE FROM DataverseFeaturedItem item WHERE item.id=:id"),
+        @NamedQuery(name = "DataverseFeaturedItem.findByDataverseOrderedByDisplayOrder",
+                query = "SELECT item FROM DataverseFeaturedItem item WHERE item.dataverse = :dataverse ORDER BY item.displayOrder ASC")
 })
 @Entity
+@Table(indexes = @Index(columnList = "displayOrder"))
 public class DataverseFeaturedItem {
 
     public static final int MAX_FEATURED_ITEM_CONTENT_SIZE = 15000;

diff --git a/src/main/java/edu/harvard/iq/dataverse/DataverseFeaturedItemServiceBean.java b/src/main/java/edu/harvard/iq/dataverse/DataverseFeaturedItemServiceBean.java
@@ -50,6 +50,13 @@ public void delete(Long id) {
                 .executeUpdate();
     }
 
+    public List<DataverseFeaturedItem> findAllByDataverseOrdered(Dataverse dataverse) {
+        return em
+                .createNamedQuery("DataverseFeaturedItem.findByDataverseOrderedByDisplayOrder", DataverseFeaturedItem.class)
+                .setParameter("dataverse", dataverse)
+                .getResultList();
+    }
+
     public InputStream getImageFileAsInputStream(DataverseFeaturedItem dataverseFeaturedItem) throws IOException {
         Path imagePath = Path.of(JvmSettings.DOCROOT_DIRECTORY.lookup(),
                 JvmSettings.FEATURED_ITEMS_IMAGE_UPLOADS_DIRECTORY.lookup(),

diff --git a/.../java/edu/harvard/iq/dataverse/engine/command/impl/ListDataverseFeaturedItemsCommand.java b/.../java/edu/harvard/iq/dataverse/engine/command/impl/ListDataverseFeaturedItemsCommand.java
@@ -8,10 +8,7 @@
 import edu.harvard.iq.dataverse.engine.command.DataverseRequest;
 import edu.harvard.iq.dataverse.engine.command.exception.CommandException;
 
-import java.util.Collections;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 /**
  * Lists the featured items {@link DataverseFeaturedItem} of a {@link Dataverse}.
@@ -27,7 +24,7 @@ public ListDataverseFeaturedItemsCommand(DataverseRequest request, Dataverse dat
 
     @Override
     public List<DataverseFeaturedItem> execute(CommandContext ctxt) throws CommandException {
-        return dataverse.getDataverseFeaturedItems();
+        return ctxt.dataverseFeaturedItems().findAllByDataverseOrdered(dataverse);
     }
 
     @Override

diff --git a/...ava/edu/harvard/iq/dataverse/engine/command/impl/UpdateDataverseFeaturedItemsCommand.java b/...ava/edu/harvard/iq/dataverse/engine/command/impl/UpdateDataverseFeaturedItemsCommand.java
@@ -38,7 +38,7 @@ public UpdateDataverseFeaturedItemsCommand(DataverseRequest request, Dataverse d
     public List<DataverseFeaturedItem> execute(CommandContext ctxt) throws CommandException {
         List<DataverseFeaturedItem> dataverseFeaturedItems = updateOrDeleteExistingFeaturedItems(ctxt);
         dataverseFeaturedItems.addAll(createNewFeaturedItems(ctxt));
-        dataverseFeaturedItems.sort(Comparator.comparingLong(DataverseFeaturedItem::getId));
+        dataverseFeaturedItems.sort(Comparator.comparingInt(DataverseFeaturedItem::getDisplayOrder));
         return dataverseFeaturedItems;
     }
 

diff --git a/src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java b/src/test/java/edu/harvard/iq/dataverse/api/DataversesIT.java
@@ -1586,6 +1586,7 @@ public void testCreateFeaturedItem() {
         String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse);
 
         // Should not return any error when not passing a file
+
         Response createFeatureItemResponse = UtilIT.createDataverseFeaturedItem(dataverseAlias, apiToken, "test", 0, null);
         createFeatureItemResponse.then().assertThat()
                 .statusCode(OK.getStatusCode())
@@ -1594,6 +1595,7 @@ public void testCreateFeaturedItem() {
                 .body("data.displayOrder", equalTo(0));
 
         // Should not return any error when passing correct file and data
+
         String pathToTestFile = "src/test/resources/images/coffeeshop.png";
         createFeatureItemResponse = UtilIT.createDataverseFeaturedItem(dataverseAlias, apiToken, "test", 1, pathToTestFile);
         createFeatureItemResponse.then().assertThat()
@@ -1603,25 +1605,76 @@ public void testCreateFeaturedItem() {
                 .statusCode(OK.getStatusCode());
 
         // Should return bad request error when passing incorrect file type
+
         pathToTestFile = "src/test/resources/tab/test.tab";
         createFeatureItemResponse = UtilIT.createDataverseFeaturedItem(dataverseAlias, apiToken, "test", 0, pathToTestFile);
         createFeatureItemResponse.then().assertThat()
                 .body("message", equalTo(BundleUtil.getStringFromBundle("dataverse.create.featuredItem.error.invalidFileType")))
                 .statusCode(BAD_REQUEST.getStatusCode());
 
         // Should return unauthorized error when user has no permissions
+
         Response createRandomUser = UtilIT.createRandomUser();
         String randomUserApiToken = UtilIT.getApiTokenFromResponse(createRandomUser);
         createFeatureItemResponse = UtilIT.createDataverseFeaturedItem(dataverseAlias, randomUserApiToken, "test", 0, pathToTestFile);
         createFeatureItemResponse.then().assertThat().statusCode(UNAUTHORIZED.getStatusCode());
 
         // Should return not found error when dataverse does not exist
+
         createFeatureItemResponse = UtilIT.createDataverseFeaturedItem("thisDataverseDoesNotExist", apiToken, "test", 0, pathToTestFile);
         createFeatureItemResponse.then().assertThat()
                 .body("message", equalTo("Can't find dataverse with identifier='thisDataverseDoesNotExist'"))
                 .statusCode(NOT_FOUND.getStatusCode());
     }
 
+    @Test
+    public void testListFeaturedItems() {
+        Response createUserResponse = UtilIT.createRandomUser();
+        String apiToken = UtilIT.getApiTokenFromResponse(createUserResponse);
+        Response createDataverseResponse = UtilIT.createRandomDataverse(apiToken);
+        createDataverseResponse.then().assertThat().statusCode(CREATED.getStatusCode());
+        String dataverseAlias = UtilIT.getAliasFromResponse(createDataverseResponse);
+
+        // Create test items
+
+        List<Long> ids = Arrays.asList(0L, 0L, 0L);
+        List<String> contents = Arrays.asList("Content 1", "Content 2", "Content 3");
+        List<Integer> orders = Arrays.asList(2, 1, 0);
+        List<Boolean> keepFiles = Arrays.asList(false, false, false);
+        List<String> pathsToFiles = Arrays.asList("src/test/resources/images/coffeeshop.png", null, null);
+
+        Response updateDataverseFeaturedItemsResponse = UtilIT.updateDataverseFeaturedItems(dataverseAlias, ids, contents, orders, keepFiles, pathsToFiles, apiToken);
+        updateDataverseFeaturedItemsResponse.then().assertThat()
+                .statusCode(OK.getStatusCode());
+
+        // Items should be retrieved with all their properties and sorted by displayOrder
+
+        Response listDataverseFeaturedItemsResponse = UtilIT.listDataverseFeaturedItems(dataverseAlias, apiToken);
+        listDataverseFeaturedItemsResponse.then().assertThat()
+                .body("data.size()", equalTo(3))
+                .body("data[0].content", equalTo("Content 3"))
+                .body("data[0].imageFileName", equalTo(null))
+                .body("data[0].imageFileUrl", equalTo(null))
+                .body("data[0].displayOrder", equalTo(0))
+                .body("data[1].content", equalTo("Content 2"))
+                .body("data[1].imageFileName", equalTo(null))
+                .body("data[1].imageFileUrl", equalTo(null))
+                .body("data[1].displayOrder", equalTo(1))
+                .body("data[2].content", equalTo("Content 1"))
+                .body("data[2].imageFileName", equalTo("coffeeshop.png"))
+                .body("data[2].imageFileUrl", containsString(UtilIT.getRestAssuredBaseUri() + "/api/access/dataverseFeatureItemImage/"))
+                .body("data[2].displayOrder", equalTo(2))
+                .statusCode(OK.getStatusCode());
+
+        // Should return not found error when dataverse does not exist
+
+        listDataverseFeaturedItemsResponse = UtilIT.listDataverseFeaturedItems("thisDataverseDoesNotExist", apiToken);
+        listDataverseFeaturedItemsResponse.then().assertThat()
+                .body("message", equalTo("Can't find dataverse with identifier='thisDataverseDoesNotExist'"))
+                .statusCode(NOT_FOUND.getStatusCode());
+
+    }
+
     @Test
     public void testUpdateFeaturedItems() {
         Response createUserResponse = UtilIT.createRandomUser();
@@ -1671,22 +1724,22 @@ public void testUpdateFeaturedItems() {
         updateDataverseFeaturedItemsResponse = UtilIT.updateDataverseFeaturedItems(dataverseAlias, ids, contents, orders, keepFiles, pathsToFiles, apiToken);
         updateDataverseFeaturedItemsResponse.then().assertThat()
                 .body("data.size()", equalTo(3))
-                .body("data[0].content", equalTo("Content 1 updated"))
-                .body("data[0].imageFileName", equalTo("coffeeshop.png"))
-                .body("data[0].imageFileUrl", containsString(baseUri + "/api/access/dataverseFeatureItemImage/"))
-                .body("data[0].displayOrder", equalTo(1))
-                .body("data[1].content", equalTo("Content 2"))
-                .body("data[1].imageFileName", equalTo(null))
-                .body("data[1].imageFileUrl", equalTo(null))
-                .body("data[1].displayOrder", equalTo(0))
+                .body("data[0].content", equalTo("Content 2"))
+                .body("data[0].imageFileName", equalTo(null))
+                .body("data[0].imageFileUrl", equalTo(null))
+                .body("data[0].displayOrder", equalTo(0))
+                .body("data[1].content", equalTo("Content 1 updated"))
+                .body("data[1].imageFileName", equalTo("coffeeshop.png"))
+                .body("data[1].imageFileUrl", containsString(baseUri + "/api/access/dataverseFeatureItemImage/"))
+                .body("data[1].displayOrder", equalTo(1))
                 .body("data[2].content", equalTo("Content 3"))
                 .body("data[2].imageFileName", equalTo(null))
                 .body("data[2].imageFileUrl", equalTo(null))
                 .body("data[2].displayOrder", equalTo(2))
                 .statusCode(OK.getStatusCode());
 
-        Long firstItemIdAfterUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[0].id");
-        Long secondItemIdAfterUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[1].id");
+        Long firstItemIdAfterUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[1].id");
+        Long secondItemIdAfterUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[0].id");
         Long thirdItemIdAfterUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[2].id");
 
         assertEquals(firstItemId, firstItemIdAfterUpdate);
@@ -1704,25 +1757,25 @@ public void testUpdateFeaturedItems() {
         updateDataverseFeaturedItemsResponse = UtilIT.updateDataverseFeaturedItems(dataverseAlias, ids, contents, orders, keepFiles, pathsToFiles, apiToken);
         updateDataverseFeaturedItemsResponse.then().assertThat()
                 .body("data.size()", equalTo(3))
-                .body("data[0].content", equalTo("Content 1 updated"))
-                .body("data[0].imageFileName", equalTo(null))
-                .body("data[0].imageFileUrl", equalTo(null))
-                .body("data[0].displayOrder", equalTo(1))
-                .body("data[1].content", equalTo("Content 2"))
-                .body("data[1].imageFileName", equalTo("coffeeshop.png"))
-                .body("data[1].imageFileUrl", containsString(baseUri + "/api/access/dataverseFeatureItemImage/"))
-                .body("data[1].displayOrder", equalTo(0))
+                .body("data[0].content", equalTo("Content 2"))
+                .body("data[0].imageFileName", equalTo("coffeeshop.png"))
+                .body("data[0].imageFileUrl", containsString(baseUri + "/api/access/dataverseFeatureItemImage/"))
+                .body("data[0].displayOrder", equalTo(0))
+                .body("data[1].content", equalTo("Content 1 updated"))
+                .body("data[1].imageFileName", equalTo(null))
+                .body("data[1].imageFileUrl", equalTo(null))
+                .body("data[1].displayOrder", equalTo(1))
                 .body("data[2].content", equalTo("Content 3"))
                 .body("data[2].imageFileName", equalTo(null))
                 .body("data[2].imageFileUrl", equalTo(null))
                 .body("data[2].displayOrder", equalTo(2))
                 .statusCode(OK.getStatusCode());
 
-        Long firstItemIdAftersSecondUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[0].id");
-        Long secondItemIdAfterSecondUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[1].id");
+        Long firstItemIdAfterSecondUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[1].id");
+        Long secondItemIdAfterSecondUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[0].id");
         Long thirdItemIdAfterSecondUpdate = JsonPath.from(updateDataverseFeaturedItemsResponse.body().asString()).getLong("data[2].id");
 
-        assertEquals(firstItemId, firstItemIdAftersSecondUpdate);
+        assertEquals(firstItemId, firstItemIdAfterSecondUpdate);
         assertEquals(secondItemIdAfterUpdate, secondItemIdAfterSecondUpdate);
         assertNotEquals(thirdItemIdAfterUpdate, thirdItemIdAfterSecondUpdate);
 
@@ -1742,6 +1795,20 @@ public void testUpdateFeaturedItems() {
                 .body("data[0].imageFileUrl", equalTo(null))
                 .body("data[0].displayOrder", equalTo(0))
                 .statusCode(OK.getStatusCode());
+
+        // Should return unauthorized error when user has no permissions
+
+        Response createRandomUser = UtilIT.createRandomUser();
+        String randomUserApiToken = UtilIT.getApiTokenFromResponse(createRandomUser);
+        updateDataverseFeaturedItemsResponse = UtilIT.updateDataverseFeaturedItems(dataverseAlias, ids, contents, orders, keepFiles, pathsToFiles, randomUserApiToken);
+        updateDataverseFeaturedItemsResponse.then().assertThat().statusCode(UNAUTHORIZED.getStatusCode());
+
+        // Should return not found error when dataverse does not exist
+
+        updateDataverseFeaturedItemsResponse = UtilIT.updateDataverseFeaturedItems("thisDataverseDoesNotExist", ids, contents, orders, keepFiles, pathsToFiles, apiToken);
+        updateDataverseFeaturedItemsResponse.then().assertThat()
+                .body("message", equalTo("Can't find dataverse with identifier='thisDataverseDoesNotExist'"))
+                .statusCode(NOT_FOUND.getStatusCode());
     }
 
     @Test
@@ -1775,5 +1842,19 @@ public void testDeleteFeaturedItems() {
         listFeaturedItemsResponse.then()
                 .body("data.size()", equalTo(0))
                 .assertThat().statusCode(OK.getStatusCode());
+
+        // Should return unauthorized error when user has no permissions
+
+        Response createRandomUser = UtilIT.createRandomUser();
+        String randomUserApiToken = UtilIT.getApiTokenFromResponse(createRandomUser);
+        deleteDataverseFeaturedItemsResponse = UtilIT.deleteDataverseFeaturedItems(dataverseAlias, randomUserApiToken);
+        deleteDataverseFeaturedItemsResponse.then().assertThat().statusCode(UNAUTHORIZED.getStatusCode());
+
+        // Should return not found error when dataverse does not exist
+
+        deleteDataverseFeaturedItemsResponse = UtilIT.deleteDataverseFeaturedItems("thisDataverseDoesNotExist", apiToken);
+        deleteDataverseFeaturedItemsResponse.then().assertThat()
+                .body("message", equalTo("Can't find dataverse with identifier='thisDataverseDoesNotExist'"))
+                .statusCode(NOT_FOUND.getStatusCode());
     }
 }