fix: default_order-code raises errors.Unauthorized() on MultiQuery (

#1299) Ok... this is one of the most creepy things I've every made. This pull request simplifies and fixes the `default_order`-feature introduced by viur-core 3.6, and also doesn't raise an unauthorized error on a MultiQuery, which is created from a `listFilter` that applies an "IN"-filter. The bug is fixed, and the code is much more readable than before.
viur-framework · Oct 25, 2024 · e33f1d9 · e33f1d9
1 parent f6862c2
commit e33f1d9
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 64 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@ This file documents any relevant changes done to ViUR-core since version 3.
 
 ## [3.6.22]
 
+- fix: `default_order`-code raises `errors.Unauthorized()` on MultiQuery (#1299)
 - fix: `UserSkel.__new__()` cannot be subSkel'ed (#1296)
 
 ## [3.6.21]

diff --git a/src/viur/core/prototypes/list.py b/src/viur/core/prototypes/list.py
@@ -184,38 +184,43 @@ def list(self, *args, **kwargs) -> t.Any:
             :raises: :exc:`viur.core.errors.Unauthorized`, if the current user does not have the required permissions.
         """
         # The general access control is made via self.listFilter()
-        query = self.listFilter(self.viewSkel().all().mergeExternalFilter(kwargs))
-        if query and query.queries and not isinstance(query.queries, list):
-            # Apply default order when specified
-            if self.default_order and not query.queries.orders and not current.request.get().kwargs.get("search"):
-                # TODO: refactor: Duplicate code in prototypes.Tree
-                if callable(default_order := self.default_order):
-                    default_order = default_order(query)
-
-                if isinstance(default_order, dict):
-                    logging.debug(f"Applying filter {default_order=}")
-                    query.mergeExternalFilter(default_order)
-
-                elif default_order:
-                    logging.debug(f"Applying {default_order=}")
-
-                    # FIXME: This ugly test can be removed when there is type that abstracts SortOrders
-                    if (
-                        isinstance(default_order, str)
-                        or (
-                            isinstance(default_order, tuple)
-                            and len(default_order) == 2
-                            and isinstance(default_order[0], str)
-                            and isinstance(default_order[1], db.SortOrder)
-                        )
-                    ):
-                        query.order(default_order)
-                    else:
-                        query.order(*default_order)
-
-            return self.render.list(query.fetch())
-
-        raise errors.Unauthorized()
+        if not (query := self.listFilter(self.viewSkel().all().mergeExternalFilter(kwargs))):
+            raise errors.Unauthorized()
+
+        # Apply default_order when possible!
+        # TODO: refactor: Duplicate code in prototypes.Tree
+        if (
+                self.default_order
+                and query.queries
+                and not isinstance(query.queries, list)
+                and not query.queries.orders
+                and not current.request.get().kwargs.get("search")
+        ):
+            if callable(default_order := self.default_order):
+                default_order = default_order(query)
+
+            if isinstance(default_order, dict):
+                logging.debug(f"Applying filter {default_order=}")
+                query.mergeExternalFilter(default_order)
+
+            elif default_order:
+                logging.debug(f"Applying {default_order=}")
+
+                # FIXME: This ugly test can be removed when there is type that abstracts SortOrders
+                if (
+                    isinstance(default_order, str)
+                    or (
+                        isinstance(default_order, tuple)
+                        and len(default_order) == 2
+                        and isinstance(default_order[0], str)
+                        and isinstance(default_order[1], db.SortOrder)
+                    )
+                ):
+                    query.order(default_order)
+                else:
+                    query.order(*default_order)
+
+        return self.render.list(query.fetch())
 
     @force_ssl
     @exposed

diff --git a/src/viur/core/prototypes/tree.py b/src/viur/core/prototypes/tree.py
@@ -292,38 +292,43 @@ def list(self, skelType: SkelType, *args, **kwargs) -> t.Any:
             raise errors.NotAcceptable("Invalid skelType provided.")
 
         # The general access control is made via self.listFilter()
-        query = self.listFilter(self.viewSkel(skelType).all().mergeExternalFilter(kwargs))
-        if query and query.queries and not isinstance(query.queries, list):
-            # Apply default order when specified
-            if self.default_order and not query.queries.orders and not current.request.get().kwargs.get("search"):
-                # TODO: refactor: Duplicate code in prototypes.List
-                if callable(default_order := self.default_order):
-                    default_order = default_order(query)
-
-                if isinstance(default_order, dict):
-                    logging.debug(f"Applying filter {default_order=}")
-                    query.mergeExternalFilter(default_order)
-
-                elif default_order:
-                    logging.debug(f"Applying {default_order=}")
-
-                    # FIXME: This ugly test can be removed when there is type that abstracts SortOrders
-                    if (
-                        isinstance(default_order, str)
-                        or (
-                            isinstance(default_order, tuple)
-                            and len(default_order) == 2
-                            and isinstance(default_order[0], str)
-                            and isinstance(default_order[1], db.SortOrder)
-                        )
-                    ):
-                        query.order(default_order)
-                    else:
-                        query.order(*default_order)
-
-            return self.render.list(query.fetch())
-
-        raise errors.Unauthorized()
+        if not (query := self.listFilter(self.viewSkel(skelType).all().mergeExternalFilter(kwargs))):
+            raise errors.Unauthorized()
+
+        # Apply default_order when possible!
+        # TODO: refactor: Duplicate code in prototypes.List
+        if (
+                self.default_order
+                and query.queries
+                and not isinstance(query.queries, list)
+                and not query.queries.orders
+                and not current.request.get().kwargs.get("search")
+        ):
+            if callable(default_order := self.default_order):
+                default_order = default_order(query)
+
+            if isinstance(default_order, dict):
+                logging.debug(f"Applying filter {default_order=}")
+                query.mergeExternalFilter(default_order)
+
+            elif default_order:
+                logging.debug(f"Applying {default_order=}")
+
+                # FIXME: This ugly test can be removed when there is type that abstracts SortOrders
+                if (
+                    isinstance(default_order, str)
+                    or (
+                        isinstance(default_order, tuple)
+                        and len(default_order) == 2
+                        and isinstance(default_order[0], str)
+                        and isinstance(default_order[1], db.SortOrder)
+                    )
+                ):
+                    query.order(default_order)
+                else:
+                    query.order(*default_order)
+
+        return self.render.list(query.fetch())
 
     @exposed
     def structure(self, skelType: SkelType, *args, **kwargs) -> t.Any: