Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Protik
I am using Django 1.11. Further, adding a new book with user field disabled 
results in the following error:

[image: Selection_046.png]


I have attached the code to reproduce the error.


On Monday, September 24, 2018 at 1:59:31 AM UTC+5:30, Todor Velichkov wrote:
>
> First thought: What is your Django version? The `disabled` attribute was 
> added in Django 1.9.
> However by looking at your code (w/o testing anything) after 
> `form.is_valid()` you should only call `form.save()`, no need to do 
> `commit=False` and manually assigning user, you have already done that in 
> the form constructor (by settings initial value + disabled attr).
>
> On Sunday, September 23, 2018 at 9:25:41 PM UTC+3, Protik wrote:
>>
>> Hi, Todor
>>
>> I have tested this solution and It looks like it works only when you 
>> don't disable the field (i.e the last line in the BookForm's `__init__()` 
>> method. My views looks like this:
>>
>>
>> def book_add(request):
>> user = get_object_or_404(User, id=1)
>>
>> if request.method == 'POST':
>>
>> f = BookForm(request.POST, user=user)
>> if f.is_valid():
>> book = f.save(commit=False)
>> book.user = user
>> book.save()
>> messages.add_message(request, messages.INFO, 'book added.')
>> return redirect('book_add')
>> else:
>> f = BookForm(user=user)
>>
>> return render(request, 'blog/book_add.html', {'f': f})
>>
>>
>> def post_update(request, post_pk):
>> user = get_object_or_404(User, id=1)
>> book = get_object_or_404(Book, pk=post_pk)
>> if request.method == 'POST':
>> f = BookForm(request.POST, instance=book, user=user)
>> if f.is_valid():
>> post = f.save(commit=False)
>> post.user = user
>> post.save()
>> messages.add_message(request, messages.INFO, 'Post added.')
>> return redirect('post_update', post.pk)
>> else:
>> f = BookForm(instance=book, user=user)
>>
>> return render(request, 'blog/book_update.html', {'f': f})
>>
>>
>> The code for models and modelform is exactly same as yours.
>>
>>
>> Am I doing something wrong?
>>
>>
>> On Sunday, September 23, 2018 at 9:11:55 PM UTC+5:30, Todor Velichkov 
>> wrote:
>>>
>>> You can use the `disabled 
>>> ` 
>>> attribute on form fields with a combination of HiddenInput 
>>> 
>>>
>>> Using the Book example from the first comment it will look like this:
>>>   
>>> class BookForm(forms.ModelForm):
>>> class Meta:
>>> model = Book
>>> fields = ('user', 'name')
>>> 
>>> def __init__(self, *args, **kwargs):
>>> user = kwargs.pop('user')
>>> super(BookForm, self).__init__(*args, **kwargs)
>>> self.fields['user'].widget = forms.HiddenInput()
>>> self.fields['user'].initial = user
>>> self.fields['user'].disabled = True
>>>
>>>
>>> First we hide the the user field because we dont want the user to see 
>>> it, and at the same time keeping it inside form fields we wont prevent the 
>>> unique_together validation.
>>> Second - we disable the field and programmatically set initial value to 
>>> be used during form validation
>>>
>>> On Sunday, September 23, 2018 at 4:57:15 PM UTC+3, Protik wrote:

 Hi  Todor

 I am experiencing the same problem. Can you please post the 
 possible solution?

 On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov 
 wrote:
>
> It does? Can you give me a link to that please?
>>
>
> Pard me, it does not explicitly say to set values programmatically, 
> but that this is the place to go when fields depend on each other, and 
> since clean is a multi-step process which does not include only field 
> validation, but settings values too, it kind of makes sense.
>
> 1) Form and field validation  
> 
>
> The form subclass’s clean() method can perform validation that 
>> requires access to multiple form fields. This is where you might put in 
>> checks such as “if field A is supplied, field B must contain a valid 
>> email address”. *This method can return a completely different 
>> dictionary if it wishes, which will be used as the cleaned_data*. 
>>
>
> 2) Cleaning and validating fields that depend on each other 
> 
>
> Suppose we add another requirement to our contact form: if the 
>> cc_myself field is True, the subject must contain the word "help". *We 
>> are performing validation on more than one field at a time, so the 
>> form’s 

Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Protik
I am using Django 1.11. Further, adding a new book with user field disabled 
results in the following error:

