Yes, it is working now.

Thank you very much.

On Monday, September 24, 2018 at 1:43:00 PM UTC+5:30, Todor Velichkov wrote:
>
> Protik, just use `self.fields['user'].initial = user.id` instead of just 
> `user`. Just tested it and it work fine. 
>
> On Monday, September 24, 2018 at 9:36:19 AM UTC+3, Protik wrote:
>>
>>
>> Book.objects.filter(user=form.user, name=form.cleaned_data["name"]). 
>> exists()
>>
>> This will only work for creating records, not updating.
>>
>> But what's the issue with the current solution posted by Todor. If you 
>> look at the model form it looks 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
>>
>> Logically, It should work. But it is not, so far I have checked this code 
>> with django version 1.11 and 2.1. 
>>
>> What do you think where the problem lies?
>>
>> On Monday, September 24, 2018 at 11:39:58 AM UTC+5:30, ludovic coues 
>> wrote:
>>>
>>> First, that's a discussion for the Django user list, not the Django 
>>> developers one.
>>>
>>> I would add a clean_book method on the form and use it to check if the 
>>> user already have a book with that name. For that specific problems, that's 
>>> the cleanest solution in my opinion. Simply do a 
>>> Book.objects.filter(user=form.user, name=form.cleaned_data["name"]). 
>>> exists() and if you get a hit, raise a Validation error. The error will be 
>>> associated to the book field.
>>>
>>> If someone want to write a patch for adding full validation of a 
>>> ModelForm, I wish them good luck. You could try a save commit=False but you 
>>> will only get an integrity error and a string coming from the database. 
>>> Which can be translated. You could check for your unique_together with an 
>>> extra request. Then the form will raise a Validation error saying there is 
>>> already a book with that user and that name. Confusing the user.
>>>
>>> If you think a ModelForm giving guarantee that the save method will work 
>>> make sense, the best way to proceed is a third party module. Doing so let 
>>> you do quick release for development and testing, user will be able try the 
>>> module and see if it solves their problems. In my opinion, something 
>>> generic won't help when validating unique_together relationship when one of 
>>> the fields is not exposed to the user. But I could be wrong.
>>>
>>> On Sep 24, 2018 07:34, "Protik" <prate...@gmail.com> wrote:
>>>
>>> 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 
>>>>>> <https://docs.djangoproject.com/en/1.11/ref/forms/fields/#disabled>` 
>>>>>> attribute on form fields with a combination of HiddenInput 
>>>>>> <https://docs.djangoproject.com/en/2.1/ref/forms/widgets/#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  
>>>>>>>> <https://docs.djangoproject.com/en/1.11/ref/forms/validation/#cleaning-and-validating-fields-that-depend-on-each-other>
>>>>>>>>
>>>>>>>> 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 
>>>>>>>> <https://docs.djangoproject.com/en/1.11/ref/forms/validation/#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() 
>>>>>>>> <https://docs.djangoproject.com/en/1.11/ref/models/instances/#django.db.models.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 as an anti-pattern, 
>>>>>>>>>> because 
>>>>>>>>>> the validation needs to be repeated after that (as your example in 
>>>>>>>>>> the 
>>>>>>>>>> first post).
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Repeated is a bit overreaching, it also checks new fields…
>>>>>>>>>
>>>>>>>>> Showing an error message which the user cannot correct is a 
>>>>>>>>>> horrible UX indeed, but still its a UX, and some people may find it 
>>>>>>>>>> as a 
>>>>>>>>>> better alternative to a `500 server error page`, where there is no 
>>>>>>>>>> UX at 
>>>>>>>>>> all. Even a generic message like 'sorry we messed up' would be 
>>>>>>>>>> useful, 
>>>>>>>>>> because the user data that will be preserved into the form. However, 
>>>>>>>>>> in the 
>>>>>>>>>> example shown here, this is not even the case, there is something 
>>>>>>>>>> that the 
>>>>>>>>>> user can do to prevent the error.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 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. In the example shown, 
>>>>>>>>> the 
>>>>>>>>> user can do something about the error, this is correct, but the 
>>>>>>>>> default 
>>>>>>>>> error message would be rather confusing since it would cover a field 
>>>>>>>>> which 
>>>>>>>>> is not part of the form.
>>>>>>>>>
>>>>>>>>> That said, form.save(commit=False) is there and will stay, even if 
>>>>>>>>> it is just for backwards compatibility. Same goes for the partial 
>>>>>>>>> validation of the instance, this is just something to many people 
>>>>>>>>> rely on 
>>>>>>>>> and changing is not really an option.
>>>>>>>>>
>>>>>>>>> One could add a new flag to the Meta object ala 
>>>>>>>>> validate_full_model, but for that to work you will have to tell us a 
>>>>>>>>> sensible UX approach first. I am opposed to adding something which we 
>>>>>>>>> agree 
>>>>>>>>> is horrible just because the alternative (like I've shown) requires a 
>>>>>>>>> few 
>>>>>>>>> additional lines of code.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Florian
>>>>>>>>>
>>>>>>>> -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Django developers (Contributions to Django itself)" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to django-develop...@googlegroups.com.
>>> To post to this group, send email to django-d...@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/django-developers.
>>> To view this discussion on the web visit 
>>> https://groups.google.com/d/msgid/django-developers/44b8461e-2f29-4ac4-82f4-705b52c81560%40googlegroups.com
>>>  
>>> <https://groups.google.com/d/msgid/django-developers/44b8461e-2f29-4ac4-82f4-705b52c81560%40googlegroups.com?utm_medium=email&utm_source=footer>
>>> .
>>>
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/e267ab04-6c85-4f47-91a2-414dc74d5c9a%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to