From 4cb6a69816bbe50b45c81fdaa923ebf89d88a633 Mon Sep 17 00:00:00 2001
From: Conrad Nelson <yaro@kasear.net>
Date: Thu, 18 Sep 2025 16:20:35 -0500
Subject: [PATCH] Additional form control and presentation logic.

---
 crudkit/core/service.py         |  46 ++++++-
 crudkit/ui/fragments.py         | 223 +++++++++++++++++++++++++++++---
 crudkit/ui/templates/field.html |  72 +++++++----
 crudkit/ui/templates/form.html  |  40 +++++-
 4 files changed, 333 insertions(+), 48 deletions(-)

diff --git a/crudkit/core/service.py b/crudkit/core/service.py
index e7acf1d..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
+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,6 +12,35 @@ from crudkit.core.spec import CRUDSpec
 from crudkit.core.types import OrderSpec, SeekWindow
 from crudkit.backend import BackendInfo, make_backend_info
 
+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
+    and only fetch the related PK. This is enough for preselecting <select> inputs
+    without projecting the FK column on the root model.
+    """
+    opts: list[Load] = []
+    if not fields:
+        return opts
+
+    mapper = class_mapper(model_cls)
+    for name in fields:
+        prop = mapper.relationships.get(name)
+        if not isinstance(prop, RelationshipProperty):
+            continue
+        if prop.direction.name != "MANYTOONE":
+            continue
+
+        rel_attr = getattr(root_alias, name)
+        target_cls = prop.mapper.class_
+        # load_only PK if present; else just selectinload
+        id_attr = getattr(target_cls, "id", None)
+        if id_attr is not None:
+            opts.append(selectinload(rel_attr).load_only(id_attr))
+        else:
+            opts.append(selectinload(rel_attr))
+
+    return opts
+
 @runtime_checkable
 class _HasID(Protocol):
     id: int
@@ -358,6 +387,11 @@ class CRUDService(Generic[T]):
         if params:
             root_fields, rel_field_names, root_field_names = spec.parse_fields()
 
+        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):
+                query = query.options(opt)
+
         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))
@@ -365,6 +399,11 @@ class CRUDService(Generic[T]):
         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 []
+            for opt in _loader_options_for_fields(root_alias, self.model, fields):
+                query = query.options(opt)
+
         obj = query.first()
 
         proj = []
@@ -422,6 +461,11 @@ class CRUDService(Generic[T]):
             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 []
+                for opt in _loader_options_for_fields(root_alias, self.model, fields):
+                    query = query.options(opt)
+
             if filters:
                 query = query.filter(*filters)
 
diff --git a/crudkit/ui/fragments.py b/crudkit/ui/fragments.py
index 4621df2..1f74f3a 100644
--- a/crudkit/ui/fragments.py
+++ b/crudkit/ui/fragments.py
@@ -1,10 +1,11 @@
 import os
 import re
 
+from collections import OrderedDict
 from flask import current_app, url_for
 from jinja2 import Environment, FileSystemLoader, ChoiceLoader
 from sqlalchemy import inspect
-from sqlalchemy.orm import class_mapper, RelationshipProperty, load_only, selectinload
+from sqlalchemy.orm import Load, RelationshipProperty, class_mapper, load_only, selectinload
 from sqlalchemy.orm.base import NO_VALUE
 from typing import Any, Dict, List, Optional, Tuple
 
@@ -25,6 +26,130 @@ def get_env():
         loader=ChoiceLoader([app.jinja_loader, fallback_loader])
     )
 
+def _normalize_rows_layout(layout: Optional[List[dict]]) -> Dict[str, dict]:
+    """
+    Create node dicts for each row and link parent->children.
+    Node shape:
+      {
+        'name': str,
+        'legend': Optional[str],
+        'attrs': dict,         # sanitized
+        'order': int,
+        'parent': Optional[str],
+        'children': list,      # list of node names (we'll expand later)
+        'fields': list,        # filled later
+      }
+    Always ensures a 'main' node exists.
+    """
+    nodes: Dict[str, dict] = {}
+
+    def make_node(name: str) -> dict:
+        node = nodes.get(name)
+        if node is None:
+            node = nodes[name] = {
+                "name": name,
+                "legend": None,
+                "attrs": {},
+                "order": 0,
+                "parent": None,
+                "children": [],
+                "fields": [],
+            }
+        return node
+
+    # seed nodes from layout
+    if isinstance(layout, list):
+        for item in layout:
+            name = item.get("name")
+            if not isinstance(name, str) or not name:
+                continue
+            node = make_node(name)
+            node["legend"] = item.get("legend")
+            node["attrs"] = _sanitize_attrs(item.get("attrs") or {})
+            try:
+                node["order"] = int(item.get("order") or 0)
+            except Exception:
+                node["order"] = 0
+            parent = item.get("parent")
+            node["parent"] = parent if isinstance(parent, str) and parent else None
+
+    # ensure main exists and is early-ordered
+    main = make_node("main")
+    if "order" not in main or main["order"] == 0:
+        main["order"] = -10
+
+    # default any unknown parents to main (except main itself)
+    for n in list(nodes.values()):
+        if n["name"] == "main":
+            n["parent"] = None
+            continue
+        p = n["parent"]
+        if p is None or p not in nodes or p == n["name"]:
+            n["parent"] = "main"
+
+    # detect cycles defensively; break by reparenting to main
+    visiting = set()
+    visited = set()
+
+    def visit(name: str):
+        if name in visited:
+            return
+        if name in visiting:
+            # cycle; break this node to main
+            nodes[name]["parent"] = "main"
+            return
+        visiting.add(name)
+        parent = nodes[name]["parent"]
+        if parent is not None:
+            visit(parent)
+        visiting.remove(name)
+        visited.add(name)
+
+    for nm in list(nodes.keys()):
+        visit(nm)
+
+    # compute children lists
+    for n in nodes.values():
+        n["children"] = []
+    for n in nodes.values():
+        p = n["parent"]
+        if p is not None:
+            nodes[p]["children"].append(n["name"])
+
+    # sort children by (order, name) for deterministic rendering
+    for n in nodes.values():
+        n["children"].sort(key=lambda nm: (nodes[nm]["order"], nodes[nm]["name"]))
+
+    return nodes
+
+def _assign_fields_to_rows(fields: List[dict], rows: Dict[str, dict]) -> List[dict]:
+    """
+    Put fields into their target row buckets by name (default 'main'),
+    then return a list of root nodes expanded with nested dicts ready for templates.
+    """
+    # assign fields
+    for f in fields:
+        row_name = f.get("row") or "main"
+        node = rows.get(row_name) or rows["main"]
+        node["fields"].append(f)
+
+    # expand tree into nested structures
+    def expand(name: str) -> dict:
+        n = rows[name]
+        return {
+            "name": n["name"],
+            "legend": n["legend"],
+            "attrs": n["attrs"],
+            "order": n["order"],
+            "fields": n["fields"],
+            "children": [expand(ch) for ch in n["children"]],
+        }
+
+    # roots are nodes with parent == None
+    roots = [expand(nm) for nm, n in rows.items() if n["parent"] is None]
+    roots.sort(key=lambda r: (r["order"], r["name"]))
+    return roots
+
 def _sanitize_attrs(attrs: Any) -> dict[str, Any]:
     """
     Whitelist attributes; allow data-* and aria-*; render True as boolean attr.
