Skip to content

Commit

Permalink
Fixed #31026 -- Switched form rendering to template engine.
Browse files Browse the repository at this point in the history
Thanks Carlton Gibson, Keryn Knight, Mariusz Felisiak, and Nick Pope
for reviews.

Co-authored-by: Johannes Hoppe <[email protected]>
  • Loading branch information
2 people authored and felixxm committed Sep 20, 2021
1 parent 5353e7c commit 456466d
Show file tree
Hide file tree
Showing 56 changed files with 1,048 additions and 330 deletions.
20 changes: 11 additions & 9 deletions django/forms/boundfield.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import re

from django.core.exceptions import ValidationError
from django.forms.utils import flatatt, pretty_name
from django.forms.utils import pretty_name
from django.forms.widgets import MultiWidget, Textarea, TextInput
from django.utils.functional import cached_property
from django.utils.html import conditional_escape, format_html, html_safe
from django.utils.safestring import mark_safe
from django.utils.html import format_html, html_safe
from django.utils.translation import gettext_lazy as _

__all__ = ('BoundField',)
Expand Down Expand Up @@ -75,7 +74,7 @@ def errors(self):
"""
Return an ErrorList (empty if there are no errors) for this field.
"""
return self.form.errors.get(self.name, self.form.error_class())
return self.form.errors.get(self.name, self.form.error_class(renderer=self.form.renderer))

def as_widget(self, widget=None, attrs=None, only_initial=False):
"""
Expand Down Expand Up @@ -177,11 +176,14 @@ def label_tag(self, contents=None, attrs=None, label_suffix=None):
attrs['class'] += ' ' + self.form.required_css_class
else:
attrs['class'] = self.form.required_css_class
attrs = flatatt(attrs) if attrs else ''
contents = format_html('<label{}>{}</label>', attrs, contents)
else:
contents = conditional_escape(contents)
return mark_safe(contents)
context = {
'form': self.form,
'field': self,
'label': contents,
'attrs': attrs,
'use_tag': bool(id_),
}
return self.form.render(self.form.template_name_label, context)

def css_classes(self, extra_classes=None):
"""
Expand Down
95 changes: 55 additions & 40 deletions django/forms/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,17 @@

import copy
import datetime
import warnings

from django.core.exceptions import NON_FIELD_ERRORS, ValidationError
from django.forms.fields import Field, FileField
from django.forms.utils import ErrorDict, ErrorList
from django.forms.utils import ErrorDict, ErrorList, RenderableFormMixin
from django.forms.widgets import Media, MediaDefiningClass
from django.utils.datastructures import MultiValueDict
from django.utils.deprecation import RemovedInDjango50Warning
from django.utils.functional import cached_property
from django.utils.html import conditional_escape, html_safe
from django.utils.safestring import mark_safe
from django.utils.html import conditional_escape
from django.utils.safestring import SafeString, mark_safe
from django.utils.translation import gettext as _

from .renderers import get_default_renderer
Expand Down Expand Up @@ -49,8 +51,7 @@ def __new__(mcs, name, bases, attrs):
return new_class


