pallets-eco / pallets-eco/wtforms

Passing on validators to select subfields seems wrong

Open
#715 5 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
1.6k
Forks
409
PR merge metrics
No merged PRs in 30d

Description

This was added in https://github.com/wtforms/wtforms/pull/615 but it breaks custom select field subclasses that use e.g. a list of checkboxes as they now all get the required attribute.

Here's a failing test case (since both inputs get the required flag):

def test_required_flag_custom():
    class CustomWidget:
        def __call__(self, field, **kwargs):
            html = []
            for subfield in field:
                html.append(f'{subfield.label}: {subfield()}')
            return ''.join(html)

    class CustomSelectMultipleField(SelectMultipleField):
        widget = CustomWidget()
        option_widget = CheckboxInput()

    F = make_form(
        c=CustomSelectMultipleField(
            choices=[("a", "hello"), ("b", "bye")],
            validators=[validators.DataRequired()],
        )
    )
    form = F(DummyPostData(c=["a"]))
    assert form.c() == (
        '<label for="c-0">hello</label>: <input checked id="c-0" name="c" type="checkbox" value="a">'
        '<label for="c-1">bye</label>: <input id="c-1" name="c" type="checkbox" value="b">'
    )

I have the feeling that passing validators to subfields isn't the correct thing to do - only <input type="radio"> handles the required flag correctly, but of course for multi-select you'll never have radio buttons...

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with SelectMultipleField and the supplied test_required_flag_custom reproducer, then trace how validators reach its subfields and how the custom checkbox widget renders them. Done means the regression test passes without adding required to every checkbox while multi-select validation still works.

Written by the indexing model from the issue text.

Assessment

Tech stack
html, python
Domain
frontend, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
65/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.