@@ -63,29 +188,37 @@ class _SafeObj:
 
 def _coerce_fk_value(values: dict | None, instance: Any, base: str):
     """
-    Resolve the current selection for relationship 'base':
+    Resolve current selection for relationship 'base':
       1) values['<base>_id']
-      2) values['<base>']['id'] or values['<base>'] if scalar
-      3) instance.<base> (relationship) if it's already loaded -> use its .id
-      4) instance.<base>_id if it's already loaded (column) and instance is bound
-    Never trigger a lazy load. Never touch the DB.
+      2) values['<base>']['id'] or values['<base>'] if it's an int or numeric string
+      3) instance.<base> (if already loaded) -> use its .id   [safe for detached]
+      4) instance.<base>_id (if already loaded and not expired)
+    Never trigger a lazy load.
     """
     # 1) explicit *_id from values
     if isinstance(values, dict):
         key = f"{base}_id"
         if key in values:
             return values.get(key)
-        rel = values.get(base)
-        if isinstance(rel, dict):
-            return rel.get("id") or rel.get(key)
-        if isinstance(rel, (int, str)):
-            return rel
 
-    # 3) use loaded relationship object (safe for detached instances)
+        rel = values.get(base)
+        # 2a) nested dict with id
+        if isinstance(rel, dict):
+            vid = rel.get("id") or rel.get(key)
+            if vid is not None:
+                return vid
+        # 2b) scalar id
+        if isinstance(rel, int):
+            return rel
+        if isinstance(rel, str):
+            s = rel.strip()
+            if s.isdigit():
+                return s  # template compares as strings, so this is fine
+
+    # 3) use loaded relationship object (safe even if instance is detached)
     if instance is not None:
         try:
             state = inspect(instance)
