-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement form validation & redirect back with errors #32
base: main
Are you sure you want to change the base?
Conversation
Some other notes: Type HintsThis codebase doesn't use type hints internally but I think they should be used at least for the public API so that users can see the parameter types in their IDE, that's why I used type hints for the validation error and helper. For all the rest of things I tried following the same style that was already present. Error messagesWhen Django validates a form it creates a list of error messages for each field, something like this: { "name": ["This field is required.", "Some other error"] } To avoid having to write { "name": "This field is required." } The official Laravel adapter does the same. Non-field errorsDjango has something called form.errors["__all__"] RedirectingIf the form def view(request):
if request.method == "GET":
return render(request, "Component")
if request.method == "POST":
form = SomeForm(json.loads(request.body))
if not form.is_valid():
return render(request, "Component", props={"errors": form.errors})
# ... I didn't check this but I think it works. JSONAs per issue #21, we could provide some built-in functionality to simplify this: form = SomeForm(json.loads(request.body)) |
Just got another idea regarding this:
Since middlewares run top to bottom and then bottom to top again, the Inertia middleware is always going to run before the user's view function. So it could store the if not form.is_valid():
raise InertiaValidationError(form.errors, fallback=redirect("some_route")) Since cleaned_data = inertia_validate(form) I'll wait on some feedback before writing more code, but I think this looks pretty clean, it doesn't really do that much magic except for raising an error. |
Hey @antoniosarosi, Just curious about the specific use case you're tackling here. Like you mentioned, Django apps usually rerender the form after validation fails, and that's pretty standard for all the generic views. Are you dealing with a view that's just for POST requests? I'm thinking that might be outside what this project is all about. Usually, just adding the error messages in the context sorts things out for Inertia compatibility. We also tweaked the Inertia implementation a bit in our projects to be compatible with Django's helper function My take is that it's probably better to tweak Inertia to fit into Django's way of doing things, not the other way around. This way, you get to make the most of what Django offers, especially with its generic class-based views. What do you think? Any specific challenges or reasons why a different approach might work better in your case? |
@svengt In my particular use case there's a login form that you can submit from any page available to non-authenticated users. There's a login button in the top navbar which instead of redirecting you to the login page opens up a sidebar with the form, and you submit it from wherever you are. When using Django templates you are pretty much forced to redirect the user to the login page or use Javascript to send the request from the current page, but with Vue or React you can take different approaches. The general use case is validating a form that you don't know which page it's coming from or the As I mentioned before, if you are using a frontend framework like Vue or React you are not as constrained as when you're using template rendering, so you can improve UX by doing things like this, there's less redirecting in general. And since you can't simply render the page again because you don't know which one to render, you need some solution. In the current state, either use JSON requests/responses or always redirect to specific CRUD pages like you would with templates. Both require more code and are not compatible with Inertia's way of handling form submissions. I agree that this might not be exactly the Django way of doing things, but sticking 100% to how Django handles templates seems to limit what you can do with Inertia or require you to implement custom solutions. For example, Django doesn't have a built-in way to parse JSON requests as mentioned in #21, Django doesn't automatically serialize models/queries into JSON either and that's why On the other hand, raising an error to send a response is not unseen in Django, you can raise Anyway, I'm not trying to convince you that this is a good idea, my initial thought was actually implementing this in a separate middleware so that you can opt-in by placing it into If this doesn't end up being a built-in functionality it could go into a separate Python package, probably more people who don't mind not being 100% Django compatible would find it useful. I suppose it's not necessary for simple apps where you can always re-render on the spot but I think it comes in handy for medium size apps that might get a little more complex on the frontend side. |
Following up on #30, here's a basic implementation of Inertia Validation. Quoting the docs:
That's what the code does, when
not form.is_valid()
you can raiseInertiaValidationError
and this library handles flashing validation errors in the session, redirecting and passing theerrors
prop to Inertia. For now you must explicitly provide the "previous" URL where to user should be redirected, since it's not stored anywhere.Basic usage example:
Instead of raising the error manually you can also call the
inertia_validate
helper, which will raise the error automatically:In some cases you don't actually know where the user should be redirected because the form might be located in a pop-up that you can trigger from multiple places or something similar. For example, imagine a "Login" button in the navbar that opens a side panel with a login form. For now, the best option is to use the HTTP
Referer
header to handle such cases:In order to avoid redirecting manually we could change the API so that you pass the request and an optional
fallback
:This would try to redirect to
request.headers["Referer"]
if present, otherwise use the fallback, which could be set to"/"
by default.Technically, the
Referer
header is optional so we shouldn't rely 100% on it, Laravel for example stores the "previous" URL in the session on everyGET
request to its server. Here's some pseudocode based on Laravel 10.x implementation:But I don't think implementing this is necessary, because in practice all major browsers will send the
Referer
header and even Laravel favorsReferer
over everything else.