XWIKI-21155: Allow sorting by specified field for Document Tree macro

* Make sure null values (coming from missing space home pages) are listed last in all supported databases * Have to use 'case' in order by because Oracle doesn't support sorting by boolean expression (e.g. order by is null)
xwiki · Jan 20, 2025 · a7ea4e6 · a7ea4e6
1 parent c9885ca
commit a7ea4e6
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 12 deletions.
diff --git a/...orm-index-test-docker/src/test/it/org/xwiki/index/test/ui/docker/DocumentTreeMacroIT.java b/...orm-index-test-docker/src/test/it/org/xwiki/index/test/ui/docker/DocumentTreeMacroIT.java
@@ -118,15 +118,15 @@ void sortDocumentsBy(TestUtils setup, TestReference testReference) throws Except
             Map.of("showTerminalDocuments", false, "sortDocumentsBy", "name:desc"));
         assertNodeLabels(tree.getTopLevelNodes(), "2. George", "Carol", "1. Bob", "3. Alice");
 
-        // Sort by last modification date descending.
+        // Sort by last modification date descending (null values last).
         tree = getDocumentTree(setup, testReference,
             Map.of("showTerminalDocuments", false, "sortDocumentsBy", "date:desc"));
-        assertNodeLabels(tree.getTopLevelNodes(), "Carol", "3. Alice", "1. Bob", "2. George");
+        assertNodeLabels(tree.getTopLevelNodes(), "3. Alice", "1. Bob", "2. George", "Carol");
 
-        // Sort by creation date ascending.
+        // Sort by creation date ascending (null values last).
         tree = getDocumentTree(setup, testReference,
             Map.of("showTerminalDocuments", false, "sortDocumentsBy", "creationDate"));
-        assertNodeLabels(tree.getTopLevelNodes(), "Carol", "2. George", "3. Alice", "1. Bob");
+        assertNodeLabels(tree.getTopLevelNodes(), "2. George", "3. Alice", "1. Bob", "Carol");
 
         // Let's check also the order of descendant pages (where we mix terminal and non-terminal pages).
 
@@ -143,14 +143,14 @@ void sortDocumentsBy(TestUtils setup, TestReference testReference) throws Except
             getDocumentTree(setup, testReference, Map.of("openTo", getNodeId(alice), "sortDocumentsBy", "title:desc"));
         assertNodeLabels(tree.getNode(getNodeId(alice)).getChildren(), "Fiona", "3. Eve", "2. Denis", "1. Henry");
 
-        // Sort by last modification date ascending.
+        // Sort by last modification date ascending (null values last).
         tree = getDocumentTree(setup, testReference, Map.of("openTo", getNodeId(alice), "sortDocumentsBy", "date:asc"));
-        assertNodeLabels(tree.getNode(getNodeId(alice)).getChildren(), "Fiona", "1. Henry", "2. Denis", "3. Eve");
+        assertNodeLabels(tree.getNode(getNodeId(alice)).getChildren(), "1. Henry", "2. Denis", "3. Eve", "Fiona");
 
-        // Sort by creation date descending.
+        // Sort by creation date descending (null values last).
         tree = getDocumentTree(setup, testReference,
             Map.of("openTo", getNodeId(alice), "sortDocumentsBy", "creationDate:desc"));
-        assertNodeLabels(tree.getNode(getNodeId(alice)).getChildren(), "Fiona", "2. Denis", "1. Henry", "3. Eve");
+        assertNodeLabels(tree.getNode(getNodeId(alice)).getChildren(), "2. Denis", "1. Henry", "3. Eve", "Fiona");
 
         // Verify the sort on the Children macro, which is a simple wrapper around the Document Tree macro.
 
@@ -166,9 +166,9 @@ void sortDocumentsBy(TestUtils setup, TestReference testReference) throws Except
         tree = getChildrenTree(setup, alice, Map.of("sort", "title:desc"));
         assertNodeLabels(tree.getTopLevelNodes(), "Fiona", "3. Eve", "2. Denis", "1. Henry");
 
-        // Sort by last modification date descending.
+        // Sort by last modification date descending (null values last).
         tree = getChildrenTree(setup, alice, Map.of("sort", "date:desc"));
-        assertNodeLabels(tree.getTopLevelNodes(), "Fiona", "3. Eve", "2. Denis", "1. Henry");
+        assertNodeLabels(tree.getTopLevelNodes(), "3. Eve", "2. Denis", "1. Henry", "Fiona");
     }
 
     private ViewPage createPage(TestUtils setup, DocumentReference documentReference, String title)

diff --git a/.../src/main/java/org/xwiki/index/tree/internal/nestedpages/ChildDocumentsTreeNodeGroup.java b/.../src/main/java/org/xwiki/index/tree/internal/nestedpages/ChildDocumentsTreeNodeGroup.java
@@ -197,7 +197,8 @@ private Query getNonTerminalChildDocumentsQuery(DocumentSort sort) throws QueryE
                 // We need the space home page to be able to sort by creation date and last modification date.
                 statement.append(" left outer join XWikiDocument doc on doc.space = space.reference");
                 statement.append(" and doc.name = 'WebHome' and doc.translation = 0");
-                statement.append(String.format(" order by doc.%s %s", fieldName, order.toLowerCase()));
+                // Put null values (coming from missing space home pages) last.
+                statement.append(String.format(" order by doc.%s %s nulls last", fieldName, order.toLowerCase()));
             } else {
                 statement
                     .append(String.format(" order by lower(space.name) %1$s, space.name %1$s", order.toLowerCase()));

diff --git a/...api/src/main/resources/org/xwiki/index/tree/internal/nestedpages/query/queries.hbm.xml.vm b/...api/src/main/resources/org/xwiki/index/tree/internal/nestedpages/query/queries.hbm.xml.vm
@@ -181,7 +181,7 @@
 
     ) xwikiPage
 
-    order by xwikiPage.fieldValue $order
+    order by case when xwikiPage.fieldValue is null then 1 else 0 end, xwikiPage.fieldValue $order
   </sql-query>
 #end
 #end