-            # relationship attr present?
             rel_attr = state.attrs.get(base)
             if rel_attr is not None and rel_attr.loaded_value is not NO_VALUE:
                 rel_obj = rel_attr.loaded_value
@@ -93,7 +226,7 @@ def _coerce_fk_value(values: dict | None, instance: Any, base: str):
                     rid = getattr(rel_obj, "id", None)
                     if rid is not None:
                         return rid
-            # 4) use loaded fk column if the value is present and NOT expired
+            # 4) use loaded fk column if present and not expired
             id_attr = state.attrs.get(f"{base}_id")
             if id_attr is not None and id_attr.loaded_value is not NO_VALUE:
                 return id_attr.loaded_value
@@ -113,7 +246,7 @@ def _is_many_to_one(mapper, name: str) -> Optional[RelationshipProperty]:
 
 def _rel_for_id_name(mapper, name: str) -> tuple[Optional[str], Optional[RelationshipProperty]]:
     if name.endswith("_id"):
-        base = name[":-3"]
+        base = name[:-3]
         prop = _is_many_to_one(mapper, base)
         return (base, prop) if prop else (None, None)
     else:
@@ -174,7 +307,13 @@ def _normalize_field_spec(spec, mapper, session, label_specs_model_default):
         "type": spec.get("type"),
         "options": spec.get("options"),
         "attrs": spec.get("attrs"),
+        "label_attrs": spec.get("label_attrs"),
+        "wrap": spec.get("wrap"),
+        "row": spec.get("row"),
         "help": spec.get("help"),
+        "template": spec.get("template"),
+        "template_name": spec.get("template_name"),
+        "template_ctx": spec.get("template_ctx"),
     }
 
     if rel_prop:
@@ -503,17 +642,35 @@ def get_crudkit_template(env, name):
 
 def render_field(field, value):
     env = get_env()
+
+    # 1) custom template field
+    field_type = field.get('type', 'text')
+    if field_type == 'template':
+        tname = field.get('template') or field.get('template_name')
+        if not tname:
+            return ""  # nothing to render
+        t = get_crudkit_template(env, tname)
+        # merge ctx with some sensible defaults
+        ctx = dict(field.get('template_ctx') or {})
+        # make sure templates always see these
+        ctx.setdefault('field', field)
+        ctx.setdefault('value', value)
+        return t.render(**ctx)
+
+    # 2) normal controls
     template = get_crudkit_template(env, 'field.html')
     return template.render(
         field_name=field['name'],
         field_label=field.get('label', field['name']),
         value=value,
-        field_type=field.get('type', 'text'),
+        field_type=field_type,
         options=field.get('options', None),
         attrs=_sanitize_attrs(field.get('attrs') or {}),
-        help=field.get('help')
+        label_attrs=_sanitize_attrs(field.get('label_attrs') or {}),
+        help=field.get('help'),
     )
 
+
 def render_table(objects: List[Any], columns: Optional[List[Dict[str, Any]]] = None, **opts):
     env = get_env()
     template = get_crudkit_template(env, 'table.html')
