Opened 17 years ago

Closed 15 years ago

Last modified 13 years ago

#6845 closed (fixed)

Model validation and its propagation to ModelForms

Reported by: Honza Král Owned by: Honza Král
Component: Forms Version: dev
Severity: Keywords: newforms validation model modelform ep2008
Cc: Christian Schilling, listuser@…, gabor@…, yatiohi@…, ondrej.kohohut@…, bronger@…, rajesh.dhawan@…, model-validation@…, erwin@…, django-trac@…, flori@…, gonz@…, aball@…, public@…, Jannis Leidel, Eduardo de Oliveira Padoan, Benjamin Schwarze, remco@…, piranha@…, moya@…, David Larlet, Carlo Pires, jim@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Hi all, during a sprint at PyCON 2008 I wrote first simple version of model validation.

Main features:

  • model fields can define list of validators (kwarg validators) - validator is a function that accepts one argument and throws django.core.validation.validationError if anything doesn't look right
  • models now have a validate method that
    • call the validation for fields
    • can be overridden to allow for custom validation
    • raises django.core.validation.validationError if anything goes amiss
  • formfields' clean method has been deprecated and split into two - to_python and validate
    • to_python does the type coersion, throws django.core.validation.TypeCoersionError
    • validate doesnt return anything, just raises an exception
    • new kwarg validators allows for passing validators to formfields as well
  • form method clean has been renamed to validate and also doesn't return anything.
    • hooks for validation individual fields on form should now be named validate_FIELD_NAME and take the value to validate as only parameter

Steps that remain to be done:

  • verify how much code has been broken by this
  • clean the code, update comments
  • add more tests for the new behavior
  • add documentation
  • add validation to formsets to validate for unique and unique_together across all the forms in the formset
  • allow to turn some of the validation off ???
    • unique(_together) for performance reasons

Attachments (11)

6845-against-django-7338.diff (133.7 KB ) - added by Honza Král 17 years ago.
6845-against-django-7350.diff (134.9 KB ) - added by Honza Král 17 years ago.
6845-against-7424.diff (136.5 KB ) - added by Honza Král 17 years ago.
new version
6845-against-7625.diff (136.4 KB ) - added by Honza Král 16 years ago.
new working patch
6845-against-7871.patch (142.6 KB ) - added by Honza Král 16 years ago.
6845-against-7877.patch (142.6 KB ) - added by Honza Král 16 years ago.
6845-against-7877.patch.gz (44.9 KB ) - added by Honza Král 16 years ago.
6845-against-7877.2.patch (128.8 KB ) - added by Honza Král 16 years ago.
6845-against-8090.patch (129.0 KB ) - added by Honza Král 16 years ago.
new patch after 1.0 alpha
6845-against-8198.patch (132.4 KB ) - added by Honza Král 16 years ago.
see attached comment for details about this patch
6845-against-9226.diff (37.2 KB ) - added by Honza Král 16 years ago.

Download all attachments as: .zip

Change History (83)

by Honza Král, 17 years ago

comment:1 by Honza Král, 17 years ago

After a mail from Malcolm (thanks), I changed a few things:

