Opened 2 months ago

Closed 7 weeks ago

#28620 closed Bug (invalid)

Loop on boundfields at init of inline sub-forms breaks the form set

Reported by: Jonathan Rossel Owned by: DmitriySelischev
Component: Forms Version: master
Severity: Normal Keywords: inlineformset
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When doing a loop (even a dummy one) on their boundfields at the init phase, the sub-forms of an inline form set are "broken", i.e. the parent key field is rendered instead of being hidden and many additional SQL requests are generated.

In this illustration case, I have 2 related models - Fig and Line - where each Fig can have many Line(s).

forms.py

class FigForm( ModelForm ):
    class Meta:
        model = Fig
        fields = '__all__'

class LineForm( ModelForm ):
    class Meta:
        model = Line
        fields = '__all__'
    
    def __init__(self, *args, **kwargs):
        super( LineForm, self ).__init__(*args, **kwargs)
    
        for bf in self:   # <- this screws everything
            pass          #    even with a dummy loop
    
LineFormSet = inlineformset_factory( Fig, Line, form=LineForm, extra=1 ) 

In real life, I use this loop in a custom model form class to add default CSS classes to the form widgets, but the additionnal SQL requests kill my server.


Additional information

model.py

class Fig(models.Model):
    name = models.CharField(max_length=200,unique=True)
    xlabel = models.CharField(max_length=200,blank=True)
    ylabel = models.CharField(max_length=200,blank=True)
    descr = models.TextField(blank=True)
    pub_date = models.DateTimeField('date published',auto_now=True)
    
    def __str__(self):
        return self.name
        
    def get_absolute_url(self):
        return reverse('webplot:detail', kwargs={'pk':self.pk} )
        
    
class Line(models.Model):
    fig = models.ForeignKey(Fig, on_delete=models.CASCADE)
    x = models.CharField(max_length=200)
    y = models.CharField(max_length=200)
    label = models.CharField(max_length=20)
    
    def __str__(self):
        return self.label + ': y(x)=' + self.y + ' for x=' + self.x
        

views.py

def detail(request,pk='new'):
    # Used for both CREATE and UPDATE
    # create a form instance and populate it with data from the request if any
    if pk == 'new':
        fig = Fig() # instanciate empty object
    else:
        fig = get_object_or_404( Fig, pk=pk )
    
    form = FigForm( request.POST or None, instance=fig )
    subform = LineFormSet( request.POST or None, instance=fig )
    
    if request.method == 'POST':
        if form.is_valid() and subform.is_valid():
            form.save()
            subform.save()
            return redirect(fig)
    
    ctx = {'form': form, 'subform': subform }
       
    return render(request, 'webplot/detail.html', ctx )

detail.html

      <form method="post" class="w3-container">{% csrf_token %}
            
            <h4>Figure settings</h4>
            <table class="w3-table">
                {{ form.as_table }}
            </table>
            
            <h4>Line settings</h4>
            {{ subform.management_form }}
            {{ subform.non_field_errors }}
            <table class="w3-table">
                {% for thisform in subform.forms %}
                    {% if forloop.first %}
                        <thead>
                        <tr class="w3-border-bottom">
                        {% for field in thisform.visible_fields %}
                            <th>{{ field.label_tag }}</th>
                        {% endfor %}
                        </tr>
                        </thead>
                        <tbody>
                    {% endif %}
                    <tr>
                        {{ thisform.non_field_errors }}
                        {% for field in thisform.hidden_fields %}
                            {{ field }}
                        {% endfor %}
                        {% for field in thisform.visible_fields %}
                            <td>{{ field.errors }}{{ field }}</td>
                        {% endfor %}
                    </tr>
                {% endfor %}
                </tbody>
            </table>
            <input type="submit" value="Submit" class="w3-btn" />
        </form>

Change History (5)

comment:1 Changed 8 weeks ago by DmitriySelischev

Owner: changed from nobody to DmitriySelischev
Status: newassigned

comment:2 Changed 8 weeks ago by DmitriySelischev

Version: 1.11master

comment:3 Changed 7 weeks ago by Tim Graham

I don't think I've every seen a need to iterate over bound fields in __init__() -- certainly that's not a documented pattern. Can you instead make your modifications using self.fields['field_name'].widget in Form.__init__()?

comment:4 in reply to:  3 Changed 7 weeks ago by Jonathan Rossel

Replying to Tim Graham:

I don't think I've every seen a need to iterate over bound fields in __init__() -- certainly that's not a documented pattern. Can you instead make your modifications using self.fields['field_name'].widget in Form.__init__()?

Hi,

Thanks for the hint! It does work nicely when accessing .widget from fields['field_name'] instead of passing by the bound field. I guess it must come from a difference in the modification location in the initialization sequence (?). After re-reading the docs, I still don't grasp the exact difference between fields and bound fields though, and why what I've done should go against a documented pattern. The only things I've found are:

At this point, I am not sure if this ticket should stay as a bug or should be converted to a request for documentation improvement (possibly combined with a modification of the code to raise an exception if someone does what I did).

Anyway, thanks for the help !

comment:5 Changed 7 weeks ago by Tim Graham

Resolution: invalid
Status: assignedclosed

I opened #28655 to add some documentation about your use case. I don't think trying to prevent iterating bound fields is worthwhile and I can't think of a simple way to do that, offhand.

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