[image: Selection_046.png]
I have attached the code to reproduce the error.


On Monday, September 24, 2018 at 1:59:31 AM UTC+5:30, Todor Velichkov wrote:
>
> First thought: What is your Django version? The `disabled` attribute was 
> added in Django 1.9.
> However by looking at your code (w/o testing anything) after 
> `form.is_valid()` you should only call `form.save()`, no need to do 
> `commit=False` and manually assigning user, you have already done that in 
> the form constructor (by settings initial value + disabled attr).
>
> On Sunday, September 23, 2018 at 9:25:41 PM UTC+3, Protik wrote:
>>
>> Hi, Todor
>>
>> I have tested this solution and It looks like it works only when you 
>> don't disable the field (i.e the last line in the BookForm's `__init__()` 
>> method. My views looks like this:
>>
>>
>> def book_add(request):
>> user = get_object_or_404(User, id=1)
>>
>> if request.method == 'POST':
>>
>> f = BookForm(request.POST, user=user)
>> if f.is_valid():
>> book = f.save(commit=False)
>> book.user = user
>> book.save()
>> messages.add_message(request, messages.INFO, 'book added.')
>> return redirect('book_add')
>> else:
>> f = BookForm(user=user)
>>
>> return render(request, 'blog/book_add.html', {'f': f})
>>
>>
>> def post_update(request, post_pk):
>> user = get_object_or_404(User, id=1)
>> book = get_object_or_404(Book, pk=post_pk)
>> if request.method == 'POST':
>> f = BookForm(request.POST, instance=book, user=user)
>> if f.is_valid():
>> post = f.save(commit=False)
>> post.user = user
>> post.save()
>> messages.add_message(request, messages.INFO, 'Post added.')
>> return redirect('post_update', post.pk)
>> else:
>> f = BookForm(instance=book, user=user)
>>
>> return render(request, 'blog/book_update.html', {'f': f})
>>
>>
>> The code for models and modelform is exactly same as yours.
>>
>>
>> Am I doing something wrong?
>>
>>
>> On Sunday, September 23, 2018 at 9:11:55 PM UTC+5:30, Todor Velichkov 
>> wrote:
>>>
>>> You can use the `disabled 
>>> ` 
>>> attribute on form fields with a combination of HiddenInput 
>>> 
>>>
>>> Using the Book example from the first comment it will look like this:
>>>   
>>> class BookForm(forms.ModelForm):
>>> class Meta:
>>> model = Book
>>> fields = ('user', 'name')
>>> 
>>> def __init__(self, *args, **kwargs):
>>> user = kwargs.pop('user')
>>> super(BookForm, self).__init__(*args, **kwargs)
>>> self.fields['user'].widget = forms.HiddenInput()
>>> self.fields['user'].initial = user
>>> self.fields['user'].disabled = True
>>>
>>>
>>> First we hide the the user field because we dont want the user to see 
>>> it, and at the same time keeping it inside form fields we wont prevent the 
>>> unique_together validation.
>>> Second - we disable the field and programmatically set initial value to 
>>> be used during form validation
>>>
>>> On Sunday, September 23, 2018 at 4:57:15 PM UTC+3, Protik wrote:

 Hi  Todor

 I am experiencing the same problem. Can you please post the 
 possible solution?

 On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov 
 wrote:
>
> It does? Can you give me a link to that please?
>>
>
> Pard me, it does not explicitly say to set values programmatically, 
> but that this is the place to go when fields depend on each other, and 
> since clean is a multi-step process which does not include only field 
> validation, but settings values too, it kind of makes sense.
>
> 1) Form and field validation  
> 
>
> The form subclass’s clean() method can perform validation that 
>> requires access to multiple form fields. This is where you might put in 
>> checks such as “if field A is supplied, field B must contain a valid 
>> email address”. *This method can return a completely different 
>> dictionary if it wishes, which will be used as the cleaned_data*. 
>>
>
> 2) Cleaning and validating fields that depend on each other 
> 
>
> Suppose we add another requirement to our contact form: if the 
>> cc_myself field is True, the subject must contain the word "help". *We 
>> are performing validation on more than one field at a time, so the 
>> form’s 

Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Protik
I am using Django 1.11. Further, adding a new book without disabling the 
user field results in the following error:

[image: Selection_046.png]
I have attached the code to reproduce this error.

On Monday, September 24, 2018 at 1:59:31 AM UTC+5:30, Todor Velichkov wrote:
>
> First thought: What is your Django version? The `disabled` attribute was 
> added in Django 1.9.
> However by looking at your code (w/o testing anything) after 
> `form.is_valid()` you should only call `form.save()`, no need to do 
> `commit=False` and manually assigning user, you have already done that in 
> the form constructor (by settings initial value + disabled attr).
>
> On Sunday, September 23, 2018 at 9:25:41 PM UTC+3, Protik wrote:
>>
>> Hi, Todor
>>
>> I have tested this solution and It looks like it works only when you 
>> don't disable the field (i.e the last line in the BookForm's `__init__()` 
>> method. My views looks like this:
>>
>>
>> def book_add(request):
>> user = get_object_or_404(User, id=1)
>>
>> if request.method == 'POST':
>>
>> f = BookForm(request.POST, user=user)
>> if f.is_valid():
>> book = f.save(commit=False)
>> book.user = user
>> book.save()
>> messages.add_message(request, messages.INFO, 'book added.')
>> return redirect('book_add')
>> else:
>> f = BookForm(user=user)
>>
>> return render(request, 'blog/book_add.html', {'f': f})
>>
>>
>> def post_update(request, post_pk):
>> user = get_object_or_404(User, id=1)
>> book = get_object_or_404(Book, pk=post_pk)
>> if request.method == 'POST':
>> f = BookForm(request.POST, instance=book, user=user)
>> if f.is_valid():
>> post = f.save(commit=False)
>> post.user = user
>> post.save()
>> messages.add_message(request, messages.INFO, 'Post added.')
>> return redirect('post_update', post.pk)
>> else:
>> f = BookForm(instance=book, user=user)
>>
>> return render(request, 'blog/book_update.html', {'f': f})
>>
>>
>> The code for models and modelform is exactly same as yours.
>>
>>
>> Am I doing something wrong?
>>
>>
>> On Sunday, September 23, 2018 at 9:11:55 PM UTC+5:30, Todor Velichkov 
>> wrote:
>>>
>>> You can use the `disabled 
>>> ` 
>>> attribute on form fields with a combination of HiddenInput 
>>> 
>>>
>>> Using the Book example from the first comment it will look like this:
>>>   
>>> class BookForm(forms.ModelForm):
>>> class Meta:
>>> model = Book
>>> fields = ('user', 'name')
>>> 
>>> def __init__(self, *args, **kwargs):
>>> user = kwargs.pop('user')
>>> super(BookForm, self).__init__(*args, **kwargs)
>>> self.fields['user'].widget = forms.HiddenInput()
>>> self.fields['user'].initial = user
>>> self.fields['user'].disabled = True
>>>
>>>
>>> First we hide the the user field because we dont want the user to see 
>>> it, and at the same time keeping it inside form fields we wont prevent the 
>>> unique_together validation.
>>> Second - we disable the field and programmatically set initial value to 
>>> be used during form validation
>>>
>>> On Sunday, September 23, 2018 at 4:57:15 PM UTC+3, Protik wrote:

 Hi  Todor

 I am experiencing the same problem. Can you please post the 
 possible solution?

 On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov 
 wrote:
>
> It does? Can you give me a link to that please?
>>
>
> Pard me, it does not explicitly say to set values programmatically, 
> but that this is the place to go when fields depend on each other, and 
> since clean is a multi-step process which does not include only field 
> validation, but settings values too, it kind of makes sense.
>
> 1) Form and field validation  
> 
>
> The form subclass’s clean() method can perform validation that 
>> requires access to multiple form fields. This is where you might put in 
>> checks such as “if field A is supplied, field B must contain a valid 
>> email address”. *This method can return a completely different 
>> dictionary if it wishes, which will be used as the cleaned_data*. 
>>
>
> 2) Cleaning and validating fields that depend on each other 
> 
>
> Suppose we add another requirement to our contact form: if the 
>> cc_myself field is True, the subject must contain the word "help". *We 
>> are performing validation on more than one field at a time, so the 
>> 

Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Todor Velichkov
First thought: What is your Django version? The `disabled` attribute was 
added in Django 1.9.
However by looking at your code (w/o testing anything) after 
`form.is_valid()` you should only call `form.save()`, no need to do 
`commit=False` and manually assigning user, you have already done that in 
the form constructor (by settings initial value + disabled attr).

On Sunday, September 23, 2018 at 9:25:41 PM UTC+3, Protik wrote:
>
> Hi, Todor
>
> I have tested this solution and It looks like it works only when you don't 
> disable the field (i.e the last line in the BookForm's `__init__()` method. 
> My views looks like this:
>
>
> def book_add(request):
> user = get_object_or_404(User, id=1)
>
> if request.method == 'POST':
>
> f = BookForm(request.POST, user=user)
> if f.is_valid():
> book = f.save(commit=False)
> book.user = user
> book.save()
> messages.add_message(request, messages.INFO, 'book added.')
> return redirect('book_add')
> else:
> f = BookForm(user=user)
>
> return render(request, 'blog/book_add.html', {'f': f})
>
>
> def post_update(request, post_pk):
> user = get_object_or_404(User, id=1)
> book = get_object_or_404(Book, pk=post_pk)
> if request.method == 'POST':
> f = BookForm(request.POST, instance=book, user=user)
> if f.is_valid():
> post = f.save(commit=False)
> post.user = user
> post.save()
> messages.add_message(request, messages.INFO, 'Post added.')
> return redirect('post_update', post.pk)
> else:
> f = BookForm(instance=book, user=user)
>
> return render(request, 'blog/book_update.html', {'f': f})
>
>
> The code for models and modelform is exactly same as yours.
>
>
> Am I doing something wrong?
>
>
> On Sunday, September 23, 2018 at 9:11:55 PM UTC+5:30, Todor Velichkov 
> wrote:
>>
>> You can use the `disabled 
>> ` 
>> attribute on form fields with a combination of HiddenInput 
>> 
>>
>> Using the Book example from the first comment it will look like this:
>>   
>> class BookForm(forms.ModelForm):
>> class Meta:
>> model = Book
>> fields = ('user', 'name')
>> 
>> def __init__(self, *args, **kwargs):
>> user = kwargs.pop('user')
>> super(BookForm, self).__init__(*args, **kwargs)
>> self.fields['user'].widget = forms.HiddenInput()
>> self.fields['user'].initial = user
>> self.fields['user'].disabled = True
>>
>>
>> First we hide the the user field because we dont want the user to see it, 
>> and at the same time keeping it inside form fields we wont prevent the 
>> unique_together validation.
>> Second - we disable the field and programmatically set initial value to 
>> be used during form validation
>>
>> On Sunday, September 23, 2018 at 4:57:15 PM UTC+3, Protik wrote:
>>>
>>> Hi  Todor
>>>
>>> I am experiencing the same problem. Can you please post the 
>>> possible solution?
>>>
>>> On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov 
>>> wrote:

 It does? Can you give me a link to that please?
>

 Pard me, it does not explicitly say to set values programmatically, but 
 that this is the place to go when fields depend on each other, and since 
 clean is a multi-step process which does not include only field 
 validation, 
 but settings values too, it kind of makes sense.

 1) Form and field validation  
 

 The form subclass’s clean() method can perform validation that 
> requires access to multiple form fields. This is where you might put in 
> checks such as “if field A is supplied, field B must contain a valid 
> email address”. *This method can return a completely different 
> dictionary if it wishes, which will be used as the cleaned_data*. 
>

 2) Cleaning and validating fields that depend on each other 
 

 Suppose we add another requirement to our contact form: if the 
> cc_myself field is True, the subject must contain the word "help". *We 
> are performing validation on more than one field at a time, so the form’s 
> clean() method is a good spot to do this.*
>

 3) Model.clean() 
 

 This method should be used to provide custom model validation, *and to 
