calling self.errors in a form in a formset breaks deletion

[Carles Pina i Estany]

Hi,

Yesterday I found that calling self.errors in the constructor of a Form
that is in an InlineFormSet (or any formset I guess): the "delete" of a
form doesn't work anymore. I wonder if this is a bug in Django (for this
case, if a new bug is needed, I'm happy to open it... or be referred to
an existing bug) or if I missed some bit of the documentation.

Summary:
BaseForm.errors does:
https://github.com/django/django/blob/master/django/forms/forms.py#L169

The DELETE field in self.fields is added in the form.fields via BaseFormSet 
after calling the constructor of the form:
https://github.com/django/django/blob/master/django/forms/formsets.py#L175

So calling self.errors cleans the data but DELETE doesn't end up in
self.clean_fields (because it's not in self.fields). When DELETE is
added as a field self.clean_fields is not updated.

(I'm talking about this:
https://github.com/django/django/blob/master/django/forms/formsets.py#L390
being executed too late; or self.errors being called too early)

The full story is here:
https://groups.google.com/forum/#!topic/django-users/wO9H0cNcLfI

Thank you very much,

-- 
Carles Pina i Estany
https://carles.pina.cat

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/20200619104406.GA26082%40pina.cat.

Re: calling self.errors in a form in a formset breaks deletion

[Carlton Gibson]

Bon dia Carles. 

I'm inclined to say that calling self.errors in __init__ is at least *holding 
it wrong*. 

I'm not sure what if anything we might do about that. DRF's serializers 
have all kinds of 
"you have to call is_valid() before accessing .data" type assertions, but 
they only catch the 
obvious cases: we still get reports of unusual combinations not working on 
the tracker.
So I think trying to enforce usage is probably a loosing battle. 

Without thinking about it, I don't know exactly where I'd add the field on 
the way out but 
coming in I'd look to see if allow_error was present in clean() 
https://docs.djangoproject.com/en/3.0/ref/forms/validation/#validating-fields-with-clean

I hope that helps. 

Kind Regards,

Carlton


On Friday, 19 June 2020 13:39:00 UTC+2, Carles Pina Estany wrote:
>
>
> Hi, 
>
> Yesterday I found that calling self.errors in the constructor of a Form 
> that is in an InlineFormSet (or any formset I guess): the "delete" of a 
> form doesn't work anymore. I wonder if this is a bug in Django (for this 
> case, if a new bug is needed, I'm happy to open it... or be referred to 
> an existing bug) or if I missed some bit of the documentation. 
>
> Summary: 
> BaseForm.errors does: 
> https://github.com/django/django/blob/master/django/forms/forms.py#L169 
>
> The DELETE field in self.fields is added in the form.fields via 
> BaseFormSet after calling the constructor of the form: 
> https://github.com/django/django/blob/master/django/forms/formsets.py#L175 
>
> So calling self.errors cleans the data but DELETE doesn't end up in 
> self.clean_fields (because it's not in self.fields). When DELETE is 
> added as a field self.clean_fields is not updated. 
>
> (I'm talking about this: 
> https://github.com/django/django/blob/master/django/forms/formsets.py#L390 
> being executed too late; or self.errors being called too early) 
>
> The full story is here: 
> https://groups.google.com/forum/#!topic/django-users/wO9H0cNcLfI 
>
> Thank you very much, 
>
> -- 
> Carles Pina i Estany 
> https://carles.pina.cat 
>

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/3ed8045c-52b8-4647-9fe6-c0326aaf923do%40googlegroups.com.

Re: The blacklist / master issue

[Alexander Lyabah]

Django in international framework, not US-framework. You should not change 
variable names just because meaning of some words have been changed in US 
recently. Those words have been used in source-code for years, and nobody 
put racism in those word when this framework was founded and nobody puts 
any racism in when one is using for creation something big and meaningful.

What I'm encourage you to do, is to thing farther than what is going on 
right now.

If Django Foundation really want to help in this revolution - add a banner 
on that landing page. Feel free to choose

https://eji.org/
https://org2.salsalabs.com/o/6857/p/salsa/donation/common/public/

And this kind of contribution will work much better.

Thank you, for this opportunity to share my opinion.

On Monday, June 15, 2020 at 7:28:23 PM UTC+3, Tom Carrick wrote:
>
> This ticket was closed wontfix 
>  as requiring a 
> discussion here.
>
> David Smith mentioned this Tox issue 
>  stating it had been closed, 
> but to me it seems like it hasn't been closed (maybe there's something I 
> can't see) and apparently a PR would be accepted to add aliases at the 
> least (this is more recent than the comment on the Django ticket).
>
> My impetus to bring this up mostly comes from reading this ZDNet article 
> 
>  
> - it seems like Google have already made moves in this direction and GitHub 
> is also planning to. Usually Django is somewhere near the front for these 
> types of changes.
>
> I'm leaning towards renaming the master branch and wherever else we use 
> that terminology, but I'm less sure about black/whitelist, though right now 
> it seems more positive than negative. Most arguments against use some kind 
> of etymological argument, but I don't think debates about historical terms 
> are as interesting as how they affect people in the here and now.
>
> I don't think there is an easy answer here, and I open this can of worms 
> somewhat reluctantly. I do think Luke is correct that we should be 
> concerned with our credibility if we wrongly change this, but I'm also 
> worried about our credibility if we don't.
>

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/1c9178a3-cb80-428c-bacb-e8904695f6b6o%40googlegroups.com.

Re: The blacklist / master issue

[Tom Forbes]

As an international framework I think we should make our interface as language 
and culturally agnostic as possible. ‘Allow’ and ‘Deny’ are simply semantically 
clearer than ‘white’ and ‘black’. That alone is a convincing argument for me.

> On 19 Jun 2020, at 13:55, Alexander Lyabah  wrote:
> 
> 
> 
> Django in international framework, not US-framework. You should not change 
> variable names just because meaning of some words have been changed in US 
> recently. Those words have been used in source-code for years, and nobody put 
> racism in those word when this framework was founded and nobody puts any 
> racism in when one is using for creation something big and meaningful.
> 
> What I'm encourage you to do, is to thing farther than what is going on right 
> now.
> 
> If Django Foundation really want to help in this revolution - add a banner on 
> that landing page. Feel free to choose
> 
> https://eji.org/
> https://org2.salsalabs.com/o/6857/p/salsa/donation/common/public/
> 
> And this kind of contribution will work much better.
> 
> Thank you, for this opportunity to share my opinion.
> 
>> On Monday, June 15, 2020 at 7:28:23 PM UTC+3, Tom Carrick wrote:
>> This ticket was closed wontfix as requiring a discussion here.
>> 
>> David Smith mentioned this Tox issue stating it had been closed, but to me 
>> it seems like it hasn't been closed (maybe there's something I can't see) 
>> and apparently a PR would be accepted to add aliases at the least (this is 
>> more recent than the comment on the Django ticket).
>> 
>> My impetus to bring this up mostly comes from reading this ZDNet article - 
>> it seems like Google have already made moves in this direction and GitHub is 
>> also planning to. Usually Django is somewhere near the front for these types 
>> of changes.
>> 
>> I'm leaning towards renaming the master branch and wherever else we use that 
>> terminology, but I'm less sure about black/whitelist, though right now it 
>> seems more positive than negative. Most arguments against use some kind of 
>> etymological argument, but I don't think debates about historical terms are 
>> as interesting as how they affect people in the here and now.
>> 
>> I don't think there is an easy answer here, and I open this can of worms 
>> somewhat reluctantly. I do think Luke is correct that we should be concerned 
>> with our credibility if we wrongly change this, but I'm also worried about 
>> our credibility if we don't.
> 
> -- 
> 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 view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/1c9178a3-cb80-428c-bacb-e8904695f6b6o%40googlegroups.com.

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/FB71B5CC-7224-4C17-8C9A-E4B263B4D0C8%40tomforb.es.

Re: The blacklist / master issue

[Ahmad A. Hussein]

I'd argue that since Django is an international framework, we should strive
to set an international standard. If a certain word is off-putting or
problematic to individuals in our community, and if it does not convey an
accurate and least astonishing meaning to a non-native English speaker,
then we should definitely change it.

On Fri, Jun 19, 2020 at 2:54 PM Alexander Lyabah 
wrote:

>
> Django in international framework, not US-framework. You should not change
> variable names just because meaning of some words have been changed in US
> recently. Those words have been used in source-code for years, and nobody
> put racism in those word when this framework was founded and nobody puts
> any racism in when one is using for creation something big and meaningful.
>
> What I'm encourage you to do, is to thing farther than what is going on
> right now.
>
> If Django Foundation really want to help in this revolution - add a banner
> on that landing page. Feel free to choose
>
> https://eji.org/
> https://org2.salsalabs.com/o/6857/p/salsa/donation/common/public/
>
> And this kind of contribution will work much better.
>
> Thank you, for this opportunity to share my opinion.
>
> On Monday, June 15, 2020 at 7:28:23 PM UTC+3, Tom Carrick wrote:
>>
>> This ticket was closed wontfix
>>  as requiring a
>> discussion here.
>>
>> David Smith mentioned this Tox issue
>>  stating it had been closed,
>> but to me it seems like it hasn't been closed (maybe there's something I
>> can't see) and apparently a PR would be accepted to add aliases at the
>> least (this is more recent than the comment on the Django ticket).
>>
>> My impetus to bring this up mostly comes from reading this ZDNet article
>> 
>> - it seems like Google have already made moves in this direction and GitHub
>> is also planning to. Usually Django is somewhere near the front for these
>> types of changes.
>>
>> I'm leaning towards renaming the master branch and wherever else we use
>> that terminology, but I'm less sure about black/whitelist, though right now
>> it seems more positive than negative. Most arguments against use some kind
>> of etymological argument, but I don't think debates about historical terms
>> are as interesting as how they affect people in the here and now.
>>
>> I don't think there is an easy answer here, and I open this can of worms
>> somewhat reluctantly. I do think Luke is correct that we should be
>> concerned with our credibility if we wrongly change this, but I'm also
>> worried about our credibility if we don't.
>>
> --
> 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 view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/1c9178a3-cb80-428c-bacb-e8904695f6b6o%40googlegroups.com
> 
> .
>

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAJNa-uOOWNRmAnDhiydQC-MrEsRDyED%2B_PJzFDu5gU6GjqSs1A%40mail.gmail.com.

Re: calling self.errors in a form in a formset breaks deletion

[Carles Pina i Estany]

Hi,

On Jun/19/2020, Carlton Gibson wrote:
> Bon dia Carles. 

I'm very impressed! :-)

> I'm inclined to say that calling self.errors in __init__ is at least *holding 
> it wrong*. 

:-) it almost worked! (the only known issue was for the BaseForm not
deleting the model of my ModelForm... for what I can see though, it
might have other problems!).

