diff --git a/crudkit/backend.py b/crudkit/backend.py index 2da68f8..3232b71 100644 --- a/crudkit/backend.py +++ b/crudkit/backend.py @@ -74,32 +74,18 @@ def apply_pagination(sel: Select, backend: BackendInfo, *, page: int, per_page: per_page = max(1, int(per_page)) offset = (page - 1) * per_page - if backend.requires_order_by_for_offset: - # Avoid private attribute if possible: - has_order = bool(getattr(sel, "_order_by_clauses", ())) # fallback for SA < 2.0.30 - try: - has_order = has_order or bool(sel.get_order_by()) - except Exception: - pass - - if not has_order: - if default_order_by is not None: - sel = sel.order_by(default_order_by) - else: - # Try to find a primary key from the FROMs; fall back to a harmless literal. - try: - first_from = sel.get_final_froms()[0] - pk = next(iter(first_from.primary_key.columns)) - sel = sel.order_by(pk) - except Exception: - sel = sel.order_by(text("1")) + if backend.requires_order_by_for_offset and not sel._order_by_clauses: + if default_order_by is None: + sel = sel.order_by(text("1")) + else: + sel = sel.order_by(default_order_by) return sel.limit(per_page).offset(offset) @contextmanager def maybe_identify_insert(session: Session, table, backend: BackendInfo): """ - For MSSQL tables with IDENTITY PK when you need to insert explicit IDs. + For MSSQL tables with IDENTIFY PK when you need to insert explicit IDs. No-op elsewhere. """ if not backend.is_mssql: @@ -107,7 +93,7 @@ def maybe_identify_insert(session: Session, table, backend: BackendInfo): return full_name = f"{table.schema}.{table.name}" if table.schema else table.name - session.execute(text(f"SET IDENTITY_INSERT {full_name} ON")) + session.execute(text(f"SET IDENTIFY_INSERT {full_name} ON")) try: yield finally: @@ -115,7 +101,7 @@ def maybe_identify_insert(session: Session, table, backend: BackendInfo): def chunked_in(column, values: Iterable, backend: BackendInfo, chunk_size: Optional[int] = None) -> ClauseElement: """ - Build a safe large IN() filter respecting bind param limits. + Build a safe large IN() filter respecting bund param limits. Returns a disjunction of chunked IN clauses if needed. """ vals = list(values) @@ -134,12 +120,3 @@ def chunked_in(column, values: Iterable, backend: BackendInfo, chunk_size: Optio for p in parts[1:]: expr = expr | p return expr - -def sql_trim(expr, backend: BackendInfo): - """ - Portable TRIM. SQL Server before compat level 140 lacks TRIM(). - Emit LTRIM(RTRIM(...)) there; use TRIM elsewhere - """ - if backend.is_mssql: - return func.ltrim(func.rtrim(expr)) - return func.trim(expr) diff --git a/crudkit/core/service.py b/crudkit/core/service.py index c34bc2d..1220e04 100644 --- a/crudkit/core/service.py +++ b/crudkit/core/service.py @@ -1,7 +1,7 @@ from typing import Any, Callable, Dict, Iterable, List, Tuple, Type, TypeVar, Generic, Optional, Protocol, runtime_checkable, cast from sqlalchemy import and_, func, inspect, or_, text from sqlalchemy.engine import Engine, Connection -from sqlalchemy.orm import Load, Session, raiseload, selectinload, with_polymorphic, Mapper, RelationshipProperty, class_mapper, ColumnProperty +from sqlalchemy.orm import Load, Session, raiseload, selectinload, with_polymorphic, Mapper, RelationshipProperty, class_mapper from sqlalchemy.orm.attributes import InstrumentedAttribute from sqlalchemy.orm.util import AliasedClass from sqlalchemy.sql import operators @@ -12,19 +12,6 @@ from crudkit.core.spec import CRUDSpec from crudkit.core.types import OrderSpec, SeekWindow from crudkit.backend import BackendInfo, make_backend_info -def _is_rel(model_cls, name: str) -> bool: - try: - prop = model_cls.__mapper__.relationships.get(name) - return isinstance(prop, RelationshipProperty) - except Exception: - return False - -def _is_instrumented_column(attr) -> bool: - try: - return hasattr(attr, "property") and isinstance(attr.property, ColumnProperty) - except Exception: - return False - def _loader_options_for_fields(root_alias, model_cls, fields: list[str]) -> list[Load]: """ For bare MANYTOONE names in fields (e.g. "location"), selectinload the relationship @@ -116,47 +103,43 @@ class CRUDService(Generic[T]): => selectinload(root.location).selectinload(Room.room_function) """ opts: List[Any] = [] + root_mapper: Mapper[Any] = cast(Mapper[Any], inspect(self.model)) for path, names in (rel_field_names or {}).items(): if not path: continue + current_alias = root_alias current_mapper = root_mapper rel_props: List[RelationshipProperty] = [] valid = True for step in path: rel = current_mapper.relationships.get(step) - if not isinstance(rel, RelationshipProperty): + if rel is None: valid = False break rel_props.append(rel) current_mapper = cast(Mapper[Any], inspect(rel.entity.entity)) - if not valid or not rel_props: + if not valid: continue - first = rel_props[0] - base_loader = selectinload(getattr(root_alias, first.key)) - for i in range(1, len(rel_props)): - prev_target_cls = rel_props[i - 1].mapper.class_ - hop_attr = getattr(prev_target_cls, rel_props[i].key) - base_loader = base_loader.selectinload(hop_attr) - - target_cls = rel_props[-1].mapper.class_ + target_cls = current_mapper.class_ requires = getattr(target_cls, "__crudkit_field_requires__", None) if not isinstance(requires, dict): continue for field_name in names: - needed: Iterable[str] = requires.get(field_name, []) or [] + needed: Iterable[str] = requires.get(field_name, []) for rel_need in needed: - rel_prop2 = target_cls.__mapper__.relationships.get(rel_need) - if not isinstance(rel_prop2, RelationshipProperty): - continue - dep_attr = getattr(target_cls, rel_prop2.key) - opts.append(base_loader.selectinload(dep_attr)) + loader = selectinload(getattr(root_alias, rel_props[0].key)) + for rp in rel_props[1:]: + loader = loader.selectinload(getattr(getattr(root_alias, rp.parent.class_.__name__.lower(), None) or rp.parent.class_, rp.key)) + + loader = loader.selectinload(getattr(target_cls, rel_need)) + opts.append(loader) return opts @@ -242,18 +225,12 @@ class CRUDService(Generic[T]): root_fields, rel_field_names, root_field_names = spec.parse_fields() - seen_rel_roots = set() for path, names in (rel_field_names or {}).items(): - if not path: - continue - rel_name = path[0] - if rel_name in seen_rel_roots: - continue - if _is_rel(self.model, rel_name): + if "label" in names: + rel_name = path[0] rel_attr = getattr(root_alias, rel_name, None) if rel_attr is not None: query = query.options(selectinload(rel_attr)) - seen_rel_roots.add(rel_name) # Soft delete filter if self.supports_soft_delete and not _is_truthy(params.get("include_deleted")): @@ -274,8 +251,8 @@ class CRUDService(Generic[T]): only_cols = [c for c in root_fields if isinstance(c, InstrumentedAttribute)] if only_cols: query = query.options(Load(root_alias).load_only(*only_cols)) - # for eager in spec.get_eager_loads(root_alias, fields_map=rel_field_names): - # query = query.options(eager) + for eager in spec.get_eager_loads(root_alias, fields_map=rel_field_names): + query = query.options(eager) for opt in self._resolve_required_includes(root_alias, rel_field_names): query = query.options(opt) @@ -410,20 +387,6 @@ class CRUDService(Generic[T]): if params: root_fields, rel_field_names, root_field_names = spec.parse_fields() - if rel_field_names: - seen_rel_roots = set() - for path, names in rel_field_names.items(): - if not path: - continue - rel_name = path[0] - if rel_name in seen_rel_roots: - continue - if _is_rel(self.model, rel_name): - rel_attr = getattr(root_alias, rel_name, None) - if rel_attr is not None: - query = query.options(selectinload(rel_attr)) - seen_rel_roots.add(rel_name) - fields = (params or {}).get("fields") if isinstance(params, dict) else None if fields: for opt in _loader_options_for_fields(root_alias, self.model, fields): @@ -433,8 +396,8 @@ class CRUDService(Generic[T]): if only_cols: query = query.options(Load(root_alias).load_only(*only_cols)) - # for eager in spec.get_eager_loads(root_alias, fields_map=rel_field_names): - # query = query.options(eager) + for eager in spec.get_eager_loads(root_alias, fields_map=rel_field_names): + query = query.options(eager) if params: fields = params.get("fields") or [] @@ -491,26 +454,12 @@ class CRUDService(Generic[T]): if params: root_fields, rel_field_names, root_field_names = spec.parse_fields() - if rel_field_names: - seen_rel_roots = set() - for path, names in rel_field_names.items(): - if not path: - continue - rel_name = path[0] - if rel_name in seen_rel_roots: - continue - if _is_rel(self.model, rel_name): - rel_attr = getattr(root_alias, rel_name, None) - if rel_attr is not None: - query = query.options(selectinload(rel_attr)) - seen_rel_roots.add(rel_name) - only_cols = [c for c in root_fields if isinstance(c, InstrumentedAttribute)] if only_cols: query = query.options(Load(root_alias).load_only(*only_cols)) - # for eager in spec.get_eager_loads(root_alias, fields_map=rel_field_names): - # query = query.options(eager) + for eager in spec.get_eager_loads(root_alias, fields_map=rel_field_names): + query = query.options(eager) if params: fields = params.get("fields") or [] diff --git a/crudkit/ui/fragments.py b/crudkit/ui/fragments.py index a26bf0b..6da9be8 100644 --- a/crudkit/ui/fragments.py +++ b/crudkit/ui/fragments.py @@ -54,30 +54,15 @@ def _get_loaded_attr(obj: Any, name: str) -> Any: """ try: st = inspect(obj) - # 1) Mapped attribute? attr = st.attrs.get(name) if attr is not None: val = attr.loaded_value return None if val is NO_VALUE else val - # 2) Already present value (e.g., eager-loaded or set on the dict)? if name in st.dict: return st.dict.get(name) - # 3) If object is detached or attr is not mapped, DO NOT eval hybrids - # or descriptors that could lazy-load. That would explode. - if st.session is None: - return None - # 4) As a last resort on attached instances only, try simple getattr, - # but guard against DetachedInstanceError anyway. - try: - return getattr(obj, name, None) - except Exception: - return None + return getattr(obj, name, None) except Exception: - # If we can't even inspect it, be conservative - try: - return getattr(obj, name, None) - except Exception: - return None + return getattr(obj, name, None) def _normalize_rows_layout(layout: Optional[List[dict]]) -> Dict[str, dict]: """ @@ -308,21 +293,7 @@ def _value_label_for_field(field: dict, mapper, values_map: dict, instance, sess rel_obj = q.get(rid) if rel_obj is not None: - try: - s = _label_from_obj(rel_obj, label_spec) - except Exception: - s = None - # If we couldn't safely render and we have a session+id, do one lean retry. - if (s is None or s == "") and session is not None and rid is not None: - mdl = rel_prop.mapper.class_ - try: - rel_obj2 = session.get(mdl, rid) # attached instance - s2 = _label_from_obj(rel_obj2, label_spec) - if s2: - return s2 - except Exception: - pass - return s + return _label_from_obj(rel_obj, label_spec) return str(rid) if rid is not None else None class _SafeObj: @@ -435,7 +406,7 @@ def _fk_options(session, related_model, label_spec): if simple_cols: first = simple_cols[0] if hasattr(related_model, first): - q = q.order_by(None).order_by(getattr(related_model, first)) + q = q.order_by(getattr(related_model, first)) rows = q.all() return [ @@ -588,10 +559,7 @@ def _label_from_obj(obj: Any, spec: Any) -> str: for f in fields: root = f.split(".", 1)[0] if root not in data: - try: - data[root] = _SafeObj(_get_loaded_attr(obj, root)) - except Exception: - data[root] = _SafeObj(None) + data[root] = _SafeObj(_get_loaded_attr(obj, root)) try: return spec.format(**data) except Exception: @@ -1039,6 +1007,11 @@ def render_form( base.update(f.get("template_ctx") or {}) f["template_ctx"] = base + for f in fields: + vl = _value_label_for_field(f, mapper, values_map, instance, session) + if vl is not None: + f["value_label"] = vl + for f in fields: # existing FK label resolution vl = _value_label_for_field(f, mapper, values_map, instance, session) diff --git a/inventory/models/user.py b/inventory/models/user.py index 241cb3d..1447ef1 100644 --- a/inventory/models/user.py +++ b/inventory/models/user.py @@ -1,15 +1,10 @@ from typing import List, Optional, TYPE_CHECKING -from flask import current_app - from sqlalchemy import Boolean, Integer, ForeignKey, Unicode, case, func, literal from sqlalchemy.ext.hybrid import hybrid_property from sqlalchemy.orm import Mapped, mapped_column, relationship from sqlalchemy.sql import expression as sql -import crudkit - -from crudkit.backend import make_backend_info, sql_trim from crudkit.core.base import Base, CRUDMixin if TYPE_CHECKING: @@ -17,10 +12,6 @@ if TYPE_CHECKING: class User(Base, CRUDMixin): __tablename__ = 'users' - __crud_label__ = "{label}" - __crudkit_field_requires__ = { - "label": ["first_name", "last_name", "title"] # whatever the hybrid touches - } first_name: Mapped[Optional[str]] = mapped_column(Unicode(255), nullable=True, index=True) last_name: Mapped[Optional[str]] = mapped_column(Unicode(255), nullable=True, index=True) @@ -57,19 +48,17 @@ class User(Base, CRUDMixin): @label.expression def label(cls): - engine = current_app.extensions["crudkit"]["runtime"].engine - backend = make_backend_info(engine) first = func.coalesce(cls.first_name, "") last = func.coalesce(cls.last_name, "") title = func.coalesce(cls.title, "") - have_first = func.length(sql_trim(first, backend)) > 0 - have_last = func.length(sql_trim(last, backend)) > 0 + have_first = func.length(func.trim(first)) > 0 + have_last = func.length(func.trim(last)) > 0 space = case((have_first & have_last, literal(" ")), else_=literal("")) title_part = case( - (func.length(sql_trim(title, backend)) > 0, func.concat(" (", title, ")")), + (func.length(func.trim(title)) > 0, func.concat(" (", title, ")")), else_=literal("") ) diff --git a/inventory/routes/entry.py b/inventory/routes/entry.py index dba0426..4e57cd1 100644 --- a/inventory/routes/entry.py +++ b/inventory/routes/entry.py @@ -42,7 +42,7 @@ def init_entry_routes(app): {"name": "owner", "row": "status", "label": "Contact", "wrap": {"class": "col"}, "attrs": {"class": "form-control"}, "label_attrs": {"class": "form-label"}, - "label_spec": "{label}"}, + "label_spec": "{first_name} {last_name}"}, {"name": "location", "row": "status", "label": "Location", "wrap": {"class": "col"}, "attrs": {"class": "form-control"}, "label_attrs": {"class": "form-label"}, "label_spec": "{name} - {room_function.description}"}, @@ -50,11 +50,10 @@ def init_entry_routes(app): "attrs": {"class": "form-control"}, "label_attrs": {"class": "form-label"}}, {"name": "image", "label": "", "row": "image", "type": "template", "label_spec": "{filename}", - "template": "image_display.html", "attrs": {"class": "img-fluid img-thumbnail h-auto"}, - "wrap": {"class": "h-100 w-100"}}, + "template": "image_display.html", "attrs": {"class": "img-fluid img-thumbnail"}}, {"name": "notes", "type": "textarea", "label": "Notes", "row": "notes", "wrap": {"class": "col"}, - "attrs": {"class": "form-control", "rows": 10}, "label_attrs": {"class": "form-label"}}, + "attrs": {"class": "form-control", "rows": 10}}, ] layout = [ {"name": "label", "order": 5}, @@ -85,7 +84,7 @@ def init_entry_routes(app): "row": "name", "wrap": {"class": "col-3"}}, {"name": "supervisor", "label": "Supervisor", "label_attrs": {"class": "form-label"}, - "label_spec": "{label}", "row": "details", "wrap": {"class": "col-3"}, + "label_spec": "{first_name} {last_name}", "row": "details", "wrap": {"class": "col-3"}, "attrs": {"class": "form-control"}}, {"name": "location", "label": "Room", "label_attrs": {"class": "form-label"}, @@ -111,7 +110,7 @@ def init_entry_routes(app): "attrs": {"class": "display-6 mb-3"}, "row": "label"}, {"name": "contact", "row": "ownership", "wrap": {"class": "col"}, "label": "Contact", - "label_spec": "{label}", "attrs": {"class": "form-control"}, + "label_spec": "{first_name} {last_name}", "attrs": {"class": "form-control"}, "label_attrs": {"class": "form-label"}}, {"name": "work_item", "row": "ownership", "wrap": {"class": "col"}, "label": "Work Item", "label_spec": "{label}", "attrs": {"class": "form-control"}, "label_attrs": {"class": "form-label"}}, diff --git a/inventory/static/images/noimage.svg b/inventory/static/images/noimage.svg deleted file mode 100644 index ffc22b9..0000000 --- a/inventory/static/images/noimage.svg +++ /dev/null @@ -1 +0,0 @@ - \ No newline at end of file diff --git a/inventory/templates/image_display.html b/inventory/templates/image_display.html index ca32078..6df86e8 100644 --- a/inventory/templates/image_display.html +++ b/inventory/templates/image_display.html @@ -1,6 +1,4 @@ {% if value %} {{ value }} -{% else %} - {% endif %} \ No newline at end of file