> modify attributes on your model if desired*. For instance, you could 
> use it to automatically provide a value for a field, or 

Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Protik


On Sunday, September 23, 2018 at 11:55:41 PM UTC+5:30, Protik wrote:
>
> Hi, Todor
>
> I have tested this solution and It looks like it works only when you don't 
> disable the field (i.e the last line in the BookForm's `__init__()` method. 
> My views looks like this:
>
>
>
> def book_add(request): user = get_object_or_404(User, id=1) if request.method 
> == 'POST': f = BookForm(request.POST, user=user) if f.is_valid(): book = 
> f.save(commit=False) book.user = user book.save() 
> messages.add_message(request, messages.INFO, 'book added.') return 
> redirect('book_add') else: f = BookForm(user=user) return render(request, 
> 'blog/book_add.html', {'f': f}) def post_update(request, post_pk): user = 
> get_object_or_404(User, id=1) book = get_object_or_404(Book, pk=post_pk) if 
> request.method == 'POST': f = BookForm(request.POST, instance=book, 
> user=user) if f.is_valid(): post = f.save(commit=False) post.user = user 
> post.save() messages.add_message(request, messages.INFO, 'Post added.') 
> return redirect('post_update', post.pk) else: f = BookForm(instance=book, 
> user=user) return render(request, 'blog/book_update.html', {'f': f})
>
>
>
> The code for models and modelform is exactly same as yours.
>
>
> Am I doing something wrong?
>
>
> On Sunday, September 23, 2018 at 9:11:55 PM UTC+5:30, Todor Velichkov 
> wrote:
>>
>> You can use the `disabled 
>> ` 
>> attribute on form fields with a combination of HiddenInput 
>> 
>>
>> Using the Book example from the first comment it will look like this:
>>   
>> class BookForm(forms.ModelForm):
>> class Meta:
>> model = Book
>> fields = ('user', 'name')
>> 
>> def __init__(self, *args, **kwargs):
>> user = kwargs.pop('user')
>> super(BookForm, self).__init__(*args, **kwargs)
>> self.fields['user'].widget = forms.HiddenInput()
>> self.fields['user'].initial = user
>> self.fields['user'].disabled = True
>>
>>
>> First we hide the the user field because we dont want the user to see it, 
>> and at the same time keeping it inside form fields we wont prevent the 
>> unique_together validation.
>> Second - we disable the field and programmatically set initial value to 
>> be used during form validation
>>
>> On Sunday, September 23, 2018 at 4:57:15 PM UTC+3, Protik wrote:
>>>
>>> Hi  Todor
>>>
>>> I am experiencing the same problem. Can you please post the 
>>> possible solution?
>>>
>>> On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov 
>>> wrote:

 It does? Can you give me a link to that please?
>

 Pard me, it does not explicitly say to set values programmatically, but 
 that this is the place to go when fields depend on each other, and since 
 clean is a multi-step process which does not include only field 
 validation, 
 but settings values too, it kind of makes sense.

 1) Form and field validation  
 

 The form subclass’s clean() method can perform validation that 
> requires access to multiple form fields. This is where you might put in 
> checks such as “if field A is supplied, field B must contain a valid 
> email address”. *This method can return a completely different 
> dictionary if it wishes, which will be used as the cleaned_data*. 
>

 2) Cleaning and validating fields that depend on each other 
 

 Suppose we add another requirement to our contact form: if the 
> cc_myself field is True, the subject must contain the word "help". *We 
> are performing validation on more than one field at a time, so the form’s 
> clean() method is a good spot to do this.*
>

 3) Model.clean() 
 

 This method should be used to provide custom model validation, *and to 
> modify attributes on your model if desired*. For instance, you could 
> use it to automatically provide a value for a field, or to do validation 
> that requires access to more than a single field.
>

 Please, don't get me wrong, I'm far away from the idea of deprecating 
 `commit=False`. I just personally try to void it and trying to explain my 
 arguments. 

 The generic error message is something that I supply in some forms of 
