Skip to content

Commit

Permalink
Allow model columns to bear the same name as reserved WTForms attributes
Browse files Browse the repository at this point in the history
For example, a Model with a mapped column named `data` would collide with the
[`wtforms.Form.data`](https://github.com/aminalaee/sqladmin/blob/main/sqladmin/forms.py#L619-L634)
property, which would generate errors when processing or rendering the form.

What we do here is silently rename the column when entering and leaving WTForms territory.
Taking the example of a column named `data`, the associated `Field` object will:
- be listed under the form key `data_`
- be passed a `name='data'` kwarg so that it is still displayed with the `data` label in the admin web UI

Instead of directly passing `form.data` to `model_view.insert_model` or `model_view.update_model`, we first restore the original name (e.g. `data_` to `data`) so that the underlying SQLAlchemy can them work on the originalcolumn.

> **Note**: this is a bit crude and I think the naming could be improved. Feel free to refactor at will.

Fixes #656
  • Loading branch information
brouberol committed Oct 27, 2023
1 parent f62c38a commit 5fcf7d8
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 6 deletions.
53 changes: 48 additions & 5 deletions sqladmin/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
from sqladmin._types import ENGINE_TYPE
from sqladmin.ajax import QueryAjaxModelLoader
from sqladmin.authentication import AuthenticationBackend, login_required
from sqladmin.forms import (
WTFORMS_RESERVED_ATTRIBUTES_MAPPING,
WTFORMS_RESERVED_ATTRIBUTES_REVERSED_MAPPING,
)
from sqladmin.helpers import (
get_object_identifier,
is_async_session_maker,
Expand Down Expand Up @@ -509,8 +513,12 @@ async def create(self, request: Request) -> Response:
request, model_view.create_template, context, status_code=400
)

# we restore the original name of fields colliding with WTForms reserved attrs
form_data_dict = self._restore_reserved_attribute_names(
form.data, model_view.model
)
try:
obj = await model_view.insert_model(request, form.data)
obj = await model_view.insert_model(request, form_data_dict)
except Exception as e:
logger.exception(e)
context["error"] = str(e)
Expand Down Expand Up @@ -540,10 +548,11 @@ async def edit(self, request: Request) -> Response:
raise HTTPException(status_code=404)

Form = await model_view.scaffold_form()
replacement_data = self._compute_replacement_data_for_reserved_attributes(model)
context = {
"obj": model,
"model_view": model_view,
"form": Form(obj=model),
"form": Form(obj=model, data=replacement_data),
}

if request.method == "GET":
Expand All @@ -559,12 +568,13 @@ async def edit(self, request: Request) -> Response:
request, model_view.edit_template, context, status_code=400
)