@html_safe
class BaseForm:
class BaseForm(RenderableFormMixin):
"""
The main implementation of all the Form logic. Note that this class is
different than Form. See the comments by the Form class for more info. Any
Expand All @@ -62,6 +63,12 @@ class BaseForm:
prefix = None
use_required_attribute = True

template_name = 'django/forms/default.html'
template_name_p = 'django/forms/p.html'
template_name_table = 'django/forms/table.html'
template_name_ul = 'django/forms/ul.html'
template_name_label = 'django/forms/label.html'

def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, label_suffix=None,
empty_permitted=False, field_order=None, use_required_attribute=None, renderer=None):
Expand Down Expand Up @@ -129,9 +136,6 @@ def order_fields(self, field_order):
fields.update(self.fields) # add remaining fields in original order
self.fields = fields

def __str__(self):
return self.as_table()

def __repr__(self):
if self._errors is None:
is_valid = "Unknown"
Expand Down Expand Up @@ -206,6 +210,12 @@ def _widget_data_value(self, widget, html_name):

def _html_output(self, normal_row, error_row, row_ender, help_text_html, errors_on_separate_row):
"Output HTML. Used by as_table(), as_ul(), as_p()."
warnings.warn(
'django.forms.BaseForm._html_output() is deprecated. '
'Please use .render() and .get_context() instead.',
RemovedInDjango50Warning,
stacklevel=2,
)
# Errors that should be displayed above all fields.
top_errors = self.non_field_errors().copy()
output, hidden_fields = [], []
Expand Down Expand Up @@ -282,43 +292,48 @@ def _html_output(self, normal_row, error_row, row_ender, help_text_html, errors_
output.append(str_hidden)
return mark_safe('\n'.join(output))

def as_table(self):
"Return this form rendered as HTML <tr>s -- excluding the <table></table>."
return self._html_output(
normal_row='<tr%(html_class_attr)s><th>%(label)s</th><td>%(errors)s%(field)s%(help_text)s</td></tr>',
error_row='<tr><td colspan="2">%s</td></tr>',
row_ender='</td></tr>',
help_text_html='<br><span class="helptext">%s</span>',
errors_on_separate_row=False,
)

def as_ul(self):
"Return this form rendered as HTML <li>s -- excluding the <ul></ul>."
return self._html_output(
normal_row='<li%(html_class_attr)s>%(errors)s%(label)s %(field)s%(help_text)s</li>',
error_row='<li>%s</li>',
row_ender='</li>',
help_text_html=' <span class="helptext">%s</span>',
errors_on_separate_row=False,
)

def as_p(self):
"Return this form rendered as HTML <p>s."
return self._html_output(
normal_row='<p%(html_class_attr)s>%(label)s %(field)s%(help_text)s</p>',
error_row='%s',
row_ender='</p>',
help_text_html=' <span class="helptext">%s</span>',
errors_on_separate_row=True,
)
def get_context(self):
fields = []
hidden_fields = []
top_errors = self.non_field_errors().copy()
for name, bf in self._bound_items():
bf_errors = self.error_class(bf.errors, renderer=self.renderer)
if bf.is_hidden:
if bf_errors:
top_errors += [
_('(Hidden field %(name)s) %(error)s') % {'name': name, 'error': str(e)}
for e in bf_errors
]
hidden_fields.append(bf)
else:
errors_str = str(bf_errors)
# RemovedInDjango50Warning.
if not isinstance(errors_str, SafeString):
warnings.warn(
f'Returning a plain string from '
f'{self.error_class.__name__} is deprecated. Please '
f'customize via the template system instead.',
RemovedInDjango50Warning,
)
errors_str = mark_safe(errors_str)
fields.append((bf, errors_str))
return {
'form': self,
'fields': fields,
'hidden_fields': hidden_fields,
'errors': top_errors,
}

def non_field_errors(self):
"""
Return an ErrorList of errors that aren't associated with a particular
field -- i.e., from Form.clean(). Return an empty ErrorList if there
are none.
"""
return self.errors.get(NON_FIELD_ERRORS, self.error_class(error_class='nonfield'))
return self.errors.get(
NON_FIELD_ERRORS,
self.error_class(error_class='nonfield', renderer=self.renderer),
)

def add_error(self, field, error):
"""
Expand Down Expand Up @@ -360,9 +375,9 @@ def add_error(self, field, error):
raise ValueError(
"'%s' has no field named '%s'." % (self.__class__.__name__, field))
if field == NON_FIELD_ERRORS:
self._errors[field] = self.error_class(error_class='nonfield')
self._errors[field] = self.error_class(error_class='nonfield', renderer=self.renderer)
else:
self._errors[field] = self.error_class()
self._errors[field] = self.error_class(renderer=self.renderer)
self._errors[field].extend(error_list)
if field in self.cleaned_data:
del self.cleaned_data[field]
Expand Down
65 changes: 29 additions & 36 deletions django/forms/formsets.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
from django.core.exceptions import ValidationError
from django.forms import Form
from django.forms.fields import BooleanField, IntegerField
from django.forms.utils import ErrorList
from django.forms.renderers import get_default_renderer
from django.forms.utils import ErrorList, RenderableFormMixin
from django.forms.widgets import CheckboxInput, HiddenInput, NumberInput
from django.utils.functional import cached_property
from django.utils.html import html_safe
from django.utils.safestring import mark_safe
from django.utils.translation import gettext_lazy as _, ngettext