> mine where race conditions can happen due to high concurrency. This is 
> why 
> I guard form.save, catch database errors and then use form.add_error to 
> add 
> a generic error message asking for a retry.
>

 Yes, 

Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Protik
Hi, Todor

I have tested this solution and It looks like it works only when you don't 
disable the field (i.e the last line in the BookForm's `__init__()` method. 
My views looks like this:


def book_add(request):
user = get_object_or_404(User, id=1)

if request.method == 'POST':

f = BookForm(request.POST, user=user)
if f.is_valid():
book = f.save(commit=False)
book.user = user
book.save()
messages.add_message(request, messages.INFO, 'book added.')
return redirect('book_add')
else:
f = BookForm(user=user)

return render(request, 'blog/book_add.html', {'f': f})


def post_update(request, post_pk):
user = get_object_or_404(User, id=1)
book = get_object_or_404(Book, pk=post_pk)
if request.method == 'POST':
f = BookForm(request.POST, instance=book, user=user)
if f.is_valid():
post = f.save(commit=False)
post.user = user
post.save()
messages.add_message(request, messages.INFO, 'Post added.')
return redirect('post_update', post.pk)
else:
f = BookForm(instance=book, user=user)

return render(request, 'blog/book_update.html', {'f': f})


The code for models and modelform is exactly same as yours.


Am I doing something wrong?


On Sunday, September 23, 2018 at 9:11:55 PM UTC+5:30, Todor Velichkov wrote:
>
> You can use the `disabled 
> ` 
> attribute on form fields with a combination of HiddenInput 
> 
>
> Using the Book example from the first comment it will look like this:
>   
> class BookForm(forms.ModelForm):
> class Meta:
> model = Book
> fields = ('user', 'name')
> 
> def __init__(self, *args, **kwargs):
> user = kwargs.pop('user')
> super(BookForm, self).__init__(*args, **kwargs)
> self.fields['user'].widget = forms.HiddenInput()
> self.fields['user'].initial = user
> self.fields['user'].disabled = True
>
>
> First we hide the the user field because we dont want the user to see it, 
> and at the same time keeping it inside form fields we wont prevent the 
> unique_together validation.
> Second - we disable the field and programmatically set initial value to be 
> used during form validation
>
> On Sunday, September 23, 2018 at 4:57:15 PM UTC+3, Protik wrote:
>>
>> Hi  Todor
>>
>> I am experiencing the same problem. Can you please post the 
>> possible solution?
>>
>> On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov 
>> wrote:
>>>
>>> It does? Can you give me a link to that please?

>>>
>>> Pard me, it does not explicitly say to set values programmatically, but 
>>> that this is the place to go when fields depend on each other, and since 
>>> clean is a multi-step process which does not include only field validation, 
>>> but settings values too, it kind of makes sense.
>>>
>>> 1) Form and field validation  
>>> 
>>>
>>> The form subclass’s clean() method can perform validation that requires 
 access to multiple form fields. This is where you might put in checks such 
 as “if field A is supplied, field B must contain a valid email 
 address”. *This method can return a completely different dictionary if 
 it wishes, which will be used as the cleaned_data*. 

>>>
>>> 2) Cleaning and validating fields that depend on each other 
>>> 
>>>
>>> Suppose we add another requirement to our contact form: if the cc_myself 
 field is True, the subject must contain the word "help". *We are 
 performing validation on more than one field at a time, so the form’s 
 clean() method is a good spot to do this.*

>>>
>>> 3) Model.clean() 
>>> 
>>>
>>> This method should be used to provide custom model validation, *and to 
 modify attributes on your model if desired*. For instance, you could 
 use it to automatically provide a value for a field, or to do validation 
 that requires access to more than a single field.

>>>
>>> Please, don't get me wrong, I'm far away from the idea of deprecating 
>>> `commit=False`. I just personally try to void it and trying to explain my 
>>> arguments. 
>>>
>>> The generic error message is something that I supply in some forms of 
 mine where race conditions can happen due to high concurrency. This is why 
 I guard form.save, catch database errors and then use form.add_error to 
 add 
 a generic error message asking for a retry.