@@ -556,7 +713,10 @@ def render_form(
     label_specs: Optional[Dict[str, Any]] = None,
     exclude: Optional[set[str]] = None,
     overrides: Optional[Dict[str, Dict[str, Any]]] = None,
-    instance: Any = None,   # NEW: pass the ORM object so we can read *_id
+    instance: Any = None,
+    layout: Optional[list[dict]] = None,
+    submit_attrs: Optional[dict[str, Any]] = None,
+    submit_label: Optional[str] = None,
 ):
     """
     fields_spec: list of dicts describing fields in order. Each dict supports:
@@ -571,6 +731,8 @@ def render_form(
     exclude: set of field names to hide.
     overrides: legacy quick overrides keyed by field name (label/type/etc.)
     instance: the ORM object backing the form; used to populate *_id values
+    layout: A list of dicts describing layouts for fields.
+    submit_attrs: A dict of attributes to apply to the submit button.
     """
     env = get_env()
     template = get_crudkit_template(env, "form.html")
@@ -649,6 +811,29 @@ def render_form(
                 "type": "checkbox" if getattr(col.type, "python_type", None) is bool else "text",
             }
             field = {**base_field, **overrides.get(col.name, {})}
+            if field.get("wrap"):
+                field["wrap"] = _sanitize_attrs(field["wrap"])
             fields.append(field)
 