__all__ = ('BaseFormSet', 'formset_factory', 'all_valid')
Expand Down Expand Up @@ -50,8 +49,7 @@ def clean(self):
return cleaned_data


@html_safe
class BaseFormSet:
class BaseFormSet(RenderableFormMixin):
"""
A collection of instances of the same Form class.
"""
Expand All @@ -63,6 +61,10 @@ class BaseFormSet:
'%(field_names)s. You may need to file a bug report if the issue persists.'
),
}
template_name = 'django/forms/formsets/default.html'
template_name_p = 'django/forms/formsets/p.html'
template_name_table = 'django/forms/formsets/table.html'
template_name_ul = 'django/forms/formsets/ul.html'

def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
initial=None, error_class=ErrorList, form_kwargs=None,
Expand All @@ -85,9 +87,6 @@ def __init__(self, data=None, files=None, auto_id='id_%s', prefix=None,
messages.update(error_messages)
self.error_messages = messages

def __str__(self):
return self.as_table()

def __iter__(self):
"""Yield the forms in the order they should be rendered."""
return iter(self.forms)
Expand All @@ -110,15 +109,20 @@ def __bool__(self):
def management_form(self):
"""Return the ManagementForm instance for this FormSet."""
if self.is_bound:
form = ManagementForm(self.data, auto_id=self.auto_id, prefix=self.prefix)
form = ManagementForm(self.data, auto_id=self.auto_id, prefix=self.prefix, renderer=self.renderer)
form.full_clean()
else:
form = ManagementForm(auto_id=self.auto_id, prefix=self.prefix, initial={
TOTAL_FORM_COUNT: self.total_form_count(),
INITIAL_FORM_COUNT: self.initial_form_count(),
MIN_NUM_FORM_COUNT: self.min_num,
MAX_NUM_FORM_COUNT: self.max_num
})
form = ManagementForm(
auto_id=self.auto_id,
prefix=self.prefix,
initial={
TOTAL_FORM_COUNT: self.total_form_count(),
INITIAL_FORM_COUNT: self.initial_form_count(),
MIN_NUM_FORM_COUNT: self.min_num,
MAX_NUM_FORM_COUNT: self.max_num,
},
renderer=self.renderer,
)
return form

def total_form_count(self):
Expand Down Expand Up @@ -177,6 +181,7 @@ def _construct_form(self, i, **kwargs):
# incorrect validation for extra, optional, and deleted
# forms in the formset.
'use_required_attribute': False,
'renderer': self.renderer,
}
if self.is_bound:
defaults['data'] = self.data
Expand Down Expand Up @@ -212,7 +217,8 @@ def empty_form(self):
prefix=self.add_prefix('__prefix__'),
empty_permitted=True,
use_required_attribute=False,
**self.get_form_kwargs(None)
**self.get_form_kwargs(None),
renderer=self.renderer,
)
self.add_fields(form, None)
return form
Expand Down Expand Up @@ -338,7 +344,7 @@ def full_clean(self):
self._non_form_errors.
"""
self._errors = []
self._non_form_errors = self.error_class(error_class='nonform')
self._non_form_errors = self.error_class(error_class='nonform', renderer=self.renderer)
empty_forms_count = 0

if not self.is_bound: # Stop further processing.
Expand Down Expand Up @@ -387,7 +393,8 @@ def full_clean(self):
except ValidationError as e:
self._non_form_errors = self.error_class(
e.error_list,
error_class='nonform'
error_class='nonform',
renderer=self.renderer,
)

def clean(self):
Expand Down Expand Up @@ -450,29 +457,14 @@ def media(self):
else:
return self.empty_form.media

def as_table(self):
"Return this formset rendered as HTML <tr>s -- excluding the <table></table>."
# XXX: there is no semantic division between forms here, there
# probably should be. It might make sense to render each form as a
# table row with each field as a td.
forms = ' '.join(form.as_table() for form in self)
return mark_safe(str(self.management_form) + '\n' + forms)

def as_p(self):
"Return this formset rendered as HTML <p>s."
forms = ' '.join(form.as_p() for form in self)
return mark_safe(str(self.management_form) + '\n' + forms)

def as_ul(self):
"Return this formset rendered as HTML <li>s."
forms = ' '.join(form.as_ul() for form in self)
return mark_safe(str(self.management_form) + '\n' + forms)
def get_context(self):
return {'formset': self}


def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False,
can_delete=False, max_num=None, validate_max=False,
min_num=None, validate_min=False, absolute_max=None,
can_delete_extra=True):
can_delete_extra=True, renderer=None):
"""Return a FormSet for the given form class."""
if min_num is None:
min_num = DEFAULT_MIN_NUM
Expand All @@ -498,6 +490,7 @@ def formset_factory(form, formset=BaseFormSet, extra=1, can_order=False,
'absolute_max': absolute_max,
'validate_min': validate_min,
'validate_max': validate_max,
'renderer': renderer or get_default_renderer(),
}
return type(form.__name__ + 'FormSet', (formset,), attrs)

Expand Down
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/attrs.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% for name, value in attrs.items() %}{% if value is not sameas False %} {{ name }}{% if value is not sameas True %}="{{ value }}"{% endif %}{% endif %}{% endfor %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/default.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% include "django/forms/table.html" %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/errors/dict/default.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% include "django/forms/errors/dict/ul.html" %}
3 changes: 3 additions & 0 deletions django/forms/jinja2/django/forms/errors/dict/text.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{% for field, errors in errors %}* {{ field }}
{% for error in errors %} * {{ error }}
{% endfor %}{% endfor %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/errors/dict/ul.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% if errors %}<ul class="{{ error_class }}">{% for field, error in errors %}<li>{{ field }}{{ error }}</li>{% endfor %}</ul>{% endif %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/errors/list/default.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% include "django/forms/errors/list/ul.html" %}
2 changes: 2 additions & 0 deletions django/forms/jinja2/django/forms/errors/list/text.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{% for error in errors %}* {{ error }}
{% endfor %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/errors/list/ul.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% if errors %}<ul class="{{ error_class }}">{% for error in errors %}<li>{{ error }}</li>{% endfor %}</ul>{% endif %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/formsets/default.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ formset.management_form }}{% for form in formset %}{{ form }}{% endfor %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/formsets/p.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ formset.management_form }}{% for form in formset %}{{ form.as_p() }}{% endfor %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/formsets/table.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ formset.management_form }}{% for form in formset %}{{ form.as_table() }}{% endfor %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/formsets/ul.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{{ formset.management_form }}{% for form in formset %}{{ form.as_ul() }}{% endfor %}
1 change: 1 addition & 0 deletions django/forms/jinja2/django/forms/label.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{% if use_tag %}<label{% if attrs %}{% include 'django/forms/attrs.html' %}{% endif %}>{{ label }}</label>{% else %}{{ label }}{% endif %}
Loading

0 comments on commit 456466d

Please sign in to comment.