>>>
>>> Yes, that's definitely the place 

Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Todor Velichkov
You can use the `disabled 
` 
attribute on form fields with a combination of HiddenInput 


Using the Book example from the first comment it will look like this:
  
class BookForm(forms.ModelForm):
class Meta:
model = Book
fields = ('user', 'name')

def __init__(self, *args, **kwargs):
user = kwargs.pop('user')
super(BookForm, self).__init__(*args, **kwargs)
self.fields['user'].widget = forms.HiddenInput()
self.fields['user'].initial = user
self.fields['user'].disabled = True


First we hide the the user field because we dont want the user to see it, 
and at the same time keeping it inside form fields we wont prevent the 
unique_together validation.
Second - we disable the field and programmatically set initial value to be 
used during form validation

On Sunday, September 23, 2018 at 4:57:15 PM UTC+3, Protik wrote:
>
> Hi  Todor
>
> I am experiencing the same problem. Can you please post the 
> possible solution?
>
> On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov wrote:
>>
>> It does? Can you give me a link to that please?
>>>
>>
>> Pard me, it does not explicitly say to set values programmatically, but 
>> that this is the place to go when fields depend on each other, and since 
>> clean is a multi-step process which does not include only field validation, 
>> but settings values too, it kind of makes sense.
>>
>> 1) Form and field validation  
>> 
>>
>> The form subclass’s clean() method can perform validation that requires 
>>> access to multiple form fields. This is where you might put in checks such 
>>> as “if field A is supplied, field B must contain a valid email 
>>> address”. *This method can return a completely different dictionary if 
>>> it wishes, which will be used as the cleaned_data*. 
>>>
>>
>> 2) Cleaning and validating fields that depend on each other 
>> 
>>
>> Suppose we add another requirement to our contact form: if the cc_myself 
>>> field is True, the subject must contain the word "help". *We are 
>>> performing validation on more than one field at a time, so the form’s 
>>> clean() method is a good spot to do this.*
>>>
>>
>> 3) Model.clean() 
>> 
>>
>> This method should be used to provide custom model validation, *and to 
>>> modify attributes on your model if desired*. For instance, you could 
>>> use it to automatically provide a value for a field, or to do validation 
>>> that requires access to more than a single field.
>>>
>>
>> Please, don't get me wrong, I'm far away from the idea of deprecating 
>> `commit=False`. I just personally try to void it and trying to explain my 
>> arguments. 
>>
>> The generic error message is something that I supply in some forms of 
>>> mine where race conditions can happen due to high concurrency. This is why 
>>> I guard form.save, catch database errors and then use form.add_error to add 
>>> a generic error message asking for a retry.
>>>
>>
>> Yes, that's definitely the place when something outside the validation 
>> process can happen, but you don't need `commit=False` here ;) 
>>
>> One alternative approach to validate the instance w/o `commit=False` 
>> right now would be to change the form like so:
>>
>> class BookForm(forms.ModelForm):
>>
>> class Meta:
>> model = Book
>> fields = ('name', 'user')
>> widgets = {'user': forms.HiddenInput()}
>>
>> def __init__(self, *args, **kwargs):
>> super(BookForm, self).__init__(*args, **kwargs)
>> self.fields['user'].disabled = True
>>
>> #in the view
>> form = BookForm(data={'name': 'Harry Potter'}, initial={'user': user})
>>
>> But if we compare this with the form in the first post, we are just 
>> Fixing/Patching it, i.e. fighting with it in order to make it work for us.
>>
>> However, there is one more little difference here, which I want to point 
>> out. Image we didn't had the unique constrain, and we just wanted to hide 
>> the user field from the form, and we do it this way (instead of the one 
>> with the `instance` kwarg as in the first post). Doing this and adding the 
>> unique_constrain later on would yield to no more core changes (except 
>> changing the error msg if too ambiguous). While using the `commit=False` 
>> approach would require further code changes and it would be multiplied 
>> by the number of views the form is being used, and by the number of forms 
>> where the field has been omitted (i.e. commit=False has been used). Its a 
>> slight difference, but can lead to a 

Re: ModelForm unique validation is not done right IMHO.

2018-09-23 Thread Protik
Hi  Todor

