Add DataTableLayout composite control for CSV import to table - #212
Merged
Conversation
A FormModel subclass whose fields describe table columns instead of a single instance's inputs, embedded as a layout field (layout_handler= "datatable") on a plain FormModel like any other layout, and rendered through the same render_form_html() pipeline every other form uses. The table is progressively enhanced client-side by a vendored, MIT-licensed copy of DataTables.js (search/sort/paging/csv+copy export), and CSV rows are validated row-by-row against the same Pydantic model used for the columns. The demo (examples/datatable_import_example.py) uses one form with a file field and the table as sibling fields, and a two-step Load/Submit flow so a CSV import can be reviewed and corrected before it's ever committed. Along the way, fixed two real bugs surfaced while building this: - Field()'s input_type/layout_handler kwargs were silently dropped by pydantic's Field() shim when combined with other ui_* params. - render_form_html() never set enctype="multipart/form-data" for forms with a file field, so a real browser submit would have sent only the chosen file's name, never its contents. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
github-actions
Bot
changed the title
DataTableLayout, model_list, and form_layouts.py's orchestrators had grown into three genuinely different mechanisms for "a field renders something bigger than itself". Consolidate the two field-level ones (form_layouts.py stays separate on purpose -- it orchestrates multiple FormModels, a different problem shape): - CompositeLayoutModel (composite_layout.py): shared base for a FormModel subclass whose own fields ARE the composite's structure (e.g. table columns). DataTableLayout now subclasses it instead of duplicating the render_form() -> NotImplementedError guard itself. - LayoutEngine.register_layout_renderer gained a builtin flag (and a @LayoutEngine.layout_renderer(name) decorator defaulting to it) so a library-owned renderer survives reset_layout_renderers() -- previously this only worked for DataTableLayout by luck of test-file ordering. - model_list's rendering moved out of field_renderer.py's hardcoded ui_element dispatch into its own rendering/model_list_renderer.py, registered through the same LayoutEngine registry (a second, ui_element- keyed lookup, independent from the layout_handler keyspace so SchemaMetadata's layout-field grouping/auto-tabbing stays unchanged). Public API is untouched: FormField(..., input_type='model_list') on list[ItemModel] behaves exactly as before. docs/plugin_hooks.md and CLAUDE.md now document this as the pattern to reuse for future composite controls, replacing plugin_hooks.md's old aspirational (never-shipped) WizardForm example with the two real ones. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ful Submit Only the department dropdown was ever declared with input_type=..., so every other column (name, email, project_team_name, project_description) rendered read-only -- fixing a typo required re-uploading a whole new CSV. Declare every column with FormField(..., input_type=...) so each cell is a real editable widget. Submit previously always looped back to the same table regardless of outcome. Now, matching every other form in this demo app: a clean submit (no row errors) renders success.html with the saved rows as JSON, same as the rest of the showcase; if any row still fails validation it stays on the table with errors highlighted so they can be fixed and resubmitted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ositives
Real fixes:
- layout_engine.py: replace list(cls._custom_renderers) with a set-based
diff for reset_layout_renderers() -- avoids the "unnecessary list()"
complaint while staying correct (iterating the dict directly while
deleting from it would raise RuntimeError).
- model_list_renderer.py: extract _normalize_model_list_value() out of
render_model_list_field (cognitive complexity 17 -> under 15); extract
render_schema_list_item (complexity 21) into five focused helpers
(_resolve_item_title, _render_item_toggle, _column_class_for_field_count,
_render_item_field_cell, _render_item_body_wrapper); rename the genuinely
unused field_renderer parameter on extract_nested_errors_for_field to
_field_renderer (documented why -- it's the one function of the four
that never calls back into the renderer).
- test_benchmarks.py: parametrize the two 4-test benchmark groups.
- test_datatable_layout.py: split one composite assert into two.
The remaining ~19 "unused local variable/parameter" findings are false
positives: SonarCloud's Python analyzer doesn't parse PEP 750 t-strings
yet, so it can't see a variable read only via {expr} interpolation inside
a t"..." literal. Annotated with # NOSONAR + an explanatory comment,
matching the convention already established in inputs/specialized_inputs.py
for the same tool limitation.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous demo required hand-declaring a separate file field, hand- building Load/Submit button HTML, string-splicing it into the rendered form, manually branching on a datatable_action discriminator, and manually re-merging valid/invalid rows into original order -- all just to get "a datatable you can load a CSV into." That's now owned by the library: - model_config['csv_upload'] (new, defaults True): the layout field's own rendered output includes the file input and Load/Submit buttons. Set False to opt out for a bare table with no upload UI. - DataTableLayout.handle_import_post() (new, async): reads which button was clicked and parses accordingly, returning rows/row_errors already merged into original order. - DataTableLayout.merge_rows() (new): the merge logic every caller needed, now owned by the library instead of copy-pasted per app. - as_layout_value() gained reviewing/notice/discard_url to drive the built-in UI's banners. examples/datatable_import_example.py shrinks to one model, a one-field FormModel, and a POST route that's just `await EmployeeImport .handle_import_post(form)` plus the three outcomes. docs/recipes.md rewritten to document this as the primary pattern. Full suite (1486 tests) passing; badges/coverage regenerated from that run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
model_config['editable'] (new, defaults True): set False to force every cell read-only regardless of its own input_type, and drop the "Submit" button entirely -- the built-in UI keeps just one "Reload CSV" button that parses and commits in a single click (handle_import_post() returns action='reload'), since there's no separate review step to wait on when nothing in the table can be hand-corrected anyway. UI contrast: "Load CSV"/"Reload CSV" and "Download CSV template" were outline buttons (btn-outline-primary/btn-outline-secondary), nearly invisible against a plain white background -- switched to solid btn-secondary. The file picker itself had zero CSS at all (FileInput's .file-drop-zone wrapper has never had any styling anywhere in the codebase); added a scoped .datatable-upload-zone stylesheet so it doesn't look broken next to the solid "Submit" button. Bug fix: choosing a file (drop or picker) and clicking Load/Reload did nothing -- the form was missing enctype="multipart/form-data" because the file input now lives inside the layout field's own rendered output, not as a literal field on the embedding FormModel, so the generic file-field scan could never see it. LayoutEngine.register_layout_renderer() gained a requires_multipart predicate so a layout composite can declare "check my current value for a nested file input"; DataTableLayout wires this to its csv_upload setting. Regression tests added for both the generic mechanism and the exact end-to-end scenario. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…po root --reload-dir .. watched the entire repo root, including coverage.xml, report.xml, htmlcov/*.html, and the coverage/test badges -- all of which get rewritten by every pytest run. Each one looked like a source change and triggered a full app reload, paying the whole import cost again on the next request. Scope the watch to examples/ and pydantic_schemaforms/ (the only two directories with actual application code) instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ocally Add a RequestTimingMiddleware logging total wall-clock time for every request, plus finer-grained log_timing breakdowns (form parsing, CSV import handling, render, template) around the DataTableLayout CSV-import flow -- used to track down a reported slow page load / slow return from POST. That investigation found shared_base.html was loading Bootstrap CSS/JS from cdn.jsdelivr.net as a render-blocking dependency on every page; vendor it locally via new /vendor/bootstrap.min.css and /vendor/bootstrap.bundle.min.js routes instead, mirroring the existing bootstrap-icons/htmx vendoring. Also guard examples/main.py's config_log() call (added earlier for Loguru-based logging) so it only runs outside pytest -- it reconfigures the global logging module, which was leaking into tests/test_integration.py and tests/test_layouts.py (both import examples.main for their TestClient) and breaking test_package_logger_uses_null_handler. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
demo_app's Docker image builds examples/ directly (COPY examples ./examples, uvicorn examples.main:app) rather than a duplicated copy, so it inherits whatever examples/ imports -- but demo_app/requirements.template.txt never picked up loguru/devsetgo-lib after they were added to examples/main.py and examples/fastapi_routes.py for request-timing logging. Verified in an isolated venv with only the demo requirements installed: the app failed to import with ModuleNotFoundError. Added both packages and re-rendered requirements.txt. Also anchor config_log()'s logging_directory to examples/main.py's own location instead of a bare relative 'logs' -- that resolved fine under `make ex-run` (cd's into examples/ first) but not under the Docker image, which runs uvicorn from /app with no examples/logs directory. Verified the app now boots cleanly and serves requests with only the demo requirements installed, cwd outside examples/. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
_normalize_model_list_value used hasattr(item, 'model_dump') to accept any model-dump-able object, not just pydantic BaseModel instances -- but pyright can't narrow through hasattr, so it inferred item as plain dict and flagged item.model_dump() as an attribute-access error in CI. Replace the hasattr checks with isinstance against a new @runtime_checkable _ModelDumpable Protocol: same duck-typed runtime behavior (any object with a model_dump() method, verified by the existing test using a bare non-BaseModel class), but isinstance against a runtime_checkable Protocol is something pyright can actually narrow through. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
'# NOSONAR -- reason' (marker with a trailing explanation on the same line) isn't a format Sonar's suppression parser accepts. Move the explanation to a standalone comment on the preceding line and leave a bare '# NOSONAR' on the suppressed line -- the format this file (and datatable_renderer.py) already use successfully everywhere else. Fixes the 3 occurrences of the malformed pattern: model_list_renderer.py's _render_item_toggle (collapse_id/item_title params) and _render_item_body_wrapper (collapse_id param), and datatable_renderer.py's render_datatable_table (table_id param). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous fix (76c3e22) moved each justification to a standalone comment line but phrased it as '# NOSONAR: <reason>' (colon immediately after NOSONAR), which Sonar's suppression-comment parser also rejects -- it got flagged again, on the new lines this time. Comparing against the NOSONAR comments elsewhere in these same files that were never flagged (e.g. "# NOSONAR comments below: each variable is read via...") shows the working pattern needs a word between NOSONAR and any colon, not NOSONAR immediately followed by ':'. Rephrased all three standalone comments to match that proven pattern; the actual suppressed lines keep their bare '# NOSONAR'. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
CSV import is a common, high-value workflow for data-driven applications, but robust, user-friendly table import with validation and editing is tedious to build from scratch and was not previously available as a reusable, declarative control in this library. This PR introduces a first-class
DataTableLayoutcomposite control, enabling easy CSV import-to-table with per-cell validation, editing, review, and export, all as a drop-in layout field.Why this improves the project
model_list), and prevents future fragmentation.Summary of changes
DataTableLayout(for CSV import-to-table) and supporting renderer modules..gitignore, requirements, and coverage badge as needed.This change empowers users to add robust CSV import flows with minimal effort, while providing a maintainable foundation for future composite controls.