Models now have clean, to_python and validate methods that behave the same way as on forms:

  • to_python loops through the fields and does type coercion (and save the new value)
  • validate loops through the fields and validates them, if new_data attribute is supplied, it would look for the data there, do ad-hoc type coercion (which it wouldn't save -- not sure about this) and then run it through field's validate
  • clean calls to_python and validate

Consequences are that if to_python raises an error, validate doesn't run when to_python fails

FormField's clean method is now called from form.full_clean instead of to_python and validate, this should allow for easy backwards compatibility without the need of any special field class.

I changed fi and br localflavors to conform with the rest of django - fields should return unicode strings. I didn't do that for FISocialSecurityNumber because it would break localflavor_fi_tests - it would throw UnicodeDecodeError before ValidationError, in my opinion that's what it should do when passed incorrect data (or perhaps TypeCoercionError). What do you think?

Corrected the typo in TypeCoer(s/c)ionError

by Honza Král, 17 years ago

by Honza Král, 17 years ago

Attachment: 6845-against-7424.diff added

new version

comment:2 by BazzaniMarco <alfred.einstein@…>, 17 years ago

the question that everybody wants to ask but nobody do:
when will it be merged into trunk ?

comment:3 by BazzaniMarco <alfred.einstein@…>, 17 years ago

and another stupid question
is it me or diffs are all empty here?

in reply to:  3 comment:4 by Honza Král, 17 years ago

Replying to BazzaniMarco <alfred.einstein@gmail.com>:

and another stupid question
is it me or diffs are all empty here?

the diffs are not empty, they are just not parsed by TRAC, try downloading them

Replying to BazzaniMarco <alfred.einstein@gmail.com>:

the question that everybody wants to ask but nobody do:
when will it be merged into trunk ?

when its ready, so far very few people has tested this and/or expressed their opinion on the matter.

the missing parts:

  1. *feedback* !! (works, does what everybody expects it to do, has all the options neede)
  2. *documentation* and code clenup
  3. tests (the existing tests were adapted, but more are needed)

comment:5 by mrts, 17 years ago

Marked #4895 as duplicate of this.

comment:6 by nix, 17 years ago

Cc: listuser@… added

comment:7 by anonymous, 17 years ago

Cc: gabor@… added

comment:8 by Karen Tracey <kmtracey@…>, 17 years ago

I'm confused by the patch 6845-against-7625.diff and can't get it to apply. I'm using an svn working copy at r7625, no local mods, yet I get some failures and many applies with offsets and one "Reversed (or previously applied) patch detected!". The first failure (django/contrib/auth/create_superuser.py) appears to reference code that was moved out of that file in r7590. Is this patch really against r7625 and if so what am I doing wrong?

kmt@lbox:~/tmp/django/trunk$ svn info
Path: .
URL: http://code.djangoproject.com/svn/django/trunk
Repository Root: http://code.djangoproject.com/svn
Repository UUID: bcc190cf-cafb-0310-a4f2-bffc1f526a37
Revision: 7625
Node Kind: directory
Schedule: normal
Last Changed Author: russellm
Last Changed Rev: 7625
Last Changed Date: 2008-06-12 09:19:37 -0400 (Thu, 12 Jun 2008)

kmt@lbox:~/tmp/django/trunk$ svn status -u
       *     7625   django/core/management/commands/dumpdata.py
?                   6845-against-7625.diff
Status against revision:   7630
kmt@lbox:~/tmp/django/trunk$ patch -p1 --dry-run <6845-against-7625.diff 
patching file AUTHORS
Hunk #1 succeeded at 387 (offset 6 lines).
patching file django/contrib/admin/views/template.py
patching file django/contrib/auth/create_superuser.py
Hunk #1 FAILED at 61.
1 out of 1 hunk FAILED -- saving rejects to file django/contrib/auth/create_superuser.py.rej
patching file django/contrib/auth/forms.py
patching file django/contrib/auth/models.py
Hunk #2 succeeded at 135 (offset 7 lines).
patching file django/contrib/comments/views/comments.py
patching file django/contrib/flatpages/models.py
patching file django/contrib/localflavor/br/forms.py
patching file django/contrib/localflavor/fi/forms.py
patching file django/contrib/localflavor/jp/forms.py
patching file django/core/exceptions.py
Reversed (or previously applied) patch detected!  Assume -R? [n]  
Apply anyway? [n] 
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file django/core/exceptions.py.rej
patching file django/core/validation.py
patching file django/core/validators.py
patching file django/db/models/__init__.py
patching file django/db/models/base.py
Hunk #1 FAILED at 1.
Hunk #2 succeeded at 19 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 347 with fuzz 2 (offset 69 lines).
1 out of 3 hunks FAILED -- saving rejects to file django/db/models/base.py.rej
patching file django/db/models/fields/__init__.py
Hunk #1 succeeded at 12 with fuzz 1 (offset 2 lines).
Hunk #2 FAILED at 81.
Hunk #3 succeeded at 105 (offset 6 lines).
Hunk #4 succeeded at 173 with fuzz 2 (offset 18 lines).
Hunk #5 succeeded at 348 (offset 21 lines).
Hunk #6 succeeded at 544 (offset 21 lines).
Hunk #7 succeeded at 736 (offset 21 lines).
Hunk #8 succeeded at 744 (offset 21 lines).
Hunk #9 succeeded at 777 (offset 21 lines).
Hunk #10 succeeded at 788 (offset 21 lines).
Hunk #11 succeeded at 946 (offset 21 lines).
Hunk #12 succeeded at 958 (offset 21 lines).
Hunk #13 succeeded at 988 (offset 21 lines).
Hunk #14 succeeded at 1027 (offset 21 lines).
Hunk #15 succeeded at 1125 (offset 21 lines).
1 out of 15 hunks FAILED -- saving rejects to file django/db/models/fields/__init__.py.rej
patching file django/db/models/fields/related.py
Hunk #1 succeeded at 792 (offset 45 lines).
patching file django/newforms/__init__.py
patching file django/newforms/fields.py
patching file django/newforms/forms.py
patching file django/newforms/models.py
patching file django/newforms/util.py
patching file django/oldforms/__init__.py
patching file django/oldforms/validators.py
patching file tests/modeltests/manipulators/models.py
patching file tests/modeltests/model_forms/models.py
patching file tests/modeltests/validation/models.py
patching file tests/regressiontests/core/models.py
patching file tests/regressiontests/core/tests.py
patching file tests/regressiontests/forms/error_messages.py
patching file tests/regressiontests/forms/extra.py
patching file tests/regressiontests/forms/fields.py
patching file tests/regressiontests/forms/localflavor/br.py
patching file tests/regressiontests/forms/localflavor/generic.py
patching file tests/regressiontests/forms/util.py
kmt@lbox:~/tmp/django/trunk$ 


comment:9 by alfred.einstein@…, 17 years ago

same problems here patching with latest trunk 7629

bzr patch ../../6845-against-7625.diff --strip=1
1 out of 1 hunk FAILED -- saving rejects to file django/contrib/auth/create_superuser.py.rej
1 out of 1 hunk FAILED -- saving rejects to file django/core/exceptions.py.rej
Patch attempted to create file django/core/validation.py, which already exists.
1 out of 1 hunk FAILED -- saving rejects to file django/core/validation.py.rej
1 out of 3 hunks FAILED -- saving rejects to file django/db/models/base.py.rej
1 out of 15 hunks FAILED -- saving rejects to file django/db/models/fields/__init__.py.rej
Patch attempted to create file django/oldforms/validators.py, which already exists.
1 out of 1 hunk FAILED -- saving rejects to file django/oldforms/validators.py.rej
Patch attempted to create file tests/regressiontests/core/models.py, which already exists.
1 out of 1 hunk FAILED -- saving rejects to file tests/regressiontests/core/models.py.rej
Patch attempted to create file tests/regressiontests/core/tests.py, which already exists.
1 out of 1 hunk FAILED -- saving rejects to file tests/regressiontests/core/tests.py.rej
bzr: ERROR: Patch application failed

comment:10 by Jeff Anderson, 16 years ago

Triage Stage: UnreviewedAccepted

by Honza Král, 16 years ago

Attachment: 6845-against-7625.diff added

new working patch

comment:11 by Honza Král, 16 years ago

Needs documentation: set
Needs tests: set

Should work now, sorry for the mixup:

king all % git reset HEAD --hard
HEAD is now at c5699f6 Fixed #7327 -- Added documentation and test case for defining subqueries. Thanks, Sebastian Noack.
king all % patch -p1 < 6845-against-7625.diff
patching file AUTHORS
patching file django/contrib/admin/views/template.py
patching file django/contrib/auth/forms.py
patching file django/contrib/auth/management/commands/createsuperuser.py
patching file django/contrib/auth/models.py
patching file django/contrib/comments/views/comments.py
patching file django/contrib/flatpages/models.py
patching file django/contrib/localflavor/br/forms.py
patching file django/contrib/localflavor/fi/forms.py
patching file django/contrib/localflavor/jp/forms.py
patching file django/core/validation.py
patching file django/core/validators.py
patching file django/db/models/__init__.py
patching file django/db/models/base.py
patching file django/db/models/fields/__init__.py
patching file django/db/models/fields/related.py
patching file django/newforms/__init__.py
patching file django/newforms/fields.py
patching file django/newforms/forms.py
patching file django/newforms/models.py
patching file django/newforms/util.py
patching file django/oldforms/__init__.py
patching file django/oldforms/validators.py
patching file tests/modeltests/manipulators/models.py
patching file tests/modeltests/model_forms/models.py
patching file tests/modeltests/validation/models.py
patching file tests/regressiontests/core/models.py
patching file tests/regressiontests/core/tests.py
patching file tests/regressiontests/forms/error_messages.py
patching file tests/regressiontests/forms/extra.py
patching file tests/regressiontests/forms/fields.py
patching file tests/regressiontests/forms/localflavor/br.py
patching file tests/regressiontests/forms/localflavor/generic.py
patching file tests/regressiontests/forms/util.py

comment:13 by mrts, 16 years ago

Tagging related tickets as model-validation.

<mrts> jacobkm: there are several tickets related to model validation (#1021, #702), should we create a keyword for them? They can't be marked as duplicates of #6845 as they describe various corner cases that can be easily missed by model validation patch.
<DjangoBot> http://code.djangoproject.com/ticket/1021
 http://code.djangoproject.com/ticket/702
 http://code.djangoproject.com/ticket/6845
<jacobkm> mrts: yeah, just wanna tag them "model-validation" or something?

comment:14 by mrts, 16 years ago

#4895 is another duplicate

comment:15 by ctrochalakis, 16 years ago

Cc: yatiohi@… added

by Honza Král, 16 years ago

Attachment: 6845-against-7871.patch added

comment:16 by Honza Král, 16 years ago

Keywords: ep2008 added

by Honza Král, 16 years ago

Attachment: 6845-against-7877.patch added

comment:17 by Christian Schilling, 16 years ago

Cc: Christian Schilling added

comment:18 by anonymous, 16 years ago

Cc: ondrej.kohohut@… added

by Honza Král, 16 years ago

Attachment: 6845-against-7877.patch.gz added

comment:19 by Honza Král, 16 years ago

Just uploaded new patch that deals with the outstanding issues of stuff placement: ValidationError is moved to django.core.exceptions, I got rid of TypeCoercionError (the distinction between validation errors and type coercion error was too unclear for it to make sense) and ErrorList has moved to django.newforms.util. I introduced FormValidationError that takes inherits from ValidationError and uses ErrorList to represent itself.

by Honza Král, 16 years ago

Attachment: 6845-against-7877.2.patch added

comment:20 by Honza Král, 16 years ago

I stopped for a moment, talked on the IRC and realized that I am an idiot and I don't need the ErrorList to be inside the ValidationError but can exist separately.

enjoy

comment:21 by Torsten Bronger <bronger@…>, 16 years ago

Cc: bronger@… added

comment:22 by Victor Andrée <victor.andree@…>, 16 years ago

Uh, would it still be possible to manipulate cleaned_data in Form.validate? I currently have a clean-method which creates a cleaned_data-item out of several fields, i.e. it constructs a Python value from several fields. I consider it a clean solution in my code (albeit unorthodox).

in reply to:  22 comment:23 by Honza Král, 16 years ago

Replying to Victor Andrée <victor.andree@gmail.com>:

Uh, would it still be possible to manipulate cleaned_data in Form.validate? I currently have a clean-method which creates a cleaned_data-item out of several fields, i.e. it constructs a Python value from several fields. I consider it a clean solution in my code (albeit unorthodox).

No, validate shouldn't manipulate data, but you can still override clean() when necessary.

comment:24 by Malcolm Tredinnick, 16 years ago

We probably do need to find a way to let the "normalise + validate" pass change the data, Honza. This is possible now and quite a useful feature (particularly in Form.clean(). It's not entirely clear where the natural responsibility lies in theto_python and validate split, since you kind of want to validate before munging. We should think about this. It might a "phase 2" feature if it becomes clear that there is a solution but we bump the implementation a bit, but I'd like to be sure there is a way to do this.

Example use cases are things where you combine multiple fields into one value to validate them as a pair and then, since you've done all that work, make them available via the form's data. Now it's not a showstopper if this isn't possible, since you can make a method/property that does the conversion from multiple form fields to one, but it's a bit of an arbitrary restriction.

Anyway, something to think about. I'd forgotten about this situation, so I'll have to reread the patch with that situation in mind.

comment:25 by anonymous, 16 years ago

Cc: rajesh.dhawan@… added

by Honza Král, 16 years ago

Attachment: 6845-against-8090.patch added

new patch after 1.0 alpha

comment:26 by Honza Král, 16 years ago

Patch needs improvement: set

I attached new version of the patch, no changes in functionality, just maybe some typos and stuff introduced during the hectic rebasing on trunk.

I haven't had any time to think about Malcolms comment, but I agree that there are some issues we still need to resolve.

Some tests are failing - and it is IMHO a DDN: the auth tests are failing because UserCreationForm doesn't contain all the required fields and so the model.validate shoots it down.

I have no idea why the formset tests are failing, I will work on it.

This is by no means a patch suitable in any way for merging, it is work in progress only meant for people who wish to work on this issue.

I hope to find some time tomorrow to do it properly, but I cannot promise anything.

comment:27 by Nikolay, 16 years ago

Cc: model-validation@… added

comment:28 by Jökull Sólberg Auðunsson <jokullsolberg@…>, 16 years ago

Shouldn't this also address unique_for_[date,month,year] validation in ModelForms?

in reply to:  28 comment:29 by Honza Král, 16 years ago

Replying to Jökull Sólberg Auðunsson <jokullsolberg@gmail.com>:

Shouldn't this also address unique_for_[date,month,year] validation in ModelForms?

It should indeed, thanks for the catch. I will try and incorporate that into the next patch

by Honza Král, 16 years ago

Attachment: 6845-against-8198.patch added

see attached comment for details about this patch

comment:30 by Honza Král, 16 years ago

I attached new version of this ticket, some tests fail. Issues:

  • Failed test auth should ModelForm restricted to some fields validate even if some required (blank=False) fields were omitted?
  • Failed test model_formsets I haven't figure out how to run validation on InlineFormset with new instance (I don't have all the data needed to run validation)
  • UNTESTED should we move validation for unique, unique_for_date etc. to model's validate__FIELD ?? They need access to more than the field's value and that would make them candidates for the move. We could do this in contribute_to_class.
  • How do we deal with value-altering clean procedures? I say we keep it as is - anybody can override the current clean implementation and do their thing
  • No validation for unique and unique_together in formset

comment:31 by anonymous, 16 years ago

Cc: erwin@… added

comment:32 by Django Trac, 16 years ago

Cc: django-trac@… added

comment:33 by flosch, 16 years ago

Cc: flori@… added

comment:34 by Jacob, 16 years ago

milestone: 1.0 betapost-1.0

comment:35 by Gonzalo Saavedra <gonz@…>, 16 years ago

Cc: gonz@… added

comment:36 by Gary Wilson, 16 years ago

(In [8348]) Fixed #8206 -- Removed validate methods of Model and Model fields. They are are unsupported for 1.0 and will be replaced with more complete model validation (refs #6845).

comment:37 by anonymous, 16 years ago

Cc: aball@… added

comment:38 by Milan Holzäpfel, 16 years ago

Cc: public@… added

comment:39 by Jannis Leidel, 16 years ago

Cc: Jannis Leidel added

comment:40 by Eduardo de Oliveira Padoan, 16 years ago

Cc: Eduardo de Oliveira Padoan added

comment:41 by Benjamin Schwarze, 16 years ago

Cc: Benjamin Schwarze added

comment:42 by Remco Wendt, 16 years ago

Cc: remco@… added

by Honza Král, 16 years ago

Attachment: 6845-against-9226.diff added

comment:43 by Honza Král, 16 years ago

Another version of the patch.

Implemented:

  • moved ValidationError to django.core.exceptions, leaving ErrorList in django.forms
  • field.validate and field.clean on db fields
  • model.clean and model.validate on models
  • moved validate_unique to models
  • have formsets call form.save instead of save_instance (split save_instance into two functions for that purpose)

Not yet implemented:

  • validators and their use in both db and form fields
  • custom error messages (being able to override the messages from models in forms etc.)

Broken:

  • save_as_new on formsets - the model without the FK doesn't validate well, I don't know what to do with this one - on one hand it shouldn't validate (the model IS invalid), on the otherhand it's useful feature. Maybe when I get around to enable turning model validation off for a form instance, that might help a bit. Or some sort of delayed validation. suggestions welcome.

Changed and untested:

  • BaseGenericInlineFormSet

stay tuned for more.

comment:44 by Alexander Solovyov, 16 years ago

Cc: piranha@… added

comment:45 by alerades, 16 years ago

Hi Honza, Do you have any new code? I'd like to help here (this feature is critical for my current project)

Thank you

comment:46 by moya, 16 years ago

Cc: moya@… added

comment:47 by David Larlet, 16 years ago

Cc: David Larlet added

comment:48 by Carlo Pires, 16 years ago

Is anyone working on this? I need to implement some form of model validation and could help with this.

comment:49 by mrts, 16 years ago

I'll semi-champion and coordinate this (see http://groups.google.com/group/django-developers/msg/23e95656e30195ef ).

http://github.com/mrts/django/tree/master created with the latest patch. Everybody is most welcome to contribute.

A short howto (fork-based process seems best to me):

  1. register an account at http://github.com
  2. click the fork button at http://github.com/mrts/django/tree/master
  3. clone the fork to your machine:
    git clone git@github.com:YOUR_NICK_HERE/django.git
    
  4. add my "upstream" repo for tracking:
    cd django
    git remote add upstream git://github.com/mrts/django.git
    git fetch upstream
    
  5. work on code, commit locally
    git commit -a -m "Implemented foo."
    
  6. publish changes to your GitHub repo
    git push
    
  7. when my "upstream" branch changes, pull updates from it (fetch and merge in one step):
    git pull upstream master
    
  8. when you want to get your changes merged back to my tree, send me a pull request by clicking pull request at your fork's home page.

comment:50 by mrts, 16 years ago

Be sure to check the overview/task list: http://wiki.github.com/mrts/django

comment:51 by Malcolm Tredinnick, 16 years ago

@mrts: This is already well under control. Honza is working on it and coordinating with myself and Jacob. Let's not please have parallel implementations running, since Honza has spent a lot of time gathering requirements already.

comment:52 by mrts, 16 years ago

Good to hear that Honza is working on it, but it's unfortunate that this wasn't mentioned neither here nor on the mailing list -- what a waste of my time...

comment:53 by Malcolm Tredinnick, 16 years ago

Except that it has been mentioned on the mailing list multiple times (most recently last week when ericflo asked about it) and has been on the Version1.1Features page for ages. A quick glance at the producer of all the patches would show some evidence of Honza working on it as well.

comment:54 by mrts, 16 years ago

Arguing is not on my Favourite Pastimes List, but last update here is from October (followed by two requests for status updates) and the only mention since ages on the mailing list is a line "I'll check in with Honza and make sure that validation is under control" by yourself a few days ago -- which doesn't really say much (whether it is or is not under control remains open). Thus, there were sufficient grounds to conclude that no progress has been made since the last patch.

Anyhow, good luck, Honza, and thanks for working on this!

comment:55 by Honza Král, 16 years ago

I am the last one to say that everything is moving according to plan, but I AM working on this - I have set aside almost entire next week to update the patch and rebase it to new version of django.

The least I would expect is that somebody would contact me directly and ask about the status before assuming my death. Only Malcolm did that, and I answered that I should have new version within a couple of days.

btw. the version you based your wiki on is obsolete, there is a new one availible:

http://groups.google.com/group/django-developers/browse_thread/thread/6143408977f7ad/d09c52e1fb561469

comment:56 by mrts, 16 years ago

Aha, good. Do you want me to assist you? Perhaps you could fork the django svn clone on github yourself and publish your work so that all who are interested can contribute (the fork-based workflow is really nice and flexible)? The automatically updated django svn clone is at http://github.com/django/django/tree/master.

comment:57 by Carlo Pires, 16 years ago

Cc: Carlo Pires added

@Honza, I didn't understand if your last code is 6845-against-9226.diff or you are working on new one for a few days.

comment:58 by Honza Král, 16 years ago

Owner: changed from nobody to Honza Král

I will post my commits here: http://github.com/HonzaKral/django/tree/master

After this weekend I hope to have a lot of tasks for anybody willing to help, now I have to update the base of the code to match current state of django.

If you have time now, you can definitely start working on validators (not glamorous task, I agree, but it needs to be done) so that we can later integrate them.

@carlopires: I am working on new code, but the main thing I am doing now is cleaning and updating the last patch to trunk, will push to github as soon as that's done.

comment:59 by mrts, 16 years ago

I have nothing against gruntwork :). I'll take care of porting 0.96 validators to django.db.models.validators.

in reply to:  59 comment:60 by Honza Král, 16 years ago

Replying to mrts:

I have nothing against gruntwork :). I'll take care of porting 0.96 validators to django.db.models.validators.