I am experiencing the same problem. Can you please post the 
possible solution?

On Tuesday, October 10, 2017 at 8:26:32 AM UTC+5:30, Todor Velichkov wrote:
>
> It does? Can you give me a link to that please?
>>
>
> Pard me, it does not explicitly say to set values programmatically, but 
> that this is the place to go when fields depend on each other, and since 
> clean is a multi-step process which does not include only field validation, 
> but settings values too, it kind of makes sense.
>
> 1) Form and field validation  
> 
>
> The form subclass’s clean() method can perform validation that requires 
>> access to multiple form fields. This is where you might put in checks such 
>> as “if field A is supplied, field B must contain a valid email address”. 
>> *This 
>> method can return a completely different dictionary if it wishes, which 
>> will be used as the cleaned_data*. 
>>
>
> 2) Cleaning and validating fields that depend on each other 
> 
>
> Suppose we add another requirement to our contact form: if the cc_myself 
>> field is True, the subject must contain the word "help". *We are 
>> performing validation on more than one field at a time, so the form’s 
>> clean() method is a good spot to do this.*
>>
>
> 3) Model.clean() 
> 
>
> This method should be used to provide custom model validation, *and to 
>> modify attributes on your model if desired*. For instance, you could use 
>> it to automatically provide a value for a field, or to do validation that 
>> requires access to more than a single field.
>>
>
> Please, don't get me wrong, I'm far away from the idea of deprecating 
> `commit=False`. I just personally try to void it and trying to explain my 
> arguments. 
>
> The generic error message is something that I supply in some forms of mine 
>> where race conditions can happen due to high concurrency. This is why I 
>> guard form.save, catch database errors and then use form.add_error to add a 
>> generic error message asking for a retry.
>>
>
> Yes, that's definitely the place when something outside the validation 
> process can happen, but you don't need `commit=False` here ;) 
>
> One alternative approach to validate the instance w/o `commit=False` right 
> now would be to change the form like so:
>
> class BookForm(forms.ModelForm):
>
> class Meta:
> model = Book
> fields = ('name', 'user')
> widgets = {'user': forms.HiddenInput()}
>
> def __init__(self, *args, **kwargs):
> super(BookForm, self).__init__(*args, **kwargs)
> self.fields['user'].disabled = True
>
> #in the view
> form = BookForm(data={'name': 'Harry Potter'}, initial={'user': user})
>
> But if we compare this with the form in the first post, we are just 
> Fixing/Patching it, i.e. fighting with it in order to make it work for us.
>
> However, there is one more little difference here, which I want to point 
> out. Image we didn't had the unique constrain, and we just wanted to hide 
> the user field from the form, and we do it this way (instead of the one 
> with the `instance` kwarg as in the first post). Doing this and adding the 
> unique_constrain later on would yield to no more core changes (except 
> changing the error msg if too ambiguous). While using the `commit=False` 
> approach would require further code changes and it would be multiplied by 
> the number of views the form is being used, and by the number of forms 
> where the field has been omitted (i.e. commit=False has been used). Its a 
> slight difference, but can lead to a big wins.
>
> About the error message, to be honest I don't know, gonna keep thinking 
> about it, one thing that came to mind is to strip the missing fields, i.e. 
> instead of "Book with this User and Name already exists." to become: "Book 
> with this Name already exists." but it really depends on the context. The 
> one thing that I know for sure is that personally I would definitely prefer 
> an ambiguous message, rather than a 500 server error.
>
>
>
> On Monday, October 9, 2017 at 10:52:50 AM UTC+3, Florian Apolloner wrote:
>>
>> Hi,
>>
>> On Monday, October 9, 2017 at 8:52:53 AM UTC+2, Todor Velichkov wrote:
>>>
>>> Settings values programmatically is a cumulative operation most of the 
>>> time, however when its not and things depend on each other (like your 
>>> example), then even the docs suggests than one can use the form.clean 
>>> method. 
>>>
>>
>> It does? Can you give me a link to that please?
>>
>> If there is some other dependency outside form.cleaned_data I would 
>>> prefer to use dependency injection in order to get this data and do the job 
>>> done. I'm sorry I just see commit=False