Opened 7 years ago

Closed 6 years ago

Last modified 4 years ago

#6845 closed (fixed)

Model validation and its propagation to ModelForms

Reported by: Honza_Kral Owned by: Honza_Kral
Component: Forms Version: master
Severity: Keywords: newforms validation model modelform ep2008
Cc: christian, listuser@…, gabor@…, yatiohi@…, ondrej.kohohut@…, bronger@…, rajesh.dhawan@…, model-validation@…, erwin@…, django-trac@…, flori@…, gonz@…, aball@…, public@…, jezdez, edcrypt, benjixx, remco@…, piranha@…, moya@…, david, carlopires, jim@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

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_Kral 7 years ago.
6845-against-django-7350.diff (134.9 KB) - added by Honza_Kral 7 years ago.
6845-against-7424.diff (136.5 KB) - added by Honza_Kral 7 years ago.
new version
6845-against-7625.diff (136.4 KB) - added by Honza_Kral 7 years ago.
new working patch
6845-against-7871.patch (142.6 KB) - added by Honza_Kral 7 years ago.
6845-against-7877.patch (142.6 KB) - added by Honza_Kral 7 years ago.
6845-against-7877.patch.gz (44.9 KB) - added by Honza_Kral 7 years ago.
6845-against-7877.2.patch (128.8 KB) - added by Honza_Kral 7 years ago.
6845-against-8090.patch (129.0 KB) - added by Honza_Kral 7 years ago.
new patch after 1.0 alpha
6845-against-8198.patch (132.4 KB) - added by Honza_Kral 7 years ago.
see attached comment for details about this patch
6845-against-9226.diff (37.2 KB) - added by Honza_Kral 7 years ago.

Download all attachments as: .zip

Change History (83)

Changed 7 years ago by Honza_Kral

comment:1 Changed 7 years ago by Honza_Kral

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

Changed 7 years ago by Honza_Kral

Changed 7 years ago by Honza_Kral

new version

comment:2 Changed 7 years ago by BazzaniMarco <alfred.einstein@…>

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

comment:3 follow-up: Changed 7 years ago by BazzaniMarco <alfred.einstein@…>

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

comment:4 in reply to: ↑ 3 Changed 7 years ago by Honza_Kral

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 Changed 7 years ago by mrts

Marked #4895 as duplicate of this.

comment:6 Changed 7 years ago by nix

  • Cc listuser@… added

comment:7 Changed 7 years ago by anonymous

  • Cc gabor@… added

comment:8 Changed 7 years ago by Karen Tracey <kmtracey@…>

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 Changed 7 years ago by alfred.einstein@…

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 Changed 7 years ago by programmerq

  • Triage Stage changed from Unreviewed to Accepted

Changed 7 years ago by Honza_Kral

new working patch

comment:11 Changed 7 years ago by Honza_Kral

  • 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:12 Changed 7 years ago by mrts

See #702

comment:13 Changed 7 years ago by mrts

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 Changed 7 years ago by mrts

#4895 is another duplicate

comment:15 Changed 7 years ago by ctrochalakis

  • Cc yatiohi@… added

Changed 7 years ago by Honza_Kral

comment:16 Changed 7 years ago by Honza_Kral

  • Keywords ep2008 added

Changed 7 years ago by Honza_Kral

comment:17 Changed 7 years ago by christian

  • Cc christian added

comment:18 Changed 7 years ago by anonymous

  • Cc ondrej.kohohut@… added

Changed 7 years ago by Honza_Kral

comment:19 Changed 7 years ago by Honza_Kral

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.

Changed 7 years ago by Honza_Kral

comment:20 Changed 7 years ago by Honza_Kral

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 Changed 7 years ago by Torsten Bronger <bronger@…>

  • Cc bronger@… added

comment:22 follow-up: Changed 7 years ago by Victor Andrée <victor.andree@…>

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).

comment:23 in reply to: ↑ 22 Changed 7 years ago by Honza_Kral

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 Changed 7 years ago by mtredinnick

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 Changed 7 years ago by anonymous

  • Cc rajesh.dhawan@… added

Changed 7 years ago by Honza_Kral

new patch after 1.0 alpha

comment:26 Changed 7 years ago by Honza_Kral

  • 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 Changed 7 years ago by niksite

  • Cc model-validation@… added

comment:28 follow-up: Changed 7 years ago by Jökull Sólberg Auðunsson <jokullsolberg@…>

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

comment:29 in reply to: ↑ 28 Changed 7 years ago by Honza_Kral

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

Changed 7 years ago by Honza_Kral

see attached comment for details about this patch

comment:30 Changed 7 years ago by Honza_Kral

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 Changed 7 years ago by anonymous

  • Cc erwin@… added

comment:32 Changed 7 years ago by mikeblake

  • Cc django-trac@… added

comment:33 Changed 7 years ago by flosch

  • Cc flori@… added

comment:34 Changed 7 years ago by jacob

  • milestone changed from 1.0 beta to post-1.0

comment:35 Changed 7 years ago by Gonzalo Saavedra <gonz@…>

  • Cc gonz@… added

comment:36 Changed 7 years ago by gwilson

(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 Changed 7 years ago by anonymous

  • Cc aball@… added

comment:38 Changed 7 years ago by m_holzaepfel

  • Cc public@… added

comment:39 Changed 7 years ago by jezdez

  • Cc jezdez added

comment:40 Changed 7 years ago by edcrypt

  • Cc edcrypt added

comment:41 Changed 7 years ago by benjixx

  • Cc benjixx added

comment:42 Changed 7 years ago by shanx

  • Cc remco@… added

Changed 7 years ago by Honza_Kral

comment:43 Changed 7 years ago by Honza_Kral

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 Changed 7 years ago by piranha

  • Cc piranha@… added

comment:45 Changed 7 years ago by alerades

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 Changed 7 years ago by moya

  • Cc moya@… added

comment:47 Changed 7 years ago by david

  • Cc david added

comment:48 Changed 7 years ago by carlopires

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

comment:49 Changed 7 years ago by mrts

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 Changed 7 years ago by mrts

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

comment:51 Changed 7 years ago by mtredinnick

@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 Changed 7 years ago by mrts

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 Changed 7 years ago by mtredinnick

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 Changed 7 years ago by mrts

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 Changed 7 years ago by Honza_Kral

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 Changed 7 years ago by mrts

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 Changed 7 years ago by carlopires

  • Cc carlopires 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 Changed 7 years ago by Honza_Kral

  • Owner changed from nobody to Honza_Kral

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 follow-up: Changed 7 years ago by mrts

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

comment:60 in reply to: ↑ 59 Changed 7 years ago by Honza_Kral

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 Changed 7 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:63 Changed 7 years ago by jacob

  • milestone set to 1.2

comment:64 Changed 6 years ago by garrison

  • Cc jim@… added

comment:65 follow-up: Changed 6 years ago by 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

?

Many thanks.

comment:66 Changed 6 years ago by sampablokuper

Typo in the above:

s/an a handful/an arbitrary handful/

comment:67 in reply to: ↑ 65 Changed 6 years ago by Honza_Kral

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 follow-up: Changed 6 years ago by visik7

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

comment:69 in reply to: ↑ 68 Changed 6 years ago by Honza_Kral

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 Changed 6 years ago by tobias

closed #11998 as duplicate of this case

comment:71 Changed 6 years ago by jkocherhans

  • Resolution set to fixed
  • Status changed from new to closed

Closed by [12098].

comment:72 Changed 4 years ago by jacob

  • milestone 1.2 deleted

Milestone 1.2 deleted

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