From 57104824a1194ee504ff2661e84e76d98d50e97f Mon Sep 17 00:00:00 2001 From: Yaro Kasear Date: Fri, 3 Oct 2025 16:26:04 -0500 Subject: [PATCH] WIP on getting work_log support on inventory page. --- crudkit/core/service.py | 124 ++++++++++++------------ crudkit/ui/fragments.py | 39 +++++++- inventory/routes/entry.py | 9 +- inventory/routes/listing.py | 5 +- inventory/templates/inventory_logs.html | 8 ++ 5 files changed, 117 insertions(+), 68 deletions(-) create mode 100644 inventory/templates/inventory_logs.html diff --git a/crudkit/core/service.py b/crudkit/core/service.py index 07514ce..60e3aef 100644 --- a/crudkit/core/service.py +++ b/crudkit/core/service.py @@ -247,32 +247,30 @@ class CRUDService(Generic[T]): # Detect first hops that have deeper, nested tails requested (e.g. "contact.supervisor") nested_first_hops = { path[0] for path in (rel_field_names or {}).keys() if len(path) > 1 } - used_contains_eager = False + # IMPORTANT: + # - Only attach loader options for first-hop relations from the root. + # - Always use selectinload here (avoid contains_eager joins). + # - Let compile_projections() supply deep chained options. for base_alias, rel_attr, target_alias in join_paths: - is_collection = bool(getattr(getattr(rel_attr, "property", None), "uselist", False)) + is_firsthop_from_root = (base_alias is root_alias) + if not is_firsthop_from_root: + # Deeper hops are handled by proj_opts below + continue + prop = getattr(rel_attr, "property", None) + is_collection = bool(getattr(prop, "uselist", False)) is_nested_firsthop = rel_attr.key in nested_first_hops - if is_collection or is_nested_firsthop: - # Use selectinload so deeper hops can chain cleanly (and to avoid - # contains_eager/loader conflicts on nested paths). - opt = selectinload(rel_attr) - - # Narrow columns for collections if we know child scalar names - if is_collection: - child_names = (collection_field_names or {}).get(rel_attr.key, []) - if child_names: - target_cls = rel_attr.property.mapper.class_ - cols = [getattr(target_cls, n, None) for n in child_names] - cols = [c for c in cols if isinstance(c, InstrumentedAttribute)] - if cols: - opt = opt.load_only(*cols) - - query = query.options(opt) - else: - # Simple first-hop scalar rel with no deeper tails: safe to join + contains_eager - query = query.join(target_alias, rel_attr.of_type(target_alias), isouter=True) - query = query.options(contains_eager(rel_attr, alias=target_alias)) - used_contains_eager = True + opt = selectinload(rel_attr) + # Optional narrowng for collections + if is_collection: + child_names = (collection_field_names or {}).get(rel_attr.key, []) + if child_names: + target_cls = prop.mapper.class_ + cols = [getattr(target_cls, n, None) for n in child_names] + cols = [c for c in cols if isinstance(c, InstrumentedAttribute)] + if cols: + opt = opt.load_only(*cols) + query = query.options(opt) # Filters AFTER joins → no cartesian products if filters: @@ -365,6 +363,10 @@ class CRUDService(Generic[T]): last_key = None # Count DISTINCT ids with mirrored joins + + # Apply deep projection loader options (safe: we avoided contains_eager) + if proj_opts: + query = query.options(*proj_opts) total = None if include_total: base = session.query(getattr(root_alias, "id")) @@ -466,26 +468,25 @@ class CRUDService(Generic[T]): nested_first_hops = { path[0] for path in (rel_field_names or {}).keys() if len(path) > 1 } - used_contains_eager = False + # First-hop only; use selectinload (no contains_eager) for base_alias, rel_attr, target_alias in join_paths: - is_collection = bool(getattr(getattr(rel_attr, "property", None), "uselist", False)) - is_nested_firsthop = rel_attr.key in nested_first_hops + is_firsthop_from_root = (base_alias is root_alias) + if not is_firsthop_from_root: + continue + prop = getattr(rel_attr, "property", None) + is_collection = bool(getattr(prop, "uselist", False)) + _is_nested_firsthop = rel_attr.key in nested_first_hops - if is_collection or is_nested_firsthop: - opt = selectinload(rel_attr) - if is_collection: - child_names = (collection_field_names or {}).get(rel_attr.key, []) - if child_names: - target_cls = rel_attr.property.mapper.class_ - cols = [getattr(target_cls, n, None) for n in child_names] - cols = [c for c in cols if isinstance(c, InstrumentedAttribute)] - if cols: - opt = opt.load_only(*cols) - query = query.options(opt) - else: - query = query.join(target_alias, rel_attr.of_type(target_alias), isouter=True) - query = query.options(contains_eager(rel_attr, alias=target_alias)) - used_contains_eager = True + opt = selectinload(rel_attr) + if is_collection: + child_names = (collection_field_names or {}).get(rel_attr.key, []) + if child_names: + target_cls = prop.mapper.class_ + cols = [getattr(target_cls, n, None) for n in child_names] + cols = [c for c in cols if isinstance(c, InstrumentedAttribute)] + if cols: + opt = opt.load_only(*cols) + query = query.options(opt) # Apply filters (joins are in place → no cartesian products) if filters: @@ -497,7 +498,7 @@ class CRUDService(Generic[T]): # Projection loader options compiled from requested fields. # Skip if we used contains_eager to avoid loader-strategy conflicts. expanded_fields, proj_opts = compile_projection(self.model, req_fields) if req_fields else ([], []) - if proj_opts and not used_contains_eager: + if proj_opts: query = query.options(*proj_opts) obj = query.first() @@ -565,26 +566,25 @@ class CRUDService(Generic[T]): nested_first_hops = { path[0] for path in (rel_field_names or {}).keys() if len(path) > 1 } - used_contains_eager = False - for _base_alias, rel_attr, target_alias in join_paths: - is_collection = bool(getattr(getattr(rel_attr, "property", None), "uselist", False)) - is_nested_firsthop = rel_attr.key in nested_first_hops + # First-hop only; use selectinload + for base_alias, rel_attr, target_alias in join_paths: + is_firsthop_from_root = (base_alias is root_alias) + if not is_firsthop_from_root: + continue + prop = getattr(rel_attr, "property", None) + is_collection = bool(getattr(prop, "uselist", False)) + _is_nested_firsthop = rel_attr.key in nested_first_hops - if is_collection or is_nested_firsthop: - opt = selectinload(rel_attr) - if is_collection: - child_names = (collection_field_names or {}).get(rel_attr.key, []) - if child_names: - target_cls = rel_attr.property.mapper.class_ - cols = [getattr(target_cls, n, None) for n in child_names] - cols = [c for c in cols if isinstance(c, InstrumentedAttribute)] - if cols: - opt = opt.load_only(*cols) - query = query.options(opt) - else: - query = query.join(target_alias, rel_attr.of_type(target_alias), isouter=True) - query = query.options(contains_eager(rel_attr, alias=target_alias)) - used_contains_eager = True + opt = selectinload(rel_attr) + if is_collection: + child_names = (collection_field_names or {}).get(rel_attr.key, []) + if child_names: + target_cls = prop.mapper.class_ + cols = [getattr(target_cls, n, None) for n in child_names] + cols = [c for c in cols if isinstance(c, InstrumentedAttribute)] + if cols: + opt = opt.load_only(*cols) + query = query.options(opt) # Filters AFTER joins → no cartesian products if filters: @@ -608,7 +608,7 @@ class CRUDService(Generic[T]): # Projection loaders only if we didn’t use contains_eager expanded_fields, proj_opts = compile_projection(self.model, req_fields) if req_fields else ([], []) - if proj_opts and not used_contains_eager: + if proj_opts: query = query.options(*proj_opts) else: diff --git a/crudkit/ui/fragments.py b/crudkit/ui/fragments.py index c443d8b..b4db83f 100644 --- a/crudkit/ui/fragments.py +++ b/crudkit/ui/fragments.py @@ -66,13 +66,46 @@ _ENUMS = { } def get_env(): + """ + Return an overlay Jinja Environment that knows how to load crudkit templates + and has our helper functions available as globals. + """ app = current_app default_path = os.path.join(os.path.dirname(__file__), 'templates') fallback_loader = FileSystemLoader(default_path) - return app.jinja_env.overlay( - loader=ChoiceLoader([app.jinja_loader, fallback_loader]) - ) + env = app.jinja_env.overlay(loader=ChoiceLoader([app.jinja_loader, fallback_loader])) + # Ensure helpers are available even when we render via this overlay env. + # These names are resolved at *call time* (not at def time), so it's safe. + try: + env.globals.setdefault("render_table", render_table) + env.globals.setdefault("render_form", render_form) + env.globals.setdefault("render_field", render_field) + except NameError: + # Functions may not be defined yet at import time; later calls will set them. + pass + + return env + +def register_template_globals(app=None): + """ + Register crudkit helpers as app-wide Jinja globals so they can be used + directly in any template via {{ render_table(...) }}, {{ render_form(...) }}, + and {{ render_field(...) }}. + """ + if app is None: + app = current_app + # Idempotent install using an extension flag + installed = app.extensions.setdefault("crudkit_ui_helpers", set()) + to_register = { + "render_table": render_table, + "render_form": render_form, + "render_field": render_field, + } + for name, fn in to_register.items(): + if name not in installed: + app.add_template_global(fn, name) + installed.add(name) def expand_projection(model_cls, fields): req = getattr(model_cls, "__crudkit_field_requires__", {}) or {} diff --git a/inventory/routes/entry.py b/inventory/routes/entry.py index 1763e02..7800fdd 100644 --- a/inventory/routes/entry.py +++ b/inventory/routes/entry.py @@ -4,7 +4,7 @@ from sqlalchemy.inspection import inspect from typing import Any, Dict, List, Tuple, Callable, Optional import crudkit -from crudkit.ui.fragments import render_form +from crudkit.ui.fragments import render_form, register_template_globals from crudkit.core import normalize_payload bp_entry = Blueprint("entry", __name__) @@ -17,7 +17,8 @@ def _fields_for_model(model: str): if model == "inventory": fields = ["label", "name", "serial", "barcode", "brand", "model", "device_type", "owner", "location", "condition", "image", - "notes", "work_logs"] + "notes", "work_logs.start_time", "work_logs.work_item.label", + "work_logs.contact.label", "work_logs.complete"] fields_spec = [ {"name": "label", "type": "display", "label": "", "row": "label", "attrs": {"class": "display-6 mb-3"}, "wrap": {"class": "col"}}, @@ -57,6 +58,7 @@ def _fields_for_model(model: str): "wrap": {"class": "h-100 w-100"}}, {"name": "notes", "type": "template", "label": "Notes", "row": "notes", "wrap": {"class": "col"}, "template": "inventory_note.html"}, + {"name": "work_logs", "type": "template", "row": "notes", "wrap": {"class": "col"}, "template": "inventory_logs.html"}, ] layout = [ {"name": "label", "order": 5, "attrs": {"class": "row align-items-center"}}, @@ -103,6 +105,7 @@ def _fields_for_model(model: str): ] elif model == "worklog": + # tell the service to eager-load precisely what the template needs fields = [ "id", "contact.label", @@ -165,6 +168,8 @@ def _apply_worklog_updates(worklog, updates, delete_ids): note_svc.delete(uid, actor="bulk_child_delete", commit=False) def init_entry_routes(app): + # Make helpers available in all templates + register_template_globals(app) @bp_entry.get("/entry//") def entry(model: str, id: int): diff --git a/inventory/routes/listing.py b/inventory/routes/listing.py index b7837f3..241e3b8 100644 --- a/inventory/routes/listing.py +++ b/inventory/routes/listing.py @@ -3,11 +3,14 @@ from flask import Blueprint, render_template, abort, request import crudkit from crudkit.api._cursor import decode_cursor, encode_cursor -from crudkit.ui.fragments import render_table +from crudkit.ui.fragments import render_table, register_template_globals bp_listing = Blueprint("listing", __name__) def init_listing_routes(app): + # Make helpers available in all templates + register_template_globals(app) + @bp_listing.get("/listing/") def show_list(model): if model.lower() not in {"inventory", "user", "worklog"}: diff --git a/inventory/templates/inventory_logs.html b/inventory/templates/inventory_logs.html new file mode 100644 index 0000000..00f54f1 --- /dev/null +++ b/inventory/templates/inventory_logs.html @@ -0,0 +1,8 @@ + +

+ {{ field }} +

+

+ {{ field['template_ctx']['values']['work_logs'] }} +

+{% include "table.html" %} \ No newline at end of file