> I'm not sure what if anything we might do about that. DRF's serializers 
> have all kinds of 
> "you have to call is_valid() before accessing .data" type assertions, but 

I'm not familiar with the internals of DRF (Django REST Framework I
understand) :-( yet :-)

In Django I see that is_valid() is only "return is_bound and not self.errors:"
https://github.com/django/django/blob/master/django/forms/forms.py#L177

So I didn't think that to access .data I should call is_valid(). Anyway,
what I want to do is to avoid using .data at all.

> they only catch the 
> obvious cases: we still get reports of unusual combinations not working on 
> the tracker.
> So I think trying to enforce usage is probably a loosing battle. 
> 
> Without thinking about it, I don't know exactly where I'd add the field on 
> the way out but 
> coming in I'd look to see if allow_error was present in clean() 
> https://docs.djangoproject.com/en/3.0/ref/forms/validation/#validating-fields-with-clean

Yes, I did this (if there is a BooleanField enabled I ignore some checks
in the clean)

My main problem was: ModelSet has a form that is not valid. When the
BaseForm (ModelForm in my case) is constructed again (because is_valid() is
False so the view reloads the form again showing errors to the user): if
it's not valid for a specific reason I need to add a BooleanField (to do
what you said in the clean(): ignore one of the validations).

Yesterday I implemented: in the __init__() get the data using .data (I
don't like it though) and execute my logic again to determine the cause
of the error. My first idea was to access .errors and look for a
specific error (then the problem is that the FormSet was not deleting
the Model if the user clicked on deleting a form from the formset)

For what I can see in Django code: If BaseFormSet.add_fields() called
form.full_clean() (only needed if add_fields had added a new field): I
think that if the user deletes a form (so f{prefix}-DELETE='on' is
received) the Model that the Form holds (in my case it is a ModelForm)
would have been deleted as expected. I don't know if this is going to
backfire in some places or some use cases.

(BaseFormSet.add_fields for reference: 
https://github.com/django/django/blob/master/django/forms/formsets.py#L373 )

Sorry if this sounds like a brain twist!

If using .errors in the form __init__ is unsupported: no problem!
Thanks to this email I have an idea on how to avoid it!
I'll check if the documentation says that and I've missed it. 

If using .errors in the form __init__ is supported I might look a bit
more into this.

Cheers,

> 
> I hope that helps. 
> 
> Kind Regards,
> 
> Carlton
> 
> 
> On Friday, 19 June 2020 13:39:00 UTC+2, Carles Pina Estany wrote:
> >
> >
> > Hi, 
> >
> > Yesterday I found that calling self.errors in the constructor of a Form 
> > that is in an InlineFormSet (or any formset I guess): the "delete" of a 
> > form doesn't work anymore. I wonder if this is a bug in Django (for this 
> > case, if a new bug is needed, I'm happy to open it... or be referred to 
> > an existing bug) or if I missed some bit of the documentation. 
> >
> > Summary: 
> > BaseForm.errors does: 
> > https://github.com/django/django/blob/master/django/forms/forms.py#L169 
> >
> > The DELETE field in self.fields is added in the form.fields via 
> > BaseFormSet after calling the constructor of the form: 
> > https://github.com/django/django/blob/master/django/forms/formsets.py#L175 
> >
> > So calling self.errors cleans the data but DELETE doesn't end up in 
> > self.clean_fields (because it's not in self.fields). When DELETE is 
> > added as a field self.clean_fields is not updated. 
> >
> > (I'm talking about this: 
> > https://github.com/django/django/blob/master/django/forms/formsets.py#L390 
> > being executed too late; or self.errors being called too early) 
> >
> > The full story is here: 
> > https://groups.google.com/forum/#!topic/django-users/wO9H0cNcLfI 
> >
> > Thank you very much, 
> >
> > -- 
> > Carles Pina i Estany 
> > https://carles.pina.cat 
> >
> 
> -- 
> 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 view this discussion on the web visit 
> https://groups.google.com/d/msgid/django-developers/3ed8045c-52b8-4647-9fe6-c0326aaf923do%40googlegroups.com.

-- 
Carles Pina i Estany
https://carles.pina.cat

-- 
You received this message because you are subscribed to the Google Groups 
"Django d

Re: calling self.errors in a form in a formset breaks deletion

[Carlton Gibson]

Hi Carles. 

Supported/unsupported isn't always clear cut... if you bend it far it far 
enough maybe it's just undefined what the behaviour is. 
Certainly, adding fields based on whether the field data is itself if 
invalid is going to lead into sticky territory. When you add FormSets to 
the mix all the more so. 

My reference to DRF was just that there are attempts to narrow down 
(incorrect) calling patterns there but, given issues still come up, it's 
clear that it's not feasible to cover all cases. 

I think this is probably a usage issue really, "how do I...?", rather than 
a development question... (Thought: would adding the field in 
form_invalid() before re-rendering, then checking for that in clean() save 
the whole __init__() issue...? — I don't know: you have to play :)

Sorry I don't have an instant answer. 

Bon cap de setmana!

Carlton

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/a26ebb63-6ca5-4dbe-910a-d2126161488do%40googlegroups.com.

Re: calling self.errors in a form in a formset breaks deletion

[David Smith]

Hi Carles,

Hope you are well.

You mentioned you are using crispy-forms. :-)

Have you seen this part of the docs, it allows you to update forms 'on the go'.

I've not really played with this but it sounds similar to what you need?

https://django-crispy-forms.readthedocs.io/en/latest/dynamic_layouts.html

-- 
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/41289ee1-6c5b-4699-bc15-cec57322af7fo%40googlegroups.com.