starlette-admin icon indicating copy to clipboard operation
starlette-admin copied to clipboard

Proposal - Explicit Data Validation: `validate_create` and `validate_edit` instead of just `validate` in `contrib.sqla`

Open hasansezertasan opened this issue 1 year ago • 2 comments

It's a simple draft of what I propose.

Use cases:

  • I am creating a book record, the title attribute of the book record can't be changed, so I removed the title attribute from the edit form. Using the same validate method for both create and edit processes doesn't help me very much.
  • I have a model that is quite complex with over 50 attributes. I kept the create form minimal and it has only some basic attributes to get started. The edit form has all 50 attributes.

The validation process is not the same in both create and edit processes for these scenarios.

I had these scenarios in my projects and I have overridden the create and edit methods.

I think if we are going to have a validate method at the end of the day, we better have two separate validate methods for both create and edit functionality.

@jowilf If this sounds good, I can keep working on it.

hasansezertasan avatar Dec 27 '23 14:12 hasansezertasan

Have you considered checking the request.state.action? For example:

    async def validate(self, request: Request, data: Dict[str, Any]) -> None:
        if request.state.action == RequestAction.CREATE:
            ...
        else: # RequestAction.EDIT
            ...

jowilf avatar Dec 28 '23 00:12 jowilf

Have you considered checking the request.state.action? For example:

That solution didn't come to my mind but we could have implemented the same approach for before_ hooks, instead of two different methods like before_create or before_edit. Why didn't we? What was the motivation for having two separate before_ hooks instead of the following example?

We could have just one method and use request.state.action... For example:

    async def before(self, request: Request, data: Dict[str, Any], obj: Any) -> None:
        if request.state.action == RequestAction.CREATE:
            ...
        else: # RequestAction.EDIT
            ...

I know it's not the same but I think if we are meant to have two separate before_ methods for both create and edit, we might also have two separate validate_ methods for both create and edit.

In the end, what's the downside? In my point of view, it only gives a better developer experience.

Edit: There is also contrib.sqla.ext.pydantic which accepts only one pydantic model for validation. If this pattern gets accepted, shouldn't we have two validate methods over there too? Or we might remove that ModelView since it only adds one more parameter (pydantic_model) to the ModelView and overrides the validate method with 4 lines of code. I think documenting how to use pydantic_error_to_form_validation_errors. could be good enough.

Note: These are arguable ideas from me, I'm not here to criticize anything, just trying to help.

hasansezertasan avatar Dec 28 '23 02:12 hasansezertasan

Closing due to inactivity.

hasansezertasan avatar May 06 '24 16:05 hasansezertasan