please don't put it there - it should server for form validation as well.

Validators are functions like this:

def validate_something( value, all_values={}, instance=None ): 
  if i_dont_like( value ): 
    raise ValidationError( 'I dont like %r' % value, error_code=validators.SOME_CODE ) 

where error code is some constant that will later be used to remap the default errors in custom_messages

comment:62 by (none), 16 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:63 by Jacob, 16 years ago

milestone: 1.2

comment:64 by Jim Garrison, 16 years ago

Cc: jim@… added

comment:65 by sampablokuper, 15 years ago

This might not be the right place to ask the question, but can anyone point me towards a document explaining the rationale under which Django's developers decided that:

  • most validation rules must be specified at the "forms" layer, although an a handful of them, e.g. max_length, can be specified at the model layer if desired

rather than deciding that:

  • validation rules should all be specified at the "model" layer and bubble up from there, with the option to override them for specific forms if needed

?

Many thanks.

comment:66 by sampablokuper, 15 years ago

Typo in the above:

s/an a handful/an arbitrary handful/

in reply to:  65 comment:67 by Honza Král, 15 years ago

Replying to sampablokuper:

This might not be the right place to ask the question, but can anyone point me towards a document explaining the rationale under which Django's developers decided that:

  • most validation rules must be specified at the "forms" layer, although an a handful of them, e.g. max_length, can be specified at the model layer if desired

rather than deciding that:

  • validation rules should all be specified at the "model" layer and bubble up from there, with the option to override them for specific forms if needed

This was not decided, it just wasn't implemented. This ticked and the accompanying patch fixes that for you. After this ticket is resolved you should be able to define all your model validation on Models and only use form validation for some extra stuff or for forms that aren't ModelForms.

And yes, this isn't exactly a good place to ask a question like this, try the django-dev mailing list if you want more information.

comment:68 by Marco Bazzani, 15 years ago

hi all are there any news about this patch ?
is there a branch for it ?
what could we do to improve it ?
cheers

in reply to:  68 comment:69 by Honza Král, 15 years ago

Replying to visik7:

hi all are there any news about this patch ?
is there a branch for it ?
what could we do to improve it ?
cheers

The patch is ready for testing in soc2009/model-validation branch

comment:70 by Tobias McNulty, 15 years ago

closed #11998 as duplicate of this case

comment:71 by jkocherhans, 15 years ago

Resolution: fixed
Status: newclosed

Closed by [12098].

comment:72 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top