Hi @jacobtylerwalls — Thanks for the work here. I know this ticket is difficult and has dragged on.

I think I'm missing the point of your approach here. It doesn't look to address the issue.

This change...

diff --git a/tests/forms_tests/field_tests/test_multivaluefield.py b/tests/forms_tests/field_tests/test_multivaluefield.py
index bcbdf0bc61..69d8cecb02 100644
--- a/tests/forms_tests/field_tests/test_multivaluefield.py
+++ b/tests/forms_tests/field_tests/test_multivaluefield.py
@@ -183,6 +183,6 @@ class MultiValueFieldTest(SimpleTestCase):
             'field1_2_0': '2007-04-25',
             'field1_2_1': '',  # not required
         })
-        form.is_valid()
+        self.assertTrue(form.is_valid())
         self.assertInHTML('<input type="text" name="f_0" required id="id_f_0">', form.as_p())
         self.assertInHTML('<input type="text" name="f_2_1" id="id_f_2_1">', form.as_p())

... fails...

======================================================================
FAIL: test_mixed_required_attributes (forms_tests.field_tests.test_multivaluefield.MultiValueFieldTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "...django/tests/forms_tests/field_tests/test_multivaluefield.py", line 186, in test_mixed_required_attributes
    self.assertTrue(form.is_valid())
AssertionError: False is not true

AFAICS the whole point of this ticket is that the required value on the sub-fields is not taken into account when require_all_fields is false.

The require_all_fields=True cases seems clear enough.

But when False (surely, no?) we need to defer to the individual fields — it's not enough to just adjust the rendered HTML. I think we need to look again. 🤔

Hi all,

I'll try and recall some of my thoughts on this, hope it is more help than hinderence. 🤞

I opened #14738 to show the current behaviour with the phonefield tests which I think cover all of the scenarios. Looking at the change here, Jacobs patch removes requied for the HTML for phone_3_3 (field required=True, require_all_fields=False, sub field required=False).

This fixes the 5th line on the truth table I posted a while back on the forum.

In the truth table I had suggested that the second row (field required=false, require_all_fields=False, sub field required=True) also needed adjusting. It's this scenario where I'm more uncertain what should happen. There are three aspects here:

a) HTML 5 required I had suggested that the html5 required labels should appear, I now think that's not the case as it's ok for all of the field to be empty. This PR meets this requirement. 👍

b) Field validation - all fields empty. Again, on the truth-table I had suggested that this scenario should raise an incomplete error if the entire field is not submitted. I now think this is incorrect. This addresses Carlton's observation here. This PR meets this requirement (i.e. doens't introduce a change) 👍

c) Field validation - some fields empty. So here, I'll refer to Carlton's previous comment as he can say it much more eloquently than me. "The MVF itself is optional. If submitted, and require_all_fields=False, we defer to the individual fields to determine if they can be omitted". I think the tests here which cover this scenario and don't look like they need adjusting. However, Calton -- you had pointed at the batch of tests above. Maybe this is where we have a different trail of thought? 🤔

I've not looked at the docs, do they need any work? To elaborate on Jacob's last point, some folk may find the above mentioned scenario surprising where required field is False but is True on a sub field and they can get an incomplete error?

Finally, one small backward(s) (I'm british so I can continue adding an "S", right?) comment inline below.

p.s. Carlton -- it amused me that this got picked up today 😉

OK, you've convinced me. 😅

Can you squash down @jacobtylerwalls? (Something like, Fixed #29205 -- Corrected required attr rendering for MultiValueField subfields.)

I think we should take the extra tests in #14738 as well.

Maybe this is where we have a different trail of thought? 🤔

No. I think I was pointing to the whole set of tests, rather than the specific case — I agree it does cover it.

I've not looked at the docs, do they need any work? To elaborate on Jacob's last point, some folk may find the above mentioned scenario surprising where required field is False but is True on a sub field and they can get an incomplete error?

I think they're OK. In particular I think the require_all_fields doc is pretty clear:

When set to False, the Field.required attribute can be set to False for individual fields to make them optional. If no value is supplied for a required field, an incomplete validation error will be raised.

https://docs.djangoproject.com/en/3.2/ref/forms/fields/#django.forms.MultiValueField.require_all_fields

Thanks both! 🎁