Merge pull request #92 from dlcs/fix/orderByDescending

Fix `orderByDescending` behaviour
dlcs · Oct 11, 2024 · 230bc95 · 230bc95
2 parents 549bdf4 + 14f78e2
commit 230bc95
Show file tree

Hide file tree

Showing 3 changed files with 15 additions and 9 deletions.
diff --git a/src/IIIFPresentation/API.Tests/Integration/GetCollectionTests.cs b/src/IIIFPresentation/API.Tests/Integration/GetCollectionTests.cs
@@ -337,14 +337,15 @@ public async Task Get_ChildFlat_ReturnsCorrectItem_WhenCalledWithSmallPageSizeAn
         collection.View.Page.Should().Be(1);
         collection.View.TotalPages.Should().Be(3);
         collection.Items!.Count.Should().Be(1);
+
         collection.Items.OfType<Collection>().First().Id.Should().Be("http://localhost/1/collections/FirstChildCollection");
     }
 
     [Theory]
-    [InlineData("id")]
-    [InlineData("slug")]
-    [InlineData("created")]
-    public async Task Get_RootFlat_ReturnsFirstPageWithSecondItem_WhenCalledWithSmallPageSizeAndOrderByDescending(string field)
+    [InlineData("id", "NonPublic")]
+    [InlineData("slug", "NonPublic")]
+    [InlineData("created", "IiifCollection")]
+    public async Task Get_RootFlat_ReturnsFirstPageWithSecondItem_WhenCalledWithSmallPageSizeAndOrderByDescending(string field, string expectedItemId)
     {
         // Arrange
         var requestMessage =
@@ -363,7 +364,8 @@ public async Task Get_RootFlat_ReturnsFirstPageWithSecondItem_WhenCalledWithSmal
         collection.View.Page.Should().Be(1);
         collection.View.TotalPages.Should().Be(3);
         collection.Items!.Count.Should().Be(1);
-        collection.Items.OfType<Collection>().First().Id.Should().Be("http://localhost/1/collections/IiifCollection");
+
+        collection.Items.OfType<Collection>().First().Id.Should().Be($"http://localhost/1/collections/{expectedItemId}");
     }
 
     [Fact]

diff --git a/src/IIIFPresentation/API/Features/Storage/StorageController.cs b/src/IIIFPresentation/API/Features/Storage/StorageController.cs
@@ -71,14 +71,18 @@ public async Task<IActionResult> Get(int customerId, string id, int? page = 1, i
 
         var orderByField = this.GetOrderBy(orderBy, orderByDescending, out var descending);
         var storageRoot =
-            await Mediator.Send(new GetCollection(customerId, id, page.Value, pageSize.Value, orderBy, descending));
+            await Mediator.Send(new GetCollection(customerId, id, page.Value, pageSize.Value, orderByField, descending));
 
         if (storageRoot.Collection == null) return this.PresentationNotFound();
 
         if (Request.ShowExtraProperties())
         {
+            var orderByParameter = orderByField != null
+                ? $"{(descending ? nameof(orderByDescending) : nameof(orderBy))}={orderByField}" 
+                : null;
+
             return Ok(storageRoot.Collection.ToFlatCollection(GetUrlRoots(), pageSize.Value, page.Value,
-                storageRoot.TotalItems, storageRoot.Items, orderByField));
+                storageRoot.TotalItems, storageRoot.Items, orderByParameter));
         }
 
         return SeeOther(storageRoot.Collection.GenerateHierarchicalCollectionId(GetUrlRoots()));

diff --git a/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs b/src/IIIFPresentation/API/Infrastructure/ControllerBaseX.cs
@@ -102,11 +102,11 @@ public static ObjectResult ValidationFailed(this ControllerBase controller, Vali
         descending = false;
         if (orderBy.HasText() && OrderByHelper.AllowedOrderByFields.Contains(orderBy.ToLower()))
         {
-            orderByField = $"orderBy={orderBy}";
+            orderByField = orderBy;
         }
         else if (orderByDescending.HasText() && OrderByHelper.AllowedOrderByFields.Contains(orderByDescending.ToLower()))
         {
-            orderByField = $"orderByDescending={orderByDescending}";
+            orderByField = orderByDescending;
             descending = true;
         }