form_data_dict = self._restore_reserved_attribute_names(form.data, model)
try:
if model_view.save_as and form_data.get("save") == "Save as new":
obj = await model_view.insert_model(request, form.data)
obj = await model_view.insert_model(request, form_data_dict)
else:
obj = await model_view.update_model(
request, pk=request.path_params["pk"], data=form.data
request, pk=request.path_params["pk"], data=form_data_dict
)
except Exception as e:
logger.exception(e)
Expand Down Expand Up @@ -659,7 +669,7 @@ def get_save_redirect_url(

async def _handle_form_data(self, request: Request, obj: Any = None) -> FormData:
"""
Handle form data and modify in case of UplaodFile.
Handle form data and modify in case of UploadFile.
This is needed since in edit page
there's no way to show current file of object.
"""
Expand All @@ -683,6 +693,39 @@ async def _handle_form_data(self, request: Request, obj: Any = None) -> FormData
form_data.append((key, value))
return FormData(form_data)

def _compute_replacement_data_for_reserved_attributes(self, obj: Any) -> dict:
"""Handle fields which name collide with WTForms reserved Form attribute names.
Return a mapping of replacement name -> original value, that will be passed to
the ModelView associated Form.
"""
replacement_data = {}
for (
reserved_field_name,
replacement_name,
) in WTFORMS_RESERVED_ATTRIBUTES_MAPPING.items():
if model_attr := getattr(obj, reserved_field_name, None):
replacement_data[replacement_name] = model_attr
return replacement_data

def _restore_reserved_attribute_names(self, form_data: dict, obj: Any) -> dict:
"""Restore the original names of fields clashing with reserved WTForms attrs"""
consolidated_form_data = form_data.copy()
for (
replacement_name,
reserved_field_name,
) in WTFORMS_RESERVED_ATTRIBUTES_REVERSED_MAPPING.items():
if (
replacement_name in consolidated_form_data
and not getattr(obj, replacement_name, None)
and getattr(obj, reserved_field_name, None)
):
consolidated_form_data[
reserved_field_name
] = consolidated_form_data.pop(replacement_name)
return consolidated_form_data


def expose(
path: str,
Expand Down
25 changes: 24 additions & 1 deletion sqladmin/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,17 @@ def __call__(

T_CC = TypeVar("T_CC", bound=ConverterCallable)

# If the model has a mapped column named e.g. 'data', this will utltimately
# shadow the wtforms.Form.data attribute, thus causing issues
# (see https://github.com/aminalaee/sqladmin/issues/656).
# To circumvent the issue, we maintain a list of reserved attribute names that
# sqlmodel will silently rename between when converting a sqladmin mapped column
# into its associated wtform form field (and inversely).
WTFORMS_RESERVED_ATTRIBUTES_MAPPING = {"data": "data_"}
WTFORMS_RESERVED_ATTRIBUTES_REVERSED_MAPPING = {
v: k for k, v in WTFORMS_RESERVED_ATTRIBUTES_MAPPING.items()
}


@no_type_check
def converts(*args: str) -> Callable[[T_CC], T_CC]:
Expand Down Expand Up @@ -618,6 +629,18 @@ async def get_model_form(
field_dict = {}
for name, attr in attributes:
field_args = form_args.get(name, {})

# if the model has a mapped column with a name matching one of the
# wtforms reserved names, we silently replace it in the destination Form.
# However, we pass a 'name' attribute to the FormField with the original
# column name, so that the user does not see the replacement name in the
# admin web UI.
if name in WTFORMS_RESERVED_ATTRIBUTES_MAPPING:
field_dict_key = WTFORMS_RESERVED_ATTRIBUTES_MAPPING[name]
field_args["name"] = name
else:
field_dict_key = name

field_widget_args = form_widget_args.get(name, {})
label = column_labels.get(name, None)
override = form_overrides.get(name, None)
Expand All @@ -633,6 +656,6 @@ async def get_model_form(
form_ajax_refs=form_ajax_refs,
)
if field is not None:
field_dict[name] = field
field_dict[field_dict_key] = field

return type(type_name, (form_class,), field_dict)
34 changes: 34 additions & 0 deletions tests/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@
Base = declarative_base() # type: ignore


class DataModel(Base):
__tablename__ = "datamodel"
id = Column(Integer, primary_key=True)
data = Column(String)


class User(Base):
__tablename__ = "users"

Expand Down Expand Up @@ -121,3 +127,31 @@ class UserAdmin(ModelView, model=User):
admin.add_view(UserAdmin)

admin._menu.items.pop().name = "Accounts"


def test_compute_replacement_data_for_reserved_attributes() -> None:
app = Starlette()
admin = Admin(app=app, engine=engine)

class DataModelAdmin(ModelView, model=DataModel):
...

datamodel = DataModel(id=1, data="abcdef")
admin.add_view(DataModelAdmin)
assert admin._compute_replacement_data_for_reserved_attributes(datamodel) == {
"data_": "abcdef"
}


def test_restore_reserved_attribute_names() -> None:
app = Starlette()
admin = Admin(app=app, engine=engine)

class DataModelAdmin(ModelView, model=DataModel):
...

datamodel = DataModel(id=1, data="abcdef")
admin.add_view(DataModelAdmin)
assert admin._restore_reserved_attribute_names({"data_": "abcdef"}, datamodel) == {
"data": "abcdef"
}
17 changes: 17 additions & 0 deletions tests/test_forms/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -298,3 +298,20 @@ class MyModel(Base):

assert isinstance(Form()._fields["email"], EmailField)
assert isinstance(Form()._fields["number"], IntegerField)


async def test_model_field_clashing_with_wtforms_reserved_attribute() -> None:
class DataModel(Base):
__tablename__ = "model_with_wtforms_reserved_attribute"
id = Column(Integer, primary_key=True)
data = Column(String)

Form = await get_model_form(
model=DataModel,
session_maker=session_maker,
)
form = Form(obj=DataModel(id=1, data="abcdef"))
assert Form.data_.field_class == StringField
assert Form.data_.name == "data"
assert isinstance(form.data, dict)
assert isinstance(Form.data, property)

0 comments on commit 5fcf7d8

Please sign in to comment.