Rebase on origin/master + other minors#79
Merged
hasansezertasan merged 16 commits intoJun 1, 2026
Merged
Conversation
Reviewer's GuideReplaces the legacy x-editable-based inline editing with a new HTMX-powered editable cell system across Flask-Admin, including widget, AJAX endpoints, JS behavior, templates, and tests, and adds a comprehensive SQLAlchemy example for column_editable_list. Sequence diagram for HTMX-powered inline cell editing flowsequenceDiagram
actor User
participant Browser as Browser_HTMX
participant View as BaseModelView
participant DB as Database
User->>Browser: Click editable cell (editable_cell_display.html span)
Browser->>View: GET /ajax/edit/ (ajax_edit)
View->>View: list_form(obj=record)
View->>View: _restore_original_widget(form, field_name)
View-->>Browser: render(editable_cell_edit.html)
User->>Browser: Submit inline edit form
Browser->>View: POST /ajax/update/ (ajax_update)
View->>View: list_form()
View->>View: validate_form(form)
alt valid
View->>DB: update_model(form, record)
alt update success
View->>DB: get_one(pk)
View->>View: get_list_value(None, record, field_name)
View-->>Browser: render(editable_cell_display.html)
else update failed
View-->>Browser: render(editable_cell_edit.html, errors)
end
else invalid
View-->>Browser: render(editable_cell_edit.html, errors), 500
end
Browser->>Browser: htmx:beforeSwap (allow 4xx/5xx swap for editable cells)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The HTMX editable cell markup is now defined both in
HTMXEditableWidget.__call__and ineditable_cell_display.html; consider consolidating this into a single rendering path to avoid future drift between the two. - The inline styles for
.editable-popoverand.editable-cellinbootstrap4/admin/lib.htmlmake the admin look harder to customize; moving these rules into a dedicated CSS file understatic(and including it only wheneditable_columnsis set) would keep them more maintainable and theme-friendly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The HTMX editable cell markup is now defined both in `HTMXEditableWidget.__call__` and in `editable_cell_display.html`; consider consolidating this into a single rendering path to avoid future drift between the two.
- The inline styles for `.editable-popover` and `.editable-cell` in `bootstrap4/admin/lib.html` make the admin look harder to customize; moving these rules into a dedicated CSS file under `static` (and including it only when `editable_columns` is set) would keep them more maintainable and theme-friendly.
## Individual Comments
### Comment 1
<location path="flask_admin/static/admin/js/form.js" line_range="625-626" />
<code_context>
+document.addEventListener('htmx:afterSwap', function(event) {
+ var target = event.detail.target;
+
+ // Initialize all data-role widgets (select2, timepicker, datepicker, etc.)
+ faForm.applyGlobalStyles(target);
+
+ // Position and focus the popover
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `faForm` here will fail because it is not defined in this global scope.
Because this listener is outside the IIFE where `faForm` is defined, `faForm` will be undefined at runtime and trigger a `ReferenceError` when popovers are swapped in. Either attach `faForm` to `window` inside the IIFE and reference `window.faForm` here, or move these HTMX handlers into the IIFE so they share the same scope.
</issue_to_address>
### Comment 2
<location path="flask_admin/model/base.py" line_range="2760-2768" />
<code_context>
+ if not field_name or field_name not in self.column_editable_list:
+ abort(404)
+
+ # Prevent validation issues: delete all fields except the submitted
+ # field, primary key, and csrf token
+ keep = {field_name, "list_form_pk", "csrf_token"}
for field in list(form):
- if (field.name in request.form) or (field.name == "csrf_token"):
- pass
- else:
+ if field.name not in keep and field.name not in request.form:
form.__delitem__(field.name)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Field-pruning logic could keep unexpected fields and subtly affect validation.
Because of `if field.name not in keep and field.name not in request.form`, any field submitted in `request.form` but not in `keep` will remain on the form. That contradicts the intent to keep only the edited field, PK, and CSRF, and could let unintended fields affect validation. Consider basing the pruning solely on membership in `keep` (or an explicit allowlist) to enforce the inline-edit contract more strictly.
```suggestion
if not field_name or field_name not in self.column_editable_list:
abort(404)
# Prevent validation issues: delete all fields except the submitted
# field, primary key, and csrf token
keep = {field_name, "list_form_pk", "csrf_token"}
for field in list(form):
if field.name not in keep:
form.__delitem__(field.name)
```
</issue_to_address>
### Comment 3
<location path="flask_admin/tests/sqla/test_basic.py" line_range="929-939" />
<code_context>
+ assert 'name="int_field"' in data
+ assert "42" in data
+
+ # -- FloatField: edit and save --
+ rv = client.post(
+ "/admin/model2/ajax/update/",
+ data={
+ "list_form_pk": "1",
+ "float_field": "3.14",
+ },
+ )
+ data = rv.data.decode("utf-8")
+ assert rv.status_code == 200
+ assert 'class="editable-cell"' in data
+
+ # -- FloatField: edit form renders input --
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen FloatField test by asserting the updated value is rendered
For the FloatField "edit and save" case, you currently only assert the 200 status and presence of `editable-cell`. Please also assert that `"3.14"` appears in the response (as with the IntegerField case) to confirm the updated float value is actually rendered in the list view.
```suggestion
# -- FloatField: edit and save --
rv = client.post(
"/admin/model2/ajax/update/",
data={
"list_form_pk": "1",
"float_field": "3.14",
},
)
data = rv.data.decode("utf-8")
assert rv.status_code == 200
assert 'class="editable-cell"' in data
assert "3.14" in data
```
</issue_to_address>
### Comment 4
<location path="flask_admin/tests/mongoengine/test_basic.py" line_range="243" />
<code_context>
+ rv = client.get(f"/admin/editable_model/ajax/edit/?pk={obj1.pk}&field=test2")
+ assert rv.status_code == 404
+
+ # Test relation editing
+ rv = client.post(
+ "/admin/editable_related/ajax/update/",
</code_context>
<issue_to_address>
**suggestion (testing):** Extend relation editing test to assert the related object actually changed
This test currently only checks for the presence of `editable-cell` in the response. To verify the relation edit actually applies, please also assert that the updated related object is reflected—for example, by checking for `obj2`’s name in the response or by reloading `RelatedModel` from MongoDB and asserting its `ref` is `obj2`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+625
to
+626
| // Initialize all data-role widgets (select2, timepicker, datepicker, etc.) | ||
| faForm.applyGlobalStyles(target); |
There was a problem hiding this comment.
issue (bug_risk): Using faForm here will fail because it is not defined in this global scope.
Because this listener is outside the IIFE where faForm is defined, faForm will be undefined at runtime and trigger a ReferenceError when popovers are swapped in. Either attach faForm to window inside the IIFE and reference window.faForm here, or move these HTMX handlers into the IIFE so they share the same scope.
| rv = client.get(f"/admin/editable_model/ajax/edit/?pk={obj1.pk}&field=test2") | ||
| assert rv.status_code == 404 | ||
|
|
||
| # Test relation editing |
There was a problem hiding this comment.
suggestion (testing): Extend relation editing test to assert the related object actually changed
This test currently only checks for the presence of editable-cell in the response. To verify the relation edit actually applies, please also assert that the updated related object is reflected—for example, by checking for obj2’s name in the response or by reloading RelatedModel from MongoDB and asserting its ref is obj2.
ElLorans
changed the title
ElLorans
changed the title
Author
|
Now err msgs appear |
Author
Where? Is it fixed now? |
…o#1724) (pallets-eco#2902) * fix(sqla): coerce ARRAY element type in `conv_ARRAY` (closes pallets-eco#1724) `AdminModelConverter.conv_ARRAY` returned a `Select2TagsField` with no `coerce` callable, so every submitted value was cast via the default `text_type` and the resulting Python list was always `list[str]`. For a Postgres `ARRAY(Integer)` column that round-trip fails on save with column "x" is of type integer[] but expression is of type text[] (reported in pallets-eco#1724 in 2018 and still reproducible on `master` as of v2.2.0; the long-standing workaround in that thread is to subclass `Select2TagsField` and override `process_formdata` to call `int()` on each entry). --------- Co-authored-by: ElLorans <lorenzo.cerreta@gmail.com>
Owner
|
Hey @ElLorans, thanks for the work. I just merged it. Should I close pallets-eco#2847 and open a fresh PR (pallets-eco/flask-admin@master...hasansezertasan:flask-admin:master) or would you like to proceed by yourself? I am OK both ways. |
Author
Urgh, I opened a PR to the wrong branch. I will open a PR vs the right one, my bad. |
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
3 participants
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.
Closes pallets-eco#2909.
TODO: Errors are not rendered. E.g.: if the user submits a non unique field on a unique constrained column, only a console error is registered.
High-level PR Summary
This PR replaces the unmaintained
x-editablelibrary with a custom HTMX-based implementation for inline editing in Flask-Admin list views. TheXEditableWidgetis now deprecated (with a backward compatibility alias) and replaced byHTMXEditableWidget, which uses HTMX to fetch edit forms and submit updates via dedicatedajax_editandajax_updateendpoints. The change removes dependencies on x-editable and its associated CSS/JS files, replacing them with HTMX and custom inline styling. A comprehensive example covering all supported field types (StringField, TextAreaField, IntegerField, FloatField, BooleanField, DateField, TimeField, DateTimeField, Enum, and ForeignKey) is added. Tests are updated to verify the new HTMX-based editing flow, including validation error handling and widget restoration. The TODO notes that error rendering still requires work for constraint violations like unique field constraints.⏱️ Estimated Review Time: 1-3 hours
💡 Review Order Suggestion
doc/changelog.rstexamples/sqla_column_editable/README.mdexamples/sqla_column_editable/pyproject.tomlexamples/sqla_column_editable/.python-versionexamples/sqla_column_editable/__init__.pyexamples/sqla_column_editable/main.pyflask_admin/model/widgets.pyflask_admin/_types.pyflask_admin/model/base.pyflask_admin/static/admin/js/form.jsflask_admin/templates/bootstrap4/admin/lib.htmlflask_admin/templates/bootstrap4/admin/model/editable_cell_edit.htmlflask_admin/templates/bootstrap4/admin/model/list.htmlflask_admin/model/form.pyflask_admin/contrib/sqla/view.pyflask_admin/contrib/mongoengine/view.pyflask_admin/contrib/peewee/view.pyflask_admin/static/vendor/htmx/htmx.min.jsflask_admin/tests/sqla/test_basic.pyflask_admin/tests/peeweemodel/test_basic.pyflask_admin/tests/mongoengine/test_basic.py