From e28f3fbd6ea70172b2edd78ddf58d2b45bd0c444 Mon Sep 17 00:00:00 2001 From: borhanst Date: Tue, 8 Sep 2026 18:27:57 +0600 Subject: [PATCH 01/12] Refactor object label handling and search functionality - Introduced `get_display_label` and `get_default_search_fields` functions to centralize label retrieval and search field defaults via the introspection backend. - Updated `model_display_name` to utilize the new label retrieval method, ensuring consistent label rendering across the application. - Refactored `ModelAdmin` and related views to use introspection for displaying object labels in dropdowns and search results. - Enhanced JavaScript utility functions for normalizing option objects and handling label/value extraction. - Updated templates to reflect changes in label handling, ensuring that displayed labels are derived from the introspection backend. - Improved multi-relation and permission selection widgets to use the new label structure, maintaining backward compatibility with existing IDs. --- fastapi_admin_kit/backends/memory.py | 40 +++++++ fastapi_admin_kit/backends/protocols.py | 24 ++++ fastapi_admin_kit/backends/sqlalchemy.py | 86 ++++++++++++++ fastapi_admin_kit/export_import/csv.py | 9 +- fastapi_admin_kit/inspection.py | 16 ++- fastapi_admin_kit/inspection/__init__.py | 44 +++++-- fastapi_admin_kit/modeladmin.py | 15 ++- fastapi_admin_kit/router.py | 27 +++-- fastapi_admin_kit/static/js/admin.js | 107 ++++++++++++++---- .../templates/macros/form_fields.html | 18 +-- fastapi_admin_kit/templates/pages/search.html | 8 +- .../templates/pages/users/form.html | 17 +-- .../templates/partials/detail_field.html | 17 +++ .../templates/partials/inline_stacked.html | 5 +- .../templates/partials/inline_tabular.html | 5 +- .../partials/perm_select_widget.html | 8 +- .../templates/partials/permission_widget.html | 8 +- fastapi_admin_kit/views.py | 4 +- fastapi_admin_kit/views/class_views.py | 92 +++++++++++---- fastapi_admin_kit/views/context.py | 10 +- fastapi_admin_kit/views/factory.py | 52 +++++++-- fastapi_admin_kit/views/list_context.py | 10 +- fastapi_admin_kit/views/roles.py | 20 ++-- fastapi_admin_kit/views/users.py | 2 +- 24 files changed, 505 insertions(+), 139 deletions(-) diff --git a/fastapi_admin_kit/backends/memory.py b/fastapi_admin_kit/backends/memory.py index 7028ac1..21edd93 100644 --- a/fastapi_admin_kit/backends/memory.py +++ b/fastapi_admin_kit/backends/memory.py @@ -442,6 +442,46 @@ def get_pk_columns(self, model: type) -> list[Any]: pk = _pk_field_name(model) return [pk] if pk else [] + _DISPLAY_CANDIDATES: tuple[str, ...] = ("name", "title", "email", "username", "label") + + def get_display_label(self, obj: Any) -> str | None: + """Short label for *obj* (custom ``__str__`` or display field).""" + if type(obj).__str__ is not object.__str__: + try: + text = str(obj) + except Exception: + text = "" + if text and "=" not in text: + return text + schema: Schema = getattr(type(obj), "__schema__", None) + available = {f.name for f in schema.fields} if schema is not None else None + for attr in self._DISPLAY_CANDIDATES: + if available is not None and attr not in available: + continue + try: + label = getattr(obj, attr, None) + except Exception: + label = None + if label is not None and str(label).strip(): + return str(label) + return None + + def get_default_search_fields(self, model: type) -> list[str]: + """Default ``search_fields`` for *model* (existing candidates first).""" + schema: Schema = getattr(model, "__schema__", None) + if schema is None: + return [] + by_name = {f.name: f for f in schema.fields} + found = [a for a in self._DISPLAY_CANDIDATES if a in by_name] + if found: + return found + for f in schema.fields: + if any( + hint in str(f.type).lower() for hint in ("string", "str", "text", "char", "email") + ): + return [f.name] + return [] + # --------------------------------------------------------------------------- # Audit backend diff --git a/fastapi_admin_kit/backends/protocols.py b/fastapi_admin_kit/backends/protocols.py index 573ec45..a17db24 100644 --- a/fastapi_admin_kit/backends/protocols.py +++ b/fastapi_admin_kit/backends/protocols.py @@ -75,6 +75,30 @@ def get_pk_columns(self, model: type) -> list[Any]: """Return the primary key column(s) for a model.""" ... + def get_display_label(self, obj: Any) -> str | None: + """Return a short human-readable label for *obj*, or None. + + Used for relation-picker options, list/detail relation values, and + anywhere a related object must render as ``label`` instead of full + row data. Backends must ignore framework-default ``__str__`` + implementations that dump every field (e.g. SQLModel/Pydantic + ``BaseModel.__str__``) and prefer a custom ``__str__``, falling + back to ``name`` / ``title`` / ``email`` / ``username`` attributes. + Callers apply their own final fallback (``#id``, ``ClassName:pk``). + """ + ... + + def get_default_search_fields(self, model: type) -> list[str]: + """Return default searchable field names for *model*. + + Used when ``ModelAdmin.search_fields`` is unset. Backends return + the ``name`` / ``title`` / ``email`` / ``username`` columns that + actually exist on the model, falling back to the first text-like + column so models without those fields (e.g. feedback/comment + tables) are still searchable instead of silently unfiltered. + """ + ... + @runtime_checkable class SessionBackend(Protocol): diff --git a/fastapi_admin_kit/backends/sqlalchemy.py b/fastapi_admin_kit/backends/sqlalchemy.py index f5a0d79..5445fc7 100644 --- a/fastapi_admin_kit/backends/sqlalchemy.py +++ b/fastapi_admin_kit/backends/sqlalchemy.py @@ -347,6 +347,92 @@ def get_pk_columns(self, model: type) -> list[Any]: mapper = sa_inspect(model) return list(mapper.primary_key) + # Display candidates in priority order. Only attributes that exist as + # real columns are considered, so proxies/deferred loaders on missing + # attributes are never triggered. + _DISPLAY_CANDIDATES: tuple[str, ...] = ("name", "title", "email", "username", "label") + + def get_display_label(self, obj: Any) -> str | None: + """Short label for *obj* (custom ``__str__`` or display column).""" + if self._has_custom_str(obj): + try: + text = str(obj) + except Exception: + text = "" + # Guard against __str__ implementations that still dump fields + # (e.g. "id=1 name='x'"): prefer a clean column value instead. + if text and "=" not in text: + return text + try: + columns, _ = self.inspect_model(type(obj)) + available = {c.name for c in columns} + except Exception: + # Not a mapped model (plain object, memory record, ...): + # fall back to a plain attribute probe. + available = None + for attr in self._DISPLAY_CANDIDATES: + if available is not None and attr not in available: + continue + try: + label = getattr(obj, attr, None) + except Exception: + label = None + if label is not None and str(label).strip(): + return str(label) + if self._has_custom_str(obj): + try: + return str(obj) + except Exception: + return None + return None + + def get_default_search_fields(self, model: type) -> list[str]: + """Default ``search_fields`` for *model* (existing candidates first).""" + try: + columns, _ = self.inspect_model(model) + except Exception: + return [] + by_name = {c.name: c for c in columns} + found = [a for a in self._DISPLAY_CANDIDATES if a in by_name] + if found: + return found + for col in columns: + # ColumnMeta.type may be a type *class* (SQLModel-resolved) or + # an instance (plain SQLAlchemy) — handle both. + col_type = col.type + if col_type is None: + continue + type_name = ( + col_type.__name__ if isinstance(col_type, type) else type(col_type).__name__ + ).lower() + if any(hint in type_name for hint in ("string", "text", "char", "unicode")): + return [col.name] + return [] + + @staticmethod + def _has_custom_str(obj: Any) -> bool: + """True if the model defines a real custom ``__str__``. + + Framework-default ``__str__`` implementations that dump every + field (SQLModel/Pydantic ``BaseModel.__str__``) are explicitly + ignored so they never leak full row data into labels. + """ + str_fn = type(obj).__str__ + if str_fn is object.__str__: + return False + owner = getattr(str_fn, "__objclass__", None) + if owner is None: + # Plain function defined on a class in the MRO — find it. + for klass in type(obj).__mro__: + if "__str__" in klass.__dict__: + owner = klass + break + if owner is not None: + module = getattr(owner, "__module__", "") or "" + if module.split(".")[0] in ("sqlmodel", "pydantic"): + return False + return True + # -- internal helpers --------------------------------------------------- def _is_sqlmodel(self, model: type) -> bool: diff --git a/fastapi_admin_kit/export_import/csv.py b/fastapi_admin_kit/export_import/csv.py index 2624a9e..53704a9 100644 --- a/fastapi_admin_kit/export_import/csv.py +++ b/fastapi_admin_kit/export_import/csv.py @@ -98,12 +98,13 @@ def export_filtered( # Apply search filter if q: + from fastapi_admin_kit.inspection import get_default_search_fields from fastapi_admin_kit.search_utils import apply_search_filter - search_fields = getattr(self.admin, "search_fields", None) or [ - "name", - "title", - ] + search_fields = getattr(self.admin, "search_fields", None) or get_default_search_fields( + self.registered.model, + getattr(request.app.state, "admin_introspection_adapter", None), + ) from sqlalchemy import select queryset = apply_search_filter( diff --git a/fastapi_admin_kit/inspection.py b/fastapi_admin_kit/inspection.py index 3a7cde3..77d80cb 100644 --- a/fastapi_admin_kit/inspection.py +++ b/fastapi_admin_kit/inspection.py @@ -104,12 +104,18 @@ def is_required(col: ColumnMeta) -> bool: def model_display_name(obj: Any) -> str: """Return a human-readable label for an ORM object. - Uses the model's ``__str__`` if it has a custom implementation. - Falls back to ``ClassName:pk`` when ``__str__`` is the default - ``object.__str__``. + Delegates to the introspection backend + (``IntrospectionBackend.get_display_label``) so display logic lives + behind the multi-ORM seam; falls back to ``ClassName:pk``. """ - if type(obj).__str__ is not object.__str__: - return str(obj) + from fastapi_admin_kit.backends.sqlalchemy import SqlAlchemyIntrospectionAdapter + + try: + label = SqlAlchemyIntrospectionAdapter().get_display_label(obj) + except Exception: + label = None + if label: + return label pk = getattr(obj, "id", None) return f"{type(obj).__name__}:{pk}" if pk is not None else type(obj).__name__ diff --git a/fastapi_admin_kit/inspection/__init__.py b/fastapi_admin_kit/inspection/__init__.py index 3041d6e..b98da28 100644 --- a/fastapi_admin_kit/inspection/__init__.py +++ b/fastapi_admin_kit/inspection/__init__.py @@ -109,17 +109,47 @@ def is_required(col: ColumnMeta) -> bool: ) +def get_display_label(obj: Any, introspection: Any | None = None) -> str | None: + """Best-effort short label for *obj* via the introspection backend. + + Delegates to ``IntrospectionBackend.get_display_label`` (SQLAlchemy by + default) so display logic lives behind the multi-ORM seam instead of + inline ``getattr`` chains. Returns None when no clean label exists — + callers apply their own final fallback (``#id``, ``ClassName:pk``). + """ + adapter = introspection if introspection is not None else _inspector + try: + return adapter.get_display_label(obj) + except Exception: + return None + + +def get_default_search_fields(model: type, introspection: Any | None = None) -> list[str]: + """Default ``search_fields`` for *model* via the introspection backend. + + Falls back to ``["name", "title", "email"]`` when the backend cannot + determine fields (e.g. third-party backends predating this method), + preserving historical behaviour. + """ + adapter = introspection if introspection is not None else _inspector + try: + fields = adapter.get_default_search_fields(model) + except Exception: + fields = [] + return list(fields) if fields else ["name", "title", "email"] + + def model_display_name(obj: Any) -> str: """Return a human-readable label for an ORM object. - Uses the model's ``__str__`` if it has a custom implementation. - Falls back to ``name``, ``title``, or ``ClassName:pk``. + Resolved through the introspection backend (custom ``__str__``, else + ``name`` / ``title`` / ``email`` / ``username``), falling back to + ``ClassName:pk`` — so related objects always render as a short label, + never full row data. """ - if type(obj).__str__ is not object.__str__: - return str(obj) - label = getattr(obj, "name", None) or getattr(obj, "title", None) - if label is not None: - return str(label) + label = get_display_label(obj) + if label: + return label pk = getattr(obj, "id", None) return f"{type(obj).__name__}:{pk}" if pk is not None else type(obj).__name__ diff --git a/fastapi_admin_kit/modeladmin.py b/fastapi_admin_kit/modeladmin.py index af81565..7cdc4cb 100644 --- a/fastapi_admin_kit/modeladmin.py +++ b/fastapi_admin_kit/modeladmin.py @@ -224,12 +224,15 @@ def get_nav_badge(self, request: Any = None) -> str | None: # Object display def __str__(self, obj: Any) -> str: - """How to display an object in dropdowns/links.""" - return str( - getattr(obj, "name", None) - or getattr(obj, "title", None) - or f"#{getattr(obj, 'id', '?')}" - ) + """How to display an object in dropdowns/links. + + Label resolution goes through the introspection backend + (custom ``__str__``, else ``name`` / ``title`` / ``email`` / + ``username``); falls back to ``#id``. + """ + from fastapi_admin_kit.inspection import get_display_label + + return str(get_display_label(obj) or f"#{getattr(obj, 'id', '?')}") def get_model(self) -> Any: return self.model diff --git a/fastapi_admin_kit/router.py b/fastapi_admin_kit/router.py index 98db39c..510c1a5 100644 --- a/fastapi_admin_kit/router.py +++ b/fastapi_admin_kit/router.py @@ -228,9 +228,13 @@ async def export_data( # Apply search/filter from query params q = request.query_params.get("q", "") if q: + from fastapi_admin_kit.inspection import get_default_search_fields from fastapi_admin_kit.search_utils import apply_search_filter - search_fields = getattr(admin, "search_fields", None) or ["name", "title"] + search_fields = getattr(admin, "search_fields", None) or get_default_search_fields( + registered.model, + getattr(request.app.state, "admin_introspection_adapter", None), + ) base = apply_search_filter(base, registered.model, search_fields, q) # Execute query @@ -857,11 +861,20 @@ async def autocomplete( """Search-as-you-type endpoint for relation pickers.""" from fastapi.responses import JSONResponse + from fastapi_admin_kit.inspection import ( + get_default_search_fields, + model_display_name, + ) + session = get_db_session(request) model = registered.model results = [] - search_fields = getattr(registered.admin, "search_fields", None) or ["name", "title"] + search_fields = getattr( + registered.admin, "search_fields", None + ) or get_default_search_fields( + model, getattr(request.app.state, "admin_introspection_adapter", None) + ) from sqlalchemy import select @@ -869,12 +882,10 @@ async def autocomplete( query = apply_search_filter(request, select(model), model, search_fields, q).limit(20) for obj in await session.all(query): - label = str( - getattr(obj, "name", None) - or getattr(obj, "title", None) - or f"#{getattr(obj, 'id', '?')}" - ) - results.append({"id": str(obj.id), "label": label}) + # Label via the introspection backend — never full row data. + label = model_display_name(obj) + # Standard option shape: label/value (+ id for back-compat). + results.append({"id": str(obj.id), "value": str(obj.id), "label": label}) return JSONResponse(content=results) diff --git a/fastapi_admin_kit/static/js/admin.js b/fastapi_admin_kit/static/js/admin.js index f22362d..3599373 100644 --- a/fastapi_admin_kit/static/js/admin.js +++ b/fastapi_admin_kit/static/js/admin.js @@ -69,6 +69,32 @@ document.addEventListener('alpine:init', () => { }, }); + /* ── Relation option helpers (label/value only, never full JSON) ─── */ + function _normOption(raw) { + if (raw == null) return { id: '', value: '', label: '' }; + if (typeof raw === 'string') return { id: raw, value: raw, label: raw }; + const value = raw.value ?? raw.id ?? ''; + const label = raw.label ?? raw.name ?? raw.title ?? String(value ?? ''); + return { ...raw, id: value, value: value, label: label }; + } + + function _normList(data) { + if (!Array.isArray(data)) return []; + return data.map(_normOption); + } + + function _optLabel(opt) { + if (opt == null) return ''; + if (typeof opt === 'string') return opt; + return opt.label ?? opt.name ?? opt.title ?? String(opt.value ?? opt.id ?? ''); + } + + function _optValue(opt) { + if (opt == null) return ''; + if (typeof opt === 'string') return opt; + return String(opt.value ?? opt.id ?? ''); + } + /* ── Relation Picker ─────────────────────────────────────────────── */ Alpine.data('relationPicker', (initialId, initialLabel, searchUrl, fixed = false) => ({ @@ -132,7 +158,7 @@ document.addEventListener('alpine:init', () => { try { const resp = await fetch(`${searchUrl}?q=${encodeURIComponent(this.searchQuery)}`); if (resp.ok) { - this.results = await resp.json(); + this.results = _normList(await resp.json()); if (this.fixed) this._setPosition(); } } catch (e) { @@ -142,8 +168,9 @@ document.addEventListener('alpine:init', () => { }, select(result) { - this.selectedId = result.id; - this.searchQuery = result.label; + const opt = _normOption(result); + this.selectedId = _optValue(opt); + this.searchQuery = _optLabel(opt); this.results = []; this.open = false; }, @@ -180,9 +207,9 @@ document.addEventListener('alpine:init', () => { try { this.selectedIds = JSON.parse(initialIds); } catch (e) { this.selectedIds = []; } } if (Array.isArray(initialItems)) { - this.selectedItems = initialItems; + this.selectedItems = _normList(initialItems); } else if (typeof initialItems === 'string' && initialItems) { - try { this.selectedItems = JSON.parse(initialItems); } catch (e) { this.selectedItems = []; } + try { this.selectedItems = _normList(JSON.parse(initialItems)); } catch (e) { this.selectedItems = []; } } if (this.selectedIds.length > 0 && this.selectedItems.length === 0) { this._loadSelected(); @@ -207,6 +234,9 @@ document.addEventListener('alpine:init', () => { return []; }, + optLabel(opt) { return _optLabel(opt); }, + optValue(opt) { return _optValue(opt); }, + async _loadSelected() { try { const ids = this._ensureArray(this.selectedIds); @@ -216,7 +246,7 @@ document.addEventListener('alpine:init', () => { } const resp = await fetch(`${searchUrl}?ids=${ids.join(',')}`); if (resp.ok) { - this.selectedItems = await resp.json(); + this.selectedItems = _normList(await resp.json()); } } catch (e) { console.error('Multi-relation load error:', e); @@ -231,10 +261,10 @@ document.addEventListener('alpine:init', () => { const url = q ? `${searchUrl}?q=${encodeURIComponent(q)}` : `${searchUrl}`; const resp = await fetch(url); if (resp.ok) { - const all = await resp.json(); + const all = _normList(await resp.json()); const ids = this._ensureArray(this.selectedIds); const idStrs = ids.map(String); - this.results = all.filter(r => !idStrs.includes(String(r.id))); + this.results = all.filter(r => !idStrs.includes(String(_optValue(r)))); } } catch (e) { console.error('Multi-relation search error:', e); @@ -243,11 +273,13 @@ document.addEventListener('alpine:init', () => { }, add(result) { + const opt = _normOption(result); + const val = _optValue(opt); const ids = this._ensureArray(this.selectedIds); const idStrs = ids.map(String); - if (!idStrs.includes(String(result.id))) { - this.selectedIds.push(result.id); - this.selectedItems.push(result); + if (!idStrs.includes(String(val))) { + this.selectedIds.push(val); + this.selectedItems.push(opt); } this.searchQuery = ''; this.results = []; @@ -268,11 +300,19 @@ document.addEventListener('alpine:init', () => { open: false, _debounce: null, + optLabel(opt) { return opt.label ?? opt.name ?? _optLabel(opt); }, + init() { if (initialPermData && Array.isArray(initialPermData)) { - this.selectedPerms = initialPermData.map(p => ({ - id: p.id, name: p.name, table_name: p.table_name - })); + this.selectedPerms = initialPermData.map(p => { + const o = _normOption(p); + return { + id: _optValue(o), value: _optValue(o), + label: p.label ?? p.name ?? _optLabel(o), + name: p.label ?? p.name ?? _optLabel(o), + table_name: p.table_name, + }; + }); } }, @@ -284,17 +324,23 @@ document.addEventListener('alpine:init', () => { const url = q ? `${searchUrl}?q=${encodeURIComponent(q)}` : searchUrl; const resp = await fetch(url); if (resp.ok) { - const all = await resp.json(); - const selected = new Set(this.selectedPerms.map(p => p.id)); - this.results = all.filter(r => !selected.has(r.id)); + const all = _normList(await resp.json()).map(o => ({ + ...o, + name: o.label ?? o.name, + })); + const selected = new Set(this.selectedPerms.map(p => String(p.id))); + this.results = all.filter(r => !selected.has(String(_optValue(r)))); } } catch (e) { console.error('Permission search error:', e); } }, 250); }, addPerm(perm) { - if (!this.selectedPerms.find(p => p.id === perm.id)) { - this.selectedPerms.push({ id: perm.id, name: perm.name, table_name: perm.table_name }); + const o = _normOption(perm); + const val = _optValue(o); + const label = perm.label ?? perm.name ?? _optLabel(o); + if (!this.selectedPerms.find(p => String(p.id) === String(val))) { + this.selectedPerms.push({ id: val, value: val, label: label, name: label, table_name: perm.table_name }); } this.searchQuery = ''; this.results = []; @@ -330,12 +376,18 @@ document.addEventListener('alpine:init', () => { } }, + optLabel(opt) { return opt.label ?? opt.name ?? _optLabel(opt); }, + optValue(opt) { return _optValue(opt); }, + async _loadSelected() { try { const ids = this.selectedIds.join(','); const resp = await fetch(`${searchUrl}?ids=${ids}`); if (resp.ok) { - this.selectedItems = await resp.json(); + this.selectedItems = _normList(await resp.json()).map(o => ({ + ...o, + label: o.label ?? o.name ?? _optLabel(o), + })); } } catch (e) { console.error('Permission load error:', e); @@ -350,9 +402,12 @@ document.addEventListener('alpine:init', () => { const url = q ? `${searchUrl}?q=${encodeURIComponent(q)}` : searchUrl; const resp = await fetch(url); if (resp.ok) { - const all = await resp.json(); + const all = _normList(await resp.json()).map(o => ({ + ...o, + label: o.label ?? o.name ?? _optLabel(o), + })); const idSet = new Set(this.selectedIds.map(String)); - this.results = all.filter(r => !idSet.has(String(r.id))); + this.results = all.filter(r => !idSet.has(String(_optValue(r)))); } } catch (e) { console.error('Permission search error:', e); @@ -361,9 +416,11 @@ document.addEventListener('alpine:init', () => { }, add(result) { - if (!this.selectedIds.includes(result.id)) { - this.selectedIds.push(result.id); - this.selectedItems.push(result); + const opt = _normOption(result); + const val = _optValue(opt); + if (!this.selectedIds.map(String).includes(String(val))) { + this.selectedIds.push(val); + this.selectedItems.push({ ...opt, label: opt.label ?? opt.name ?? _optLabel(opt) }); } this.searchQuery = ''; this.results = []; diff --git a/fastapi_admin_kit/templates/macros/form_fields.html b/fastapi_admin_kit/templates/macros/form_fields.html index e0d4175..006e6f8 100644 --- a/fastapi_admin_kit/templates/macros/form_fields.html +++ b/fastapi_admin_kit/templates/macros/form_fields.html @@ -584,9 +584,10 @@ placeholder="Search {{ field_ctx.meta.label | lower }}..." class="form-input">
-