-    return template.render(fields=fields, values=values_map, render_field=render_field)
+        if submit_attrs:
+            submit_attrs = _sanitize_attrs(submit_attrs)
+
+        common_ctx = {"values": values_map, "instance": instance, "model_cls": model_cls, "session": session}
+        for f in fields:
+            if f.get("type") == "template":
+                base = dict(common_ctx)
+                base.update(f.get("template_ctx") or {})
+                f["template_ctx"] = base
+
+    # Build rows (supports nested layout with parents)
+    rows_map = _normalize_rows_layout(layout)
+    rows_tree = _assign_fields_to_rows(fields, rows_map)
+
+    return template.render(
+        rows=rows_tree,
+        fields=fields,  # keep for backward compatibility
+        values=values_map,
+        render_field=render_field,
+        submit_attrs=submit_attrs,
+        submit_label=submit_label
+    )
diff --git a/crudkit/ui/templates/field.html b/crudkit/ui/templates/field.html
index c60242a..0c0a39b 100644
--- a/crudkit/ui/templates/field.html
+++ b/crudkit/ui/templates/field.html
@@ -1,37 +1,59 @@
-<label for="{{ field_name }}">{{ field_label }}</label>
+{# show label unless hidden/custom #}
+{% if field_type != 'hidden' and field_label %}
+  <label for="{{ field_name }}"
+    {% if label_attrs %}{% for k,v in label_attrs.items() %}
+      {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+    {% endfor %}{% endif %}>
+    {{ field_label }}
+  </label>
+{% endif %}
 
 {% if field_type == 'select' %}
-    <select name="{{ field_name }}" id="{{ field_name }}"
-        {% if attrs %}{% for k,v in attrs.items() %} {{k}}-"{{v}}" {% endfor %}{% endif %}
-        {%- if not options %} disabled{% endif %}>
-        {% if options %}
-            <option value="">-- Select --</option>
-            {% for opt in options %}
-                <option value="{{ opt.value }}" {% if opt.value|string == value|string %}selected{% endif %}>
-                    {{ opt.label }}
-                </option>
-            {% endfor %}
-        {% else %}
-            <option value="">-- No selection available --</option>
-        {% endif %}
-    </select>
+  <select name="{{ field_name }}" id="{{ field_name }}"
+          {% if attrs %}{% for k,v in attrs.items() %}
+            {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+          {% endfor %}{% endif %}
+          {%- if not options %} disabled{% endif %}>
+    {% if options %}
+      <option value="">-- Select --</option>
+      {% for opt in options %}
+        <option value="{{ opt.value }}" {% if opt.value|string == value|string %}selected{% endif %}>
+          {{ opt.label }}
+        </option>
+      {% endfor %}
+    {% else %}
+      <option value="">-- No selection available --</option>
+    {% endif %}
+  </select>
 
 {% elif field_type == 'textarea' %}
-    <textarea name="{{ field_name }}" id="{{ field_name }}"
-        {% if attrs %}{% for k,v in attrs.items() %} {{k}}="{{v}}"{% endfor %}{% endif %}>{{ value }}</textarea>
+  <textarea name="{{ field_name }}" id="{{ field_name }}"
+            {% if attrs %}{% for k,v in attrs.items() %}
+              {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+            {% endfor %}{% endif %}>{{ value }}</textarea>
 
 {% elif field_type == 'checkbox' %}
-    <input type="checkbox" name="{{ field_name }}" id="{{ field_name }}"
-        value="1"
-        {% if value %}checked{% endif %}
-        {% if attrs %}{% for k,v in attrs.items() %} {{k}}="{{v}}"{% endfor %}{% endif %}>
+  <input type="checkbox" name="{{ field_name }}" id="{{ field_name }}" value="1"
+         {% if value %}checked{% endif %}
+         {% if attrs %}{% for k,v in attrs.items() %}
+           {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+         {% endfor %}{% endif %}>
+
+{% elif field_type == 'hidden' %}
+  <input type="hidden" name="{{ field_name }}" id="{{ field_name }}" value="{{ value }}">
+
+{% elif field_type == 'display' %}
+  <div {% if attrs %}{% for k,v in attrs.items() %}
+         {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+       {% endfor %}{% endif %}>{{ value }}</div>
 
 {% else %}
-    <input type="text" name="{{ field_name }}" id="{{ field_name }}"
-        value="{{ value }}"
-        {% if attrs %}{% for k,v in attrs.items() %} {{k}}="{{v}}"{% endfor %}{% endif %}>
+  <input type="text" name="{{ field_name }}" id="{{ field_name }}" value="{{ value }}"
+         {% if attrs %}{% for k,v in attrs.items() %}
+           {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+         {% endfor %}{% endif %}>
 {% endif %}
 
 {% if help %}
-    <div>{{ help }}</div>
+  <div class="form-text">{{ help }}</div>
 {% endif %}
diff --git a/crudkit/ui/templates/form.html b/crudkit/ui/templates/form.html
index 7ab1d50..b073fc3 100644
--- a/crudkit/ui/templates/form.html
+++ b/crudkit/ui/templates/form.html
@@ -1,6 +1,40 @@
 <form method="POST">
-    {% for field in fields %}
-        {{ render_field(field, values.get(field.name, '')) | safe }}
+  {% macro render_row(row) %}
+    <!-- {{ row.name }} -->
+    {% if row.fields or row.children or row.legend %}
+      {% if row.legend %}<legend>{{ row.legend }}</legend>{% endif %}
+      <fieldset
+        {% if row.attrs %}{% for k,v in row.attrs.items() %}
+          {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+        {% endfor %}{% endif %}>
+        {% for field in row.fields %}
+          <div
+            {% if field.wrap %}{% for k,v in field.wrap.items() %}
+              {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+            {% endfor %}{% endif %}>
+            {{ render_field(field, values.get(field.name, '')) | safe }}
+          </div>
+        {% endfor %}
+        {% for child in row.children %}
+          {{ render_row(child) }}
+        {% endfor %}
+      </fieldset>
+    {% endif %}
+  {% endmacro %}
+
+  {% if rows %}
+    {% for row in rows %}
+      {{ render_row(row) }}
     {% endfor %}
-    <button type="submit">Create</button>
+  {% else %}
+    {% for field in fields %}
+      {{ render_field(field, values.get(field.name, '')) | safe }}
+    {% endfor %}
+  {% endif %}
+
+  <button type="submit"
+      {% if submit_attrs %}{% for k,v in submit_attrs.items() %}
+        {{k}}{% if v is not sameas true %}="{{ v }}"{% endif %}
+      {% endfor %}{% endif %}
+  >{{ submit_label if label else 'Save' }}</button>
 </form>