add order_by argument to piccolo user list command (#925)

piccolo/apps/user/commands/list.py

@@ -1,15 +1,43 @@
+import typing as t
+
 from piccolo.apps.user.tables import BaseUser
+from piccolo.columns import Column
 from piccolo.utils.printing import print_dict_table
 
+ORDER_BY_COLUMN_NAMES = [
+    i._meta.name for i in BaseUser.all_columns(exclude=[BaseUser.password])
+]
+
+
+async def get_users(
+    order_by: Column, ascending: bool, limit: int, page: int
+) -> t.List[t.Dict[str, t.Any]]:
+    return (
+        await BaseUser.select(
+            *BaseUser.all_columns(exclude=[BaseUser.password])
+        )
+        .order_by(
+            order_by,
+            ascending=ascending,
+        )
+        .limit(limit)
+        .offset(limit * (page - 1))
+    )
 
-def list_users(limit: int = 20, page: int = 1):
+
+async def list_users(
+    limit: int = 20, page: int = 1, order_by: str = "username"
+):
     """
     List existing users.
 
     :param limit:
         The maximum number of users to list.
     :param page:
         Lets you paginate through the list of users.
+    :param order_by:
+        The column used to order the results. Prefix with '-' for descending
+        order.
 
     """
     if page < 1:
@@ -18,12 +46,22 @@ def list_users(limit: int = 20, page: int = 1):
     if limit < 1:
         raise ValueError("The limit number must be > 0.")
 
-    users = (
-        BaseUser.select(*BaseUser.all_columns(exclude=[BaseUser.password]))
-        .order_by(BaseUser.username)
-        .limit(limit)
-        .offset(limit * (page - 1))
-        .run_sync()
+    ascending = True
+    if order_by.startswith("-"):
+        ascending = False
+        order_by = order_by[1:]
+
+    if order_by not in ORDER_BY_COLUMN_NAMES:
+        raise ValueError(
+            "The order_by argument must be one of the following: "
+            + ", ".join(ORDER_BY_COLUMN_NAMES)
+        )
+
+    users = await get_users(
+        order_by=BaseUser._meta.get_column_by_name(order_by),
+        ascending=ascending,
+        limit=limit,
+        page=page,
     )
 
     if len(users) == 0:

tests/apps/user/commands/test_list.py

@@ -1,8 +1,9 @@
 from unittest import TestCase
-from unittest.mock import MagicMock, patch
+from unittest.mock import AsyncMock, MagicMock, patch
 
 from piccolo.apps.user.commands.list import list_users
 from piccolo.apps.user.tables import BaseUser
+from piccolo.utils.sync import run_sync
 
 
 class TestList(TestCase):
@@ -22,7 +23,7 @@ def test_list(self, print_mock: MagicMock):
         """
         Make sure the user information is listed, excluding the password.
         """
-        list_users()
+        run_sync(list_users())
 
         output = "\n".join(i.args[0] for i in print_mock.call_args_list)
 
@@ -31,19 +32,66 @@ def test_list(self, print_mock: MagicMock):
         assert self.user.password not in output
 
 
-class TestListArgs(TestCase):
-    def test_limit(self):
+class TestLimit(TestCase):
+    def test_non_positive(self):
         """
         Make sure non-positive `limit` values are rejected.
         """
         for value in (0, -1):
             with self.assertRaises(ValueError):
-                list_users(page=value)
+                run_sync(list_users(page=value))
 
-    def test_page(self):
+
+class TestPage(TestCase):
+    def test_non_positive(self):
         """
         Make sure non-positive `page` values are rejected.
         """
         for value in (0, -1):
             with self.assertRaises(ValueError):
-                list_users(limit=value)
+                run_sync(list_users(limit=value))
+
+
+class TestOrder(TestCase):
+    @patch("piccolo.apps.user.commands.list.get_users")
+    def test_order(self, get_users: AsyncMock):
+        """
+        Make sure valid column names are accepted.
+        """
+        get_users.return_value = []
+        run_sync(list_users(order_by="email"))
+
+        self.assertDictEqual(
+            get_users.call_args.kwargs,
+            {
+                "order_by": BaseUser.email,
+                "ascending": True,
+                "limit": 20,
+                "page": 1,
+            },
+        )
+
+    @patch("piccolo.apps.user.commands.list.get_users")
+    def test_descending(self, get_users: AsyncMock):
+        """
+        Make sure a colume name prefixed with '-' works.
+        """
+        get_users.return_value = []
+        run_sync(list_users(order_by="-email"))
+
+        self.assertDictEqual(
+            get_users.call_args.kwargs,
+            {
+                "order_by": BaseUser.email,
+                "ascending": False,
+                "limit": 20,
+                "page": 1,
+            },
+        )
+
+    def test_unrecognised_column(self):
+        """
+        Make sure invalid column names are rejected.
+        """
+        with self.assertRaises(ValueError):
+            run_sync(list_users(order_by="abc123"))