From 093d0f926f969982a4060bb9a13f9dac4e6e6d5e Mon Sep 17 00:00:00 2001 From: "pullapprove5-fix[bot]" <4489445+pullapprove5-fix[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:15:05 +0000 Subject: [PATCH] Fix: plain-admin view URL methods duplicated 5x instead of declared on AdminView base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding confirmed and fixed. plain-admin/plain/admin/views/objects.py duplicated identical `get_list_url`/`get_create_url`/`get_detail_url`/`get_update_url`/`get_delete_url` `return ""` stubs across 5 AdminView subclasses (25 copies of 5 methods), and viewsets.py/registry.py carried 7 `# ty: ignore` comments only because AdminViewset.get_views() was typed to return `list[type[View]]` (the framework base View) instead of `list[type[AdminView]]`, and registry.py's `register_view` TypeVar `T` was unbound. Reproduced by stripping the 7 `# ty: ignore` comments from viewsets.py/registry.py and running `uv run ty check plain-admin` (had to `pip install --user uv` first since it wasn't on PATH in this sandbox) — got exactly 7 diagnostics: 6 `unresolved-attribute` in viewsets.py (`viewset`, `get_list_url`, `get_create_url`, `get_detail_url`, `get_update_url`, `get_delete_url` on `type[View[Response]]`) plus 1 `invalid-argument-type` in registry.py:44 (`set.add` expecting `type[AdminView]`, got `type[T@register_view]`). Restored the ignores and confirmed baseline is clean, masking the issue. Fix: declared the five `get_*_url` methods directly on `AdminView` (base.py), deleted all 25 duplicate stubs from objects.py, changed `AdminViewset.get_views()` to iterate/return `list[type[AdminView]]` (checking `issubclass(attr, AdminView)` instead of the framework's `View`), and bound registry.py's `TypeVar("T")` to `AdminView` (needed for the 7th ignore — `register_view`'s generic `T` was unbound, so even after fixing `get_views()`'s return type, `self.registered_views.add(view)` still failed under `ty` until `T` had an upper bound). Removed all 7 `# ty: ignore` comments; none remain in the touched files (one pre-existing, unrelated ignore remains in models.py, out of scope). Verified the flip: same `uv run ty check plain-admin` command that produced 7 diagnostics on the pre-fix tree (with ignores stripped) now reports "All checks passed!" on the fixed tree with real ignores permanently removed (not just re-added). Also ran `./scripts/fix plain-admin` (ruff, oxlint/oxfmt, prettier) and `uv run plain-code check plain-admin` (ruff, ty, oxlint, annotation coverage) — all clean. Could not run `./scripts/test plain-admin` or `./scripts/test`: the suite requires a Postgres database via `scripts/start-postgres`, which falls back to Docker or a local Postgres on 5432, and neither is available in this sandbox (`docker info` fails, port 5432 refuses connections, and `apt-get install postgresql` 404s on the mirror for the required packages). No behavioral/runtime test coverage could be exercised for this change; static analysis (ty/ruff/oxlint) is what's verified here. The change is a pure type/structure refactor (moving identical method bodies up the class hierarchy) with no behavioral change, so this risk is low, but it's an honest gap in verification. --- plain-admin/plain/admin/views/base.py | 17 +++++ plain-admin/plain/admin/views/objects.py | 75 ----------------------- plain-admin/plain/admin/views/registry.py | 4 +- plain-admin/plain/admin/views/viewsets.py | 21 +++---- 4 files changed, 29 insertions(+), 88 deletions(-) diff --git a/plain-admin/plain/admin/views/base.py b/plain-admin/plain/admin/views/base.py index f221ecad50..9f23c58894 100644 --- a/plain-admin/plain/admin/views/base.py +++ b/plain-admin/plain/admin/views/base.py @@ -68,6 +68,23 @@ def has_permission(cls, user: Model) -> bool: # Set dynamically by AdminViewset.get_views() viewset: type[AdminViewset] | None = None + # Set dynamically by AdminViewset.get_views() to the sibling view's + # get_view_url, when that sibling view exists on the viewset. + def get_list_url(self) -> str: + return "" + + def get_create_url(self) -> str: + return "" + + def get_detail_url(self, obj: Any) -> str: + return "" + + def get_update_url(self, obj: Any) -> str: + return "" + + def get_delete_url(self, obj: Any) -> str: + return "" + template_name = "admin/page.html" cards: tuple[Card, ...] = () diff --git a/plain-admin/plain/admin/views/objects.py b/plain-admin/plain/admin/views/objects.py index c3deebe195..88bc7f58dc 100644 --- a/plain-admin/plain/admin/views/objects.py +++ b/plain-admin/plain/admin/views/objects.py @@ -246,21 +246,6 @@ def get_filter_names(self) -> tuple[str, ...]: def get_object_id(self, obj: Any) -> Any: return self.get_field_value(obj, "id") - def get_list_url(self) -> str: - return "" - - def get_create_url(self) -> str: - return "" - - def get_detail_url(self, obj: Any) -> str: - return "" - - def get_update_url(self, obj: Any) -> str: - return "" - - def get_delete_url(self, obj: Any) -> str: - return "" - def get_object_url(self, obj: Any) -> str: if url := self.get_detail_url(obj): return url @@ -294,21 +279,6 @@ class AdminCreateView(AdminView, CreateView): template_name = None nav_section = None - def get_list_url(self) -> str: - return "" - - def get_create_url(self) -> str: - return "" - - def get_detail_url(self, obj: Any) -> str: - return "" - - def get_update_url(self, obj: Any) -> str: - return "" - - def get_delete_url(self, obj: Any) -> str: - return "" - def get_success_url(self, form: "BaseForm") -> str: if list_url := self.get_list_url(): return list_url @@ -335,21 +305,6 @@ def get_template_names(self) -> list[str]: "admin/detail.html", # A generic detail view for rendering any object ] - def get_list_url(self) -> str: - return "" - - def get_create_url(self) -> str: - return "" - - def get_detail_url(self, obj: Any) -> str: - return "" - - def get_update_url(self, obj: Any) -> str: - return "" - - def get_delete_url(self, obj: Any) -> str: - return "" - def get_fields(self) -> tuple[str, ...]: return self.fields @@ -372,21 +327,6 @@ class AdminUpdateView(AdminView, UpdateView): template_name = None nav_section = None - def get_list_url(self) -> str: - return "" - - def get_create_url(self) -> str: - return "" - - def get_detail_url(self, obj: Any) -> str: - return "" - - def get_update_url(self, obj: Any) -> str: - return "" - - def get_delete_url(self, obj: Any) -> str: - return "" - def get_links(self) -> dict[str, str]: links = super().get_links() @@ -418,21 +358,6 @@ class AdminDeleteView(AdminView, DeleteView): template_name = "admin/delete.html" nav_section = None - def get_list_url(self) -> str: - return "" - - def get_create_url(self) -> str: - return "" - - def get_detail_url(self, obj: Any) -> str: - return "" - - def get_update_url(self, obj: Any) -> str: - return "" - - def get_delete_url(self, obj: Any) -> str: - return "" - def get_links(self) -> dict[str, str]: links = super().get_links() diff --git a/plain-admin/plain/admin/views/registry.py b/plain-admin/plain/admin/views/registry.py index c6cf140dcc..171d3fe06d 100644 --- a/plain-admin/plain/admin/views/registry.py +++ b/plain-admin/plain/admin/views/registry.py @@ -15,7 +15,7 @@ from .base import AdminView from .viewsets import AdminViewset -T = TypeVar("T") +T = TypeVar("T", bound="AdminView") VS = TypeVar("VS", bound="AdminViewset") @@ -41,7 +41,7 @@ def register_view( self, view: type[T] | None = None ) -> type[T] | Callable[[type[T]], type[T]]: def inner(view: type[T]) -> type[T]: - self.registered_views.add(view) # ty: ignore[invalid-argument-type] + self.registered_views.add(view) # Invalidate lookup caches self.__dict__.pop("slug_to_view", None) self.__dict__.pop("path_to_view", None) diff --git a/plain-admin/plain/admin/views/viewsets.py b/plain-admin/plain/admin/views/viewsets.py index c50e7d4292..ee359d960e 100644 --- a/plain-admin/plain/admin/views/viewsets.py +++ b/plain-admin/plain/admin/views/viewsets.py @@ -1,9 +1,9 @@ -from plain.views import View +from .base import AdminView class AdminViewset: @classmethod - def get_views(cls) -> list[type[View]]: + def get_views(cls) -> list[type[AdminView]]: """Views are defined as inner classes on the viewset class.""" # Primary views that we can interlink automatically @@ -27,29 +27,28 @@ def get_views(cls) -> list[type[View]]: DeleteView.parent_view_class = DetailView # Now iterate all inner view classes - views: list[type[View]] = [] + views: list[type[AdminView]] = [] for attr in cls.__dict__.values(): - if isinstance(attr, type) and issubclass(attr, View): + if isinstance(attr, type) and issubclass(attr, AdminView): views.append(attr) for view in views: - # Dynamic attributes stamped onto the view class by the viewset. - view.viewset = cls # ty: ignore[unresolved-attribute] + view.viewset = cls if ListView: - view.get_list_url = ListView.get_view_url # ty: ignore[unresolved-attribute] + view.get_list_url = ListView.get_view_url if CreateView: - view.get_create_url = CreateView.get_view_url # ty: ignore[unresolved-attribute] + view.get_create_url = CreateView.get_view_url if DetailView: - view.get_detail_url = DetailView.get_view_url # ty: ignore[unresolved-attribute] + view.get_detail_url = DetailView.get_view_url if UpdateView: - view.get_update_url = UpdateView.get_view_url # ty: ignore[unresolved-attribute] + view.get_update_url = UpdateView.get_view_url if DeleteView: - view.get_delete_url = DeleteView.get_view_url # ty: ignore[unresolved-attribute] + view.get_delete_url = DeleteView.get_view_url return views