From 8de6482a383283bf4784affc34dfc36402a88e44 Mon Sep 17 00:00:00 2001 From: "Randy S. Hunsaker" Date: Sun, 7 Jan 2024 14:21:43 -0800 Subject: [PATCH 1/4] Fix for issue #85. Modified Postgrest.Table.GenerateUrl() to produce a single order query parameter with comma-separated expressions for each chained .Order() method rather than producing a separate order query parameter for each. --- Postgrest/Table.cs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/Postgrest/Table.cs b/Postgrest/Table.cs index 2e10811..d4b9b5e 100644 --- a/Postgrest/Table.cs +++ b/Postgrest/Table.cs @@ -655,18 +655,26 @@ public string GenerateUrl() foreach (var parsedFilter in _filters.Select(PrepareFilter)) query.Add(parsedFilter.Key, parsedFilter.Value); - foreach (var orderer in _orderers) + if (_orderers.Count > 0) { - var nullPosAttr = orderer.NullPosition.GetAttribute(); - var orderingAttr = orderer.Ordering.GetAttribute(); + var order = new StringBuilder(); - if (nullPosAttr == null || orderingAttr == null) continue; + foreach (var orderer in _orderers) + { + var nullPosAttr = orderer.NullPosition.GetAttribute(); + var orderingAttr = orderer.Ordering.GetAttribute(); - var key = !string.IsNullOrEmpty(orderer.ForeignTable) ? $"{orderer.ForeignTable}.order" : "order"; - query.Add(key, $"{orderer.Column}.{orderingAttr.Mapping}.{nullPosAttr.Mapping}"); - } + if (nullPosAttr == null || orderingAttr == null) continue; + + if (order.Length > 0) + order.Append(","); + order.Append($"{(!string.IsNullOrEmpty(orderer.ForeignTable) ? orderer.ForeignTable + "(" + orderer.Column + ")" : orderer.Column)}.{orderingAttr.Mapping}.{nullPosAttr.Mapping}"); + } + + query.Add("order", order.ToString()); + } - if (!string.IsNullOrEmpty(_columnQuery)) + if (!string.IsNullOrEmpty(_columnQuery)) query["select"] = Regex.Replace(_columnQuery!, @"\s", ""); if (_references.Count > 0) From 47cffd186e560c402e6cc45c6047e0d534c9c316 Mon Sep 17 00:00:00 2001 From: Joseph Schultz Date: Mon, 8 Jan 2024 17:36:44 +0100 Subject: [PATCH 2/4] Add test to confirm implementation --- PostgrestTests/ClientTests.cs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/PostgrestTests/ClientTests.cs b/PostgrestTests/ClientTests.cs index 9fe1ed4..e47846b 100644 --- a/PostgrestTests/ClientTests.cs +++ b/PostgrestTests/ClientTests.cs @@ -424,13 +424,26 @@ public async Task TestOrderBy() { var client = new Client(BaseUrl); + // Test with a single orderer specified var orderedResponse = await client.Table().Order("username", Ordering.Descending).Get(); var unorderedResponse = await client.Table().Get(); - var supaOrderedUsers = orderedResponse.Models; var linqOrderedUsers = unorderedResponse.Models.OrderByDescending(u => u.Username).ToList(); - CollectionAssert.AreEqual(linqOrderedUsers, supaOrderedUsers); + CollectionAssert.AreEqual(linqOrderedUsers, orderedResponse.Models); + + // Test with multiple orderers specified + var multipleOrderedResponse = await client.Table() + .Order(u => u.Username!, Ordering.Descending) + .Order(u => u.Status!, Ordering.Descending) + .Get(); + + linqOrderedUsers = unorderedResponse.Models + .OrderByDescending(u => u.Username) + .OrderByDescending(u => u.Status) + .ToList(); + + CollectionAssert.AreEqual(linqOrderedUsers, multipleOrderedResponse.Models); } [TestMethod("limit: basic")] From ecb04c8591635c3dfc35d42f1fb6823b9fa4fb2d Mon Sep 17 00:00:00 2001 From: Joseph Schultz Date: Mon, 8 Jan 2024 17:42:10 +0100 Subject: [PATCH 3/4] Clarify ternary operation by assigning a variable --- Postgrest/Table.cs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Postgrest/Table.cs b/Postgrest/Table.cs index d4b9b5e..f92357d 100644 --- a/Postgrest/Table.cs +++ b/Postgrest/Table.cs @@ -668,13 +668,18 @@ public string GenerateUrl() if (order.Length > 0) order.Append(","); - order.Append($"{(!string.IsNullOrEmpty(orderer.ForeignTable) ? orderer.ForeignTable + "(" + orderer.Column + ")" : orderer.Column)}.{orderingAttr.Mapping}.{nullPosAttr.Mapping}"); + + var selector = !string.IsNullOrEmpty(orderer.ForeignTable) + ? orderer.ForeignTable + "(" + orderer.Column + ")" + : orderer.Column; + + order.Append($"{selector}.{orderingAttr.Mapping}.{nullPosAttr.Mapping}"); } - query.Add("order", order.ToString()); - } + query.Add("order", order.ToString()); + } - if (!string.IsNullOrEmpty(_columnQuery)) + if (!string.IsNullOrEmpty(_columnQuery)) query["select"] = Regex.Replace(_columnQuery!, @"\s", ""); if (_references.Count > 0) From 99704ec24be0f2cbc993d10651d2f7f990e739ca Mon Sep 17 00:00:00 2001 From: Joseph Schultz Date: Mon, 8 Jan 2024 17:44:09 +0100 Subject: [PATCH 4/4] Fix the test that's supposed to confirm the implementation! --- PostgrestTests/ClientTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PostgrestTests/ClientTests.cs b/PostgrestTests/ClientTests.cs index e47846b..f99b3e8 100644 --- a/PostgrestTests/ClientTests.cs +++ b/PostgrestTests/ClientTests.cs @@ -440,7 +440,7 @@ public async Task TestOrderBy() linqOrderedUsers = unorderedResponse.Models .OrderByDescending(u => u.Username) - .OrderByDescending(u => u.Status) + .ThenByDescending(u => u.Status) .ToList(); CollectionAssert.AreEqual(linqOrderedUsers, multipleOrderedResponse.Models);