Fixed #30758 -- Made RangeFields use multiple hidden inputs for initial data.

[nasirhjafri]

https://code.djangoproject.com/ticket/30758

[felixxm]

@nasirhjafri Thanks for this PR, however I don't think that it's a proper approach. IMO we should rather fix passing initial data for range fields. Currently we use HiddenInput which ends with

<input type="hidden"
       name="initial-range_field"
       value="[datetime.datetime(2019, 9, 13, 9, 53, 31, 531040), None]"
...

IMO we should change hidden_widget in BaseRangeField to get

<input type="hidden" name="initial-range_field_0" value="2019-09-13 09:53:31" id="initial-id_range_field_0">
<input type="hidden" name="initial-range_field_1" id="initial-id_range_field_1">

e.g.

diff --git a/django/contrib/postgres/forms/ranges.py b/django/contrib/postgres/forms/ranges.py
index 47a16a1bbd..2d55cbce57 100644
--- a/django/contrib/postgres/forms/ranges.py
+++ b/django/contrib/postgres/forms/ranges.py
@@ -2,7 +2,7 @@ from psycopg2.extras import DateRange, DateTimeTZRange, NumericRange
 
 from django import forms
 from django.core import exceptions
-from django.forms.widgets import MultiWidget
+from django.forms.widgets import MultiWidget, MultipleHiddenInput
 from django.utils.translation import gettext_lazy as _
 
 __all__ = [
@@ -16,6 +16,7 @@ class BaseRangeField(forms.MultiValueField):
         'invalid': _('Enter two valid values.'),
         'bound_ordering': _('The start of the range must not exceed the end of the range.'),
     }
+    hidden_widget = MultipleHiddenInput
 
     def __init__(self, **kwargs):
         if 'widget' not in kwargs:

[nasirhjafri]

@felixxm That's how it was being done in 1.11.x https://github.com/django/django/blob/stable/1.11.x/django/forms/fields.py#L398

I'll try the solution you provided, thanks.

[felixxm]

@nasirhjafri Yes but it's clunky, IMO.

[nasirhjafri]

@felixxm Any ideas on how test cases should look like? This solution works but I can't think of a test case.

[felixxm]

We should test the way how RangeFields widgets are rendered with initial data (tests.postgres_tests.TestWidget) and how has_changed() works for these fields with and without initial data (see similar tests for MultiValueField).

[nasirhjafri]

@felixxm I've pushed the tests, but they are not running on circle ci. Can you please retrigger the tests on PR?

[felixxm]

@nasirhjafri Please move tests to the postgres_tests.test_ranges.TestWidget. Moreover tests that are not related with this patch should be in a separate commit, i.e. first commit with extra tests and second (Fixed #30758 -- Made RangeFields use multiple hidden inputs for initial data.) with fix and tests that falling without it.

[felixxm]

Please use hanging indentation per our coding style (in all tests).

[nasirhjafri]

@felixxm This one is failing without the fix and working with the fix.

django/tests/postgres_tests/test_ranges.py

Lines 802 to 813 in 3b7febf

	def test_range_fields_form_with_initial_data(self):
	data = QueryDict(mutable=True)
	data.update({
	'date_field_0': '2010-01-01',
	'date_field_1': '',
	'datetime_field_0': '2010-01-01 00:00:00',
	'datetime_field_1': '',
	'initial-date_field': '[datetime.date(2019, 9, 14), None]',
	'initial-datetime_field': '[datetime.datetime(2019, 9, 14, 15, 52, 33), None]'
	})
	form = self.RangeFieldsForm(data=data)
	self.assertTrue(form.has_changed())

I've created two separate commits. Please review.

[felixxm]

@nasirhjafri Thanks! I simplified tests, moved them between test classes and added some extra tests. I also notice that with MultipleHiddenInput it doesn't detect changes as expected, so I decided to add a new HiddenRangeWidget which works fine, IMO. Can you take a look?

[nasirhjafri]

@felixxm looks good to me, working fine with the new HiddenRangeWidget.

[felixxm]

Great, I squashed commits and rebased from master.

[felixxm]

Merged in faf4b98 and 733dbb2.

[yeppus]

Hi! Thanks a lot for fixing this!

@felixxm do you know if this patch will be able to make it into 2.2.6?

[felixxm]

@yeppus No, it doesn't qualify for a backport to 2.2.x, but it's included in Django 3.0.

[yeppus]

@felixxm thanks for letting me know,,, we need the LTS so I guess we need to rework the code around this bug in that case.