Code

Opened 4 years ago

Closed 2 years ago

#14184 closed Bug (fixed)

Validators not called on MultiValueField

Reported by: anonymous Owned by: paulcollins
Component: Forms Version: 1.2
Severity: Normal Keywords: MultiValueField Validators
Cc: paulcollins Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Validators are not called for the compressed value of a MultiValueField.

If, say, I have a MultiValueField subclass which takes a first name and a last name and compresses the two into a "Lastname, Firstname" and this MultiValueField subclass was instantiated using validators = [disallow_john_doe] , one would expect that disallow_john_doe(compressed value) would be executed. However, this is not the case.

Looking at the source, it appears that while the clean() method of django.forms.fields.Field calls run_validators() , the clean() method of django.forms.fields.MultiValueField does not. This is different from expected behaviour and is not documented to my knowledge.

The workaround is to implement validate() in the subclass.

Attachments (2)

add_run_validators_call.patch (2.2 KB) - added by paulcollins 3 years ago.
add_run_validators_call.2.patch (2.0 KB) - added by paulcollins 3 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 3 years ago by dmoisset

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Either the documentation or MultiValueField are wrong. (I'd say that MultiValueField is). Accepting

comment:2 Changed 3 years ago by julien

  • Severity set to Normal
  • Type set to Bug

Changed 3 years ago by paulcollins

comment:3 follow-up: Changed 3 years ago by paulcollins

  • Cc paulcollins added
  • Easy pickings unset
  • Has patch set
  • Owner changed from nobody to paulcollins
  • UI/UX unset

Was a bit surprised by this one myself. I'm working on the idea that the MultiValueField is wrong rather than the docs since the validators arg is a core Field arg. Patch includes a regression test for this and a fix which appears to resolve the issue.

Version 0, edited 3 years ago by paulcollins (next)

Changed 3 years ago by paulcollins

comment:4 in reply to: ↑ 3 Changed 3 years ago by paulcollins

Replying to paulcollins:

On a related note MultiValueField.validate is overridden to be a noop, but the parent class Field checks if the field is required. I've left it alone but, should MultiValueField.validate be removed so that the Field.validate gets called instead?

Never mind, eventually rereading the clean function for the MultiValueField answered my question. Seems the clean method already does the work of the Field.validate method.

comment:5 Changed 2 years ago by claudep

  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 2 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

In [17430]:

Fixed #14184 -- Enabled running the validators in MultiValueFields. Thanks, paulcollins.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.