fix: fixed the 'order' method for 'BaseSelectRequestBuilder' (#495)

Co-authored-by: Andrew Smith <a.smith@silentworks.co.uk>
supabase · Aug 22, 2024 · 97d520e · 97d520e
1 parent b5b7b2f
commit 97d520e
Show file tree

Hide file tree

Showing 5 changed files with 110 additions and 13 deletions.
diff --git a/postgrest/base_request_builder.py b/postgrest/base_request_builder.py
@@ -563,9 +563,20 @@ def order(
         .. versionchanged:: 0.10.3
            Allow ordering results for foreign tables with the foreign_table parameter.
         """
+
+        new_order_parameter = (
+            f"{foreign_table + '(' if foreign_table else ''}{column}{')' if foreign_table else ''}"
+            f"{'.desc' if desc else ''}{'.nullsfirst' if nullsfirst else ''}"
+        )
+
+        existing_order_parameter = self.params.get("order")
+        if existing_order_parameter:
+            self.params = self.params.remove("order")
+            new_order_parameter = f"{existing_order_parameter},{new_order_parameter}"
+
         self.params = self.params.add(
-            f"{foreign_table}.order" if foreign_table else "order",
-            f"{column}{'.desc' if desc else ''}{'.nullsfirst' if nullsfirst else ''}",
+            "order",
+            new_order_parameter,
         )
         return self
 

diff --git a/tests/_async/test_filter_request_builder_integration.py b/tests/_async/test_filter_request_builder_integration.py
@@ -481,3 +481,20 @@ async def test_rpc_with_range():
         {"nicename": "Albania", "iso": "AL"},
         {"nicename": "Algeria", "iso": "DZ"},
     ]
+
+
+async def test_order():
+    res = (
+        await rest_client()
+        .from_("countries")
+        .select("country_name, iso")
+        .limit(3)
+        .order("nicename", desc=True)
+        .execute()
+    )
+
+    assert res.data == [
+        {"country_name": "ZIMBABWE", "iso": "ZW"},
+        {"country_name": "UNITED STATES", "iso": "US"},
+        {"country_name": "UNITED KINGDOM", "iso": "GB"},
+    ]
diff --git a/tests/_async/test_request_builder.py b/tests/_async/test_request_builder.py
@@ -190,6 +190,32 @@ def test_explain_options(self, request_builder: AsyncRequestBuilder):
         assert "options=analyze|verbose|buffers|wal" in str(builder.headers.get("accept"))
 
 
+class TestOrder:
+    def test_order(self, request_builder: AsyncRequestBuilder):
+        builder = request_builder.select().order("country_name", desc=True)
+        assert str(builder.params) == "order=country_name.desc"
+
+    def test_multiple_orders(self, request_builder: AsyncRequestBuilder):
+        builder = (
+            request_builder.select()
+            .order("country_name", desc=True)
+            .order("iso", desc=True)
+        )
+        assert str(builder.params) == "order=country_name.desc%2Ciso.desc"
+
+    def test_multiple_orders_on_foreign_table(self, request_builder: AsyncRequestBuilder):
+        foreign_table = "cities"
+        builder = (
+            request_builder.select()
+            .order("city_name", desc=True, foreign_table=foreign_table)
+            .order("id", desc=True, foreign_table=foreign_table)
+        )
+        assert (
+            str(builder.params)
+            == "order=cities%28city_name%29.desc%2Ccities%28id%29.desc"
+        )
+
+
 class TestRange:
     def test_range_on_own_table(self, request_builder: AsyncRequestBuilder):
         builder = request_builder.select("*").range(0, 1)

diff --git a/tests/_sync/test_filter_request_builder_integration.py b/tests/_sync/test_filter_request_builder_integration.py
@@ -474,3 +474,20 @@ def test_rpc_with_range():
         {"nicename": "Albania", "iso": "AL"},
         {"nicename": "Algeria", "iso": "DZ"},
     ]
+
+
+def test_order():
+    res = (
+        rest_client()
+        .from_("countries")
+        .select("country_name, iso")
+        .limit(3)
+        .order("nicename", desc=True)
+        .execute()
+    )
+
+    assert res.data == [
+        {"country_name": "ZIMBABWE", "iso": "ZW"},
+        {"country_name": "UNITED STATES", "iso": "US"},
+        {"country_name": "UNITED KINGDOM", "iso": "GB"},
+    ]
diff --git a/tests/_sync/test_request_builder.py b/tests/_sync/test_request_builder.py
@@ -71,6 +71,17 @@ def test_insert_with_upsert(self, request_builder: SyncRequestBuilder):
         assert builder.http_method == "POST"
         assert builder.json == {"key1": "val1"}
 
+    def test_upsert_with_default_single(self, request_builder: SyncRequestBuilder):
+        builder = request_builder.upsert([{"key1": "val1"}], default_to_null=False)
+        assert builder.headers.get_list("prefer", True) == [
+            "return=representation",
+            "resolution=merge-duplicates",
+            "missing=default",
+        ]
+        assert builder.http_method == "POST"
+        assert builder.json == [{"key1": "val1"}]
+        assert builder.params.get("columns") == '"key1"'
+
     def test_bulk_insert_using_default(self, request_builder: SyncRequestBuilder):
         builder = request_builder.insert(
             [{"key1": "val1", "key2": "val2"}, {"key3": "val3"}], default_to_null=False
@@ -95,17 +106,6 @@ def test_upsert(self, request_builder: SyncRequestBuilder):
         assert builder.http_method == "POST"
         assert builder.json == {"key1": "val1"}
 
-    def test_upsert_with_default_single(self, request_builder: SyncRequestBuilder):
-        builder = request_builder.upsert([{"key1": "val1"}], default_to_null=False)
-        assert builder.headers.get_list("prefer", True) == [
-            "return=representation",
-            "resolution=merge-duplicates",
-            "missing=default",
-        ]
-        assert builder.http_method == "POST"
-        assert builder.json == [{"key1": "val1"}]
-        assert builder.params.get("columns") == '"key1"'
-
     def test_bulk_upsert_with_default(self, request_builder: SyncRequestBuilder):
         builder = request_builder.upsert(
             [{"key1": "val1", "key2": "val2"}, {"key3": "val3"}], default_to_null=False
@@ -190,6 +190,32 @@ def test_explain_options(self, request_builder: SyncRequestBuilder):
         assert "options=analyze|verbose|buffers|wal" in str(builder.headers.get("accept"))
 
 
+class TestOrder:
+    def test_order(self, request_builder: SyncRequestBuilder):
+        builder = request_builder.select().order("country_name", desc=True)
+        assert str(builder.params) == "order=country_name.desc"
+
+    def test_multiple_orders(self, request_builder: SyncRequestBuilder):
+        builder = (
+            request_builder.select()
+            .order("country_name", desc=True)
+            .order("iso", desc=True)
+        )
+        assert str(builder.params) == "order=country_name.desc%2Ciso.desc"
+
+    def test_multiple_orders_on_foreign_table(self, request_builder: SyncRequestBuilder):
+        foreign_table = "cities"
+        builder = (
+            request_builder.select()
+            .order("city_name", desc=True, foreign_table=foreign_table)
+            .order("id", desc=True, foreign_table=foreign_table)
+        )
+        assert (
+            str(builder.params)
+            == "order=cities%28city_name%29.desc%2Ccities%28id%29.desc"
+        )
+
+
 class TestRange:
     def test_range_on_own_table(self, request_builder: SyncRequestBuilder):
         builder = request_builder.select("*").range(0, 1)